KEYCLOAK-11417 Internal server error on front channel logout with expired session

services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java

@@ -54,7 +54,11 @@ import org.keycloak.services.resources.Cors;
 import org.keycloak.services.util.MtlsHoKTokenUtil;
 import org.keycloak.util.TokenUtil;
 
-import javax.ws.rs.*;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
@@ -65,6 +69,9 @@ import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Stream;
 
+import static org.keycloak.models.UserSessionModel.State.LOGGED_OUT;
+import static org.keycloak.models.UserSessionModel.State.LOGGING_OUT;
+
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
  */
@@ -152,11 +159,15 @@ public class LogoutEndpoint {
             if (idToken != null && idToken.getSessionState().equals(AuthenticationManager.getSessionIdFromSessionCookie(session))) {
                 return initiateBrowserLogout(userSession, redirect, state, initiatingIdp);
             }
+            // check if the user session is not logging out or already logged out
+            // this might happen when a backChannelLogout is already initiated from AuthenticationManager.authenticateIdentityCookie
+            if (userSession.getState() != LOGGING_OUT && userSession.getState() != LOGGED_OUT) {
                 // 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();
             }
+        }
 
         if (redirect != null) {
             UriBuilder uriBuilder = UriBuilder.fromUri(redirect);

services/src/main/java/org/keycloak/services/util/CookieHelper.java

@@ -25,13 +25,10 @@ import org.keycloak.common.util.ServerCookie;
 
 import javax.ws.rs.core.Cookie;
 import javax.ws.rs.core.HttpHeaders;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import static org.keycloak.common.util.ServerCookie.SameSiteAttributeValue;
 
@@ -100,7 +97,7 @@ public class CookieHelper {
         Set<String> ret = getInternalCookieValue(name);
         if (ret.size() == 0) {
             String legacy = name + LEGACY_COOKIE;
-            logger.debugv("Couldn't find any cookies with name '{0}', trying '{1}'", name, legacy);
+            logger.debugv("Could not find any cookies with name '{0}', trying '{1}'", name, legacy);
             ret = getInternalCookieValue(legacy);
         }
         return ret;
@@ -116,7 +113,7 @@ public class CookieHelper {
         // get cookies from the cookie field
         Cookie cookie = headers.getCookies().get(name);
         if (cookie != null) {
-            logger.debugv("{0} cookie found in the cookie's field", name);
+            logger.debugv("{0} cookie found in the cookie field", name);
             cookiesVal.add(cookie.getValue());
         }
 
@@ -134,7 +131,7 @@ public class CookieHelper {
 
         for (Cookie cookie : CookieParser.parseCookies(header)) {
             if (name.equals(cookie.getName())) {
-                logger.debugv("{0} cookie found in the request's header", name);
+                logger.debugv("{0} cookie found in the request header", name);
                 values.add(cookie.getValue());
             }
         }
@@ -149,7 +146,7 @@ public class CookieHelper {
         }
         else {
             String legacy = name + LEGACY_COOKIE;
-            logger.debugv("Couldn't find cookie {0}, trying {1}", name, legacy);
+            logger.debugv("Could not find cookie {0}, trying {1}", name, legacy);
             return cookies.get(legacy);
         }
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LogoutTest.java

@@ -17,8 +17,10 @@
 package org.keycloak.testsuite.forms;
 
 import org.jboss.arquillian.graphene.page.Page;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.keycloak.OAuth2Constants;
 import org.keycloak.common.Profile;
 import org.keycloak.events.Details;
 import org.keycloak.models.Constants;
@@ -31,10 +33,12 @@ import org.keycloak.common.util.Retry;
 import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
 import org.keycloak.testsuite.pages.AppPage;
+import org.keycloak.testsuite.pages.ErrorPage;
 import org.keycloak.testsuite.pages.LoginPage;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -44,6 +48,10 @@ import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlEquals;
 
 import org.keycloak.testsuite.auth.page.account.AccountManagement;
 import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
+import org.keycloak.testsuite.updaters.RealmAttributeUpdater;
+import org.keycloak.testsuite.util.ClientManager;
+import org.keycloak.testsuite.util.OAuthClient;
+import org.keycloak.testsuite.util.WaitUtils;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -63,10 +71,18 @@ public class LogoutTest extends AbstractTestRealmKeycloakTest {
     @Page
     protected AccountManagement accountManagementPage;
 
+    @Page
+    private ErrorPage errorPage;
+
     @Override
     public void configureTestRealm(RealmRepresentation testRealm) {
     }
 
+    @Before
+    public void clientConfiguration() {
+        ClientManager.realm(adminClient.realm("test")).clientId("test-app").directAccessGrant(true);
+    }
+
     @Test
     public void logoutRedirect() {
         loginPage.open();
@@ -118,6 +134,37 @@ public class LogoutTest extends AbstractTestRealmKeycloakTest {
         assertNotEquals(sessionId, sessionId2);
     }
 
+    @Test
+    public void logoutWithExpiredSession() throws Exception {
+        try (AutoCloseable c = new RealmAttributeUpdater(adminClient.realm("test"))
+                .updateWith(r -> r.setSsoSessionMaxLifespan(2))
+                .update()) {
+
+            oauth.doLogin("test-user@localhost", "password");
+
+            String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+
+            oauth.clientSessionState("client-session");
+            OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "password");
+            String idTokenString = tokenResponse.getIdToken();
+
+            // wait for a timeout
+            // setTimeOffset doesn't work because session cookie is not invalidated thus the logout flow would continue with browser logout
+            TimeUnit.SECONDS.sleep(3);
+
+            String logoutUrl = oauth.getLogoutUrl().redirectUri(oauth.APP_AUTH_ROOT).idTokenHint(idTokenString).build();
+            driver.navigate().to(logoutUrl);
+
+            // should not throw an internal server error
+            appPage.assertCurrent();
+
+            // check if the back channel logout succeeded
+            driver.navigate().to(oauth.getLoginFormUrl());
+            WaitUtils.waitForPageToLoad();
+            loginPage.assertCurrent();
+        }
+    }
+
     @Test
     public void logoutMultipleSessions() throws IOException {
         // Login session 1