diff --git a/services/src/main/java/org/keycloak/services/resources/LogoutSessionCodeChecks.java b/services/src/main/java/org/keycloak/services/resources/LogoutSessionCodeChecks.java index 21eb09da1e..fd74584416 100644 --- a/services/src/main/java/org/keycloak/services/resources/LogoutSessionCodeChecks.java +++ b/services/src/main/java/org/keycloak/services/resources/LogoutSessionCodeChecks.java @@ -42,7 +42,7 @@ import org.keycloak.sessions.RootAuthenticationSessionModel; public class LogoutSessionCodeChecks extends SessionCodeChecks { public LogoutSessionCodeChecks(RealmModel realm, UriInfo uriInfo, HttpRequest request, ClientConnection clientConnection, KeycloakSession session, EventBuilder event, - String code, String clientId, String tabId) { + String code, String clientId, String tabId) { super(realm, uriInfo, request, clientConnection, session, event, null, code, null, clientId, tabId, null); } @@ -67,4 +67,9 @@ public class LogoutSessionCodeChecks extends SessionCodeChecks { } return true; } + + @Override + protected boolean checkClientDisabled(ClientModel client) { + return !client.isEnabled() && getClientCode() != null; + } } 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 5c8db8e8a9..a670dacf3a 100644 --- a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java +++ b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java @@ -18,6 +18,7 @@ package org.keycloak.services.resources; import java.net.URI; + import javax.ws.rs.core.Cookie; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; @@ -151,7 +152,8 @@ public class SessionCodeChecks { // object retrieve AuthenticationSessionManager authSessionManager = new AuthenticationSessionManager(session); AuthenticationSessionModel authSession = null; - if (authSessionId != null) authSession = authSessionManager.getAuthenticationSessionByIdAndClient(realm, authSessionId, client, tabId); + if (authSessionId != null) + authSession = authSessionManager.getAuthenticationSessionByIdAndClient(realm, authSessionId, client, tabId); AuthenticationSessionModel authSessionCookie = authSessionManager.getCurrentAuthenticationSession(realm, client, tabId); if (authSession != null && authSessionCookie != null && !authSession.getParentSession().getId().equals(authSessionCookie.getParentSession().getId())) { @@ -222,9 +224,9 @@ public class SessionCodeChecks { setClientToEvent(client); session.getContext().setClient(client); - if (!client.isEnabled()) { + if (checkClientDisabled(client)) { event.error(Errors.CLIENT_DISABLED); - response = ErrorPage.error(session,authSession, Response.Status.BAD_REQUEST, Messages.LOGIN_REQUESTER_NOT_ENABLED); + response = ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.LOGIN_REQUESTER_NOT_ENABLED); clientCode.removeExpiredClientSession(); return false; } @@ -236,7 +238,7 @@ public class SessionCodeChecks { String lastFlow = authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH); // Check if we transitted between flows (eg. clicking "register" on login screen) - if (execution==null && !flowPath.equals(lastFlow)) { + if (execution == null && !flowPath.equals(lastFlow)) { logger.debugf("Transition between flows! Current flow: %s, Previous flow: %s", flowPath, lastFlow); // Don't allow moving to different flow if I am on requiredActions already @@ -378,7 +380,7 @@ public class SessionCodeChecks { AuthenticationSessionModel authSession = null; Cookie cook = RestartLoginCookie.getRestartCookie(session); - if(cook == null){ + if (cook == null) { event.error(Errors.COOKIE_NOT_FOUND); return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.COOKIE_NOT_FOUND); } @@ -449,4 +451,8 @@ public class SessionCodeChecks { protected EventBuilder getEvent() { return event; } + + protected boolean checkClientDisabled(ClientModel client) { + return !client.isEnabled(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java index 75cc501e49..bb5d87e362 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java @@ -142,4 +142,9 @@ public class ClientAttributeUpdater extends ServerResourceUpdater * This is handled on server-side by the LogoutEndpoint.logout method * * @author Stian Thorgersen @@ -141,6 +141,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { @Test public void logoutRedirect() { + OAuthClient.AccessTokenResponse tokenResponse = loginUser(); String sessionId = tokenResponse.getSessionState(); @@ -292,7 +293,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { assertEquals("test-user@localhost", loginPage.getUsername()); loginPage.login("test-user@localhost", "password"); - + //log out appPage.openAccount(); accountManagementPage.signOut(); @@ -506,8 +507,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { try { logoutConfirmPage.clickBackToApplicationLink(); fail(); - } - catch (NoSuchElementException ex) { + } catch (NoSuchElementException ex) { // expected } @@ -562,8 +562,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { try { errorPage.clickBackToApplication(); fail(); - } - catch (NoSuchElementException ex) { + } catch (NoSuchElementException ex) { // expected } } @@ -760,8 +759,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { try { logoutConfirmPage.clickBackToApplicationLink(); fail(); - } - catch (NoSuchElementException ex) { + } catch (NoSuchElementException ex) { // expected } @@ -880,8 +878,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { try { logoutConfirmPage.clickBackToApplicationLink(); fail(); - } - catch (NoSuchElementException ex) { + } catch (NoSuchElementException ex) { // expected } @@ -944,7 +941,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { ClientRepresentation rep = clients.findByClientId(oauth.getClientId()).get(0); rep.setFrontchannelLogout(true); rep.getAttributes().put(OIDCConfigAttributes.FRONT_CHANNEL_LOGOUT_URI, oauth.APP_ROOT + "/admin/frontchannelLogout"); - clients.get(rep.getId()).update(rep); + clients.get(rep.getId()).update(rep); try { oauth.clientSessionState("client-session"); oauth.doLogin("test-user@localhost", "password"); @@ -1029,7 +1026,49 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { } } + @Test + public void logoutWithIdTokenAndDisabledClientMustWork() throws Exception { + OAuthClient.AccessTokenResponse tokenResponse = loginUser(); + + try (Closeable accountClientUpdater = ClientAttributeUpdater.forClient(adminClient, "test", oauth.getClientId()) + .setEnabled(false).update()) { + + String logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(APP_REDIRECT_URI).clientId("test-app").build(); + driver.navigate().to(logoutUrl); + MatcherAssert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState()))); + events.assertEmpty(); + + logoutConfirmPage.confirmLogout(); + events.expectLogout(tokenResponse.getSessionState()).assertEvent(); + MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); + } + + } + + //Login and logout with account client disabled after login + @Test + public void testLogoutWhenAccountClientIsDisabled() throws IOException { + + OAuthClient.AccessTokenResponse tokenResponse = loginUser(); + String sessionId = tokenResponse.getSessionState(); + + try (Closeable accountClientUpdater = ClientAttributeUpdater.forClient(adminClient, "test", Constants.ACCOUNT_MANAGEMENT_CLIENT_ID) + .setEnabled(false) + .update()) { + String logoutUrl = oauth.getLogoutUrl().build(); + driver.navigate().to(logoutUrl); + + events.assertEmpty(); + logoutConfirmPage.assertCurrent(); + logoutConfirmPage.confirmLogout(); + + MatcherAssert.assertThat(false, is(isSessionActive(sessionId))); + MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); + } + } + + // SUPPORT METHODS private OAuthClient.AccessTokenResponse loginUser() { return loginUser(false); }