Enhancing Lightweight access token M2 #25716

skabano · 2023-12-20T06:53:04Z

Lightweight access token support follows milestone #21186.
Since M0,1 have already been implemented, I will implement milestone 2.
Details of the M2 specification can be found in #23724.

closes #23724

mposolda

@skabano This looks great to me, Thanks!

I've added one minor comment inline. Could you please check it?

@tnorimat Could you please review as well?

Besides that, we will likely need some documentation? I wonder if it is ok to add documentation for Lightweight access tokens in this PR or use it in the follow-up PR? Whatever of those is fine with me.

For the documentation, I suggest to:

Add some small paragraph in the https://www.keycloak.org/docs/latest/server_admin/index.html#_protocol-mappers about lightweight access tokens and how to use them (together with the executors). It can be also nice to redo the screenshots in this section due the new configuration switches "Add to introspection" and "Add to lightweight token", which you added.
Also it can be nice to list the executor here where all the other executors are: https://www.keycloak.org/docs/latest/server_admin/index.html#executor . Possibly with the link to the new lightweight tokens section.

@skabano @tnorimat WDYT? Do you prefer follow-up PR for the documentation or do it directly in this one?

server-spi-private/src/main/java/org/keycloak/models/ClientPolicyConstants.java

cypress · 2023-12-20T09:12:30Z

1 flaky test on run #10453 ↗︎

0	558	54	0	1

Details:

Merge `c27d29d` into `7337871`...
Project: Keycloak Admin UI	Commit: `d8390702a1 ℹ️`
Status: Passed	Duration: 07:17 💡
Started: Jan 9, 2024 2:16 AM	Ended: Jan 9, 2024 2:23 AM

cypress/e2e/masthead_test.spec.ts • 1 flaky test • chrome

View Output

Test		Artifacts
Masthead tests > Desktop view > Go to account console and back to admin console		`Test Replay` `Screenshots`

Review all test suite changes for PR #25716 ↗︎

tnorimat · 2023-12-21T02:42:36Z

@mposolda Yes, I will review the PR.

tnorimat

@skabano I added some review comments. Could you check them?

tnorimat · 2023-12-21T05:26:28Z

...uillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/LightWeightAccessTokenTest.java

+            String introspectResponse = oauth.introspectAccessTokenWithClientCredential(RESOURCE_SERVER_CLIENT_ID, RESOURCE_SERVER_CLIENT_PASSWORD, accessToken);
+            System.out.println("tokenResponse:"   introspectResponse);
+            assertTokenIntrospectionResponse(JsonSerialization.readValue(introspectResponse, AccessToken.class), true, true, false);
+


How about adding a test here to check whether a client get an access token instead of a lightweight access token after removing a policy to apply UseLightweightAccessTokenExecutor to the client?

Added test.

tnorimat · 2023-12-21T05:28:49Z

...uillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/LightWeightAccessTokenTest.java

+            OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(authsEndpointResponse.getCode(), TEST_CLIENT_SECRET);
+            String accessToken = tokenResponse.getAccessToken();
+            assertAccessToken(oauth.verifyToken(accessToken), true, false);
+            System.out.println("access token:"   accessToken);


How about replacing System.out.println with other logging method? It seems that Keycloak tends not to use System.out.println. Could you check other lines using System.out.println in this test class?

Changed to use logger.debug.

tnorimat

@skabano I added one review comment. Could you check it?

tnorimat · 2023-12-21T19:18:27Z

...uillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/LightWeightAccessTokenTest.java

@@ -1,6  1,7 @@
 package org.keycloak.testsuite.oidc;


Could you add header comments?

Added header.

tnorimat · 2023-12-22T04:52:33Z

@skabano
In LightWeightAccessTokenTest.java how about using Assert.assertNull instead of Assert.assertTrue for null check?
For example,

Assert.assertTrue(token.getNonce() == null);

to

Assert.assertNull(token.getNonce());

skabano · 2023-12-22T06:18:48Z

@skabano In LightWeightAccessTokenTest.java how about using Assert.assertNull instead of Assert.assertTrue for null check? For example,
Assert.assertTrue(token.getNonce() == null);
to
Assert.assertNull(token.getNonce());

@tnorimat
Changed to use Assert.assertNull(token.getNonce()).

tnorimat

LGTM

tnorimat · 2023-12-22T06:19:30Z

@mposolda Could you check the PR?

mposolda

@skabano @tnorimat Thanks!

mposolda · 2024-01-05T09:33:09Z

@skabano There are some conflicts, could you please resolve them? Hopefully just rebase would help. Thanks!

Closes keycloak#23724 Signed-off-by: shigeyuki kabano <[email protected]>

skabano · 2024-01-09T03:07:21Z

@skabano There are some conflicts, could you please resolve them? Hopefully just rebase would help. Thanks!
@mposolda
I resolved the conflict and rebased.

mposolda

@skabano Thanks!

Closes keycloak#23724 Signed-off-by: shigeyuki kabano <[email protected]> Signed-off-by: Stefan Wiedemann <[email protected]>

Closes keycloak#23724 Signed-off-by: shigeyuki kabano <[email protected]> Signed-off-by: ShefeeqPM <86718986 [email protected]>

skabano requested review from a team as code owners December 20, 2023 06:53

skabano requested a review from a team December 20, 2023 06:53

skabano requested a review from a team as a code owner December 20, 2023 06:53

ghost added team/ui team/core team/cloud-native labels Dec 20, 2023

skabano force-pushed the LighweightAccessTokenM2 branch from de3489e to cc0ba88 Compare December 20, 2023 07:08

