Skip link only when client is not system when logout (#24595)

Signed-off-by: Lex Cao <lexcao@foxmail.com>

server-spi-private/src/main/java/org/keycloak/models/utils/SystemClientUtil.java

@@ -18,9 +18,14 @@
 package org.keycloak.models.utils;
 
 import org.jboss.logging.Logger;
+import org.keycloak.forms.login.LoginFormsProvider;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
+import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
+import org.keycloak.sessions.AuthenticationSessionModel;
+
+import java.util.Optional;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -57,4 +62,16 @@ public class SystemClientUtil {
 
     }
 
+    /**
+     * Cleanup system client URL to avoid links to account management
+     */
+    public static void checkSkipLink(KeycloakSession session, AuthenticationSessionModel authSession) {
+        String usedClientId = Optional.ofNullable(authSession)
+                .map(it -> it.getClient().getClientId())
+                .orElseGet(() -> session.getContext().getUri().getQueryParameters().getFirst(Constants.CLIENT_ID));
+
+        if (usedClientId != null && usedClientId.equals(getSystemClient(session.getContext().getRealm()).getClientId())) {
+            session.getProvider(LoginFormsProvider.class).setAttribute(Constants.SKIP_LINK, true);
+        }
+    }
 }

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

@@ -371,10 +371,7 @@ public class LogoutEndpoint {
             logger.debugf("Failed verification during logout. logoutSessionId=%s, clientId=%s, tabId=%s",
                     logoutSession != null ? logoutSession.getParentSession().getId() : "unknown", clientId, tabId);
 
-            if (logoutSession == null || logoutSession.getClient().equals(SystemClientUtil.getSystemClient(logoutSession.getRealm()))) {
+            SystemClientUtil.checkSkipLink(session, logoutSession);
-                // Cleanup system client URL to avoid links to account management
-                session.getProvider(LoginFormsProvider.class).setAttribute(Constants.SKIP_LINK, true);
-            }
 
             event.error(Errors.SESSION_EXPIRED);
 
@@ -405,11 +402,7 @@ public class LogoutEndpoint {
         if (logoutSession == null) {
             logger.debugf("Failed verification when changing locale logout. clientId=%s, tabId=%s", clientId, tabId);
 
-            LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class);
+            SystemClientUtil.checkSkipLink(session, logoutSession);
-            if (clientId == null || clientId.equals(SystemClientUtil.getSystemClient(realm).getClientId())) {
-                // Cleanup system client URL to avoid links to account management
-                loginForm.setAttribute(Constants.SKIP_LINK, true);
-            }
 
             AuthenticationManager.AuthResult authResult = AuthenticationManager.authenticateIdentityCookie(session, realm, false);
             if (authResult != null) {
@@ -417,7 +410,7 @@ public class LogoutEndpoint {
                 return ErrorPage.error(session, logoutSession, Response.Status.BAD_REQUEST, Messages.FAILED_LOGOUT);
             } else {
                 // Probably changing locale on logout screen after logout was already performed. If there is no session in the browser, we can just display that logout was already finished
-                return loginForm.setSuccess(Messages.SUCCESS_LOGOUT).createInfoPage();
+                return session.getProvider(LoginFormsProvider.class).setSuccess(Messages.SUCCESS_LOGOUT).createInfoPage();
             }
         }
 

services/src/main/java/org/keycloak/protocol/oidc/utils/LogoutUtil.java

@@ -24,7 +24,6 @@ import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.UriBuilder;
 
 import org.keycloak.forms.login.LoginFormsProvider;
-import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.utils.SystemClientUtil;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
@@ -45,12 +44,10 @@ public class LogoutUtil {
             return Response.status(302).location(finalRedirectUri).build();
         }
 
-        LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class).setSuccess(Messages.SUCCESS_LOGOUT);
+        SystemClientUtil.checkSkipLink(session, logoutSession);
-        boolean usedSystemClient = logoutSession.getClient().equals(SystemClientUtil.getSystemClient(logoutSession.getRealm()));
+
-        if (usedSystemClient) {
+        return session.getProvider(LoginFormsProvider.class)
-            loginForm.setAttribute(Constants.SKIP_LINK, true);
+                .setSuccess(Messages.SUCCESS_LOGOUT)
-        }
-        return loginForm
                 .setDetachedAuthSession()
                 .createInfoPage();
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java

@@ -594,6 +594,40 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
         Assert.assertEquals("Logout failed", errorPage.getError());
 
         events.expectLogoutError(Errors.SESSION_EXPIRED).assertEvent();
+
+        // Link not present
+        try {
+            errorPage.clickBackToApplication();
+            fail();
+        } catch (NoSuchElementException ex) {
+            // expected
+        }
+    }
+
+    // Test for the scenario when "authenticationSession" itself is expired without system client
+    @Test
+    public void logoutExpiredConfirmationAuthSessionWithClient() {
+        OAuthClient.AccessTokenResponse tokenResponse = loginUser();
+
+        driver.navigate().to(oauth.getLogoutUrl().clientId("test-app").build());
+
+        // Assert logout confirmation page. Session still exists
+        logoutConfirmPage.assertCurrent();
+        MatcherAssert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState())));
+        events.assertEmpty();
+
+        // Set time offset to expire "action" inside logoutSession
+        setTimeOffset(1810);
+        logoutConfirmPage.confirmLogout();
+
+        errorPage.assertCurrent();
+        Assert.assertEquals("Logout failed", errorPage.getError());
+
+        events.expectLogoutError(Errors.SESSION_EXPIRED).assertEvent();
+
+        // Link "Back to application" present
+        errorPage.clickBackToApplication();
+        MatcherAssert.assertThat(driver.getCurrentUrl(), endsWith("/app/auth"));
     }
 
     // Test logout with "consentRequired" . All of "post_logout_redirect_uri", "id_token_hint" and "state" parameters are present in the logout request