Clarify user session limits documentation and test SSO scenario (#19372)

Closes #17374


Co-authored-by: andymunro <48995441+andymunro@users.noreply.github.com>

docs/documentation/server_admin/topics/authentication/flows.adoc

@@ -401,7 +401,7 @@ one of the specified levels. If it is not able to return one of the specified le
 in the authentication flow), then {project_name} will throw an error.
 
 [[_user_session_limits]]
-==== User session limits
+=== User session limits
 
 Limits on the number of session that a user can have can be configured. Sessions can be limited per realm or per client.
 
@@ -425,14 +425,28 @@ If both session limits and client session limits are enabled, it makes sense to
 
 Note that the user session limits should be added to your bound *Browser flow*, *Direct grant flow*, *Reset credentials* and also to any *Post broker login flow*.
 The authenticator should be added at the point when the user is already known during authentication (usually at the end of the authentication flow) and should be typically REQUIRED. Note that it is not possible to have
-ALTERNATIVE and REQUIRED executions at the same level. For example for the default browser flow, it may be necessary to wrap the existing flow as a REQUIRED level-1 subflow and
-add `User Session Count Limiter` to the same level as this new subflow. Example of such flow is below.
+ALTERNATIVE and REQUIRED executions at the same level.
 
-image:images/authentication-user-session-limits.png[Authentication User Session Limits Flow]
+For most of authenticators like `Direct grant flow`, `Reset credentials` or `Post broker login flow`, it is recommended to add the authenticator as REQUIRED at the end of the authentication flow.
+Here is an example for the `Reset credentials` flow:
 
-Currently, the administrator is responsible for maintaining consistency between the different configurations.
+image:images/authentication-user-session-limits-resetcred.png[Authentication User Session Limits Reset Credentials Flow]
 
-Note also that the user session limit feature is not available for CIBA.
+For `Browser` flow, consider not adding the Session Limits authenticator at the top level flow. This recommendation is due to the `Cookie` authenticator, which automatically re-authenticates users based
+on SSO cookie. It is at the top level and it is better to not check session limits during SSO re-authentication because a user session already exists. So instead, consider adding a separate ALTERNATIVE
+subflow, such as the following `authenticate-user-with-session-limit` example at the same level like `Cookie`. Then you can add a REQUIRED subflow, in the following `real-authentication-subflow`example, as a nested subflow of `authenticate-user-with-session-limit` and add a `User Session Limit` at the same level as well. Inside the `real-authentication-subflow`,
+you can add real authenticators in a similar fashion to the default browser flow. The following example flow allows to users to authenticate with an identity provider or
+with password and OTP:
+
+image:images/authentication-user-session-limits-browser.png[Authentication User Session Limits Browser Flow]
+
+Regarding `Post Broker login flow`,  you can add the `User Session Limits` as the only authenticator in the authentication flow  as long as you have no other authenticators that you  trigger after authentication with your identity provider.  However, make sure that this flow is configured as `Post Broker Flow` at your identity providers. This requirement exists needed so that 
+the authentication with Identity providers also participates in the session limits.
+
+NOTE: Currently, the administrator is responsible for maintaining consistency between the different configurations. So make sure that all your flows use same the configuration
+of `User Session Limits`.
+
+NOTE: User session limit feature is not available for CIBA.
 
 ifeval::[{project_community}==true]
 === Script Authenticator

services/src/main/java/org/keycloak/authentication/authenticators/sessionlimits/UserSessionLimitsAuthenticator.java

@@ -2,6 +2,7 @@ package org.keycloak.authentication.authenticators.sessionlimits;
 
 import java.util.Collections;
 import org.jboss.logging.Logger;
+import org.keycloak.authentication.AuthenticationFlowException;
 import org.keycloak.authentication.Authenticator;
 import org.keycloak.authentication.AuthenticationFlowContext;
 import org.keycloak.authentication.AuthenticationFlowError;
