Improve SessionCodeChecks to detect better the ALREADY_LOGGED_IN situation

Closes https://github.com/keycloak/keycloak/issues/19677

services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java

@@ -867,6 +867,13 @@ public class AuthenticationManager {
         expireCookie(realm, AuthenticationSessionManager.AUTH_SESSION_ID, oldPath, true, connection, SameSiteAttributeValue.NONE, session);
     }
 
+    public static void expireAuthSessionCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) {
+        logger.debugv("Expire {1} cookie .", AuthenticationSessionManager.AUTH_SESSION_ID);
+        ClientConnection connection = session.getContext().getConnection();
+        String oldPath = getRealmCookiePath(realm, uriInfo);
+        expireCookie(realm, AuthenticationSessionManager.AUTH_SESSION_ID, oldPath, true, connection, SameSiteAttributeValue.NONE, session);
+    }
+
     protected static String getIdentityCookiePath(RealmModel realm, UriInfo uriInfo) {
         return getRealmCookiePath(realm, uriInfo);
     }

services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java

@@ -18,7 +18,6 @@
 package org.keycloak.services.resources;
 
 import static org.keycloak.services.managers.AuthenticationManager.authenticateIdentityCookie;
-import static org.keycloak.services.managers.AuthenticationSessionManager.AUTH_SESSION_ID;
 import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification;
 
 import java.net.URI;
@@ -52,7 +51,6 @@ import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.services.util.BrowserHistoryHelper;
 import org.keycloak.services.util.AuthenticationFlowURLHelper;
-import org.keycloak.services.util.CookieHelper;
 import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.sessions.RootAuthenticationSessionModel;
 
@@ -181,21 +179,15 @@ public class SessionCodeChecks {
 
         }
 
-        // See if we are already authenticated and userSession with same ID exists.
+        // Otherwise just try to restart from the cookie
-        UserSessionModel userSession = authSessionManager.getUserSessionFromAuthCookie(realm);
+        RootAuthenticationSessionModel existingRootAuthSession = authSessionManager.getCurrentRootAuthenticationSession(realm);
+        response = restartAuthenticationSessionFromCookie(existingRootAuthSession);
 
-        boolean authenticating = !CookieHelper.getCookieValue(session, AUTH_SESSION_ID).isEmpty();
+        // if restart from cookie was not found check if the user is already authenticated
-
+        if (response.getStatus() != Response.Status.FOUND.getStatusCode()) {
-        if (authenticating) {
-            // if there is an auth session, make sure the user is not yet authenticated
             AuthenticationManager.AuthResult authResult = lockUserSessionsForModification(session, () -> authenticateIdentityCookie(session, realm, false));
 
-            if (authResult != null) {
+            if (authResult != null && authResult.getSession() != null) {
-                userSession = authResult.getSession();
-            }
-        }
-
-        if (userSession != null) {
                 LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class).setAuthenticationSession(authSession)
                         .setSuccess(Messages.ALREADY_LOGGED_IN);
 
@@ -204,12 +196,9 @@ public class SessionCodeChecks {
                 }
 
                 response = loginForm.createInfoPage();
-            return null;
+            }
         }
 
-        // Otherwise just try to restart from the cookie
-        RootAuthenticationSessionModel existingRootAuthSession = authSessionManager.getCurrentRootAuthenticationSession(realm);
-        response = restartAuthenticationSessionFromCookie(existingRootAuthSession);
         return null;
     }
 

services/src/main/java/org/keycloak/services/resources/admin/UserResource.java

@@ -340,6 +340,7 @@ public class UserResource {
             UserSessionModel userSession = lockUserSessionsForModification(session, () -> session.sessions().getUserSession(authenticatedRealm, sessionState));
             AuthenticationManager.expireIdentityCookie(realm, session.getContext().getUri(), session);
             AuthenticationManager.expireRememberMeCookie(realm, session.getContext().getUri(), session);
+            AuthenticationManager.expireAuthSessionCookie(realm, session.getContext().getUri(), session);
             AuthenticationManager.backchannelLogout(session, authenticatedRealm, userSession, session.getContext().getUri(), clientConnection, headers, true);
         }
         EventBuilder event = new EventBuilder(realm, session, clientConnection);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java

@@ -47,6 +47,7 @@ import java.security.cert.Certificate;
 import java.security.cert.CertificateFactory;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -87,24 +88,31 @@ import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathExpression;
 import javax.xml.xpath.XPathFactory;
 
+import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.client.methods.RequestBuilder;
 import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.client.BasicCookieStore;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.util.EntityUtils;
 
 import org.jboss.arquillian.container.test.api.Deployment;
 import org.jboss.arquillian.graphene.page.Page;
+import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder;
 import org.jboss.shrinkwrap.api.asset.StringAsset;
 import org.jboss.shrinkwrap.api.spec.WebArchive;
 
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.keycloak.admin.client.Keycloak;
+import org.keycloak.admin.client.KeycloakBuilder;
 import org.keycloak.admin.client.resource.ClientResource;
 import org.keycloak.admin.client.resource.ProtocolMappersResource;
+import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.admin.client.resource.RoleScopeResource;
 import org.keycloak.admin.client.resource.UserResource;
 import org.keycloak.common.util.Base64;
@@ -119,6 +127,7 @@ import org.keycloak.dom.saml.v2.protocol.StatusResponseType;
 import org.keycloak.keys.Attributes;
 import org.keycloak.keys.ImportedRsaKeyProviderFactory;
 import org.keycloak.keys.KeyProvider;
+import org.keycloak.models.Constants;
 import org.keycloak.protocol.saml.SamlConfigAttributes;
 import org.keycloak.protocol.saml.SamlProtocol;
 import org.keycloak.representations.idm.ClientRepresentation;
@@ -135,17 +144,23 @@ import org.keycloak.saml.common.util.XmlKeyInfoKeyNameTransformer;
 import org.keycloak.saml.processing.core.parsers.saml.SAMLParser;
 import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
 import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
+import org.keycloak.services.managers.AuthenticationManager;
+import org.keycloak.services.managers.AuthenticationSessionManager;
 import org.keycloak.services.resources.RealmsResource;
+import org.keycloak.services.util.CookieHelper;
 import org.keycloak.testsuite.adapter.page.*;
 import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
 import org.keycloak.testsuite.util.ServerURLs;
 import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
 import org.keycloak.testsuite.auth.page.login.Login;
+import org.keycloak.testsuite.auth.page.login.OneTimeCode;
 import org.keycloak.testsuite.auth.page.login.SAMLIDPInitiatedLogin;
 import org.keycloak.testsuite.auth.page.login.SAMLPostLoginTenant1;
 import org.keycloak.testsuite.auth.page.login.SAMLPostLoginTenant2;
 import org.keycloak.testsuite.page.AbstractPage;
+import org.keycloak.testsuite.pages.ErrorPage;
+import org.keycloak.testsuite.pages.InfoPage;
 import org.keycloak.testsuite.saml.AbstractSamlTest;
 import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
 import org.keycloak.testsuite.updaters.Creator;
@@ -159,6 +174,7 @@ import org.keycloak.testsuite.util.WaitUtils;
 import org.keycloak.testsuite.utils.io.IOUtil;
 
 import org.openqa.selenium.By;