skabano force-pushed the LighweightAccessTokenM2 branch from cc0ba88 to 39e9ea7 Compare December 20, 2023 08:57

mposolda reviewed Dec 20, 2023

View reviewed changes

server-spi-private/src/main/java/org/keycloak/models/ClientPolicyConstants.java Outdated Show resolved Hide resolved

mposolda self-assigned this Dec 20, 2023

mposolda requested a review from tnorimat December 20, 2023 08:59

skabano force-pushed the LighweightAccessTokenM2 branch from 39e9ea7 to 2075c13 Compare December 20, 2023 09:55

tnorimat suggested changes Dec 21, 2023

View reviewed changes

skabano force-pushed the LighweightAccessTokenM2 branch from 2075c13 to 1aa310c Compare December 21, 2023 09:25

ghost mentioned this pull request Dec 21, 2023

Flaky test: org.keycloak.testsuite.admin.concurrency.ConcurrencyTest#createGroup #24337

Closed

tnorimat suggested changes Dec 21, 2023

View reviewed changes

skabano force-pushed the LighweightAccessTokenM2 branch from 1aa310c to f12c172 Compare December 22, 2023 01:35

skabano force-pushed the LighweightAccessTokenM2 branch from f12c172 to 6fa74a8 Compare December 22, 2023 05:58

tnorimat approved these changes Dec 22, 2023

View reviewed changes

mposolda previously approved these changes Jan 5, 2024

View reviewed changes

Enhancing Lightweight access token M2(keycloak#25716)

c27d29d

Closes keycloak#23724 Signed-off-by: shigeyuki kabano <[email protected]>

skabano dismissed mposolda’s stale review via c27d29d January 9, 2024 02:08

skabano force-pushed the LighweightAccessTokenM2 branch from 6fa74a8 to c27d29d Compare January 9, 2024 02:08

ghost mentioned this pull request Jan 9, 2024

Flaky test: org.keycloak.testsuite.forms.BrowserFlowTest#testAlternativeNonInteractiveExecutorInSubflow #22644

Closed

mposolda approved these changes Jan 9, 2024

View reviewed changes

mposolda merged commit 67e73d3 into keycloak:main Jan 9, 2024
70 checks passed

skabano deleted the LighweightAccessTokenM2 branch January 9, 2024 08:53

wistefan pushed a commit to wistefan/keycloak that referenced this pull request Jan 17, 2024

Enhancing Lightweight access token M2(keycloak#25716)

7edf8ed

Closes keycloak#23724 Signed-off-by: shigeyuki kabano <[email protected]> Signed-off-by: Stefan Wiedemann <[email protected]>

ShefeeqPM pushed a commit to ShefeeqPM/keycloak that referenced this pull request Jan 27, 2024

Enhancing Lightweight access token M2(keycloak#25716)

cf7f60a

Closes keycloak#23724 Signed-off-by: shigeyuki kabano <[email protected]> Signed-off-by: ShefeeqPM <86718986 [email protected]>

elkman mentioned this pull request May 15, 2024

Upgrade Keycloak to 24.0.4 elkman/keycloak-radius-plugin#102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancing Lightweight access token M2 #25716

Enhancing Lightweight access token M2 #25716

skabano commented Dec 20, 2023 •

edited by mposolda

Loading

mposolda left a comment •

edited

Loading

cypress bot commented Dec 20, 2023 •

edited

Loading

tnorimat commented Dec 21, 2023

tnorimat left a comment

tnorimat Dec 21, 2023

skabano Dec 21, 2023

tnorimat Dec 21, 2023

skabano Dec 21, 2023

tnorimat left a comment

tnorimat Dec 21, 2023

skabano Dec 22, 2023

tnorimat commented Dec 22, 2023

skabano commented Dec 22, 2023

tnorimat left a comment

tnorimat commented Dec 22, 2023

mposolda left a comment

mposolda commented Jan 5, 2024

skabano commented Jan 9, 2024

mposolda left a comment

Enhancing Lightweight access token M2 #25716

Enhancing Lightweight access token M2 #25716

Conversation

skabano commented Dec 20, 2023 • edited by mposolda Loading

mposolda left a comment • edited Loading

Choose a reason for hiding this comment

cypress bot commented Dec 20, 2023 • edited Loading

1 flaky test on run #10453 ↗︎

Review all test suite changes for PR #25716 ↗︎

tnorimat commented Dec 21, 2023

tnorimat left a comment

Choose a reason for hiding this comment

tnorimat Dec 21, 2023

Choose a reason for hiding this comment

skabano Dec 21, 2023

Choose a reason for hiding this comment

tnorimat Dec 21, 2023

Choose a reason for hiding this comment

skabano Dec 21, 2023

Choose a reason for hiding this comment

tnorimat left a comment

Choose a reason for hiding this comment

tnorimat Dec 21, 2023

Choose a reason for hiding this comment

skabano Dec 22, 2023

Choose a reason for hiding this comment

tnorimat commented Dec 22, 2023

skabano commented Dec 22, 2023

tnorimat left a comment

Choose a reason for hiding this comment

tnorimat commented Dec 22, 2023

mposolda left a comment

Choose a reason for hiding this comment

mposolda commented Jan 5, 2024

skabano commented Jan 9, 2024

mposolda left a comment

Choose a reason for hiding this comment

skabano commented Dec 20, 2023 •

edited by mposolda

Loading

mposolda left a comment •

edited

Loading

cypress bot commented Dec 20, 2023 •

edited

Loading