From 5344aada5ee06b02ec3a9e0f52fa381d085b6282 Mon Sep 17 00:00:00 2001 From: Giuseppe Graziano Date: Mon, 23 Sep 2024 15:28:40 +0200 Subject: [PATCH] Remove root auth session after backchannel logout Closes #32197 Signed-off-by: Giuseppe Graziano (cherry picked from commit b46fab230824a2304daafe74be019e8bd4ee590a) --- .../managers/AuthenticationManager.java | 3 +- .../testsuite/util/BrowserTabUtil.java | 6 ++- .../testsuite/AbstractKeycloakTest.java | 2 + .../testsuite/forms/ReAuthenticationTest.java | 37 +++++++++++++++++++ .../keycloak/testsuite/oauth/LogoutTest.java | 23 ++---------- .../testsuite/oauth/RefreshTokenTest.java | 12 +++--- .../oauth/TokenIntrospectionTest.java | 4 +- .../keycloak/testsuite/oidc/UserInfoTest.java | 18 ++++----- 8 files changed, 65 insertions(+), 40 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index c8abe2a0708..f2224782084 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -305,8 +305,7 @@ public class AuthenticationManager { checkUserSessionOnlyHasLoggedOutClients(realm, userSession, logoutAuthSession); } finally { logger.tracef("Removing logout session '%s' after backchannel logout", logoutAuthSession.getParentSession().getId()); - RootAuthenticationSessionModel rootAuthSession = logoutAuthSession.getParentSession(); - rootAuthSession.removeAuthenticationSessionByTabId(logoutAuthSession.getTabId()); + session.authenticationSessions().removeRootAuthenticationSession(realm, logoutAuthSession.getParentSession()); } userSession.setState(UserSessionModel.State.LOGGED_OUT); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/BrowserTabUtil.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/BrowserTabUtil.java index 7fc4144c2fa..7d6d0c7bbbd 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/BrowserTabUtil.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/BrowserTabUtil.java @@ -79,6 +79,10 @@ public class BrowserTabUtil implements AutoCloseable { return instance; } + public static void cleanup() { + instances = new ArrayList<>(); + } + public WebDriver getDriver() { return driver; } @@ -155,4 +159,4 @@ public class BrowserTabUtil implements AutoCloseable { public void close() { destroy(); } -} \ No newline at end of file +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java index fc2bbe463c9..3e1ecdd0ec3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java @@ -56,6 +56,7 @@ import org.keycloak.testsuite.auth.page.login.OIDCLogin; import org.keycloak.testsuite.auth.page.login.UpdatePassword; import org.keycloak.testsuite.client.KeycloakTestingClient; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; +import org.keycloak.testsuite.util.BrowserTabUtil; import org.keycloak.testsuite.util.CryptoInitRule; import org.keycloak.testsuite.util.DroneUtils; import org.keycloak.testsuite.util.OAuthClient; @@ -251,6 +252,7 @@ public abstract class AbstractKeycloakTest { // Remove all browsers from queue DroneUtils.resetQueue(); + BrowserTabUtil.cleanup(); } protected TestCleanup getCleanup(String realmName) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ReAuthenticationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ReAuthenticationTest.java index 9b1a36c8538..c387a00b723 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ReAuthenticationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ReAuthenticationTest.java @@ -49,6 +49,7 @@ import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginTotpPage; import org.keycloak.testsuite.pages.LoginUsernameOnlyPage; import org.keycloak.testsuite.pages.PasswordPage; +import org.keycloak.testsuite.util.BrowserTabUtil; import org.keycloak.testsuite.util.FederatedIdentityBuilder; import org.keycloak.testsuite.util.FlowUtil; import org.keycloak.testsuite.util.OAuthClient; @@ -366,6 +367,42 @@ public class ReAuthenticationTest extends AbstractTestRealmKeycloakTest { realmsResouce().realm(rep.getRealm()).update(rep); } + @Test + public void loginAfterLogoutWithDifferentSessionId() { + BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver); + + assertThat(tabUtil.getCountOfTabs(), Matchers.is(1)); + oauth.openLoginForm(); + loginPage.assertCurrent(); + + tabUtil.newTab(oauth.getLoginFormUrl()); + assertThat(tabUtil.getCountOfTabs(), Matchers.equalTo(2)); + oauth.openLoginForm(); + + tabUtil.closeTab(tabUtil.getCountOfTabs() - 1); + assertThat(tabUtil.getCountOfTabs(), Matchers.equalTo(1)); + + tabUtil.switchToTab(0); + loginPage.assertCurrent(); + + loginPage.login("test-user@localhost", "password"); + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response1 = oauth.doAccessTokenRequest(code, "password"); + AccessToken accessToken1 = oauth.verifyToken(response1.getAccessToken()); + + oauth.doLogout(response1.getRefreshToken(), "password"); + + oauth.openLoginForm(); + loginPage.assertCurrent(); + loginPage.login("test-user@localhost", "password"); + code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response2 = oauth.doAccessTokenRequest(code, "password"); + AccessToken accessToken2 = oauth.verifyToken(response2.getAccessToken()); + + Assert.assertNotEquals(accessToken1.getId(), accessToken2.getId()); + Assert.assertNotEquals(accessToken1.getSessionId(), accessToken2.getSessionId()); + } + private void setupIdentityFirstFlow() { String newFlowAlias = "browser - identity first"; testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java index 46928a6971b..a49c5120de6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java @@ -146,30 +146,14 @@ public class LogoutTest extends AbstractKeycloakTest { setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); Assert.assertFalse(loginPage.isCurrent()); String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); OAuthClient.AccessTokenResponse tokenResponse2 = oauth.doAccessTokenRequest(code, "password"); - // POST logout with token should fail - try (CloseableHttpResponse response = oauth.doLogout(refreshToken1, "password")) { - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatusLine().getStatusCode()); - } - - String logoutUrl = oauth.getLogoutUrl() - .idTokenHint(accessTokenResponse.getIdToken()) - .postLogoutRedirectUri(oauth.APP_AUTH_ROOT) - .build(); - - // GET logout with ID token should fail as well - try (CloseableHttpClient c = HttpClientBuilder.create().disableRedirectHandling().build(); - CloseableHttpResponse response = c.execute(new HttpGet(logoutUrl))) { - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatusLine().getStatusCode()); - } - // finally POST logout with VALID token should succeed try (CloseableHttpResponse response = oauth.doLogout(tokenResponse2.getRefreshToken(), "password")) { MatcherAssert.assertThat(response, Matchers.statusCodeIsHC(Status.NO_CONTENT)); @@ -178,7 +162,6 @@ public class LogoutTest extends AbstractKeycloakTest { } } - @Test public void postLogoutFailWithCredentialsOfDifferentClient() throws Exception { oauth.doLogin("test-user@localhost", "password"); @@ -247,7 +230,7 @@ public class LogoutTest extends AbstractKeycloakTest { .idTokenHint(idTokenString) .postLogoutRedirectUri(oauth.APP_AUTH_ROOT) .build(); - + try (CloseableHttpClient c = HttpClientBuilder.create().disableRedirectHandling().build(); CloseableHttpResponse response = c.execute(new HttpGet(logoutUrl))) { MatcherAssert.assertThat(response, Matchers.statusCodeIsHC(Status.FOUND)); 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 e9aeecee03c..f828cf1724e 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 @@ -942,8 +942,8 @@ public class RefreshTokenTest extends AbstractKeycloakTest { try { // Continue with login setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); assertFalse(loginPage.isCurrent()); @@ -976,8 +976,8 @@ public class RefreshTokenTest extends AbstractKeycloakTest { try { // Continue with login setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); assertFalse(loginPage.isCurrent()); @@ -1009,8 +1009,8 @@ public class RefreshTokenTest extends AbstractKeycloakTest { // Continue with login setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); assertFalse(loginPage.isCurrent()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenIntrospectionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenIntrospectionTest.java index 69bd6d930eb..eba63167589 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenIntrospectionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenIntrospectionTest.java @@ -244,8 +244,8 @@ public class TokenIntrospectionTest extends AbstractTestRealmKeycloakTest { setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); events.expectLogin().assertEvent(); Assert.assertFalse(loginPage.isCurrent()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java index e143cd8d361..0022020c0c7 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java @@ -207,7 +207,7 @@ public class UserInfoTest extends AbstractKeycloakTest { client.close(); } } - + @Test public void testSuccess_postMethod_charset_body() throws Exception { Client client = AdminClientUtil.createResteasyClient(); @@ -531,7 +531,7 @@ public class UserInfoTest extends AbstractKeycloakTest { public void testSuccessSignedResponseRS256AcceptJWT() throws Exception { testSuccessSignedResponse(Algorithm.RS256, MediaType.APPLICATION_JWT); } - + @Test public void testSessionExpired() { Client client = AdminClientUtil.createResteasyClient(); @@ -607,8 +607,8 @@ public class UserInfoTest extends AbstractKeycloakTest { setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); events.expectLogin().assertEvent(); Assert.assertFalse(loginPage.isCurrent()); @@ -630,7 +630,7 @@ public class UserInfoTest extends AbstractKeycloakTest { response.close(); events.expect(EventType.USER_INFO_REQUEST_ERROR) - .error(Errors.INVALID_TOKEN) + .error(Errors.USER_SESSION_NOT_FOUND) .user(Matchers.nullValue(String.class)) .session(Matchers.nullValue(String.class)) .detail(Details.AUTH_METHOD, Details.VALIDATE_ACCESS_TOKEN) @@ -1088,23 +1088,23 @@ public class UserInfoTest extends AbstractKeycloakTest { assertNull(userInfo.getOtherClaims().get("realm_access")); assertNull(userInfo.getOtherClaims().get("resource_access")); } - + @Test public void test_noContentType() throws Exception { Client client = AdminClientUtil.createResteasyClient(); try { AccessTokenResponse accessTokenResponse = executeGrantAccessTokenRequest(client); - + WebTarget userInfoTarget = UserInfoClientUtil.getUserInfoWebTarget(client); Response response = userInfoTarget.request() .header(HttpHeaders.AUTHORIZATION, "bearer " + accessTokenResponse.getToken()) .build("POST") .invoke(); - + Assert.assertEquals(200, response.getStatus()); Assert.assertEquals("OK", response.getStatusInfo().toString()); - + } finally { client.close(); }