From d0f8fedba3901994d4914b6721e4ab22755056ce 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 acba5312c3b5..679c9a48c8e8 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -307,8 +307,7 @@ public static BackchannelLogoutResponse backchannelLogout(KeycloakSession sessio 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 a949b952d3a3..5f91394bb5e9 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 static BrowserTabUtil getInstanceAndSetEnv(WebDriver driver) { return instance; } + public static void cleanup() { + instances = new ArrayList<>(); + } + public WebDriver getDriver() { return driver; } @@ -155,4 +159,4 @@ private void assertValidIndex(int index) { 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 fc2bbe463c99..3e1ecdd0ec3c 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.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 void afterAbstractKeycloakTest() throws Exception { // 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 9b1a36c8538a..c387a00b7237 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.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 void loginAfterExpiredUserSession() { 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 fd53ec562cbe..121ca3029667 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 void postLogoutWithRefreshTokenAfterUserSessionLogoutAndLoginAgain() thro 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 void postLogoutWithRefreshTokenAfterUserSessionLogoutAndLoginAgain() thro } } - @Test public void postLogoutFailWithCredentialsOfDifferentClient() throws Exception { oauth.doLogin("test-user@localhost", "password"); @@ -247,7 +230,7 @@ private void backchannelLogoutRequest(String expectedRefreshAlg, String expected .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 362589a03018..009dbdbf435b 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 @@ -1067,8 +1067,8 @@ public void refreshTokenAfterUserLogoutAndLoginAgain() { try { // Continue with login setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); assertFalse(loginPage.isCurrent()); @@ -1101,8 +1101,8 @@ public void refreshTokenAfterAdminLogoutAllAndLoginAgain() { try { // Continue with login setTimeOffset(2); - WaitUtils.waitForPageToLoad(); - loginPage.login("password"); + driver.navigate().refresh(); + oauth.fillLoginForm("test-user@localhost", "password"); assertFalse(loginPage.isCurrent()); @@ -1134,8 +1134,8 @@ public void refreshTokenAfterUserAdminLogoutEndpointAndLoginAgain() { // 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 1836cf3ceb5a..3ece6c2b0930 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 @@ -250,8 +250,8 @@ public void testIntrospectRefreshTokenAfterUserSessionLogoutAndLoginAgain() thro 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 e143cd8d361c..0022020c0c71 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 void testSuccess_postMethod_body() throws Exception { client.close(); } } - + @Test public void testSuccess_postMethod_charset_body() throws Exception { Client client = AdminClientUtil.createResteasyClient(); @@ -531,7 +531,7 @@ public void testSuccessSignedResponsePS256() throws Exception { public void testSuccessSignedResponseRS256AcceptJWT() throws Exception { testSuccessSignedResponse(Algorithm.RS256, MediaType.APPLICATION_JWT); } - + @Test public void testSessionExpired() { Client client = AdminClientUtil.createResteasyClient(); @@ -607,8 +607,8 @@ public void testAccessTokenAfterUserSessionLogoutAndLoginAgain() { 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 void testAccessTokenAfterUserSessionLogoutAndLoginAgain() { 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 @@ private void testRolesAreNotInUserInfoResponse(UserInfo userInfo) { 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(); }