From 1987c942f527b9f3bbf2a86ba71ba8ae0154ac37 Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Mon, 12 Dec 2022 12:46:59 +0100 Subject: [PATCH 1/3] Avoid path traversal vis double-url encoding of redirect URI (#8) (cherry picked from commit a2128fb9e940d96c2f9a64edcd4fbcc768eedb4f) --- .../protocol/oidc/utils/RedirectUtils.java | 39 +++++++++++++++++++ .../testsuite/oauth/OAuthRedirectUriTest.java | 17 +++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java index 01d6cc4c0d5e..9cb314b2e483 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java @@ -18,6 +18,8 @@ package org.keycloak.protocol.oidc.utils; import org.jboss.logging.Logger; +import org.keycloak.common.util.Encode; +import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.common.util.UriUtils; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; @@ -91,6 +93,7 @@ public static String verifyRedirectUri(KeycloakSession session, String rootUrl, KeycloakUriInfo uriInfo = session.getContext().getUri(); RealmModel realm = session.getContext().getRealm(); + redirectUri = decodeRedirectUri(redirectUri); if (redirectUri != null) { try { URI uri = URI.create(redirectUri); @@ -152,6 +155,42 @@ public static String verifyRedirectUri(KeycloakSession session, String rootUrl, } } + // Decode redirectUri. We don't decode query and fragment as those can be encoded in the original URL. + // URL can be decoded multiple times (in case it was encoded multiple times, or some of it's parts were encoded multiple times) + private static String decodeRedirectUri(String redirectUri) { + if (redirectUri == null) return null; + int MAX_DECODING_COUNT = 5; // Max count of attempts for decoding URL (in case it was encoded multiple times) + + try { + KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri); + String origQuery = uriBuilder.getQuery(); + String origFragment = uriBuilder.getFragment(); + String encodedRedirectUri = uriBuilder + .replaceQuery(null) + .fragment(null) + .buildAsString(); + String decodedRedirectUri = null; + + for (int i = 0; i < MAX_DECODING_COUNT; i++) { + decodedRedirectUri = Encode.decode(encodedRedirectUri); + if (decodedRedirectUri.equals(encodedRedirectUri)) { + // URL is decoded. We can return it (after attach original query and fragment) + return KeycloakUriBuilder.fromUri(decodedRedirectUri) + .replaceQuery(origQuery) + .fragment(origFragment) + .buildAsString(); + } else { + // Next attempt + encodedRedirectUri = decodedRedirectUri; + } + } + } catch (IllegalArgumentException iae) { + logger.debugf("Illegal redirect URI used: %s, Details: %s", redirectUri, iae.getMessage()); + } + logger.debugf("Was not able to decode redirect URI: %s", redirectUri); + return null; + } + private static String lowerCaseHostname(String redirectUri) { int n = redirectUri.indexOf('/', 7); if (n == -1) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java index 93022679d9a1..2959c4977b84 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java @@ -337,6 +337,20 @@ public void testWildcard() throws IOException { checkRedirectUri("http://localhost:8280/foo/bar", true, true); checkRedirectUri("http://example.com/foobar", false); checkRedirectUri("http://localhost:8280/foobar", false, true); + + checkRedirectUri("http://example.com/foo/../", false); + checkRedirectUri("http://example.com/foo/%2E%2E/", false); // url-encoded "http://example.com/foobar/../" + checkRedirectUri("http://example.com/foo%2F%2E%2E%2F", false); // url-encoded "http://example.com/foobar/../" + checkRedirectUri("http://example.com/foo/%252E%252E/", false); // double-encoded "http://example.com/foobar/../" + checkRedirectUri("http://example.com/foo/%252E%252E/?some_query_param=some_value", false); // double-encoded "http://example.com/foobar/../?some_query_param=some_value" + checkRedirectUri("http://example.com/foo/%252E%252E/?encodeTest=a%3Cb", false); // double-encoded "http://example.com/foobar/../?encodeTest=a Date: Mon, 12 Dec 2022 04:07:07 -0800 Subject: [PATCH 2/3] Do not resolve user session if corresponding auth session does not exist (#7) --- .../oidc/endpoints/LogoutEndpoint.java | 8 +- .../AuthenticationSessionManager.java | 14 +- .../managers/UserSessionCrossDCManager.java | 14 ++ .../services/resources/SessionCodeChecks.java | 14 ++ .../oauth/RPInitiatedLogoutTest.java | 12 +- .../testsuite/oauth/RefreshTokenTest.java | 123 ++++++++++++++++++ 6 files changed, 177 insertions(+), 8 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java index 14a2941c5b4f..42582f35debc 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java @@ -370,6 +370,8 @@ public Response logoutConfirmAction() { session.getProvider(LoginFormsProvider.class).setAttribute(Constants.SKIP_LINK, true); } + event.error(Errors.SESSION_EXPIRED); + return ErrorPage.error(session, logoutSession, Response.Status.BAD_REQUEST, Messages.FAILED_LOGOUT); } @@ -405,6 +407,7 @@ public Response logoutConfirmGet() { AuthenticationManager.AuthResult authResult = AuthenticationManager.authenticateIdentityCookie(session, realm, false); if (authResult != null) { + event.error(Errors.LOGOUT_FAILED); return ErrorPage.error(session, logoutSession, Response.Status.BAD_REQUEST, Messages.FAILED_LOGOUT); } else { // Probably changing locale on logout screen after logout was already performed. If there is no session in the browser, we can just display that logout was already finished @@ -431,7 +434,10 @@ private Response doBrowserLogout(AuthenticationSessionModel logoutSession) { try { userSession = lockUserSessionsForModification(session, () -> session.sessions().getUserSession(realm, userSessionIdFromIdToken)); - if (userSession != null) { + if (userSession == null) { + event.event(EventType.LOGOUT); + event.error(Errors.SESSION_EXPIRED); + } else { Integer idTokenIssuedAt = Integer.parseInt(idTokenIssuedAtStr); checkTokenIssuedAt(idTokenIssuedAt, userSession); } diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java index fc07aaf14ff3..f815259185e9 100644 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java @@ -34,6 +34,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Collectors; import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification; @@ -196,7 +197,18 @@ List getAuthSessionCookies(RealmModel realm) { log.debugf("Not found AUTH_SESSION_ID cookie"); } - return authSessionIds; + return authSessionIds.stream().filter(new Predicate() { + @Override + public boolean test(String id) { + StickySessionEncoderProvider encoder = session.getProvider(StickySessionEncoderProvider.class); + // in case the id is encoded with a route when running in a cluster + String decodedId = encoder.decodeSessionId(cookiesVal.iterator().next()); + // we can't blindly trust the cookie and assume it is valid and referencing a valid root auth session + // but make sure the root authentication session actually exists + // without this check there is a risk of resolving user sessions from invalid root authentication sessions as they share the same id + return session.authenticationSessions().getRootAuthenticationSession(realm, decodedId) != null; + } + }).collect(Collectors.toList()); } diff --git a/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java b/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java index 9e3eda42be2a..ceca28908256 100644 --- a/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java +++ b/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java @@ -17,6 +17,7 @@ package org.keycloak.services.managers; +import java.util.Collections; import java.util.List; import java.util.Objects; @@ -26,6 +27,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; +import static org.keycloak.services.managers.AuthenticationManager.authenticateIdentityCookie; import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification; /** @@ -65,6 +67,18 @@ public UserSessionModel getUserSessionWithClient(RealmModel realm, String id, St public UserSessionModel getUserSessionIfExistsRemotely(AuthenticationSessionManager asm, RealmModel realm) { List sessionCookies = asm.getAuthSessionCookies(realm); + if (sessionCookies.isEmpty()) { + // ideally, we should not rely on auth session id to retrieve user sessions + // in case the auth session was removed, we fall back to the identity cookie + // we are here doing the user session lookup twice, however the second lookup is going to make sure the + // session exists in remote caches + AuthenticationManager.AuthResult authResult = authenticateIdentityCookie(kcSession, realm, true); + + if (authResult != null && authResult.getSession() != null) { + sessionCookies = Collections.singletonList(authResult.getSession().getId()); + } + } + return sessionCookies.stream().map(oldEncodedId -> { AuthSessionId authSessionId = asm.decodeAuthSessionId(oldEncodedId); String sessionId = authSessionId.getDecodedId(); diff --git a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java index e7e6c82c7324..55ab61da4782 100644 --- a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java +++ b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java @@ -17,6 +17,9 @@ package org.keycloak.services.resources; +import static org.keycloak.services.managers.AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE; +import static org.keycloak.services.managers.AuthenticationManager.authenticateIdentityCookie; + import java.net.URI; import javax.ws.rs.core.Cookie; @@ -42,11 +45,13 @@ import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.services.ErrorPage; import org.keycloak.services.ServicesLogger; +import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.messages.Messages; import org.keycloak.services.util.BrowserHistoryHelper; import org.keycloak.services.util.AuthenticationFlowURLHelper; +import org.keycloak.services.util.CookieHelper; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; @@ -178,6 +183,15 @@ public AuthenticationSessionModel initialVerifyAuthSession() { // See if we are already authenticated and userSession with same ID exists. UserSessionModel userSession = authSessionManager.getUserSessionFromAuthCookie(realm); + if (userSession == null) { + // fallback to check if there is an identity cookie + AuthenticationManager.AuthResult authResult = authenticateIdentityCookie(session, realm, false); + + if (authResult != null) { + userSession = authResult.getSession(); + } + } + if (userSession != null) { LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class).setAuthenticationSession(authSession) .setSuccess(Messages.ALREADY_LOGGED_IN); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java index a2f2112e79dc..ac93e73a388a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java @@ -238,7 +238,7 @@ public void logoutRedirectWithIdTokenHintPointToDifferentSession() { driver.navigate().to(logoutUrl); logoutConfirmPage.assertCurrent(); logoutConfirmPage.confirmLogout(); - events.expectLogout(sessionId2).detail(Details.REDIRECT_URI, redirectUri).assertEvent(); + events.expectLogoutError(Errors.SESSION_EXPIRED); MatcherAssert.assertThat(false, is(isSessionActive(sessionId2))); assertCurrentUrlEquals(redirectUri + "&state=something"); } @@ -261,7 +261,7 @@ public void logoutWithExpiredSession() throws Exception { // should not throw an internal server error. But no logout event is sent as nothing was logged-out appPage.assertCurrent(); - events.assertEmpty(); + events.expectLogoutError(Errors.SESSION_EXPIRED); MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); // check if the back channel logout succeeded @@ -318,7 +318,7 @@ public void logoutSessionWhenLoggedOutByAdmin() { // Try logout even if user already logged-out by admin. Should redirect back to the application, but no logout-event should be triggered String logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(APP_REDIRECT_URI).idTokenHint(idTokenString).build(); driver.navigate().to(logoutUrl); - events.assertEmpty(); + events.expectLogoutError(Errors.SESSION_EXPIRED); assertCurrentUrlEquals(APP_REDIRECT_URI); // Login again in the browser. Ensure to use newest idTokenHint after logout @@ -378,7 +378,7 @@ public void logoutWithExpiredIdToken() throws Exception { assertThat(response, Matchers.statusCodeIsHC(Response.Status.FOUND)); assertThat(response.getFirstHeader(HttpHeaders.LOCATION).getValue(), is(APP_REDIRECT_URI)); } - events.assertEmpty(); + events.expectLogoutError(Errors.SESSION_EXPIRED); MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); } @@ -401,7 +401,7 @@ public void logoutWithValidIdTokenWhenLoggedOutByAdmin() throws Exception { assertThat(response, Matchers.statusCodeIsHC(Response.Status.FOUND)); assertThat(response.getFirstHeader(HttpHeaders.LOCATION).getValue(), is(APP_REDIRECT_URI)); } - events.assertEmpty(); + events.expectLogoutError(Errors.SESSION_EXPIRED); MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); } @@ -932,7 +932,7 @@ public void testIncorrectChangingParameters() throws IOException { errorPage.assertCurrent(); Assert.assertEquals("Logout failed", errorPage.getError()); - events.expectLogoutError(Errors.SESSION_EXPIRED).assertEvent(); + events.expectLogoutError(Errors.LOGOUT_FAILED).assertEvent(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java index d91537d78d53..3427fef83346 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java @@ -17,7 +17,9 @@ package org.keycloak.testsuite.oauth; import com.fasterxml.jackson.databind.JsonNode; +import com.gargoylesoftware.htmlunit.WebClient; import org.hamcrest.CoreMatchers; +import org.jboss.arquillian.drone.webdriver.htmlunit.DroneHtmlUnitDriver; import org.jboss.arquillian.graphene.page.Page; import org.junit.Assert; import org.junit.Before; @@ -27,6 +29,7 @@ import org.keycloak.OAuthErrorException; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.admin.client.resource.RealmsResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.enums.SslRequired; import org.keycloak.crypto.Algorithm; @@ -36,6 +39,7 @@ import org.keycloak.jose.jws.JWSInput; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.SessionTimeoutHelper; import org.keycloak.protocol.oidc.OIDCConfigAttributes; import org.keycloak.protocol.oidc.OIDCLoginProtocol; @@ -47,6 +51,7 @@ import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserSessionRepresentation; +import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.admin.ApiUtil; @@ -59,11 +64,13 @@ import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.RealmManager; import org.keycloak.testsuite.util.TokenSignatureUtil; +import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.UserInfoClientUtil; import org.keycloak.testsuite.util.UserManager; import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.util.BasicAuthHelper; import org.keycloak.util.JsonSerialization; +import org.openqa.selenium.Cookie; import javax.ws.rs.client.Client; import javax.ws.rs.client.Entity; @@ -74,6 +81,7 @@ import javax.ws.rs.core.UriBuilder; import java.net.URI; import java.util.List; +import java.util.concurrent.TimeUnit; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.greaterThan; @@ -83,6 +91,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -283,6 +292,120 @@ public void refreshTokenWithAccessToken() throws Exception { Assert.assertNotEquals(200, response.getStatusCode()); } + @Test + public void testDoNotResolveOfflineUserSessionIfAuthenticationSessionIsInvalidated() { + oauth.scope("offline_access"); + testDoNotResolveUserSessionIfAuthenticationSessionIsInvalidated(); + } + + @Test + public void testDoNotResolveUserSessionIfAuthenticationSessionIsInvalidated() { + String realmName = KeycloakModelUtils.generateId(); + RealmsResource realmsResource = realmsResouce(); + realmsResource.create(RealmBuilder.create().name(realmName).build()); + RealmResource realmResource = realmsResource.realm(realmName); + RealmRepresentation realm = realmResource.toRepresentation(); + + try { + realm.setSsoSessionMaxLifespan((int) TimeUnit.MINUTES.toSeconds(2)); + realm.setSsoSessionIdleTimeout((int) TimeUnit.MINUTES.toSeconds(2)); + realm.setAccessTokenLifespan((int) TimeUnit.MINUTES.toSeconds(1)); + realmResource.update(realm); + + realmResource.clients().create(org.keycloak.testsuite.util.ClientBuilder.create() + .clientId("public-client") + .redirectUris("*") + .publicClient() + .build()); + + realmResource.users() + .create(UserBuilder.create().username("alice").password("alice").addRoles("offline_access").build()); + realmResource.users() + .create(UserBuilder.create().username("bob").password("bob").addRoles("offline_access").build()); + + oauth.realm(realmName); + oauth.clientId("public-client"); + + oauth.doLogin("alice", "alice"); + String aliceCode = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(aliceCode, "password"); + AccessToken aliceAt = oauth.verifyToken(tokenResponse.getAccessToken()); + + setTimeOffset((int) TimeUnit.MINUTES.toSeconds(2)); + + oauth.doLogin("bob", "bob"); + String bobCode = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + + assertNotEquals(aliceCode, bobCode); + + tokenResponse = oauth.doAccessTokenRequest(bobCode, "password"); + String refreshToken = tokenResponse.getRefreshToken(); + tokenResponse = oauth.doRefreshTokenRequest(refreshToken, null); + AccessToken bobAt = oauth.verifyToken(tokenResponse.getAccessToken()); + + assertNotEquals(aliceAt.getSessionId(), bobAt.getSessionId()); + assertEquals("bob", bobAt.getPreferredUsername()); + } finally { + setTimeOffset(0); + + realm.setSsoSessionMaxLifespan(null); + realm.setSsoSessionIdleTimeout(null); + realm.setAccessTokenLifespan(null); + realmResource.update(realm); + } + } + + @Test + public void testTimeoutWhenReUsingPreviousAuthenticationSession() { + String realmName = KeycloakModelUtils.generateId(); + RealmsResource realmsResource = realmsResouce(); + realmsResource.create(RealmBuilder.create().name(realmName).build()); + RealmResource realmResource = realmsResource.realm(realmName); + RealmRepresentation realm = realmResource.toRepresentation(); + + try { + realm.setSsoSessionMaxLifespan((int) TimeUnit.MINUTES.toSeconds(2)); + realm.setSsoSessionIdleTimeout((int) TimeUnit.MINUTES.toSeconds(2)); + realm.setAccessTokenLifespan((int) TimeUnit.MINUTES.toSeconds(1)); + realmResource.update(realm); + + realmResource.clients().create(org.keycloak.testsuite.util.ClientBuilder.create() + .clientId("public-client") + .redirectUris("*") + .publicClient() + .build()); + + realmResource.users() + .create(UserBuilder.create().username("alice").password("alice").addRoles("offline_access").build()); + realmResource.users() + .create(UserBuilder.create().username("bob").password("bob").addRoles("offline_access").build()); + + oauth.realm(realmName); + oauth.clientId("public-client"); + + oauth.openLoginForm(); + + Cookie authSessionCookie = driver.manage().getCookieNamed(AuthenticationSessionManager.AUTH_SESSION_ID); + + oauth.fillLoginForm("alice", "alice"); + + String aliceCode = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + WebClient webClient = DroneHtmlUnitDriver.class.cast(driver).getWebClient(); + webClient.getCookieManager().clearCookies(); + oauth.openLoginForm(); + driver.manage().addCookie(authSessionCookie); + oauth.fillLoginForm("bob", "bob"); + assertEquals("Your login attempt timed out. Login will start from the beginning.", loginPage.getError()); + } finally { + setTimeOffset(0); + + realm.setSsoSessionMaxLifespan(null); + realm.setSsoSessionIdleTimeout(null); + realm.setAccessTokenLifespan(null); + realmResource.update(realm); + } + } + /** * KEYCLOAK-15437 */ From 560620e121f98d9a4254c6a7effd5930febc361e Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Tue, 13 Dec 2022 10:32:50 +0100 Subject: [PATCH 3/3] Stabilizing the ConcurrentLoginTest when running with JPA map storage by locking user sessions (#9) --- .../keycloak/services/managers/UserSessionCrossDCManager.java | 2 +- .../org/keycloak/services/resources/SessionCodeChecks.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java b/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java index ceca28908256..6d6cacbdda6c 100644 --- a/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java +++ b/services/src/main/java/org/keycloak/services/managers/UserSessionCrossDCManager.java @@ -72,7 +72,7 @@ public UserSessionModel getUserSessionIfExistsRemotely(AuthenticationSessionMana // in case the auth session was removed, we fall back to the identity cookie // we are here doing the user session lookup twice, however the second lookup is going to make sure the // session exists in remote caches - AuthenticationManager.AuthResult authResult = authenticateIdentityCookie(kcSession, realm, true); + AuthenticationManager.AuthResult authResult = lockUserSessionsForModification(kcSession, () -> authenticateIdentityCookie(kcSession, realm, true)); if (authResult != null && authResult.getSession() != null) { sessionCookies = Collections.singletonList(authResult.getSession().getId()); diff --git a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java index 55ab61da4782..999de87625af 100644 --- a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java +++ b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java @@ -19,6 +19,7 @@ import static org.keycloak.services.managers.AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE; import static org.keycloak.services.managers.AuthenticationManager.authenticateIdentityCookie; +import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification; import java.net.URI; @@ -185,7 +186,7 @@ public AuthenticationSessionModel initialVerifyAuthSession() { if (userSession == null) { // fallback to check if there is an identity cookie - AuthenticationManager.AuthResult authResult = authenticateIdentityCookie(session, realm, false); + AuthenticationManager.AuthResult authResult = lockUserSessionsForModification(session, () -> authenticateIdentityCookie(session, realm, false)); if (authResult != null) { userSession = authResult.getSession();