KEYCLOAK-9050 Change LoginProtocol.authenticated to read most of the values from authenticationSession

server-spi-private/src/main/java/org/keycloak/protocol/LoginProtocol.java

@@ -68,7 +68,7 @@ public interface LoginProtocol extends Provider {
 
     LoginProtocol setEventBuilder(EventBuilder event);
 
-    Response authenticated(UserSessionModel userSession, ClientSessionContext clientSessionCtx);
+    Response authenticated(AuthenticationSessionModel authSession, UserSessionModel userSession, ClientSessionContext clientSessionCtx);
 
     Response sendError(AuthenticationSessionModel authSession, Error error);
 

server-spi/src/main/java/org/keycloak/models/ClientSessionContext.java

@@ -45,6 +45,4 @@ public interface ClientSessionContext {
 
     <T> T getAttribute(String attribute, Class<T> clazz);
 
-
-    String AUTHENTICATION_SESSION_ATTR = "AUTH_SESSION_ATTR";
 }

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

@@ -979,7 +979,7 @@ public class AuthenticationProcessor {
         event.success();
         RealmModel realm = authenticationSession.getRealm();
         ClientSessionContext clientSessionCtx = attachSession();
-        return AuthenticationManager.redirectAfterSuccessfulFlow(session, realm, userSession, clientSessionCtx, request, uriInfo, connection, event, protocol);
+        return AuthenticationManager.redirectAfterSuccessfulFlow(session, realm, userSession, clientSessionCtx, request, uriInfo, connection, event, authenticationSession, protocol);
 
     }
 

services/src/main/java/org/keycloak/protocol/docker/DockerAuthV2Protocol.java

@@ -92,7 +92,7 @@ public class DockerAuthV2Protocol implements LoginProtocol {
     }
 
     @Override
-    public Response authenticated(final UserSessionModel userSession, final ClientSessionContext clientSessionCtx) {
+    public Response authenticated(final AuthenticationSessionModel authSession, final UserSessionModel userSession, final ClientSessionContext clientSessionCtx) {
         // First, create a base response token with realm + user values populated
         final AuthenticatedClientSessionModel clientSession = clientSessionCtx.getClientSession();
         final ClientModel client = clientSession.getClient();
@@ -100,7 +100,7 @@ public class DockerAuthV2Protocol implements LoginProtocol {
         DockerResponseToken responseToken = new DockerResponseToken()
                 .id(KeycloakModelUtils.generateId())
                 .type(TokenUtil.TOKEN_TYPE_BEARER)
-                .issuer(clientSession.getNote(DockerAuthV2Protocol.ISSUER))
+                .issuer(authSession.getClientNote(DockerAuthV2Protocol.ISSUER))
                 .subject(userSession.getUser().getUsername())
                 .issuedNow()
                 .audience(client.getClientId())

services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java

@@ -181,16 +181,16 @@ public class OIDCLoginProtocol implements LoginProtocol {
 
 
     @Override
-    public Response authenticated(UserSessionModel userSession, ClientSessionContext clientSessionCtx) {
+    public Response authenticated(AuthenticationSessionModel authSession, UserSessionModel userSession, ClientSessionContext clientSessionCtx) {
         AuthenticatedClientSessionModel clientSession= clientSessionCtx.getClientSession();
 
-        String responseTypeParam = clientSession.getNote(OIDCLoginProtocol.RESPONSE_TYPE_PARAM);
+        String responseTypeParam = authSession.getClientNote(OIDCLoginProtocol.RESPONSE_TYPE_PARAM);
-        String responseModeParam = clientSession.getNote(OIDCLoginProtocol.RESPONSE_MODE_PARAM);
+        String responseModeParam = authSession.getClientNote(OIDCLoginProtocol.RESPONSE_MODE_PARAM);
         setupResponseTypeAndMode(responseTypeParam, responseModeParam);
 
-        String redirect = clientSession.getRedirectUri();
+        String redirect = authSession.getRedirectUri();
         OIDCRedirectUriBuilder redirectUri = OIDCRedirectUriBuilder.fromUri(redirect, responseMode);
-        String state = clientSession.getNote(OIDCLoginProtocol.STATE_PARAM);
+        String state = authSession.getClientNote(OIDCLoginProtocol.STATE_PARAM);
         logger.debugv("redirectAccessCode: state: {0}", state);
         if (state != null)
             redirectUri.addParam(OAuth2Constants.STATE, state);
@@ -200,12 +200,6 @@ public class OIDCLoginProtocol implements LoginProtocol {
             redirectUri.addParam(OAuth2Constants.SESSION_STATE, userSession.getId());
         }
 
-        AuthenticationSessionModel authSession = clientSessionCtx.getAttribute(ClientSessionContext.AUTHENTICATION_SESSION_ATTR, AuthenticationSessionModel.class);
-        if (authSession == null) {
-            // Shouldn't happen if correctly used
-            throw new IllegalStateException("AuthenticationSession attachement not set in the ClientSessionContext");
-        }
-
         String nonce = authSession.getClientNote(OIDCLoginProtocol.NONCE_PARAM);
         clientSessionCtx.setAttribute(OIDCLoginProtocol.NONCE_PARAM, nonce);
 

services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java

@@ -431,8 +431,6 @@ public class TokenManager {
         new AuthenticationSessionManager(session).removeAuthenticationSession(userSession.getRealm(), authSession, true);
 
         ClientSessionContext clientSessionCtx = DefaultClientSessionContext.fromClientSessionAndClientScopeIds(clientSession, clientScopeIds);
-        clientSessionCtx.setAttribute(ClientSessionContext.AUTHENTICATION_SESSION_ATTR, authSession);
-
         return clientSessionCtx;
     }
 

services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java

@@ -297,8 +297,8 @@ public class SamlProtocol implements LoginProtocol {
         return (logoutRedirectUrl == null || logoutRedirectUrl.trim().isEmpty());
     }
 
-    protected String getNameIdFormat(SamlClient samlClient, AuthenticatedClientSessionModel clientSession) {
+    protected String getNameIdFormat(SamlClient samlClient, AuthenticationSessionModel authSession) {
-        String nameIdFormat = clientSession.getNote(GeneralConstants.NAMEID_FORMAT);
+        String nameIdFormat = authSession.getClientNote(GeneralConstants.NAMEID_FORMAT);
 
         boolean forceFormat = samlClient.forceNameIDFormat();
         String configuredNameIdFormat = samlClient.getNameIDFormat();
@@ -368,20 +368,20 @@ public class SamlProtocol implements LoginProtocol {
     }
 
     @Override
-    public Response authenticated(UserSessionModel userSession, ClientSessionContext clientSessionCtx) {
+    public Response authenticated(AuthenticationSessionModel authSession, UserSessionModel userSession, ClientSessionContext clientSessionCtx) {
         AuthenticatedClientSessionModel clientSession = clientSessionCtx.getClientSession();
         ClientModel client = clientSession.getClient();
         SamlClient samlClient = new SamlClient(client);
-        String requestID = clientSession.getNote(SAML_REQUEST_ID);
+        String requestID = authSession.getClientNote(SAML_REQUEST_ID);
-        String relayState = clientSession.getNote(GeneralConstants.RELAY_STATE);
+        String relayState = authSession.getClientNote(GeneralConstants.RELAY_STATE);
-        String redirectUri = clientSession.getRedirectUri();
+        String redirectUri = authSession.getRedirectUri();
         String responseIssuer = getResponseIssuer(realm);
-        String nameIdFormat = getNameIdFormat(samlClient, clientSession);
+        String nameIdFormat = getNameIdFormat(samlClient, authSession);
-        String nameId = getNameId(nameIdFormat, clientSession, userSession);
+        String nameId = getNameId(nameIdFormat, authSession, userSession);
 
         if (nameId == null) {
             return samlErrorMessage(
-              null, samlClient, isPostBinding(clientSession),
+              null, samlClient, isPostBinding(authSession),
               redirectUri, JBossSAMLURIConstants.STATUS_INVALID_NAMEIDPOLICY, relayState
             );
         }
@@ -426,7 +426,7 @@ public class SamlProtocol implements LoginProtocol {
         Document samlDocument = null;
         KeyManager keyManager = session.keys();
         KeyManager.ActiveRsaKey keys = keyManager.getActiveRsaKey(realm);
-        boolean postBinding = isPostBinding(clientSession);
+        boolean postBinding = isPostBinding(authSession);
         String keyName = samlClient.getXmlSigKeyInfoKeyNameTransformer().getKeyName(keys.getKid(), keys.getCertificate());
 
         try {

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

@@ -733,20 +733,20 @@ public class AuthenticationManager {
     public static Response redirectAfterSuccessfulFlow(KeycloakSession session, RealmModel realm, UserSessionModel userSession,
                                                        ClientSessionContext clientSessionCtx,
                                                 HttpRequest request, UriInfo uriInfo, ClientConnection clientConnection,
-                                                EventBuilder event, String protocol) {
+                                                EventBuilder event, AuthenticationSessionModel authSession) {
-        LoginProtocol protocolImpl = session.getProvider(LoginProtocol.class, protocol);
+        LoginProtocol protocolImpl = session.getProvider(LoginProtocol.class, authSession.getProtocol());
         protocolImpl.setRealm(realm)
                 .setHttpHeaders(request.getHttpHeaders())
                 .setUriInfo(uriInfo)
                 .setEventBuilder(event);
-        return redirectAfterSuccessfulFlow(session, realm, userSession, clientSessionCtx, request, uriInfo, clientConnection, event, protocolImpl);
+        return redirectAfterSuccessfulFlow(session, realm, userSession, clientSessionCtx, request, uriInfo, clientConnection, event, authSession, protocolImpl);
 
     }
 
     public static Response redirectAfterSuccessfulFlow(KeycloakSession session, RealmModel realm, UserSessionModel userSession,
                                                        ClientSessionContext clientSessionCtx,
                                                        HttpRequest request, UriInfo uriInfo, ClientConnection clientConnection,
-                                                       EventBuilder event, LoginProtocol protocol) {
+                                                       EventBuilder event, AuthenticationSessionModel authSession, LoginProtocol protocol) {
         Cookie sessionCookie = request.getHttpHeaders().getCookies().get(AuthenticationManager.KEYCLOAK_SESSION_COOKIE);
         if (sessionCookie != null) {
 
@@ -787,7 +787,7 @@ public class AuthenticationManager {
             clientSession.removeNote(SSO_AUTH);
         }
 
-        return protocol.authenticated(userSession, clientSessionCtx);
+        return protocol.authenticated(authSession, userSession, clientSessionCtx);
 
     }
 
@@ -873,7 +873,7 @@ public class AuthenticationManager {
         event.event(EventType.LOGIN);
         event.session(userSession);
         event.success();
-        return redirectAfterSuccessfulFlow(session, realm, userSession, clientSessionCtx, request, uriInfo, clientConnection, event, authSession.getProtocol());
+        return redirectAfterSuccessfulFlow(session, realm, userSession, clientSessionCtx, request, uriInfo, clientConnection, event, authSession);
     }
 
     // Return null if action is not required. Or the name of the requiredAction in case it is required.

services/src/main/java/org/keycloak/services/resources/LoginActionsService.java

@@ -864,7 +864,7 @@ public class LoginActionsService {
         event.success();
 
         ClientSessionContext clientSessionCtx = AuthenticationProcessor.attachSession(authSession, null, session, realm, clientConnection, event);
-        return AuthenticationManager.redirectAfterSuccessfulFlow(session, realm, clientSessionCtx.getClientSession().getUserSession(), clientSessionCtx, request, session.getContext().getUri(), clientConnection, event, authSession.getProtocol());
+        return AuthenticationManager.redirectAfterSuccessfulFlow(session, realm, clientSessionCtx.getClientSession().getUserSession(), clientSessionCtx, request, session.getContext().getUri(), clientConnection, event, authSession);
     }
 
     private void initLoginEvent(AuthenticationSessionModel authSession) {

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java

@@ -760,6 +760,10 @@ public class OAuthClient {
         return redirectUri;
     }
 
+    public String getState() {
+        return state.getState();
+    }
+
     public String getNonce() {
         return nonce;
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java

@@ -326,7 +326,8 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest {
                     OAuthClient oauth1 = new OAuthClient();
                     oauth1.init(driver);
 
-                    // Add some randomness to nonce and redirectUri. Verify that login is successful and nonce will match
+                    // Add some randomness to state, nonce and redirectUri. Verify that login is successful and "state" and "nonce" will match
+                    oauth1.stateParamHardcoded(KeycloakModelUtils.generateId());
                     oauth1.nonce(KeycloakModelUtils.generateId());
                     oauth1.redirectUri(oauth.getRedirectUri() + "?some=" + new Random().nextInt(1024));
                     return oauth1;
@@ -371,7 +372,12 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest {
             Assert.assertThat(context.getRedirectLocations(), Matchers.notNullValue());
             Assert.assertThat(context.getRedirectLocations(), Matchers.not(Matchers.empty()));
             String currentUrl = context.getRedirectLocations().get(0).toString();
-            String code = getQueryFromUrl(currentUrl).get(OAuth2Constants.CODE);
+
+            Map<String, String> query = getQueryFromUrl(currentUrl);
+            String code = query.get(OAuth2Constants.CODE);
+            String state = query.get(OAuth2Constants.STATE);
+
+            Assert.assertEquals("Invalid state.", state, oauth1.getState());
 
             AtomicReference<OAuthClient.AccessTokenResponse> accessResRef = new AtomicReference<>();
             totalInvocations.incrementAndGet();

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

@@ -37,6 +37,7 @@ import org.openqa.selenium.By;
 
 import javax.ws.rs.core.UriBuilder;
 import java.io.IOException;
+import java.net.URI;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
@@ -169,4 +170,31 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
         String codeId = events.expectLogin().assertEvent().getDetails().get(Details.CODE_ID);
     }
 
+
+    @Test
+    public void authorizationRequestFragmentResponseModeNotKept() throws Exception {
+        // Set response_mode=fragment and login
+        oauth.responseMode(OIDCResponseMode.FRAGMENT.toString().toLowerCase());
+        OAuthClient.AuthorizationEndpointResponse response = oauth.doLogin("test-user@localhost", "password");
+
+        Assert.assertNotNull(response.getCode());
+        Assert.assertNotNull(response.getState());
+
+        URI currentUri = new URI(driver.getCurrentUrl());
+        Assert.assertNull(currentUri.getRawQuery());
+        Assert.assertNotNull(currentUri.getRawFragment());
+
+        // Unset response_mode. The initial OIDC AuthenticationRequest won't contain "response_mode" parameter now and hence it should fallback to "query".
+        oauth.responseMode(null);
+        oauth.openLoginForm();
+        response = new OAuthClient.AuthorizationEndpointResponse(oauth);
+
+        Assert.assertNotNull(response.getCode());
+        Assert.assertNotNull(response.getState());
+
+        currentUri = new URI(driver.getCurrentUrl());
+        Assert.assertNotNull(currentUri.getRawQuery());
+        Assert.assertNull(currentUri.getRawFragment());
+    }
+
 }