+import org.openqa.selenium.Cookie;
 
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -289,6 +305,15 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest {
     @Page
     protected SAMLPostLoginTenant2 tenant2RealmSAMLPostLoginPage;
 
+    @Page
+    protected OneTimeCode authenticate;
+
+    @Page
+    private InfoPage infoPage;
+
+    @Page
+    private ErrorPage errorPage;
+
     public static final String FORBIDDEN_TEXT = "HTTP status code: 403";
     public static final String WEBSPHERE_FORBIDDEN_TEXT = "Error reported: 403";
 
@@ -1773,6 +1798,103 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest {
 
     }
 
+    @Test
+    public void testReloginWithInvalidAuthSessionCookie() {
+        assertSuccessfulLogin(salesPostServletPage, bburkeUser, testRealmSAMLPostLoginPage, "principal=bburke");
+        assertSuccessfullyLoggedIn(salesPostSigServletPage, "principal=bburke");
+        // remove session cookies for the application to force re-login against the keycloak server
+        driver.manage().deleteAllCookies();
+
+        // go to the login-actions/authenticate page and change AUTH_SESSION_ID cookies
+        authenticate.setAuthRealm(SAMLSERVLETDEMO);
+        authenticate.navigateTo();
+        waitForPageToLoad();
+        infoPage.assertCurrent();
+        Assert.assertEquals("You are already logged in.", infoPage.getInfo());
+        Cookie identityCookie = driver.manage().getCookieNamed(AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
+        Assert.assertNotNull(identityCookie);
+        driver.manage().deleteCookieNamed(AuthenticationSessionManager.AUTH_SESSION_ID);
+        driver.manage().deleteCookieNamed(AuthenticationSessionManager.AUTH_SESSION_ID + CookieHelper.LEGACY_COOKIE);
+        driver.manage().addCookie(new Cookie(AuthenticationSessionManager.AUTH_SESSION_ID, "invalid-value", identityCookie.getPath()));
+        driver.manage().addCookie(new Cookie(AuthenticationSessionManager.AUTH_SESSION_ID + CookieHelper.LEGACY_COOKIE, "invalid-value", identityCookie.getPath()));
+
+        // go back to the app page, re-login should work with the invalid cookie
+        testRealmSAMLPostLoginPage.navigateTo();
+        waitForPageToLoad();
+        assertSuccessfullyLoggedIn(salesPostSigServletPage, "principal=bburke");
+        driver.manage().deleteAllCookies();
+
+        salesPostSigServletPage.logout();
+        checkLoggedOut(salesPostSigEmailServletPage, testRealmSAMLPostLoginPage);
+    }
+
+    private List<Cookie> impersonate(String admin, String adminPassword, String userId) throws IOException {
+        ResteasyClientBuilder resteasyClientBuilder = (ResteasyClientBuilder) ResteasyClientBuilder.newBuilder();
+        resteasyClientBuilder.connectionPoolSize(10);
+        resteasyClientBuilder.httpEngine(AdminClientUtil.getCustomClientHttpEngine(resteasyClientBuilder, 10, null));
+
+        BasicCookieStore cookieStore = new BasicCookieStore();
+        try (Keycloak client = KeycloakBuilder.builder().serverUrl(loginPage.getAuthRoot()).realm(SAMLSERVLETDEMO)
+                .username(admin).password(adminPassword).clientId(Constants.ADMIN_CLI_CLIENT_ID)
+                .resteasyClient(ResteasyClientBuilder.newBuilder().build()).build();
+                CloseableHttpClient httpClient = HttpClientBuilder.create().setDefaultCookieStore(cookieStore).build()) {
+            HttpUriRequest req = RequestBuilder.post()
+                    .setUri(loginPage.getAuthRoot() + "/admin/realms/" + SAMLSERVLETDEMO + "/users/" + userId + "/impersonation")
+                    .addHeader(HttpHeaders.AUTHORIZATION, "Bearer " + client.tokenManager().getAccessTokenString())
+                    .build();
+            HttpResponse res = httpClient.execute(req);
+            Assert.assertEquals(Response.Status.OK.getStatusCode(), res.getStatusLine().getStatusCode());
+            String resBody = EntityUtils.toString(res.getEntity());
+
+            Assert.assertNotNull(resBody);
+            Assert.assertTrue(resBody.contains("redirect"));
+
+            // return cookies not expired in the store as selenium cookies
+            final Date now = new Date();
+            return cookieStore.getCookies().stream()
+                    .filter(c -> !c.isExpired(now))
+                    .map(c -> new Cookie(c.getName(), c.getValue(), c.getDomain(), c.getPath(), c.getExpiryDate(), c.isSecure(), true))
+                    .collect(Collectors.toList());
+        }
+    }
+
+    @Test
+    public void testImpersonationForSaml() throws IOException {
+        RealmResource realm = adminClient.realm(SAMLSERVLETDEMO);
+        List<UserRepresentation> users = realm.users().search("bburke", true);
+        Assert.assertNotNull(users);
+        Assert.assertEquals(1, users.size());
+
+        // impersonate and get all returned cookies
+        List<Cookie> cookies = impersonate("admindemo", "password", users.get(0).getId());
+        Assert.assertNotNull(cookies);
+
+        // go to the authenticate page and add all the returned cookies by the impersonation
+        authenticate.setAuthRealm(SAMLSERVLETDEMO);
+        authenticate.navigateTo();
+        waitForPageToLoad();
+        errorPage.assertCurrent();
+        cookies.stream().forEach(c -> driver.manage().addCookie(c));
+        driver.navigate().refresh();
+        waitForPageToLoad();
+        infoPage.assertCurrent();
+        Assert.assertEquals("You are already logged in.", infoPage.getInfo());
+
+        // now go to the saml app with all the impersonation cookies
+        testRealmSAMLPostLoginPage.navigateTo();
+        waitForPageToLoad();
+        assertSuccessfullyLoggedIn(salesPostSigServletPage, "principal=bburke");
+        driver.manage().deleteAllCookies();
+
+        // go back to the app page a second time
+        testRealmSAMLPostLoginPage.navigateTo();
+        waitForPageToLoad();
+        assertSuccessfullyLoggedIn(salesPostSigServletPage, "principal=bburke");
+
+        salesPostSigServletPage.logout();
+        checkLoggedOut(salesPostSigEmailServletPage, testRealmSAMLPostLoginPage);
+    }
+
     public static void printDocument(Source doc, OutputStream out) throws IOException, TransformerException {
         TransformerFactory tf = TransformerFactory.newInstance();
         Transformer transformer = tf.newTransformer();

testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/testsaml.json

@@ -99,6 +99,19 @@
                 "phone": "617"
             },
             "realmRoles": ["manager", "user"]
+        },
+        {
+            "username" : "admindemo",
+            "enabled": true,
+            "email" : "admindemo@redhat.com",
+            "credentials" : [
+                { "type" : "password",
+                    "value" : "password" }
+            ],
+            "realmRoles": ["manager", "user"],
+            "clientRoles" : {
+                "realm-management" : [ "impersonation", "view-users" ]
+            }
         }
     ],
     "clients": [