Merge pull request #2022 from patriot1burke/master

KEYCLOAK-2098

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

@@ -637,23 +637,9 @@ public class AuthenticationProcessor {
     }
 
     public Response authenticate() throws AuthenticationFlowException {
-        checkClientSession();
         logger.debug("AUTHENTICATE");
-        event.client(clientSession.getClient().getClientId())
+        Response challenge = authenticateOnly();
-                .detail(Details.REDIRECT_URI, clientSession.getRedirectUri())
-                .detail(Details.AUTH_METHOD, clientSession.getAuthMethod());
-        String authType = clientSession.getNote(Details.AUTH_TYPE);
-        if (authType != null) {
-            event.detail(Details.AUTH_TYPE, authType);
-        }
-        UserModel authUser = clientSession.getAuthenticatedUser();
-        validateUser(authUser);
-        AuthenticationFlow authenticationFlow = createFlowExecution(this.flowId, null);
-        Response challenge = authenticationFlow.processFlow();
         if (challenge != null) return challenge;
-        if (clientSession.getAuthenticatedUser() == null) {
-            throw new AuthenticationFlowException(AuthenticationFlowError.UNKNOWN_USER);
-        }
         return authenticationComplete();
     }
 
@@ -668,18 +654,7 @@ public class AuthenticationProcessor {
         }
     }
 