@@ -43,6 +44,10 @@ public class UserSessionLimitsAuthenticator implements Authenticator {
     @Override
     public void authenticate(AuthenticationFlowContext context) {
         AuthenticatorConfigModel authenticatorConfig = context.getAuthenticatorConfig();
+        if (authenticatorConfig == null) {
+            throw new AuthenticationFlowException("No configuration found of 'User Session Count Limiter' authenticator. Please make sure to configure this authenticator in your authentication flow in the realm '" + context.getRealm().getName() + "'!"
+                    , AuthenticationFlowError.INTERNAL_ERROR);
+        }
         Map<String, String> config = authenticatorConfig.getConfig();
 
         // Get the configuration for this authenticator

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/sessionlimits/UserSessionLimitsTest.java

@@ -20,6 +20,7 @@ import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.keycloak.authentication.authenticators.browser.CookieAuthenticatorFactory;
 import org.keycloak.authentication.authenticators.browser.UsernamePasswordFormFactory;
 import org.keycloak.authentication.authenticators.sessionlimits.UserSessionLimitsAuthenticatorFactory;
 import org.keycloak.events.Details;
@@ -36,13 +37,17 @@ import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
 import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.admin.ApiUtil;
+import org.keycloak.testsuite.forms.BrowserFlowTest;
+import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.LoginPage;
 import org.keycloak.testsuite.pages.LoginPasswordResetPage;
 import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
+import org.keycloak.testsuite.util.FlowUtil;
 import org.keycloak.testsuite.util.GreenMailRule;
 import org.keycloak.testsuite.util.MailUtils;
 import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.pages.ErrorPage;
+
 import javax.mail.internet.MimeMessage;
 
 import static org.junit.Assert.assertEquals;
@@ -109,6 +114,9 @@ public class UserSessionLimitsTest extends AbstractTestRealmKeycloakTest {
     @Page
     protected LoginPasswordUpdatePage updatePasswordPage;
 
+    @Page
+    protected AppPage appPage;
+
     @Test
     public void testClientSessionCountExceededAndNewSessionDeniedBrowserFlow() throws Exception {
         // Login and verify login was successful
@@ -443,6 +451,54 @@ public class UserSessionLimitsTest extends AbstractTestRealmKeycloakTest {
         }
     }
 
+    // Issue 17374
+    @Test
+    public void testSSOLogin() throws Exception {
+        // Setup authentication flow
+        testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session). copyBrowserFlow("browser-session-limits"));
+        testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session)
+                .selectFlow("browser-session-limits")
+                .clear()
+                        .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.ALTERNATIVE, CookieAuthenticatorFactory.PROVIDER_ID)
+                        .addSubFlowExecution(AuthenticationExecutionModel.Requirement.ALTERNATIVE, subFlow -> {
+                            subFlow.addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernamePasswordFormFactory.PROVIDER_ID);
+                            subFlow.addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UserSessionLimitsAuthenticatorFactory.USER_SESSION_LIMITS,
+                                    config -> {
+                                        config.getConfig().put(UserSessionLimitsAuthenticatorFactory.BEHAVIOR, UserSessionLimitsAuthenticatorFactory.DENY_NEW_SESSION);
+                                        config.getConfig().put(UserSessionLimitsAuthenticatorFactory.USER_REALM_LIMIT, "1");
+                                    });
+                        })
+                .defineAsBrowserFlow()
+        );
+
+        // Login in browser1
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+        assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+        EventRepresentation loginEvent = events.expectLogin().assertEvent();
+        String sessionId1 = loginEvent.getSessionId();
+
+        // SSO login in browser1. Should be still OK (Login won't be denied even if session limit is set to 1 because we are login in same browser for SSO login)
+        oauth.openLoginForm();
+        assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+        loginEvent = events.expectLogin().removeDetail(Details.USERNAME).client("test-app").assertEvent();
+        String sessionId2 = loginEvent.getSessionId();
+        assertEquals(sessionId1, sessionId2);
+
+        // Delete cookies to emulate login in new browser
+        super.deleteCookies();
+
+        // New login should fail due the sessions limit
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+        events.expect(EventType.LOGIN_ERROR).user((String) null).error(Errors.GENERIC_AUTHENTICATION_ERROR).assertEvent();
+        errorPage.assertCurrent();
+        assertEquals("There are too many sessions", errorPage.getError()); // Default error message
+
+        // Revert config of authenticators
+        BrowserFlowTest.revertFlows(adminClient.realm("test"), "browser-session-limits");
+    }
+
     private void setAuthenticatorConfigItem(String alias, String key, String value) {
         testingClient.server().run(session -> {
             RealmModel realm = session.realms().getRealmByName("test");