diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java index 53537b5d13..0eb3f22acd 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java @@ -111,7 +111,7 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { if (tasks.containsKey(taskKey)) { throw new IllegalStateException("Can't add session: task in progress for session"); } else { - tasks.put(taskKey, new CacheTaskWithValue(value) { + tasks.put(taskKey, new CacheTaskWithValue(value, lifespan, lifespanUnit) { @Override public void execute() { decorateCache(cache).put(key, value, lifespan, lifespanUnit); @@ -160,16 +160,17 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { } public void replace(Cache cache, K key, V value, long lifespan, TimeUnit lifespanUnit) { - log.tracev("Adding cache operation: {0} on {1}", CacheOperation.REPLACE, key); + log.tracev("Adding cache operation: {0} on {1}. Lifespan {2} {3}.", CacheOperation.REPLACE, key, lifespan, lifespanUnit); Object taskKey = getTaskKey(cache, key); CacheTask current = tasks.get(taskKey); if (current != null) { if (current instanceof CacheTaskWithValue) { ((CacheTaskWithValue) current).setValue(value); + ((CacheTaskWithValue) current).updateLifespan(lifespan, lifespanUnit); } } else { - tasks.put(taskKey, new CacheTaskWithValue(value) { + tasks.put(taskKey, new CacheTaskWithValue(value, lifespan, lifespanUnit) { @Override public void execute() { decorateCache(cache).replace(key, value, lifespan, lifespanUnit); @@ -256,9 +257,17 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { public static abstract class CacheTaskWithValue implements CacheTask { protected V value; + protected long lifespan; + protected TimeUnit lifespanUnit; public CacheTaskWithValue(V value) { + this(value, -1, TimeUnit.SECONDS); + } + + public CacheTaskWithValue(V value, long lifespan, TimeUnit lifespanUnit) { this.value = value; + this.lifespan = lifespan; + this.lifespanUnit = lifespanUnit; } public V getValue() { @@ -269,6 +278,11 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { this.value = value; } + public void updateLifespan(long lifespan, TimeUnit lifespanUnit) { + this.lifespan = lifespan; + this.lifespanUnit = lifespanUnit; + } + public Operation getOperation() { return Operation.OTHER; } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/RootAuthenticationSessionAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/RootAuthenticationSessionAdapter.java index 36a9c0638a..738c3c13a9 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/RootAuthenticationSessionAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/RootAuthenticationSessionAdapter.java @@ -63,7 +63,7 @@ public class RootAuthenticationSessionAdapter implements RootAuthenticationSessi } void update() { - int expirationSeconds = SessionExpiration.getAuthSessionLifespan(realm); + int expirationSeconds = getTimestamp() - Time.currentTime() + SessionExpiration.getAuthSessionLifespan(realm); provider.tx.replace(cache, entity.getId(), entity, expirationSeconds, TimeUnit.SECONDS); } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java b/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java index 9b4d52ceeb..db597c38bb 100644 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java @@ -67,16 +67,11 @@ public class AuthenticationStateCookie { this.remainingTabs = remainingTabs; } - public static void generateAndSetCookie(KeycloakSession session, RealmModel realm, RootAuthenticationSessionModel rootAuthSession) { + public static void generateAndSetCookie(KeycloakSession session, RealmModel realm, RootAuthenticationSessionModel rootAuthSession, int cookieMaxAge) { UriInfo uriInfo = session.getContext().getHttpRequest().getUri(); String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); boolean secureOnly = realm.getSslRequired().isRequired(session.getContext().getConnection()); - // 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 immediatelly by JS script - // and login user automatically. No need to have cookie living any further - int cookieMaxAge = realm.getAccessCodeLifespan(); - AuthenticationStateCookie cookie = new AuthenticationStateCookie(); cookie.setAuthSessionId(rootAuthSession.getId()); cookie.setRemainingTabs(rootAuthSession.getAuthenticationSessions().keySet()); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java index 87877294bd..28453cfc04 100644 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java @@ -26,6 +26,7 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; +import org.keycloak.models.utils.SessionExpiration; import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.services.resources.LoginActionsService; import org.keycloak.services.util.CookieHelper; @@ -262,15 +263,22 @@ public class AuthenticationSessionManager { * @param authSession */ public void updateAuthenticationSessionAfterSuccessfulAuthentication(RealmModel realm, AuthenticationSessionModel authSession) { - // TODO: The authentication session might need to be expired in short interval (realm accessCodeLifespan, which is 1 minute by default). That should be sufficient for other browser tabs - // to finish authentication and at the same time we won't need to keep authentication sessions in storage longer than needed boolean removedRootAuthSession = removeTabIdInAuthenticationSession(realm, authSession); if (!removedRootAuthSession) { RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession(); - log.tracef("Removed authentication session of root session '%s' with tabId '%s'. But there are remaining tabs in the root session", rootAuthSession.getId(), authSession.getTabId()); + // 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 + // and login user automatically. No need to have authenticationSession and cookie living any longer + int authSessionExpiresIn = realm.getAccessCodeLifespan(); - AuthenticationStateCookie.generateAndSetCookie(session, realm, rootAuthSession); + // 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; + 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); + + AuthenticationStateCookie.generateAndSetCookie(session, realm, rootAuthSession, authSessionExpiresIn); } } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java index 114198fbea..1710c16eae 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java @@ -69,6 +69,7 @@ import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.resource.RealmResourceProvider; import org.keycloak.services.scheduled.ClearExpiredUserSessions; import org.keycloak.services.util.CookieHelper; +import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.datastore.PeriodicEventInvalidation; import org.keycloak.testsuite.components.TestProvider; @@ -1106,4 +1107,18 @@ public class TestingResourceProvider implements RealmResourceProvider { factory.setProvider(this.factory.truststoreProvider); } + @GET + @Path("/get-authentication-session-tabs-count") + @NoCache + public Integer getAuthenticationSessionTabsCount(@QueryParam("realm") String realmName, @QueryParam("authSessionId") String authSessionId) { + RealmModel realm = getRealmByName(realmName); + RootAuthenticationSessionModel rootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId); + if (rootAuthSession == null) { + return 0; + } + + return rootAuthSession.getAuthenticationSessions().size(); + } + + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java index 4b0941490d..70da333d0c 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java @@ -425,4 +425,16 @@ public interface TestingResource { @NoCache void reenableTruststoreSpi(); + /** + * Get count of tabs (child authentication sessions) for given "root authentication session" + * + * @param realm realm name (not ID) + * @param authSessionId ID of authentication session + * @return count of tabs. Return 0 if authentication session of given ID does not exists (or if it exists, but without any authenticationSessions attached, which should not happen with normal usage) + */ + @GET + @Path("/get-authentication-session-tabs-count") + @NoCache + Integer getAuthenticationSessionTabsCount(@QueryParam("realm") String realm, @QueryParam("authSessionId") String authSessionId); + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java index 5bc41f885c..f67262d79b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTest.java @@ -55,7 +55,6 @@ import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.admin.ApiUtil; -import org.keycloak.testsuite.arquillian.annotation.DisableFeature; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; @@ -894,6 +893,28 @@ public class LoginTest extends AbstractTestRealmKeycloakTest { events.expectLogin().detail(Details.USERNAME, "test-user@localhost").assertEvent(); } + @Test + public void testAuthenticationSessionExpiresEarlyAfterAuthentication() throws Exception { + // Open login form and refresh right after. This simulates creating another "tab" in rootAuthenticationSession + oauth.openLoginForm(); + driver.navigate().refresh(); + + // Assert authenticationSession in cache with 2 tabs + String authSessionId = driver.manage().getCookieNamed(AuthenticationSessionManager.AUTH_SESSION_ID).getValue(); + Assert.assertEquals((Integer) 2, getTestingClient().testing().getAuthenticationSessionTabsCount("test", authSessionId)); + + loginPage.login("test-user@localhost", "password"); + appPage.assertCurrent(); + + // authentication session should still exists with remaining browser tab + Assert.assertEquals((Integer) 1, getTestingClient().testing().getAuthenticationSessionTabsCount("test", authSessionId)); + + // authentication session should be expired after 1 minute + setTimeOffset(300); + Assert.assertEquals((Integer) 0, getTestingClient().testing().getAuthenticationSessionTabsCount("test", authSessionId)); + } + + @Test public void loginRememberMeExpiredIdle() throws Exception { try (Closeable c = new RealmAttributeUpdater(adminClient.realm("test"))