Avoid the same userSessionId after re-authentication

Closes keycloak/keycloak-private#69

Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>

services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java

@@ -43,6 +43,7 @@ import org.keycloak.models.utils.FormMessage;
 import org.keycloak.protocol.ClientData;
 import org.keycloak.protocol.LoginProtocol;
 import org.keycloak.protocol.LoginProtocol.Error;
+import org.keycloak.protocol.RestartLoginCookie;
 import org.keycloak.protocol.oidc.TokenManager;
 import org.keycloak.services.ErrorPage;
 import org.keycloak.services.ErrorPageException;
@@ -51,12 +52,14 @@ import org.keycloak.services.managers.AuthenticationManager;
 import org.keycloak.services.managers.BruteForceProtector;
 import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.managers.UserSessionManager;
+import org.keycloak.services.managers.AuthenticationSessionManager;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.services.resources.LoginActionsService;
 import org.keycloak.services.util.CacheControlUtil;
 import org.keycloak.services.util.AuthenticationFlowURLHelper;
 import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.sessions.CommonClientSessionModel;
+import org.keycloak.sessions.RootAuthenticationSessionModel;
 import org.keycloak.util.JsonSerialization;
 
 import jakarta.ws.rs.core.MultivaluedHashMap;
@@ -957,6 +960,22 @@ public class AuthenticationProcessor {
         authSession.setAuthNote(CURRENT_FLOW_PATH, flowPath);
     }
 
