Remove online session when offline access is requested as the first request (#34346)

Closes #34001

Signed-off-by: rmartinc <rmartinc@redhat.com>


Co-authored-by: andymunro <48995441+andymunro@users.noreply.github.com>
Signed-off-by: Marek Posolda <mposolda@gmail.com>

---------

Signed-off-by: rmartinc <rmartinc@redhat.com>
Signed-off-by: Marek Posolda <mposolda@gmail.com>
Co-authored-by: Marek Posolda <mposolda@gmail.com>
Co-authored-by: andymunro <48995441+andymunro@users.noreply.github.com>

docs/documentation/upgrading/topics/changes/changes-26_1_0.adoc

@@ -54,3 +54,9 @@ See the Javadoc for a detailed description.
 = Removal of robots.txt file
 
 The `robots.txt` file, previously included by default, is now removed. The default `robots.txt` file blocked all crawling, which prevented the `noindex`/`nofollow` directives from being followed. The desired default behaviour is for {project_name} pages to not show up in search engine results and this is accomplished by the existing `X-Robots-Tag` header, which is set to `none` by default. The value of this header can be overidden per-realm if a different behaviour is needed.
+
+= Offline access removes the associated online session if the `offline_scope` is requested in the initial exchange
+
+Any offline session in {project_name} is created from another online session. When the `offline_access` scope is requested, the current online session is used to create the associated offline session for the client. Therefore any `offline_access` request finished, until now, with two sessions, one online and one offline.
+
+Starting with this version, {project_name} removes the initial online session if the `offline_scope` is directly requested as the first interaction for the session. The client retrieves the offline token after the code to token exchange that is associated to the offline session, but the previous online session is removed. If the online session has been used before the `offline_scope` request, by the same or another client, the online session remains active as today. Although the new behavior makes sense because the client application is just asking for an offline token, it can affect some scenarios that rely on having the online session still active after the initial `offline_scope` token request.

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

@@ -18,6 +18,7 @@
 package org.keycloak.authentication;
 
 import org.jboss.logging.Logger;
+import org.keycloak.OAuth2Constants;
 import org.keycloak.http.HttpRequest;
 import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator;
 import org.keycloak.authentication.authenticators.client.ClientAuthUtil;
@@ -61,6 +62,7 @@ import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.sessions.CommonClientSessionModel;
 import org.keycloak.sessions.RootAuthenticationSessionModel;
 import org.keycloak.util.JsonSerialization;
+import org.keycloak.util.TokenUtil;
 
 import jakarta.ws.rs.core.MultivaluedHashMap;
 import jakarta.ws.rs.core.Response;
@@ -93,6 +95,9 @@ public class AuthenticationProcessor {
     // Boolean flag, which is true when authentication-selector screen should be rendered (typically displayed when user clicked on 'try another way' link)
     public static final String AUTHENTICATION_SELECTOR_SCREEN_DISPLAYED = "auth.selector.screen.rendered";
 
+    // Boolean note in the client session indicating it was first created for offline session
+    public static final String FIRST_OFFLINE_ACCESS = "first.offline.access";
+
     protected static final Logger logger = Logger.getLogger(AuthenticationProcessor.class);
     protected RealmModel realm;
     protected UserSessionModel userSession;
@@ -1141,7 +1146,15 @@ public class AuthenticationProcessor {
             event.detail(Details.REMEMBER_ME, "true");
         }
 
+        final int clientSessions = userSession.getAuthenticatedClientSessions().size();
         ClientSessionContext clientSessionCtx = TokenManager.attachAuthenticationSession(session, userSession, authSession);
+        if (clientSessions == 0 && userSession.getStarted() == userSession.getLastSessionRefresh()
+                && TokenUtil.hasScope(clientSessionCtx.getScopeString(), OAuth2Constants.OFFLINE_ACCESS)) {
+            // user session is just created, empty and the first access was for offline token, set the note
+            clientSessionCtx.getClientSession().setNote(FIRST_OFFLINE_ACCESS, Boolean.TRUE.toString());
+        } else {
+            clientSessionCtx.getClientSession().removeNote(FIRST_OFFLINE_ACCESS);
+        }
 
         event.user(userSession.getUser())
                 .detail(Details.USERNAME, username)

services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java

@@ -30,6 +30,7 @@ import org.jboss.logging.Logger;
 
 import org.keycloak.OAuth2Constants;
 import org.keycloak.OAuthErrorException;
+import org.keycloak.authentication.AuthenticationProcessor;
 import org.keycloak.common.ClientConnection;
 import org.keycloak.common.Profile;
 import org.keycloak.constants.AdapterConstants;
@@ -111,6 +112,11 @@ public abstract class OAuth2GrantTypeBase implements OAuth2GrantType {
         boolean useRefreshToken = clientConfig.isUseRefreshToken();
         if (useRefreshToken) {
             responseBuilder.generateRefreshToken();
+            if (TokenUtil.TOKEN_TYPE_OFFLINE.equals(responseBuilder.getRefreshToken().getType())
+                    && clientSessionCtx.getClientSession().getNote(AuthenticationProcessor.FIRST_OFFLINE_ACCESS) != null) {
+                // the online session can be removed if first created for offline access
+                session.sessions().removeUserSession(realm, userSession);
+            }
         }
 
         checkAndBindMtlsHoKToken(responseBuilder, useRefreshToken);

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

@@ -17,6 +17,7 @@
 package org.keycloak.services.managers;
 
 import org.jboss.logging.Logger;
+import org.keycloak.authentication.AuthenticationProcessor;
 import org.keycloak.common.util.Time;
 import org.keycloak.device.DeviceActivityManager;
 import org.keycloak.models.AuthenticatedClientSessionModel;
@@ -66,7 +67,8 @@ public class UserSessionManager {
         // Create and persist clientSession
         AuthenticatedClientSessionModel offlineClientSession = offlineUserSession.getAuthenticatedClientSessionByClient(clientSession.getClient().getId());
         if (offlineClientSession == null) {
-            createOfflineClientSession(user, clientSession, offlineUserSession);
+            offlineClientSession = createOfflineClientSession(user, clientSession, offlineUserSession);
+            offlineClientSession.removeNote(AuthenticationProcessor.FIRST_OFFLINE_ACCESS);
         }
     }
 
@@ -140,13 +142,13 @@ public class UserSessionManager {
         return offlineUserSession;
     }
 
-    private void createOfflineClientSession(UserModel user, AuthenticatedClientSessionModel clientSession, UserSessionModel offlineUserSession) {
+    private AuthenticatedClientSessionModel createOfflineClientSession(UserModel user, AuthenticatedClientSessionModel clientSession, UserSessionModel offlineUserSession) {
         if (logger.isTraceEnabled()) {
             logger.tracef("Creating new offline token client session. ClientSessionId: '%s', UserSessionID: '%s' , Username: '%s', Client: '%s'" ,
                     clientSession.getId(), offlineUserSession.getId(), user.getUsername(), clientSession.getClient().getClientId());
         }
 
-        kcSession.sessions().createOfflineClientSession(clientSession, offlineUserSession);
+        return kcSession.sessions().createOfflineClientSession(clientSession, offlineUserSession);
     }
 
     // Check if userSession has any offline clientSessions attached to it. Remove userSession if not

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExtendedEventTest.java

@@ -29,6 +29,7 @@ import static org.keycloak.testsuite.util.ClientPoliciesUtil.createClientRolesCo
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createClientScopesConditionConfig;
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createTestRaiseExeptionExecutorConfig;
 
+import jakarta.ws.rs.NotFoundException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedList;
@@ -529,8 +530,10 @@ public class ClientPoliciesExtendedEventTest extends AbstractClientPoliciesTest
         ).toString();
         updatePolicies(json);
 
-        // delete the non-offline session to force the NPE
+        // now the online session should be removed as it's a offline first request
-        adminClient.realm(REALM_NAME).deleteSession(token.getSessionId(), false);
+        NotFoundException nfe = Assert.assertThrows(NotFoundException.class,
+                () -> adminClient.realm(REALM_NAME).deleteSession(token.getSessionId(), false));
+        Assert.assertEquals(404, nfe.getResponse().getStatus());
 
         String refreshTokenString = res.getRefreshToken();
         OAuthClient.AccessTokenResponse accessTokenResponseRefreshed = oauth.doRefreshTokenRequest(refreshTokenString, clientSecret);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/BackchannelLogoutTest.java

@@ -510,8 +510,9 @@ public class BackchannelLogoutTest extends AbstractNestedBrokerTest {
         String sessionIdProviderRealm = assertProviderLoginEventIdpClient(userIdProviderRealm);
 
         String sessionIdConsumerRealm = assertConsumerLoginEvent(userIdConsumerRealm, OidcBackchannelLogoutBrokerConfiguration.CONSUMER_CLIENT_ID);
-        assertActiveSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
+        assertNoSessionsInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
                 sessionIdConsumerRealm);
+        assertActiveOfflineSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm);
 
         String sessionIdSubConsumerRealm =
                 assertLoginEvent(userIdSubConsumerRealm, ACCOUNT_CLIENT_NAME, nbc.subConsumerRealmName());
@@ -525,13 +526,11 @@ public class BackchannelLogoutTest extends AbstractNestedBrokerTest {
             assertThat(response, Matchers.statusCodeIsHC(Response.Status.OK));
         }
 
-        assertConsumerLogoutEvent(sessionIdConsumerRealm, userIdConsumerRealm);
+        // no logout event as there is no online session now
-        assertLogoutEvent(sessionIdSubConsumerRealm, userIdSubConsumerRealm, nbc.subConsumerRealmName());
-
         assertNoSessionsInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
                 sessionIdConsumerRealm);
         assertNoOfflineSessionsInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm);
-        assertNoSessionsInClient(nbc.subConsumerRealmName(), accountClientIdSubConsumerRealm, userIdSubConsumerRealm,
+        assertActiveSessionInClient(nbc.subConsumerRealmName(), accountClientIdSubConsumerRealm, userIdSubConsumerRealm,
                 sessionIdSubConsumerRealm);
         assertActiveSessionInClient(nbc.providerRealmName(), brokerClientIdProviderRealm, userIdProviderRealm,
                 sessionIdProviderRealm);
@@ -550,8 +549,9 @@ public class BackchannelLogoutTest extends AbstractNestedBrokerTest {
         String sessionIdProviderRealm = assertProviderLoginEventIdpClient(userIdProviderRealm);
 
         String sessionIdConsumerRealm = assertConsumerLoginEvent(userIdConsumerRealm, OidcBackchannelLogoutBrokerConfiguration.CONSUMER_CLIENT_ID);
-        assertActiveSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
+        assertNoSessionsInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
                 sessionIdConsumerRealm);
+        assertActiveOfflineSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm);
 
         String sessionIdSubConsumerRealm =
                 assertLoginEvent(userIdSubConsumerRealm, ACCOUNT_CLIENT_NAME, nbc.subConsumerRealmName());
@@ -565,13 +565,11 @@ public class BackchannelLogoutTest extends AbstractNestedBrokerTest {
             assertThat(response, Matchers.statusCodeIsHC(Response.Status.OK));
         }
 
-        assertConsumerLogoutEvent(sessionIdConsumerRealm, userIdConsumerRealm);
+        // no logout event as there is no online session now
-        assertLogoutEvent(sessionIdSubConsumerRealm, userIdSubConsumerRealm, nbc.subConsumerRealmName());
-
         assertNoSessionsInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
                 sessionIdConsumerRealm);
         assertActiveOfflineSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm);
-        assertNoSessionsInClient(nbc.subConsumerRealmName(), accountClientIdSubConsumerRealm, userIdSubConsumerRealm,
+        assertActiveSessionInClient(nbc.subConsumerRealmName(), accountClientIdSubConsumerRealm, userIdSubConsumerRealm,
                 sessionIdSubConsumerRealm);
         assertActiveSessionInClient(nbc.providerRealmName(), brokerClientIdProviderRealm, userIdProviderRealm,
                 sessionIdProviderRealm);
@@ -589,8 +587,9 @@ public class BackchannelLogoutTest extends AbstractNestedBrokerTest {
 
         String sessionIdConsumerRealm = assertConsumerLoginEvent(userIdConsumerRealm,
                 OidcBackchannelLogoutBrokerConfiguration.CONSUMER_CLIENT_ID);
-        assertActiveSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
+        assertNoSessionsInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm,
                 sessionIdConsumerRealm);
+        assertActiveOfflineSessionInClient(nbc.consumerRealmName(), consumerClientId, userIdConsumerRealm);
 
         executeLogoutFromRealm(getConsumerRoot(), nbc.consumerRealmName(), null, null, OidcBackchannelLogoutBrokerConfiguration.CONSUMER_CLIENT_ID, null);
         confirmLogout();

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java

@@ -262,6 +262,9 @@ public class OfflineTokenTest extends AbstractKeycloakTest {
 
         assertTrue(tokenResponse.getScope().contains(OAuth2Constants.OFFLINE_ACCESS));
 
+        // check only offline session is created
+        checkNumberOfSessions(userId, "offline-client", offlineToken.getSessionId(), 0, 1);
+
         String newRefreshTokenString = testRefreshWithOfflineToken(token, offlineToken, offlineTokenString, sessionId, userId);
 
         // Change offset to very big value to ensure offline session expires
@@ -282,6 +285,67 @@ public class OfflineTokenTest extends AbstractKeycloakTest {
         setTimeOffset(0);
     }
 
+    @Test
+    public void onlineOfflineTokenBrowserFlow() throws Exception {
+        // request an online token for the client
+        oauth.scope(null);
+        oauth.clientId("offline-client");
+        oauth.redirectUri(offlineClientAppUri);
+        oauth.doLogin("test-user@localhost", "password");
+        EventRepresentation loginEvent = events.expectLogin()
+                .client("offline-client")
+                .detail(Details.REDIRECT_URI, offlineClientAppUri)
+                .assertEvent();
+        final String sessionId = loginEvent.getSessionId();
+        String codeId = loginEvent.getDetails().get(Details.CODE_ID);
+        OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(oauth.getCurrentQuery().get(OAuth2Constants.CODE), "secret1");
+        RefreshToken onlineToken = assertRefreshToken(tokenResponse, TokenUtil.TOKEN_TYPE_REFRESH);
+        AccessToken token = oauth.verifyToken(tokenResponse.getAccessToken());
+        events.expectCodeToToken(codeId, sessionId)
+                .client("offline-client")
+                .detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_REFRESH)
+                .assertEvent();
+        assertEquals(TokenUtil.TOKEN_TYPE_REFRESH, onlineToken.getType());
+        Assert.assertNotNull(onlineToken.getExp());
+        // request an offline token for the same client
+        oauth.scope(OAuth2Constants.OFFLINE_ACCESS);
+        oauth.doSilentLogin();
+        events.expectLogin()
+                .client("offline-client")
+                .detail(Details.REDIRECT_URI, offlineClientAppUri)
+                .assertEvent();
+        OAuthClient.AccessTokenResponse tokenOfflineResponse = oauth.doAccessTokenRequest(
+                oauth.getCurrentQuery().get(OAuth2Constants.CODE), "secret1");
+        RefreshToken offlineToken = assertRefreshToken(tokenOfflineResponse, TokenUtil.TOKEN_TYPE_OFFLINE);
+        events.expectCodeToToken(codeId, sessionId)
+                .client("offline-client")
+                .detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_OFFLINE)
+                .assertEvent();
+        assertEquals(TokenUtil.TOKEN_TYPE_OFFLINE, offlineToken.getType());
+        Assert.assertNull(offlineToken.getExp());
+        assertTrue(tokenOfflineResponse.getScope().contains(OAuth2Constants.OFFLINE_ACCESS));
+        // check both sessions are created
+        checkNumberOfSessions(userId, "offline-client", onlineToken.getSessionId(), 1, 1);
+        // check online token can be refreshed
+        tokenResponse = oauth.doRefreshTokenRequest(tokenResponse.getRefreshToken(), "secret1");
+        assertRefreshToken(tokenResponse, TokenUtil.TOKEN_TYPE_REFRESH);
+        events.expectRefresh(token.getId(), sessionId)
+                .client("offline-client")
+                .user(userId)
+                .detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_REFRESH)
+                .detail(Details.REFRESH_TOKEN_ID, onlineToken.getId())
+                .assertEvent();
+        // check offline token can be refreshed
+        tokenOfflineResponse = oauth.doRefreshTokenRequest(tokenOfflineResponse.getRefreshToken(), "secret1");
+        assertRefreshToken(tokenOfflineResponse, TokenUtil.TOKEN_TYPE_OFFLINE);
+        events.expectRefresh(token.getId(), sessionId)
+                .client("offline-client")
+                .user(userId)
+                .detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_OFFLINE)
+                .detail(Details.REFRESH_TOKEN_ID, offlineToken.getId())
+                .assertEvent();
+    }
+
     private String testRefreshWithOfflineToken(AccessToken oldToken, RefreshToken offlineToken, String offlineTokenString,
                                                final String sessionId, String userId) {
         // Change offset to big value to ensure userSession expired
@@ -795,19 +859,28 @@ public class OfflineTokenTest extends AbstractKeycloakTest {
 
         int prev[] = null;
         try (RealmAttributeUpdater rau = new RealmAttributeUpdater(adminClient.realm("test")).setSsoSessionIdleTimeout(900).update()) {
-            // Step 1 - offline login with "offline-client"
             prev = changeOfflineSessionSettings(true, MAX_LIFESPAN, IDLE_LIFESPAN, 0, 0);
 
+            // Step 1 - online login with "tets-app"
+            oauth.scope(null);
+            oauth.clientId("test-app");
+            oauth.redirectUri(APP_ROOT + "/auth");
+            oauth.doLogin("test-user@localhost", "password");
+            String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+            OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "password");
+            assertRefreshToken(tokenResponse, TokenUtil.TOKEN_TYPE_REFRESH);
+
+            // Step 2 - offline login with "offline-client"
             oauth.scope(OAuth2Constants.OFFLINE_ACCESS);
             oauth.clientId("offline-client");
             oauth.redirectUri(offlineClientAppUri);
 
-            oauth.doLogin("test-user@localhost", "password");
+            oauth.doSilentLogin();
-            String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+            code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
-            OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "secret1");
+            tokenResponse = oauth.doAccessTokenRequest(code, "secret1");
             assertOfflineToken(tokenResponse);
 
-            // Step 2 - set some offset to refresh SSO session and offline user session. But use different client, so that we don't refresh offlineClientSession of client "offline-client"
+            // Step 3 - set some offset to refresh SSO session and offline user session. But use different client, so that we don't refresh offlineClientSession of client "offline-client"
             setTimeOffset(800);
             oauth.clientId("test-app");
             oauth.redirectUri(APP_ROOT + "/auth");
@@ -817,7 +890,7 @@ public class OfflineTokenTest extends AbstractKeycloakTest {
             tokenResponse = oauth.doAccessTokenRequest(code, "password");
             assertOfflineToken(tokenResponse);
 
-            // Step 3 - set bigger time offset and login with the original client "offline-token". Login should be successful and offline client session for "offline-client" should be re-created now
+            // Step 4 - set bigger time offset and login with the original client "offline-token". Login should be successful and offline client session for "offline-client" should be re-created now
             setTimeOffset(900 + SessionTimeoutHelper.PERIODIC_CLEANER_IDLE_TIMEOUT_WINDOW_SECONDS + 20);
             oauth.clientId("offline-client");
             oauth.redirectUri(offlineClientAppUri);
@@ -833,13 +906,17 @@ public class OfflineTokenTest extends AbstractKeycloakTest {
         }
     }
 
-    // Asserts that refresh token in the tokenResponse is offlineToken. Return parsed offline token
     private RefreshToken assertOfflineToken(OAuthClient.AccessTokenResponse tokenResponse) {
+        return assertRefreshToken(tokenResponse, TokenUtil.TOKEN_TYPE_OFFLINE);
+    }
+
+    // Asserts that refresh token in the tokenResponse is of the given type. Return parsed token
+    private RefreshToken assertRefreshToken(OAuthClient.AccessTokenResponse tokenResponse, String tokenType) {
         Assert.assertEquals(200, tokenResponse.getStatusCode());
         String offlineTokenString = tokenResponse.getRefreshToken();
-        RefreshToken offlineToken = oauth.parseRefreshToken(offlineTokenString);
+        RefreshToken refreshToken = oauth.parseRefreshToken(offlineTokenString);
-        assertEquals(TokenUtil.TOKEN_TYPE_OFFLINE, offlineToken.getType());
+        assertEquals(tokenType, refreshToken.getType());
-        return offlineToken;
+        return refreshToken;
     }
 
     @Test