KEYCLOAK-11773 Front-channel logout with identity brokering does not …

…work after browser restart
tnorimat · Dec 3, 2019 · 73d1a26 · 73d1a26
1 parent 5fc39da
commit 73d1a26
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 10 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
@@ -116,9 +116,10 @@ public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String
         }
 
         UserSessionModel userSession = null;
+        IDToken idToken = null;
         if (encodedIdToken != null) {
             try {
-                IDToken idToken = tokenManager.verifyIDTokenSignature(session, encodedIdToken);
+                idToken = tokenManager.verifyIDTokenSignature(session, encodedIdToken);
                 userSession = session.sessions().getUserSession(realm, idToken.getSessionState());
 
                 if (userSession != null) {
@@ -135,14 +136,14 @@ public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String
         AuthenticationManager.AuthResult authResult = AuthenticationManager.authenticateIdentityCookie(session, realm, false);
         if (authResult != null) {
             userSession = userSession != null ? userSession : authResult.getSession();
-            if (redirect != null) userSession.setNote(OIDCLoginProtocol.LOGOUT_REDIRECT_URI, redirect);
-            if (state != null) userSession.setNote(OIDCLoginProtocol.LOGOUT_STATE_PARAM, state);
-            userSession.setNote(AuthenticationManager.KEYCLOAK_LOGOUT_PROTOCOL, OIDCLoginProtocol.LOGIN_PROTOCOL);
-            logger.debug("Initiating OIDC browser logout");
-            Response response =  AuthenticationManager.browserLogout(session, realm, authResult.getSession(), session.getContext().getUri(), clientConnection, headers, initiatingIdp);
-            logger.debug("finishing OIDC browser logout");
-            return response;
-        } else if (userSession != null) { // non browser logout
+            return initiateBrowserLogout(userSession, redirect, state, initiatingIdp);
+        }
+        else if (userSession != null) {
+            // identity cookie is missing but there's valid id_token_hint which matches session cookie => continue with browser logout
+            if (idToken != null && idToken.getSessionState().equals(AuthenticationManager.getSessionIdFromSessionCookie(session))) {
+                return initiateBrowserLogout(userSession, redirect, state, initiatingIdp);
+            }
+            // non browser logout
             event.event(EventType.LOGOUT);
             AuthenticationManager.backchannelLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers, true);
             event.user(userSession.getUser()).session(userSession).success();
@@ -245,4 +246,14 @@ private void checkTokenIssuedAt(IDToken token, UserSessionModel userSession) thr
             throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Refresh toked issued before the user session started");
         }
     }
+
+    private Response initiateBrowserLogout(UserSessionModel userSession, String redirect, String state, String initiatingIdp ) {
+        if (redirect != null) userSession.setNote(OIDCLoginProtocol.LOGOUT_REDIRECT_URI, redirect);
+        if (state != null) userSession.setNote(OIDCLoginProtocol.LOGOUT_STATE_PARAM, state);
+        userSession.setNote(AuthenticationManager.KEYCLOAK_LOGOUT_PROTOCOL, OIDCLoginProtocol.LOGIN_PROTOCOL);
+        logger.debug("Initiating OIDC browser logout");
+        Response response =  AuthenticationManager.browserLogout(session, realm, userSession, session.getContext().getUri(), clientConnection, headers, initiatingIdp);
+        logger.debug("finishing OIDC browser logout");
+        return response;
+    }
 }
diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
@@ -800,6 +800,21 @@ public static Response redirectAfterSuccessfulFlow(KeycloakSession session, Real
 
     }
 
+    public static String getSessionIdFromSessionCookie(KeycloakSession session) {
+        Cookie cookie = session.getContext().getRequestHeaders().getCookies().get(KEYCLOAK_SESSION_COOKIE);
+        if (cookie == null || "".equals(cookie.getValue())) {
+            logger.debugv("Could not find cookie: {0}", KEYCLOAK_SESSION_COOKIE);
+            return null;
+        }
+
+        String[] parts = cookie.getValue().split("/", 3);
+        if (parts.length != 3) {
+            logger.debugv("Cannot parse session value from: {0}", KEYCLOAK_SESSION_COOKIE);
+            return null;
+        }
+        return parts[2];
+    }
+
     public static boolean isSSOAuthentication(AuthenticatedClientSessionModel clientSession) {
         String ssoAuth = clientSession.getNote(SSO_AUTH);
         return Boolean.parseBoolean(ssoAuth);

diff --git a/...illian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java b/...illian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java
@@ -253,12 +253,15 @@ protected void logoutFromRealm(String realm) {
         logoutFromRealm(realm, null);
     }
 
-    protected void logoutFromRealm(String realm, String initiatingIdp) {
+    protected void logoutFromRealm(String realm, String initiatingIdp) { logoutFromRealm(realm, initiatingIdp, null); }
+
+    protected void logoutFromRealm(String realm, String initiatingIdp, String tokenHint) {
         driver.navigate().to(BrokerTestTools.getAuthRoot(suiteContext)
                 + "/auth/realms/" + realm
                 + "/protocol/" + "openid-connect"
                 + "/logout?redirect_uri=" + encodeUrl(getAccountUrl(realm))
                 + (!StringUtils.isBlank(initiatingIdp) ? "&initiating_idp=" + initiatingIdp : "")
+                + (!StringUtils.isBlank(tokenHint) ? "&id_token_hint=" + tokenHint : "")
         );
 
         try {

diff --git a/...illian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLogoutTest.java b/...illian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLogoutTest.java
@@ -5,6 +5,8 @@
 import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
+import org.keycloak.services.managers.AuthenticationManager;
+import org.openqa.selenium.Cookie;
 
 import javax.ws.rs.core.Response;
 import java.util.List;
@@ -87,4 +89,22 @@ public void logoutWithOtherIdpAsInitiatinIdpLogsOutOfIdp() {
         driver.navigate().to(getAccountUrl(REALM_PROV_NAME));
         waitForPage(driver, "log in to provider", true);
     }
+
+    @Test
+    public void logoutAfterBrowserRestart() {
+        logInAsUserInIDPForFirstTime();
+        assertLoggedInAccountManagement();
+
+        Cookie identityCookie = driver.manage().getCookieNamed(AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
+        String idToken = identityCookie.getValue();
+
+        // simulate browser restart by deleting an identity cookie
+        log.debugf("Deleting %s cookie", AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
+        driver.manage().deleteCookieNamed(AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
+
+        logoutFromRealm(bc.consumerRealmName(), null, idToken);
+        driver.navigate().to(getAccountUrl(REALM_PROV_NAME));
+
+        waitForPage(driver, "log in to provider", true);
+    }
 }