From b88ecc023716a63a7ee2bb51101d4e8ad0618e9c Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 9 Sep 2024 09:28:48 +0200 Subject: [PATCH] Removing the extra two-minute Window for persistent user sessions (#32660) Closes #28418 Signed-off-by: Alexander Schwartz Signed-off-by: Michal Hajas Co-authored-by: Michal Hajas --- .../server_admin/topics/sessions/timeouts.adoc | 2 ++ .../upgrading/topics/changes/changes-26_0_0.adoc | 8 ++++++++ .../services/managers/AuthenticationManager.java | 6 ++++-- .../adapter/servlet/SAMLServletSessionTimeoutTest.java | 4 +++- .../java/org/keycloak/testsuite/forms/LoginTest.java | 2 +- .../org/keycloak/testsuite/oauth/OfflineTokenTest.java | 10 ++++++---- .../org/keycloak/testsuite/oauth/RefreshTokenTest.java | 8 +++++--- 7 files changed, 29 insertions(+), 11 deletions(-) diff --git a/docs/documentation/server_admin/topics/sessions/timeouts.adoc b/docs/documentation/server_admin/topics/sessions/timeouts.adoc index 472dcfbf97..22d44dad12 100644 --- a/docs/documentation/server_admin/topics/sessions/timeouts.adoc +++ b/docs/documentation/server_admin/topics/sessions/timeouts.adoc @@ -93,6 +93,8 @@ image:images/tokens-tab.png[Tokens Tab] [NOTE] ==== +The following logic is only applied if persistent user sessions are not active: + For idle timeouts, a two-minute window of time exists that the session is active. For example, when you have the timeout set to 30 minutes, it will be 32 minutes before the session expires. This action is necessary for some scenarios in cluster and cross-data center environments where the token refreshes on one cluster node a short time before the expiration and the other cluster nodes incorrectly consider the session as expired because they have not yet received the message about a successful refresh from the refreshing node. diff --git a/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc index 5eaf019fb4..18031cbdf7 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc @@ -246,6 +246,14 @@ Update your custom embedded Infinispan cache configuration file with configurati For more details proceed to the https://www.keycloak.org/server/caching[Configuring distributed caches] guide. += Grace period for idle sessions removed when persistent sessions are enabled + +Previous versions of {project_name} added a grace period of two minutes to idle times of user and client sessions. +This was added due to a previous architecture where session refresh times were replicated asynchronously in a cluster. +With persistent user sessions, this is no longer necessary, and therefore the grace period is now removed. + +To keep the old behavior, update your realm configuration and extend the session and client idle times by two minutes. + = Support for legacy `redirect_uri` parameter and SPI options has been removed Previous versions of {project_name} had supported automatic logout of the user and redirecting to the application by opening logout endpoint URL such as diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index 23c1aad5a7..396da5607a 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -185,7 +185,8 @@ public class AuthenticationManager { long idle = SessionExpirationUtils.calculateUserSessionIdleTimestamp(userSession.isOffline(), userSession.isRememberMe(), TimeUnit.SECONDS.toMillis(userSession.getLastSessionRefresh()), realm); - boolean sessionIdleOk = idle > currentTime - TimeUnit.SECONDS.toMillis(SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); + boolean sessionIdleOk = idle > currentTime - + ((Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) || Profile.isFeatureEnabled(Profile.Feature.REMOTE_CACHE)) ? 0 : TimeUnit.SECONDS.toMillis(SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS)); boolean sessionMaxOk = lifespan == -1L || lifespan > currentTime; return sessionIdleOk && sessionMaxOk; } @@ -203,7 +204,8 @@ public class AuthenticationManager { long idle = SessionExpirationUtils.calculateClientSessionIdleTimestamp(userSession.isOffline(), userSession.isRememberMe(), TimeUnit.SECONDS.toMillis(clientSession.getTimestamp()), realm, client); - boolean sessionIdleOk = idle > currentTime - TimeUnit.SECONDS.toMillis(SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); + boolean sessionIdleOk = idle > currentTime - + ((Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) || Profile.isFeatureEnabled(Profile.Feature.REMOTE_CACHE)) ? 0 : TimeUnit.SECONDS.toMillis(SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS)); boolean sessionMaxOk = lifespan == -1L || lifespan > currentTime; return sessionIdleOk && sessionMaxOk; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java index 2d9b71729e..8067ac119b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java @@ -4,6 +4,7 @@ import org.jboss.arquillian.container.test.api.Deployment; import org.jboss.arquillian.graphene.page.Page; import org.jboss.shrinkwrap.api.spec.WebArchive; import org.junit.Test; +import org.keycloak.common.Profile; import org.keycloak.dom.saml.v2.SAML2Object; import org.keycloak.dom.saml.v2.assertion.AuthnStatementType; import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; @@ -11,6 +12,7 @@ import org.keycloak.dom.saml.v2.protocol.ResponseType; import org.keycloak.models.utils.SessionTimeoutHelper; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; +import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.adapter.filter.AdapterActionsFilter; import org.keycloak.testsuite.adapter.page.Employee2Servlet; import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; @@ -46,7 +48,7 @@ public class SAMLServletSessionTimeoutTest extends AbstractSAMLServletAdapterTes } private static final int SESSION_LENGTH_IN_SECONDS = 120; - private static final int KEYCLOAK_SESSION_TIMEOUT = 1922; /** 1800 session max + 120 {@link SessionTimeoutHelper#IDLE_TIMEOUT_WINDOW_SECONDS} */ + private static final int KEYCLOAK_SESSION_TIMEOUT = 1800 + (ProfileAssume.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) ? 0 : SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); /** 1800 session max + 120 {@link SessionTimeoutHelper#IDLE_TIMEOUT_WINDOW_SECONDS} */ private AtomicReference sessionNotOnOrAfter = new AtomicReference<>(); 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 c7c6e1db20..5ed26a90c7 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 @@ -937,7 +937,7 @@ public class LoginTest extends AbstractTestRealmKeycloakTest { appPage.assertCurrent(); // expire idle timeout using the timeout window. - setTimeOffset(2 + SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); + setTimeOffset(2 + (ProfileAssume.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) ? 0 : SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS)); // trying to open the account page with an expired idle timeout should redirect back to the login page. loginPage.open(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java index 22c71ce110..0b815d8e47 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java @@ -29,6 +29,7 @@ import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RoleResource; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.common.Profile; import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.crypto.Algorithm; import org.keycloak.events.Details; @@ -56,6 +57,7 @@ import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.auth.page.AuthRealm; import org.keycloak.testsuite.pages.LoginPage; @@ -488,7 +490,7 @@ public class OfflineTokenTest extends AbstractKeycloakTest { testUser.roles().realmLevel().remove(Collections.singletonList(appRealm.roles().get("composite").toRepresentation())); appRealm.roles().get("composite").remove(); testUser.roles().realmLevel().add(Collections.singletonList(offlineAccess)); - + } /** @@ -648,7 +650,7 @@ public class OfflineTokenTest extends AbstractKeycloakTest { offlineRefresh = oauth.doRefreshTokenRequest(offlineResponse.getRefreshToken(), "secret1"); assertEquals(200, offlineRefresh.getStatusCode()); } - + @Test public void browserOfflineTokenLogoutFollowedByLoginSameSession() throws Exception { oauth.scope(OAuth2Constants.OFFLINE_ACCESS); @@ -760,7 +762,7 @@ public class OfflineTokenTest extends AbstractKeycloakTest { final int MAX_LIFESPAN = 3000; final int IDLE_LIFESPAN = 600; // Additional time window is added for the case when session was updated in different DC and the update to current DC was postponed - testOfflineSessionExpiration(IDLE_LIFESPAN, MAX_LIFESPAN, 0, IDLE_LIFESPAN + SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS + 60); + testOfflineSessionExpiration(IDLE_LIFESPAN, MAX_LIFESPAN, 0, IDLE_LIFESPAN + (ProfileAssume.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) ? 0 : SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS) + 60); } // Issue 13706 @@ -927,7 +929,7 @@ public class OfflineTokenTest extends AbstractKeycloakTest { } setTimeOffset(0); - + } finally { getTestingClient().testing().revertTestingInfinispanTimeService(); changeOfflineSessionSettings(false, prev[0], prev[1], prev[2], prev[3]); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java index aea1c0e2d5..fc394610c0 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java @@ -33,6 +33,7 @@ import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RealmsResource; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.common.Profile; import org.keycloak.common.enums.SslRequired; import org.keycloak.cookie.CookieType; import org.keycloak.crypto.Algorithm; @@ -63,6 +64,7 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserSessionRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.undertow.lb.SimpleUndertowLoadBalancer; import org.keycloak.testsuite.oidc.AbstractOIDCScopeTest; @@ -1211,7 +1213,7 @@ public class RefreshTokenTest extends AbstractKeycloakTest { events.clear(); // Needs to add some additional time due the tollerance allowed by IDLE_TIMEOUT_WINDOW_SECONDS - setTimeOffset(6 + SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); + setTimeOffset(6 + (ProfileAssume.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) ? 0 : SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS)); tokenResponse = oauth.doRefreshTokenRequest(tokenResponse.getRefreshToken(), "password"); // test idle timeout @@ -1253,7 +1255,7 @@ public class RefreshTokenTest extends AbstractKeycloakTest { String refreshId = oauth.parseRefreshToken(tokenResponse.getRefreshToken()).getId(); int last = testingClient.testing().getLastSessionRefresh("test", sessionId, false); - setTimeOffset(110 + SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); + setTimeOffset(110 + (ProfileAssume.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) ? 0 : SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS)); tokenResponse = oauth.doRefreshTokenRequest(tokenResponse.getRefreshToken(), "password"); oauth.verifyToken(tokenResponse.getAccessToken()); oauth.parseRefreshToken(tokenResponse.getRefreshToken()); @@ -1264,7 +1266,7 @@ public class RefreshTokenTest extends AbstractKeycloakTest { events.clear(); // Needs to add some additional time due the tollerance allowed by IDLE_TIMEOUT_WINDOW_SECONDS - setTimeOffset(620 + 2 * SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS); + setTimeOffset(620 + 2 * (ProfileAssume.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) ? 0 : SessionTimeoutHelper.IDLE_TIMEOUT_WINDOW_SECONDS)); tokenResponse = oauth.doRefreshTokenRequest(tokenResponse.getRefreshToken(), "password"); // test idle remember me timeout