-    public Response createSuccessRedirect() {
+    public static Response redirectToRequiredActions(RealmModel realm, ClientSessionModel clientSession, UriInfo uriInfo) {
-        // redirect to non-action url so browser refresh button works without reposting past data
-        String code = generateCode();
-
-        URI redirect = LoginActionsService.loginActionsBaseUrl(getUriInfo())
-                .path(flowPath)
-                .queryParam(OAuth2Constants.CODE, code).build(getRealm().getName());
-        return Response.status(302).location(redirect).build();
-
-    }
-
-    public static Response createRequiredActionRedirect(RealmModel realm, ClientSessionModel clientSession, UriInfo uriInfo) {
 
         // redirect to non-action url so browser refresh button works without reposting past data
         ClientSessionCode accessCode = new ClientSessionCode(realm, clientSession);
@@ -722,8 +697,8 @@ public class AuthenticationProcessor {
         String current = clientSession.getNote(CURRENT_AUTHENTICATION_EXECUTION);
         if (!execution.equals(current)) {
             logger.debug("Current execution does not equal executed execution.  Might be a page refresh");
-            logFailure();
+            //logFailure();
-            resetFlow(clientSession);
+            //resetFlow(clientSession);
             return authenticate();
         }
         UserModel authUser = clientSession.getAuthenticatedUser();
@@ -766,7 +741,7 @@ public class AuthenticationProcessor {
 
     public Response authenticateOnly() throws AuthenticationFlowException {
         logger.debug("AUTHENTICATE ONLY");
-       checkClientSession();
+        checkClientSession();
         event.client(clientSession.getClient().getClientId())
                 .detail(Details.REDIRECT_URI, clientSession.getRedirectUri())
                 .detail(Details.AUTH_METHOD, clientSession.getAuthMethod());
@@ -778,14 +753,13 @@ public class AuthenticationProcessor {
         validateUser(authUser);
         AuthenticationFlow authenticationFlow = createFlowExecution(this.flowId, null);
         Response challenge = authenticationFlow.processFlow();
+        if (challenge != null) return challenge;
+        if (clientSession.getAuthenticatedUser() == null) {
+            throw new AuthenticationFlowException(AuthenticationFlowError.UNKNOWN_USER);
+        }
         return challenge;
     }
 
-    public Response attachSessionExecutionRequiredActions() {
-        attachSession();
-        return AuthenticationManager.actionRequired(session, userSession, clientSession, connection, request, uriInfo, event);
-    }
-
     public void attachSession() {
         String username = clientSession.getAuthenticatedUser().getUsername();
         String attemptedUsername = clientSession.getNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME);
@@ -829,9 +803,16 @@ public class AuthenticationProcessor {
 
     protected Response authenticationComplete() {
         attachSession();
-        return createRequiredActionRedirect(realm, clientSession, uriInfo);
+        if (isActionRequired()) {
-        //return AuthenticationManager.nextActionAfterAuthentication(session, userSession, clientSession, connection, request, uriInfo, event);
+            return redirectToRequiredActions(realm, clientSession, uriInfo);
+        } else {
+            event.detail(Details.CODE_ID, clientSession.getId());  // todo This should be set elsewhere.  find out why tests fail.  Don't know where this is supposed to be set
+            return AuthenticationManager.finishedRequiredActions(session,  userSession, clientSession, connection, request, uriInfo, event);
+        }
+    }
 
+    public boolean isActionRequired() {
+        return AuthenticationManager.isActionRequired(session, userSession, clientSession, connection, request, uriInfo, event);
     }
 
     public AuthenticationProcessor.Result createAuthenticatorContext(AuthenticationExecutionModel model, Authenticator authenticator, List<AuthenticationExecutionModel> executions) {

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

@@ -6,17 +6,16 @@ import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientSessionModel;
 import org.keycloak.models.UserModel;
 
-import static org.keycloak.authentication.FlowStatus.*;
-
 import javax.ws.rs.core.Response;
-
 import java.util.Iterator;
 import java.util.List;
 
+import static org.keycloak.authentication.FlowStatus.SUCCESS;
+
 /**
-* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
-* @version $Revision: 1 $
+ * @version $Revision: 1 $
-*/
+ */
 public class DefaultAuthenticationFlow implements AuthenticationFlow {
     protected static Logger logger = Logger.getLogger(DefaultAuthenticationFlow.class);
     Response alternativeChallenge = null;
@@ -70,14 +69,9 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
                 authenticator.action(result);
                 Response response = processResult(result);
                 if (response == null) {
-                    if (result.status == SUCCESS && processor.isBrowserFlow()) {
+                    processor.getClientSession().removeNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION);
-                         // redirect to a non-action URL so browser refresh works without reposting.
+                    return processFlow();
-                         return processor.createSuccessRedirect();
+                } else return response;
-                    } else {
-                        return processFlow();
-                    }
-                }
-                else return response;
             }
         }
         throw new AuthenticationFlowException("action is not in current execution", AuthenticationFlowError.INTERNAL_ERROR);

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

@@ -220,9 +220,8 @@ public class FormAuthenticationFlow implements AuthenticationFlow {
 
         }
         processor.getClientSession().setExecutionStatus(actionExecution, ClientSessionModel.ExecutionStatus.SUCCESS);
-
+        processor.getClientSession().removeNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION);
-        // redirect to no execution so browser refresh button works without reposting past data
+        return null;
-        return processor.createSuccessRedirect();
     }
 
     public URI getActionUrl(String executionId, String code) {

services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java

@@ -11,6 +11,7 @@ import org.jboss.logging.Logger;
 import org.jboss.resteasy.spi.HttpRequest;
 import org.keycloak.authentication.AuthenticationProcessor;
 import org.keycloak.common.ClientConnection;
+import org.keycloak.events.Details;
 import org.keycloak.events.EventBuilder;
 import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientSessionModel;
@@ -90,33 +91,29 @@ public abstract class AuthorizationEndpointBase {
         AuthenticationFlowModel flow = getAuthenticationFlow();
         String flowId = flow.getId();
         AuthenticationProcessor processor = createProcessor(clientSession, flowId, LoginActionsService.AUTHENTICATE_PATH);
-
+        event.detail(Details.CODE_ID, clientSession.getId());
         if (isPassive) {
             // OIDC prompt == NONE or SAML 2 IsPassive flag
             // This means that client is just checking if the user is already completely logged in.
             // We cancel login if any authentication action or required action is required
-            Response challenge = null;
-            Response challenge2 = null;
             try {
-                challenge = processor.authenticateOnly();
+                if (processor.authenticateOnly() == null) {
-                if (challenge == null) {
+                    processor.attachSession();
-                    challenge2 = processor.attachSessionExecutionRequiredActions();
+                } else {
+                    Response response = protocol.sendError(clientSession, Error.PASSIVE_LOGIN_REQUIRED);
+                    session.sessions().removeClientSession(realm, clientSession);
+                    return response;
+                }
+                if (processor.isActionRequired()) {
+                    Response response = protocol.sendError(clientSession, Error.PASSIVE_INTERACTION_REQUIRED);
+                    session.sessions().removeClientSession(realm, clientSession);
+                    return response;
+
                 }
             } catch (Exception e) {
                 return processor.handleBrowserException(e);
             }
-
+            return processor.finishAuthentication(protocol);
-            if (challenge != null || challenge2 != null) {
-                if (processor.isUserSessionCreated()) {
-                    session.sessions().removeUserSession(realm, processor.getUserSession());
-                }
-                if (challenge != null)
-                    return protocol.sendError(clientSession, Error.PASSIVE_LOGIN_REQUIRED);
-                else
-                    return protocol.sendError(clientSession, Error.PASSIVE_INTERACTION_REQUIRED);
-            } else {
-                return processor.finishAuthentication(protocol);
-            }
         } else {
             try {
                 RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession);

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

@@ -421,12 +421,16 @@ public class AuthenticationManager {
         return protocol.authenticated(userSession, new ClientSessionCode(realm, clientSession));
 
     }
-
     public static Response nextActionAfterAuthentication(KeycloakSession session, UserSessionModel userSession, ClientSessionModel clientSession,
                                                   ClientConnection clientConnection,
                                                   HttpRequest request, UriInfo uriInfo, EventBuilder event) {
         Response requiredAction = actionRequired(session, userSession, clientSession, clientConnection, request, uriInfo, event);
         if (requiredAction != null) return requiredAction;
+        return finishedRequiredActions(session, userSession, clientSession, clientConnection, request, uriInfo, event);
+
+    }
+
+    public static Response finishedRequiredActions(KeycloakSession session, UserSessionModel userSession, ClientSessionModel clientSession, ClientConnection clientConnection, HttpRequest request, UriInfo uriInfo, EventBuilder event) {
         if (clientSession.getNote(END_AFTER_REQUIRED_ACTIONS) != null) {
             Response response = session.getProvider(LoginFormsProvider.class)
                     .setAttribute("skipLink", true)
@@ -439,9 +443,50 @@ public class AuthenticationManager {
         event.success();
         RealmModel realm = clientSession.getRealm();
         return redirectAfterSuccessfulFlow(session, realm , userSession, clientSession, request, uriInfo, clientConnection, event);
+    }
+
+    public static boolean isActionRequired(final KeycloakSession session, final UserSessionModel userSession, final ClientSessionModel clientSession,
+                                           final ClientConnection clientConnection,
+                                           final HttpRequest request, final UriInfo uriInfo, final EventBuilder event) {
+        final RealmModel realm = clientSession.getRealm();
+        final UserModel user = userSession.getUser();
+        final ClientModel client = clientSession.getClient();
+
+        evaluateRequiredActionTriggers(session, userSession, clientSession, clientConnection, request, uriInfo, event, realm, user);
+
+        if (!user.getRequiredActions().isEmpty() || !clientSession.getRequiredActions().isEmpty()) return true;
+
+        if (client.isConsentRequired()) {
+
+            UserConsentModel grantedConsent = user.getConsentByClient(client.getId());
+
+            ClientSessionCode accessCode = new ClientSessionCode(realm, clientSession);
+            for (RoleModel r : accessCode.getRequestedRoles()) {
+
+                // Consent already granted by user
+                if (grantedConsent != null && grantedConsent.isRoleGranted(r)) {
+                    continue;
+                }
+                return true;
+             }
+
+            for (ProtocolMapperModel protocolMapper : accessCode.getRequestedProtocolMappers()) {
+                if (protocolMapper.isConsentRequired() && protocolMapper.getConsentText() != null) {
+                    if (grantedConsent == null || !grantedConsent.isProtocolMapperGranted(protocolMapper)) {
+                        return true;
+                    }
+                }
+            }
+            String consentDetail = (grantedConsent != null) ? Details.CONSENT_VALUE_PERSISTED_CONSENT : Details.CONSENT_VALUE_NO_CONSENT_REQUIRED;
+            event.detail(Details.CONSENT, consentDetail);
+        } else {
+            event.detail(Details.CONSENT, Details.CONSENT_VALUE_NO_CONSENT_REQUIRED);
+        }
+        return false;
 
     }
 
+
     public static Response actionRequired(final KeycloakSession session, final UserSessionModel userSession, final ClientSessionModel clientSession,
                                                          final ClientConnection clientConnection,
                                                          final HttpRequest request, final UriInfo uriInfo, final EventBuilder event) {

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

@@ -523,7 +523,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
             LOGGER.debugf("Performing local authentication for user [%s].", federatedUser);
         }
 
-        return AuthenticationProcessor.createRequiredActionRedirect(realmModel, clientSession, uriInfo);
+        return AuthenticationProcessor.redirectToRequiredActions(realmModel, clientSession, uriInfo);
     }
 
 

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

@@ -697,7 +697,7 @@ public class LoginActionsService {
 
             event = event.clone().removeDetail(Details.EMAIL).event(EventType.LOGIN);
 
-            return AuthenticationProcessor.createRequiredActionRedirect(realm, clientSession, uriInfo);
+            return AuthenticationProcessor.redirectToRequiredActions(realm, clientSession, uriInfo);
         } else {
             Checks checks = new Checks();
             if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name(), ClientSessionCode.ActionType.USER)) {
@@ -740,7 +740,7 @@ public class LoginActionsService {
             clientSession.getUserSession().getUser().setEmailVerified(true);
             clientSession.setNote(AuthenticationManager.END_AFTER_REQUIRED_ACTIONS, "true");
             clientSession.setNote(ClientSessionModel.Action.EXECUTE_ACTIONS.name(), "true");
-            return AuthenticationProcessor.createRequiredActionRedirect(realm, clientSession, uriInfo);
+            return AuthenticationProcessor.redirectToRequiredActions(realm, clientSession, uriInfo);
         } else {
             event.error(Errors.INVALID_CODE);
             return ErrorPage.error(session, Messages.INVALID_CODE);
@@ -825,9 +825,9 @@ public class LoginActionsService {
         }
 
         if (!action.equals(clientSession.getNote(AuthenticationManager.CURRENT_REQUIRED_ACTION))) {
-            logger.error("required action doesn't match current required action");
+            logger.debug("required action doesn't match current required action");
-            event.error(Errors.INVALID_CODE);
+            clientSession.removeNote(AuthenticationManager.CURRENT_REQUIRED_ACTION);
-            throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
+            redirectToRequiredActions(code);
         }
 
         RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, action);
@@ -850,15 +850,17 @@ public class LoginActionsService {
         };
         provider.processAction(context);
         if (context.getStatus() == RequiredActionContext.Status.SUCCESS) {
-            event.success();
+            event.clone().success();
             // do both
             clientSession.removeRequiredAction(factory.getId());
             clientSession.getUserSession().getUser().removeRequiredAction(factory.getId());
+            clientSession.removeNote(AuthenticationManager.CURRENT_REQUIRED_ACTION);
             // redirect to a generic code URI so that browser refresh will work
-            URI redirect = LoginActionsService.loginActionsBaseUrl(uriInfo)
+            //return redirectToRequiredActions(code);
-                    .path(LoginActionsService.REQUIRED_ACTION)
+            event.removeDetail(Details.CUSTOM_REQUIRED_ACTION);
-                    .queryParam(OAuth2Constants.CODE, code).build(realm.getName());
+            initEvent(clientSession);
-            return Response.status(302).location(redirect).build();
+            event.event(EventType.LOGIN);
+            return AuthenticationManager.nextActionAfterAuthentication(session, clientSession.getUserSession(), clientSession, clientConnection, request, uriInfo, event);
        }
         if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) {
             return context.getChallenge();
@@ -880,4 +882,11 @@ public class LoginActionsService {
         throw new RuntimeException("Unreachable");
     }
 
+    public Response redirectToRequiredActions(String code) {
+        URI redirect = LoginActionsService.loginActionsBaseUrl(uriInfo)
+                .path(LoginActionsService.REQUIRED_ACTION)
+                .queryParam(OAuth2Constants.CODE, code).build(realm.getName());
+        return Response.status(302).location(redirect).build();
+    }
+
 }