From 9e8d17b3d81b3f9b3ecf60b2471e4068b51a10bf Mon Sep 17 00:00:00 2001 From: Ricardo Martin Date: Wed, 2 Oct 2024 09:44:25 +0200 Subject: [PATCH 1/2] Add the nonce attribute when the client session context is recreated (#33422) Closes #33355 Signed-off-by: rmartinc Co-authored-by: Tomas Kralik (cherry picked from commit 6e471a84773a6952a8cb3d87efcb5d61ac643ecd) --- .../keycloak/protocol/oidc/TokenManager.java | 6 +++- .../NonceBackwardsCompatibleMapperTest.java | 31 ++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index e2df13b1f9c3..914da66abcce 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -1118,8 +1118,12 @@ public AccessTokenResponseBuilder generateRefreshToken(RefreshToken oldRefreshTo String scope = oldRefreshToken.getScope(); Object reuseId = oldRefreshToken.getOtherClaims().get(Constants.REUSE_ID); boolean offlineTokenRequested = Arrays.asList(scope.split(" ")).contains(OAuth2Constants.OFFLINE_ACCESS) ; - if (offlineTokenRequested) + if (offlineTokenRequested) { clientSessionCtx = DefaultClientSessionContext.fromClientSessionAndScopeParameter(clientSession, scope, session); + if (oldRefreshToken.getNonce() != null) { + clientSessionCtx.setAttribute(OIDCLoginProtocol.NONCE_PARAM, oldRefreshToken.getNonce()); + } + } generateRefreshToken(offlineTokenRequested); if (realm.isRevokeRefreshToken()) { refreshToken.getOtherClaims().put(Constants.REUSE_ID, reuseId); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/NonceBackwardsCompatibleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/NonceBackwardsCompatibleMapperTest.java index 1f68b50e5075..0c1504559d06 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/NonceBackwardsCompatibleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/NonceBackwardsCompatibleMapperTest.java @@ -46,6 +46,7 @@ import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.updaters.ClientAttributeUpdater; import org.keycloak.testsuite.util.OAuthClient; +import org.keycloak.util.TokenUtil; /** * @@ -62,7 +63,7 @@ public void configureTestRealm(RealmRepresentation testRealm) { @Test public void testNonceWithoutMapper() throws JsonProcessingException { - testNonce(false); + testNonce(false, false); } @Test @@ -70,7 +71,18 @@ public void testNonceWithMapper() throws JsonProcessingException { ClientResource testApp = ApiUtil.findClientByClientId(testRealm(), "test-app"); String mapperId = createNonceMapper(testApp); try { - testNonce(true); + testNonce(true, false); + } finally { + testApp.getProtocolMappers().delete(mapperId); + } + } + + @Test + public void testOfflineSessionNonceWithMapper() throws JsonProcessingException { + ClientResource testApp = ApiUtil.findClientByClientId(testRealm(), "test-app"); + String mapperId = createNonceMapper(testApp); + try { + testNonce(true, true); } finally { testApp.getProtocolMappers().delete(mapperId); } @@ -142,9 +154,12 @@ private void testNonceImplicit(boolean mapper) throws JsonProcessingException { testIntrospection(idTokenString, nonce, true); } - private void testNonce(boolean mapper) throws JsonProcessingException { + private void testNonce(boolean mapper, boolean offlineSession) throws JsonProcessingException { String nonce = KeycloakModelUtils.generateId(); oauth.nonce(nonce); + if (offlineSession) { + oauth.scope(OAuth2Constants.OFFLINE_ACCESS); + } oauth.doLogin("test-user@localhost", "password"); EventRepresentation loginEvent = events.expectLogin().assertEvent(); @@ -158,12 +173,14 @@ private void testNonce(boolean mapper) throws JsonProcessingException { RefreshToken refreshToken = oauth.parseRefreshToken(response.getRefreshToken()); checkNonce(nonce, refreshToken.getNonce(), mapper); - EventRepresentation tokenEvent = events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), - loginEvent.getSessionId()).assertEvent(); + EventRepresentation tokenEvent = events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), loginEvent.getSessionId()) + .detail(Details.REFRESH_TOKEN_TYPE, offlineSession? TokenUtil.TOKEN_TYPE_OFFLINE : TokenUtil.TOKEN_TYPE_REFRESH) + .assertEvent(); response = oauth.doRefreshTokenRequest(response.getRefreshToken(), "password"); - events.expectRefresh(tokenEvent.getDetails().get(Details.REFRESH_TOKEN_ID), - loginEvent.getSessionId()).assertEvent(); + events.expectRefresh(tokenEvent.getDetails().get(Details.REFRESH_TOKEN_ID), loginEvent.getSessionId()) + .detail(Details.REFRESH_TOKEN_TYPE, offlineSession? TokenUtil.TOKEN_TYPE_OFFLINE : TokenUtil.TOKEN_TYPE_REFRESH) + .assertEvent(); token = oauth.verifyToken(response.getAccessToken()); checkNonce(nonce, token.getNonce(), mapper); From 927f77f688b92a17dc990a4eebae49d7deea0907 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Thu, 19 Sep 2024 16:23:21 -0300 Subject: [PATCH 2/2] Fix various issues holding up CI (#33086) - Disables the remote operator tests, which will have to be fixed later. - Fixes the action expired error which occurs when accessing regular registration page with Organizations enabled. - Fixes a race condition in the test suite causing sporadic failures. Closes #33064 Signed-off-by: Stefan Guilhen Signed-off-by: rmartinc Signed-off-by: Jon Koops Co-authored-by: rmartinc Co-authored-by: Jon Koops --- .../testsuite/util/ArtifactResolutionService.java | 2 +- .../ArtifactBindingWithResolutionServiceTest.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ArtifactResolutionService.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ArtifactResolutionService.java index 8b167533ba2d..408bfec50e7b 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ArtifactResolutionService.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ArtifactResolutionService.java @@ -43,7 +43,7 @@ public class ArtifactResolutionService implements Provider, Runnable { private ArtifactResponseType artifactResponseType; private final String endpointAddress; private ArtifactResolveType lastArtifactResolve; - private boolean running = true; + private volatile boolean running = true; /** * Standard constructor diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingWithResolutionServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingWithResolutionServiceTest.java index 1ef887349f8e..59f0089c51f7 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingWithResolutionServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingWithResolutionServiceTest.java @@ -55,8 +55,8 @@ public void testReceiveArtifactLoginFullWithPost() throws ParsingException, Conf ArtifactResolutionService ars = new ArtifactResolutionService("http://127.0.0.1:8082/").setResponseDocument(doc); Thread arsThread = new Thread(ars); try { - arsThread.start(); synchronized (ars) { + arsThread.start(); ars.wait(); SAMLDocumentHolder response = builder.artifactMessage(camb).build().login().user(bburkeUser).build().getSamlResponse(SamlClient.Binding.POST); assertThat(response.getSamlObject(), instanceOf(ResponseType.class)); @@ -90,8 +90,8 @@ public void testReceiveArtifactLoginFullWithRedirect() throws ParsingException, ArtifactResolutionService ars = new ArtifactResolutionService("http://127.0.0.1:8082/").setResponseDocument(doc); Thread arsThread = new Thread(ars); try { - arsThread.start(); synchronized (ars) { + arsThread.start(); ars.wait(); SAMLDocumentHolder response = builder.artifactMessage(camb).build().login().user(bburkeUser).build().getSamlResponse(REDIRECT); assertThat(response.getSamlObject(), instanceOf(ResponseType.class)); @@ -124,8 +124,8 @@ public void testReceiveArtifactNonExistingClient() throws ParsingException, Conf ArtifactResolutionService ars = new ArtifactResolutionService("http://127.0.0.1:8082/").setResponseDocument(doc); Thread arsThread = new Thread(ars); try { - arsThread.start(); synchronized (ars) { + arsThread.start(); ars.wait(); String response = builder.artifactMessage(camb).build().executeAndTransform(resp -> EntityUtils.toString(resp.getEntity())); assertThat(response, containsString("Invalid Request")); @@ -151,8 +151,8 @@ public void testReceiveEmptyArtifactResponse() throws InterruptedException { ArtifactResolutionService ars = new ArtifactResolutionService("http://127.0.0.1:8082/").setEmptyArtifactResponse(SAML_CLIENT_ID_SALES_POST); Thread arsThread = new Thread(ars); try { - arsThread.start(); synchronized (ars) { + arsThread.start(); ars.wait(); builder.artifactMessage(camb).build().execute(r -> { assertThat(r, statusCodeIsHC(400)); @@ -180,8 +180,8 @@ public void testReceiveArtifactLogoutFullWithPost() throws InterruptedException ArtifactResolutionService ars = new ArtifactResolutionService("http://127.0.0.1:8082/"); Thread arsThread = new Thread(ars); try { - arsThread.start(); synchronized (ars) { + arsThread.start(); ars.wait(); SAMLDocumentHolder samlResponse = builder.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST).build() .login().user(bburkeUser).build() @@ -220,8 +220,8 @@ public void testReceiveArtifactLogoutFullWithRedirect() throws InterruptedExcept ArtifactResolutionService ars = new ArtifactResolutionService("http://127.0.0.1:8082/"); Thread arsThread = new Thread(ars); try { - arsThread.start(); synchronized (ars) { + arsThread.start(); ars.wait(); SAMLDocumentHolder samlResponse = builder .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST,