Skip to content

Commit

Permalink
After tab1 finish authentication, make sure that rootAuthenticationSe…
Browse files Browse the repository at this point in the history
…ssion is expired shortly

closes keycloak#23880
  • Loading branch information
mposolda committed Oct 19, 2023
1 parent c718304 commit 0477729
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public <K, V> void put(BasicCache<K, V> cache, K key, V value, long lifespan, Ti
if (tasks.containsKey(taskKey)) {
throw new IllegalStateException("Can't add session: task in progress for session");
} else {
tasks.put(taskKey, new CacheTaskWithValue<V>(value) {
tasks.put(taskKey, new CacheTaskWithValue<V>(value, lifespan, lifespanUnit) {
@Override
public void execute() {
decorateCache(cache).put(key, value, lifespan, lifespanUnit);
Expand Down Expand Up @@ -160,16 +160,17 @@ public Operation getOperation() {
}

public <K, V> void replace(Cache<K, V> 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<V>) current).setValue(value);
((CacheTaskWithValue<V>) current).updateLifespan(lifespan, lifespanUnit);
}
} else {
tasks.put(taskKey, new CacheTaskWithValue<V>(value) {
tasks.put(taskKey, new CacheTaskWithValue<V>(value, lifespan, lifespanUnit) {
@Override
public void execute() {
decorateCache(cache).replace(key, value, lifespan, lifespanUnit);
Expand Down Expand Up @@ -256,9 +257,17 @@ public enum Operation { PUT, OTHER }

public static abstract class CacheTaskWithValue<V> 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() {
Expand All @@ -269,6 +278,11 @@ public void setValue(V value) {
this.value = value;
}

public void updateLifespan(long lifespan, TimeUnit lifespanUnit) {
this.lifespan = lifespan;
this.lifespanUnit = lifespanUnit;
}

public Operation getOperation() {
return Operation.OTHER;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public RootAuthenticationSessionAdapter(KeycloakSession session, InfinispanAuthe
}

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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,11 @@ public void setRemainingTabs(Set<String> remainingTabs) {
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -262,15 +263,22 @@ public boolean removeTabIdInAuthenticationSession(RealmModel realm, Authenticati
* @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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
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;
Expand Down Expand Up @@ -1106,4 +1107,18 @@ public void reenableTruststoreSpi() {
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();
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -425,4 +425,16 @@ Response simulatePostRequest(@QueryParam("postRequestUrl") String postRequestUrl
@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);

}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
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;
Expand Down Expand Up @@ -894,6 +893,28 @@ public void openLoginFormAfterExpiredCode() throws Exception {
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"))
Expand Down

0 comments on commit 0477729

Please sign in to comment.