+    // Recreate new root auth session and new auth session from the given auth session.
+    public static AuthenticationSessionModel recreate(KeycloakSession session, AuthenticationSessionModel authSession) {
+        AuthenticationSessionManager authenticationSessionManager =  new AuthenticationSessionManager(session);
+        RootAuthenticationSessionModel rootAuthenticationSession = authenticationSessionManager.createAuthenticationSession(authSession.getRealm(), true);
+        AuthenticationSessionModel newAuthSession = rootAuthenticationSession.createAuthenticationSession(authSession.getClient());
+        newAuthSession.setRedirectUri(authSession.getRedirectUri());
+        newAuthSession.setProtocol(authSession.getProtocol());
+
+        for (Map.Entry<String, String> clientNote : authSession.getClientNotes().entrySet()) {
+            newAuthSession.setClientNote(clientNote.getKey(), clientNote.getValue());
+        }
+
+        authenticationSessionManager.removeAuthenticationSession(authSession.getRealm(), authSession, true);
+        RestartLoginCookie.setRestartCookie(session, authSession);
+        return newAuthSession;
+    }
 
     // Clone new authentication session from the given authSession. New authenticationSession will have same parent (rootSession) and will use same client
     public static AuthenticationSessionModel clone(KeycloakSession session, AuthenticationSessionModel authSession) {

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

@@ -211,18 +211,23 @@ public class AuthenticationSessionManager {
     public void updateAuthenticationSessionAfterSuccessfulAuthentication(RealmModel realm, AuthenticationSessionModel authSession) {
         boolean removedRootAuthSession = removeTabIdInAuthenticationSession(realm, authSession);
         if (!removedRootAuthSession) {
-            RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession();
+            if(realm.getSsoSessionIdleTimeout() < SessionExpiration.getAuthSessionLifespan(realm) && realm.getSsoSessionMaxLifespan() < SessionExpiration.getAuthSessionLifespan(realm)) {
+                removeAuthenticationSession(realm, authSession, true);
+            }
+            else {
+                RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession();
 
-            // 1 minute by default. Same timeout, which is used for client to complete "authorization code" flow
+                // 1 minute by default. Same timeout, which is used for client to complete "authorization code" flow
-            // Very short timeout should be OK as when this cookie is set, other existing browser tabs are supposed to be refreshed immediately by JS script authChecker.js
+                // Very short timeout should be OK as when this cookie is set, other existing browser tabs are supposed to be refreshed immediately by JS script authChecker.js
-            // and login user automatically. No need to have authenticationSession and cookie living any longer
+                // and login user automatically. No need to have authenticationSession and cookie living any longer
-            int authSessionExpiresIn = realm.getAccessCodeLifespan();
+                int authSessionExpiresIn = realm.getAccessCodeLifespan();
 
-            // Set timestamp to the past to make sure that authSession is scheduled for expiration in "authSessionExpiresIn" seconds
+                // Set timestamp to the past to make sure that authSession is scheduled for expiration in "authSessionExpiresIn" seconds
-            int authSessionExpirationTime = Time.currentTime() - SessionExpiration.getAuthSessionLifespan(realm) + authSessionExpiresIn;
+                int authSessionExpirationTime = Time.currentTime() - SessionExpiration.getAuthSessionLifespan(realm) + authSessionExpiresIn;
-            rootAuthSession.setTimestamp(authSessionExpirationTime);
+                rootAuthSession.setTimestamp(authSessionExpirationTime);
 
-            log.tracef("Removed authentication session of root session '%s' with tabId '%s'. But there are remaining tabs in the root session. Root authentication session will expire in %d seconds", rootAuthSession.getId(), authSession.getTabId(), authSessionExpiresIn);
+                log.tracef("Removed authentication session of root session '%s' with tabId '%s'. But there are remaining tabs in the root session. Root authentication session will expire in %d seconds", rootAuthSession.getId(), authSession.getTabId(), authSessionExpiresIn);
+            }
         }
     }
 

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

@@ -245,12 +245,13 @@ public class LoginActionsService {
             if (userSession != null) {
                 logger.debugf("Logout of user session %s when restarting flow during re-authentication", userSession.getId());
                 AuthenticationManager.backchannelLogout(session, userSession, false);
+                authSession = AuthenticationProcessor.recreate(session, authSession);
             }
         }
 
         AuthenticationProcessor.resetFlow(authSession, flowPath);
 
-        URI redirectUri = getLastExecutionUrl(flowPath, null, authSession.getClient().getClientId(), tabId, AuthenticationProcessor.getClientData(session, authSession));
+        URI redirectUri = getLastExecutionUrl(flowPath, null, authSession.getClient().getClientId(), authSession.getTabId(), AuthenticationProcessor.getClientData(session, authSession));
         logger.debugf("Flow restart requested. Redirecting to %s", redirectUri);
         return Response.status(Response.Status.FOUND).location(redirectUri).build();
     }

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

@@ -28,10 +28,13 @@ import org.jboss.arquillian.drone.api.annotation.Drone;
 import org.jboss.arquillian.graphene.page.Page;
 import org.jboss.arquillian.test.api.ArquillianResource;
 import org.junit.Test;
+import org.keycloak.OAuth2Constants;
 import org.keycloak.admin.client.resource.UserResource;
 import org.keycloak.authentication.authenticators.browser.PasswordFormFactory;
 import org.keycloak.authentication.authenticators.browser.UsernameFormFactory;
 import org.keycloak.models.AuthenticationExecutionModel;
+import org.keycloak.protocol.oidc.OIDCLoginProtocol;
+import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.idm.FederatedIdentityRepresentation;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
@@ -302,6 +305,67 @@ public class ReAuthenticationTest extends AbstractTestRealmKeycloakTest {
         BrowserFlowTest.revertFlows(testRealm(), "browser - identity first");
     }
 
+    @Test
+    public void restartLoginWithNewRootAuthSession() {
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+        String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+        OAuthClient.AccessTokenResponse response1 = oauth.doAccessTokenRequest(code, "password");
+
+        oauth.prompt(OIDCLoginProtocol.PROMPT_VALUE_LOGIN);
+        loginPage.open();
+        loginPage.clickResetLogin();
+        loginPage.login("john-doh@localhost", "password");
+
+        code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+        OAuthClient.AccessTokenResponse response2 = oauth.doAccessTokenRequest(code, "password");
+
+
+        AccessToken accessToken1 = oauth.verifyToken(response1.getAccessToken());
+        AccessToken accessToken2 = oauth.verifyToken(response2.getAccessToken());
+
+        Assert.assertNotEquals(accessToken1.getSubject(), accessToken2.getSubject());
+        Assert.assertNotEquals(accessToken1.getSessionId(), accessToken2.getSessionId());
+    }
+
+    @Test
+    public void loginAfterExpiredUserSession() {
+        RealmRepresentation rep = testRealm().toRepresentation();
+        Integer originalSsoSessionIdleTimeout = rep.getSsoSessionIdleTimeout();
+        Integer originalSsoSessionMaxLifespan = rep.getSsoSessionMaxLifespan();
+
+        rep.setSsoSessionIdleTimeout(10);
+        rep.setSsoSessionMaxLifespan(10);
+        realmsResouce().realm(rep.getRealm()).update(rep);
+
+        loginPage.open();
+        driver.navigate().refresh();
+        loginPage.login("test-user@localhost", "password");
+
+        String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+        OAuthClient.AccessTokenResponse response1 = oauth.doAccessTokenRequest(code, "password");
+
+        //set time offset after user session expiration (10s) but before accessCodeLifespanLogin (1800s) and accessCodeLifespan (60s)
+        setTimeOffset(20);
+
+        loginPage.open();
+        loginPage.login("john-doh@localhost", "password");
+
+        code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+        OAuthClient.AccessTokenResponse response2 = oauth.doAccessTokenRequest(code, "password");
+
+        AccessToken accessToken1 = oauth.verifyToken(response1.getAccessToken());
+        AccessToken accessToken2 = oauth.verifyToken(response2.getAccessToken());
+
+        Assert.assertNotEquals(accessToken1.getSubject(), accessToken2.getSubject());
+        Assert.assertNotEquals(accessToken1.getSessionId(), accessToken2.getSessionId());
+
+        setTimeOffset(0);
+        rep.setSsoSessionIdleTimeout(originalSsoSessionIdleTimeout);
+        rep.setSsoSessionMaxLifespan(originalSsoSessionMaxLifespan);
+        realmsResouce().realm(rep.getRealm()).update(rep);
+    }
+
     private void setupIdentityFirstFlow() {
         String newFlowAlias = "browser - identity first";
         testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias));