Merge pull request #1730 from patriot1burke/master

KEYCLOAK-1908

forms/common-themes/src/main/resources/theme/base/login/login-totp.ftl

@@ -25,6 +25,7 @@
                 <div id="kc-form-buttons" class="${properties.kcFormButtonsClass!}">
                     <div class="${properties.kcFormButtonsWrapperClass!}">
                         <input class="${properties.kcButtonClass!} ${properties.kcButtonPrimaryClass!} ${properties.kcButtonLargeClass!}" name="login" id="kc-login" type="submit" value="${msg("doLogIn")}"/>
+                        <input class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonLargeClass!}" name="cancel" id="kc-cancel" type="submit" value="${msg("doCancel")}"/>
                     </div>
                 </div>
             </div>

model/api/src/main/java/org/keycloak/models/ClientSessionModel.java

@@ -98,7 +98,8 @@ public interface ClientSessionModel {
         SOCIAL_CALLBACK,
         LOGGED_OUT,
         RESET_CREDENTIALS,
-        EXECUTE_ACTIONS
+        EXECUTE_ACTIONS,
+        REQUIRED_ACTIONS
     }
 
     public enum ExecutionStatus {

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

@@ -71,6 +71,12 @@ public interface AuthenticationFlowContext extends AbstractAuthenticationFlowCon
      */
     void cancelLogin();
 
+    /**
+     * Reset the current flow to the beginning and restarts it.
+     *
+     */
+    void resetFlow();
+
     /**
      * Fork the current flow.  The client session will be cloned and set to point at the realm's browser login flow.  The Response will be the result
      * of this fork.  The previous flow will still be set at the current execution.  This is used by reset password when it sends an email.

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

@@ -55,6 +55,7 @@ public class AuthenticationProcessor {
     protected HttpRequest request;
     protected String flowId;
     protected String flowPath;
+    protected boolean browserFlow;
     /**
      * This could be an error message forwarded from another authenticator
      */
@@ -73,6 +74,15 @@ public class AuthenticationProcessor {
     public AuthenticationProcessor() {
     }
 
+    public boolean isBrowserFlow() {
+        return browserFlow;
+    }
+
+    public AuthenticationProcessor setBrowserFlow(boolean browserFlow) {
+        this.browserFlow = browserFlow;
+        return this;
+    }
+
     public RealmModel getRealm() {
         return realm;
     }
@@ -454,6 +464,11 @@ public class AuthenticationProcessor {
             forceChallenge(response);
         }
 
+        @Override
+        public void resetFlow() {
+            this.status = FlowStatus.FLOW_RESET;
+        }
+
         @Override
         public void fork() {
             this.status = FlowStatus.FORK;
@@ -640,6 +655,31 @@ public class AuthenticationProcessor {
         }
     }
 
+    public Response createSuccessRedirect() {
+        // 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);
+        accessCode.setAction(ClientSessionModel.Action.REQUIRED_ACTIONS.name());
+        clientSession.setTimestamp(Time.currentTime());
+
+        URI redirect = LoginActionsService.loginActionsBaseUrl(uriInfo)
+                .path(LoginActionsService.REQUIRED_ACTION)
+                .queryParam(OAuth2Constants.CODE, accessCode.getCode()).build(realm.getName());
+        return Response.status(302).location(redirect).build();
+
+    }
+
     public static void resetFlow(ClientSessionModel clientSession) {
         clientSession.setTimestamp(Time.currentTime());
         clientSession.setAuthenticatedUser(null);
@@ -773,7 +813,8 @@ public class AuthenticationProcessor {
 
     protected Response authenticationComplete() {
         attachSession();
-        return AuthenticationManager.nextActionAfterAuthentication(session, userSession, clientSession, connection, request, uriInfo, event);
+        return createRequiredActionRedirect(realm, clientSession, uriInfo);
+        //return AuthenticationManager.nextActionAfterAuthentication(session, userSession, clientSession, connection, request, uriInfo, event);
 
     }
 

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

@@ -1,11 +1,19 @@
 package org.keycloak.authentication;
 
+import org.keycloak.OAuth2Constants;
 import org.keycloak.models.AuthenticationExecutionModel;
 import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientSessionModel;
 import org.keycloak.models.UserModel;
+import org.keycloak.services.managers.ClientSessionCode;
+import org.keycloak.services.resources.LoginActionsService;
+import org.keycloak.util.Time;
+import org.omg.PortableInterceptor.SUCCESSFUL;
+
+import static org.keycloak.authentication.FlowStatus.*;
 
 import javax.ws.rs.core.Response;
+import java.net.URI;
 import java.util.Iterator;
 import java.util.List;
 
@@ -61,7 +69,14 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
                 AuthenticationProcessor.Result result = processor.createAuthenticatorContext(model, authenticator, executions);
                 authenticator.action(result);
                 Response response = processResult(result);
-                if (response == null) return processFlow();
+                if (response == null) {
+                    if (result.status == SUCCESS && processor.isBrowserFlow()) {
+                         // redirect to a non-action URL so browser refresh works without reposting.
+                         return processor.createSuccessRedirect();
+                    } else {
+                        return processFlow();
+                    }
+                }
                 else return response;
             }
         }
@@ -153,62 +168,65 @@ public class DefaultAuthenticationFlow implements AuthenticationFlow {
     public Response processResult(AuthenticationProcessor.Result result) {
         AuthenticationExecutionModel execution = result.getExecution();
         FlowStatus status = result.getStatus();
-        if (status == FlowStatus.SUCCESS) {
+        switch (status) {
-            AuthenticationProcessor.logger.debugv("authenticator SUCCESS: {0}", execution.getAuthenticator());
+            case SUCCESS:
-            processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.SUCCESS);
+                AuthenticationProcessor.logger.debugv("authenticator SUCCESS: {0}", execution.getAuthenticator());
-            if (execution.isAlternative()) alternativeSuccessful = true;
+                processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.SUCCESS);
-            return null;
+                if (execution.isAlternative()) alternativeSuccessful = true;
-        } else if (status == FlowStatus.FAILED) {
+                return null;
-            AuthenticationProcessor.logger.debugv("authenticator FAILED: {0}", execution.getAuthenticator());
+            case FAILED:
-            processor.logFailure();
+                AuthenticationProcessor.logger.debugv("authenticator FAILED: {0}", execution.getAuthenticator());
-            processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.FAILED);
+                processor.logFailure();
-            if (result.getChallenge() != null) {
+                processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.FAILED);
-                return sendChallenge(result, execution);
+                if (result.getChallenge() != null) {
-            }
+                    return sendChallenge(result, execution);
-            throw new AuthenticationFlowException(result.getError());
+                }
-        } else if (status == FlowStatus.FORK) {
+                throw new AuthenticationFlowException(result.getError());
-            AuthenticationProcessor.logger.debugv("reset browser login from authenticator: {0}", execution.getAuthenticator());
+            case FORK:
-            processor.getClientSession().setNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION, execution.getId());
+                AuthenticationProcessor.logger.debugv("reset browser login from authenticator: {0}", execution.getAuthenticator());
-            throw new ForkFlowException(result.getSuccessMessage(), result.getErrorMessage());
+                processor.getClientSession().setNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION, execution.getId());
-        } else if (status == FlowStatus.FORCE_CHALLENGE) {
+                throw new ForkFlowException(result.getSuccessMessage(), result.getErrorMessage());
-            processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
+            case FORCE_CHALLENGE:
-            return sendChallenge(result, execution);
-        } else if (status == FlowStatus.CHALLENGE) {
-            AuthenticationProcessor.logger.debugv("authenticator CHALLENGE: {0}", execution.getAuthenticator());
-            if (execution.isRequired()) {
                 processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
                 return sendChallenge(result, execution);
-            }
+            case CHALLENGE:
-            UserModel authenticatedUser = processor.getClientSession().getAuthenticatedUser();
+                AuthenticationProcessor.logger.debugv("authenticator CHALLENGE: {0}", execution.getAuthenticator());
-            if (execution.isOptional() && authenticatedUser != null && result.getAuthenticator().configuredFor(processor.getSession(), processor.getRealm(), authenticatedUser)) {
+                if (execution.isRequired()) {
+                    processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
+                    return sendChallenge(result, execution);
+                }
+                UserModel authenticatedUser = processor.getClientSession().getAuthenticatedUser();
+                if (execution.isOptional() && authenticatedUser != null && result.getAuthenticator().configuredFor(processor.getSession(), processor.getRealm(), authenticatedUser)) {
+                    processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
+                    return sendChallenge(result, execution);
+                }
+                if (execution.isAlternative()) {
+                    alternativeChallenge = result.getChallenge();
+                    challengedAlternativeExecution = execution;
+                } else {
+                    processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.SKIPPED);
+                }
+                return null;
+            case FAILURE_CHALLENGE:
+                AuthenticationProcessor.logger.debugv("authenticator FAILURE_CHALLENGE: {0}", execution.getAuthenticator());
+                processor.logFailure();
                 processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
                 return sendChallenge(result, execution);
-            }
+            case ATTEMPTED:
-            if (execution.isAlternative()) {
+                AuthenticationProcessor.logger.debugv("authenticator ATTEMPTED: {0}", execution.getAuthenticator());
-                alternativeChallenge = result.getChallenge();
+                if (execution.getRequirement() == AuthenticationExecutionModel.Requirement.REQUIRED) {
-                challengedAlternativeExecution = execution;
+                    throw new AuthenticationFlowException(AuthenticationFlowError.INVALID_CREDENTIALS);
-            } else {
+                }
-                processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.SKIPPED);
+                processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.ATTEMPTED);
-            }
+                return null;
-            return null;
+            case FLOW_RESET:
-        } else if (status == FlowStatus.FAILURE_CHALLENGE) {
+                AuthenticationProcessor.resetFlow(processor.getClientSession());
-            AuthenticationProcessor.logger.debugv("authenticator FAILURE_CHALLENGE: {0}", execution.getAuthenticator());
+                return processor.authenticate();
-            processor.logFailure();
+            default:
-            processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
+                AuthenticationProcessor.logger.debugv("authenticator INTERNAL_ERROR: {0}", execution.getAuthenticator());
-            return sendChallenge(result, execution);
+                AuthenticationProcessor.logger.error("Unknown result status");
-        } else if (status == FlowStatus.ATTEMPTED) {
+                throw new AuthenticationFlowException(AuthenticationFlowError.INTERNAL_ERROR);
-            AuthenticationProcessor.logger.debugv("authenticator ATTEMPTED: {0}", execution.getAuthenticator());
-            if (execution.getRequirement() == AuthenticationExecutionModel.Requirement.REQUIRED) {
-                throw new AuthenticationFlowException(AuthenticationFlowError.INVALID_CREDENTIALS);
-            }
-            processor.getClientSession().setExecutionStatus(execution.getId(), ClientSessionModel.ExecutionStatus.ATTEMPTED);
-            return null;
-        } else {
-            AuthenticationProcessor.logger.debugv("authenticator INTERNAL_ERROR: {0}", execution.getAuthenticator());
-            AuthenticationProcessor.logger.error("Unknown result status");
-            throw new AuthenticationFlowException(AuthenticationFlowError.INTERNAL_ERROR);
         }
-
     }
 
     public Response sendChallenge(AuthenticationProcessor.Result result, AuthenticationExecutionModel execution) {

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

@@ -48,6 +48,13 @@ public enum FlowStatus {
      * This flow is being forked.  The current client session is being cloned, reset, and redirected to browser login.
      *
      */
-    FORK
+    FORK,
+
+    /**
+     * This flow was reset to the beginning.  An example is hitting cancel on the OTP page which will bring you back to the
+     * username password page.
+     *
+     */
+    FLOW_RESET
 
 }

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

@@ -219,8 +219,10 @@ public class FormAuthenticationFlow implements AuthenticationFlow {
             action.setRequiredActions(processor.getSession(), processor.getRealm(), processor.getClientSession().getAuthenticatedUser());
 
         }
-        return null;
+        processor.getClientSession().setExecutionStatus(actionExecution, ClientSessionModel.ExecutionStatus.SUCCESS);
 
+        // redirect to no execution so browser refresh button works without reposting past data
+        return processor.createSuccessRedirect();
     }
 
     public URI getActionUrl(String executionId, String code) {

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

@@ -86,7 +86,7 @@ public interface RequiredActionContext {
      *
      * @return
      */
-    String generateAccessCode(String action);
+    String generateCode();
 
     Status getStatus();
 

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

@@ -12,6 +12,7 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.resources.LoginActionsService;
+import org.keycloak.util.Time;
 
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
@@ -92,13 +93,6 @@ public class RequiredActionContextResult implements RequiredActionContext {
         return httpRequest;
     }
 
-    @Override
-    public String generateAccessCode(String action) {
-        ClientSessionCode code = new ClientSessionCode(getRealm(), getClientSession());
-        code.setAction(action);
-        return code.getCode();
-    }
-
     @Override
     public Status getStatus() {
         return status;
@@ -135,16 +129,24 @@ public class RequiredActionContextResult implements RequiredActionContext {
                 .build(getRealm().getName());
     }
 
+    @Override
+    public String generateCode() {
+        ClientSessionCode accessCode = new ClientSessionCode(getRealm(), getClientSession());
+        clientSession.setTimestamp(Time.currentTime());
+        return accessCode.getCode();
+    }
+
+
     @Override
     public URI getActionUrl() {
-        String accessCode = generateAccessCode(factory.getId());
+        String accessCode = generateCode();
         return getActionUrl(accessCode);
 
     }
 
     @Override
     public LoginFormsProvider form() {
-        String accessCode = generateAccessCode(factory.getId());
+        String accessCode = generateCode();
         URI action = getActionUrl(accessCode);
         LoginFormsProvider provider = getSession().getProvider(LoginFormsProvider.class)
                 .setUser(getUser())

services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java

@@ -37,6 +37,10 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl
 
     public void validateOTP(AuthenticationFlowContext context) {
         MultivaluedMap<String, String> inputData = context.getHttpRequest().getDecodedFormParameters();
+        if (inputData.containsKey("cancel")) {
+            context.resetFlow();
+            return;
+        }
         List<UserCredentialModel> credentials = new LinkedList<>();
         String password = inputData.getFirst(CredentialRepresentation.TOTP);
         if (password == null) {

services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java

@@ -105,6 +105,8 @@ public class ResetCredentialEmail implements Authenticator, AuthenticatorFactory
             context.failure(AuthenticationFlowError.INTERNAL_ERROR, challenge);
             return;
         }
+        // We now know email is valid, so set it to valid.
+        context.getUser().setEmailVerified(true);
         context.success();
     }
 

services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java

@@ -36,9 +36,7 @@ public class VerifyEmail implements RequiredActionProvider, RequiredActionFactor
     }
     @Override
     public void requiredActionChallenge(RequiredActionContext context) {
-        // if this is EXECUTE_ACTIONS we know that the email sent is valid so we can verify it automatically
+        if (context.getUser().isEmailVerified()) {
-        if (context.getClientSession().getNote(ClientSessionModel.Action.EXECUTE_ACTIONS.name()) != null) {
-            context.getUser().setEmailVerified(true);
             context.success();
             return;
         }
@@ -52,7 +50,7 @@ public class VerifyEmail implements RequiredActionProvider, RequiredActionFactor
         LoginActionsService.createActionCookie(context.getRealm(), context.getUriInfo(), context.getConnection(), context.getUserSession().getId());
 
         LoginFormsProvider loginFormsProvider = context.getSession().getProvider(LoginFormsProvider.class)
-                .setClientSessionCode(context.generateAccessCode(UserModel.RequiredAction.VERIFY_EMAIL.name()))
+                .setClientSessionCode(context.generateCode())
                 .setUser(context.getUser());
         Response challenge = loginFormsProvider.createResponse(UserModel.RequiredAction.VERIFY_EMAIL);
         context.challenge(challenge);

services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java

@@ -22,10 +22,10 @@ import org.keycloak.protocol.RestartLoginCookie;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.utils.RedirectUtils;
 import org.keycloak.services.ErrorPageException;
+import org.keycloak.services.Urls;
 import org.keycloak.services.managers.AuthenticationManager;
 import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.messages.Messages;
-import org.keycloak.services.Urls;
 import org.keycloak.services.resources.LoginActionsService;
 
 import javax.ws.rs.GET;
@@ -174,7 +174,7 @@ public class AuthorizationEndpoint {
     private void checkClient() {
         if (clientId == null) {
             event.error(Errors.INVALID_REQUEST);
-            throw new ErrorPageException(session, Messages.MISSING_PARAMETER, OIDCLoginProtocol.CLIENT_ID_PARAM );
+            throw new ErrorPageException(session, Messages.MISSING_PARAMETER, OIDCLoginProtocol.CLIENT_ID_PARAM);
         }
 
         event.client(clientId);
@@ -182,12 +182,12 @@ public class AuthorizationEndpoint {
         client = realm.getClientByClientId(clientId);
         if (client == null) {
             event.error(Errors.CLIENT_NOT_FOUND);
-            throw new ErrorPageException(session, Messages.CLIENT_NOT_FOUND );
+            throw new ErrorPageException(session, Messages.CLIENT_NOT_FOUND);
         }
 
         if (client.isBearerOnly()) {
             event.error(Errors.NOT_ALLOWED);
-            throw new ErrorPageException(session, Messages.BEARER_ONLY );
+            throw new ErrorPageException(session, Messages.BEARER_ONLY);
         }
 
         if (client.isDirectGrantsOnly()) {
@@ -204,7 +204,7 @@ public class AuthorizationEndpoint {
                 responseType = legacyResponseType;
             } else {
                 event.error(Errors.INVALID_REQUEST);
-                throw new ErrorPageException(session, Messages.MISSING_PARAMETER, OIDCLoginProtocol.RESPONSE_TYPE_PARAM );
+                throw new ErrorPageException(session, Messages.MISSING_PARAMETER, OIDCLoginProtocol.RESPONSE_TYPE_PARAM);
             }
         }
 
@@ -216,7 +216,7 @@ public class AuthorizationEndpoint {
             }
         } else {
             event.error(Errors.INVALID_REQUEST);
-            throw new ErrorPageException(session, Messages.INVALID_PARAMETER, OIDCLoginProtocol.RESPONSE_TYPE_PARAM );
+            throw new ErrorPageException(session, Messages.INVALID_PARAMETER, OIDCLoginProtocol.RESPONSE_TYPE_PARAM);
         }
     }
 
@@ -280,29 +280,43 @@ public class AuthorizationEndpoint {
         String flowId = flow.getId();
         AuthenticationProcessor processor = createProcessor(flowId, LoginActionsService.AUTHENTICATE_PATH);
 
-        Response challenge = null;
+        if (prompt != null && prompt.equals("none")) {
-        try {
+            // OIDC prompt == NONE
-            challenge = processor.authenticateOnly();
+            // This means that client is just checking if the user is already completely logged in.
+            //
+            // here we cancel login if any authentication action or required action is required
+            Response challenge = null;
+            try {
+                challenge = processor.authenticateOnly();
+                if (challenge == null) {
+                    challenge = processor.attachSessionExecutionRequiredActions();
+                }
+            } catch (Exception e) {
+                return processor.handleBrowserException(e);
+            }
+
+            if (challenge != null) {
+                if (processor.isUserSessionCreated()) {
+                    session.sessions().removeUserSession(realm, processor.getUserSession());
+                }
+                OIDCLoginProtocol oauth = new OIDCLoginProtocol(session, realm, uriInfo, headers, event);
+                return oauth.cancelLogin(clientSession);
+            }
+
             if (challenge == null) {
-                challenge = processor.attachSessionExecutionRequiredActions();
+                return processor.finishAuthentication();
+            } else {
+                RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession);
+                return challenge;
             }
-        } catch (Exception e) {
-            return processor.handleBrowserException(e);
-        }
-
-        if (challenge != null && prompt != null && prompt.equals("none")) {
-            if (processor.isUserSessionCreated()) {
-                session.sessions().removeUserSession(realm, processor.getUserSession());
-            }
-            OIDCLoginProtocol oauth = new OIDCLoginProtocol(session, realm, uriInfo, headers, event);
-            return oauth.cancelLogin(clientSession);
-        }
-
-        if (challenge == null) {
-            return processor.finishAuthentication();
         } else {
-            RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession);
+            try {
-            return challenge;
+                RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession);
+                return processor.authenticate();
+            } catch (Exception e) {
+                return processor.handleBrowserException(e);
+            }
+
         }
     }
 

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

@@ -68,6 +68,7 @@ public class AuthenticationManager {
     public static final String KEYCLOAK_SESSION_COOKIE = "KEYCLOAK_SESSION";
     public static final String KEYCLOAK_REMEMBER_ME = "KEYCLOAK_REMEMBER_ME";
     public static final String KEYCLOAK_LOGOUT_PROTOCOL = "KEYCLOAK_LOGOUT_PROTOCOL";
+    public static final String CURRENT_REQUIRED_ACTION = "CURRENT_REQUIRED_ACTION";
 
     protected BruteForceProtector protector;
 
@@ -525,6 +526,7 @@ public class AuthenticationManager {
                 return protocol.consentDenied(context.getClientSession());
             }
             else if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) {
+                clientSession.setNote(CURRENT_REQUIRED_ACTION, model.getProviderId());
                 return context.getChallenge();
             }
             else if (context.getStatus() == RequiredActionContext.Status.SUCCESS) {

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

@@ -73,8 +73,12 @@ public class ClientSessionCode {
     }
 
     public static ParseResult parseResult(String code, KeycloakSession session, RealmModel realm) {
+        ParseResult result = new ParseResult();
+        if (code == null) {
+            result.illegalHash = true;
+            return result;
+        }
         try {
-            ParseResult result = new ParseResult();
             String[] parts = code.split("\\.");
             String id = parts[1];
 
@@ -93,7 +97,8 @@ public class ClientSessionCode {
             result.code = new ClientSessionCode(realm, clientSession);
             return result;
         } catch (RuntimeException e) {
-            return null;
+            result.illegalHash = true;
+            return result;
         }
     }
 

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

@@ -322,8 +322,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
             LOGGER.debugf("Performing local authentication for user [%s].", federatedUser);
         }
 
-        return AuthenticationManager.nextActionAfterAuthentication(this.session, userSession, clientSession, this.clientConnection, this.request,
+        return AuthenticationProcessor.createRequiredActionRedirect(realmModel, clientSession, uriInfo);
-                this.uriInfo, event);
     }
 
     @Override

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

@@ -85,6 +85,7 @@ import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 import javax.ws.rs.ext.Providers;
+import java.net.URI;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -99,6 +100,7 @@ public class LoginActionsService {
     public static final String AUTHENTICATE_PATH = "authenticate";
     public static final String REGISTRATION_PATH = "registration";
     public static final String RESET_CREDENTIALS_PATH = "reset-credentials";
+    public static final String REQUIRED_ACTION = "required-action";
 
     private RealmModel realm;
 
@@ -167,12 +169,22 @@ public class LoginActionsService {
         boolean verifyCode(String code, String requiredAction) {
             if (!verifyCode(code)) {
                 return false;
-            } else if (!clientCode.isValidAction(requiredAction)) {
+            }
+            if (!verifyAction(requiredAction)) {
+                return false;
+            } else {
+                return true;
+            }
+        }
+
+        public boolean verifyAction(String requiredAction) {
+            if (!clientCode.isValidAction(requiredAction)) {
                 event.client(clientCode.getClientSession().getClient());
                 event.error(Errors.INVALID_CODE);
                 response = ErrorPage.error(session, Messages.INVALID_CODE);
                 return false;
-            } else if (!clientCode.isActionActive(requiredAction)) {
+            }
+            if (!clientCode.isActionActive(requiredAction)) {
                 event.client(clientCode.getClientSession().getClient());
                 event.clone().error(Errors.EXPIRED_CODE);
                 if (clientCode.getClientSession().getAction().equals(ClientSessionModel.Action.AUTHENTICATE.name())) {
@@ -182,35 +194,8 @@ public class LoginActionsService {
                 }
                 response = ErrorPage.error(session, Messages.EXPIRED_CODE);
                 return false;
-            } else {
-                return true;
-            }
-        }
-
-        boolean verifyCode(String code, String requiredAction, String alternativeRequiredAction) {
-            if (!verifyCode(code)) {
-                return false;
-            } else if (!(clientCode.isValidAction(requiredAction) || clientCode.isValidAction(alternativeRequiredAction))) {
-                event.client(clientCode.getClientSession().getClient());
-                event.error(Errors.INVALID_CODE);
-                response = ErrorPage.error(session, Messages.INVALID_CODE);
-                return false;
-            } else if (!(clientCode.isActionActive(requiredAction) || clientCode.isActionActive(alternativeRequiredAction))) {
-                event.client(clientCode.getClientSession().getClient());
-                event.clone().error(Errors.EXPIRED_CODE);
-                if (clientCode.getClientSession().getAction().equals(ClientSessionModel.Action.AUTHENTICATE.name())) {
-                    AuthenticationProcessor.resetFlow(clientCode.getClientSession());
-                    response = processAuthentication(null, clientCode.getClientSession(), Messages.LOGIN_TIMEOUT);
-                } else {
-                    if (clientCode.getClientSession().getUserSession() == null) {
-                        session.sessions().removeClientSession(realm, clientCode.getClientSession());
-                    }
-                    response = ErrorPage.error(session, Messages.EXPIRED_CODE);
-                }
-                return false;
-            } else {
-                return true;
             }
+            return true;
         }
 
         public boolean verifyCode(String code) {
@@ -298,6 +283,7 @@ public class LoginActionsService {
         AuthenticationProcessor processor = new AuthenticationProcessor();
         processor.setClientSession(clientSession)
                 .setFlowPath(flowPath)
+                .setBrowserFlow(true)
                 .setFlowId(flow.getId())
                 .setConnection(clientConnection)
                 .setEventBuilder(event)
@@ -559,11 +545,17 @@ public class LoginActionsService {
         event.event(EventType.VERIFY_EMAIL);
         if (key != null) {
             Checks checks = new Checks();
-            if (!checks.verifyCode(key, ClientSessionModel.Action.VERIFY_EMAIL.name())) {
+            if (!checks.verifyCode(key, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
                 return checks.response;
             }
             ClientSessionCode accessCode = checks.clientCode;
             ClientSessionModel clientSession = accessCode.getClientSession();
+            if (!ClientSessionModel.Action.VERIFY_EMAIL.name().equals(clientSession.getNote(AuthenticationManager.CURRENT_REQUIRED_ACTION))) {
+                logger.error("required action doesn't match current required action");
+                event.error(Errors.INVALID_CODE);
+                throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
+            }
+
             UserSessionModel userSession = clientSession.getUserSession();
             UserModel user = userSession.getUser();
             initEvent(clientSession);
@@ -583,10 +575,10 @@ public class LoginActionsService {
 
             event = event.clone().removeDetail(Details.EMAIL).event(EventType.LOGIN);
 
-            return AuthenticationManager.nextActionAfterAuthentication(session, userSession, clientSession, clientConnection, request, uriInfo, event);
+            return AuthenticationProcessor.createRequiredActionRedirect(realm, clientSession, uriInfo);
         } else {
             Checks checks = new Checks();
-            if (!checks.verifyCode(code, ClientSessionModel.Action.VERIFY_EMAIL.name())) {
+            if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
                 return checks.response;
             }
             ClientSessionCode accessCode = checks.clientCode;
@@ -619,9 +611,11 @@ public class LoginActionsService {
                 return checks.response;
             }
             ClientSessionModel clientSession = checks.clientCode.getClientSession();
+            // verify user email as we know it is valid as this entry point would never have gotten here.
+            clientSession.getUserSession().getUser().setEmailVerified(true);
             clientSession.setNote(AuthenticationManager.END_AFTER_REQUIRED_ACTIONS, "true");
             clientSession.setNote(ClientSessionModel.Action.EXECUTE_ACTIONS.name(), "true");
-            return AuthenticationManager.nextActionAfterAuthentication(session, clientSession.getUserSession(), clientSession, clientConnection, request, uriInfo, event);
+            return AuthenticationProcessor.createRequiredActionRedirect(realm, clientSession, uriInfo);
         } else {
             event.error(Errors.INVALID_CODE);
             return ErrorPage.error(session, Messages.INVALID_CODE);
@@ -658,7 +652,7 @@ public class LoginActionsService {
         }
     }
 
-    @Path("required-action")
+    @Path(REQUIRED_ACTION)
     @POST
     public Response requiredActionPOST(@QueryParam("code") final String code,
                                        @QueryParam("action") String action) {
@@ -668,7 +662,7 @@ public class LoginActionsService {
 
     }
 
-    @Path("required-action")
+    @Path(REQUIRED_ACTION)
     @GET
     public Response requiredActionGET(@QueryParam("code") final String code,
                                        @QueryParam("action") String action) {
@@ -678,22 +672,8 @@ public class LoginActionsService {
     public Response processRequireAction(final String code, String action) {
         event.event(EventType.CUSTOM_REQUIRED_ACTION);
         event.detail(Details.CUSTOM_REQUIRED_ACTION, action);
-        if (action == null) {
-            logger.error("required action query param was null");
-            event.error(Errors.INVALID_CODE);
-            throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
-
-        }
-
-        RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, action);
-        if (factory == null) {
-            logger.error("required action provider was null");
-            event.error(Errors.INVALID_CODE);
-            throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
-        }
-        RequiredActionProvider provider = factory.create(session);
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, action)) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
             return checks.response;
         }
         final ClientSessionCode clientCode = checks.clientCode;
@@ -704,24 +684,31 @@ public class LoginActionsService {
             event.error(Errors.USER_SESSION_NOT_FOUND);
             throw new WebApplicationException(ErrorPage.error(session, Messages.SESSION_NOT_ACTIVE));
         }
+        if (action == null && clientSession.getUserSession() != null) { // do next required action only if user is already authenticated
+            initEvent(clientSession);
+            event.event(EventType.LOGIN);
+            return AuthenticationManager.nextActionAfterAuthentication(session, clientSession.getUserSession(), clientSession, clientConnection, request, uriInfo, event);
+        }
+
+        if (!action.equals(clientSession.getNote(AuthenticationManager.CURRENT_REQUIRED_ACTION))) {
+            logger.error("required action doesn't match current required action");
+            event.error(Errors.INVALID_CODE);
+            throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
+        }
+
+        RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, action);
+        if (factory == null) {
+            logger.error("required action provider was null");
+            event.error(Errors.INVALID_CODE);
+            throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
+        }
+        RequiredActionProvider provider = factory.create(session);
 
         initEvent(clientSession);
         event.event(EventType.CUSTOM_REQUIRED_ACTION);
 
 
         RequiredActionContextResult context = new RequiredActionContextResult(clientSession.getUserSession(), clientSession, realm, event, session, request, clientSession.getUserSession().getUser(), factory) {
-             @Override
-            public String generateAccessCode(String action) {
-                String clientSessionAction = clientSession.getAction();
-                if (action.equals(clientSessionAction)) {
-                    clientSession.setTimestamp(Time.currentTime());
-                    return code;
-                }
-                ClientSessionCode code = new ClientSessionCode(getRealm(), getClientSession());
-                code.setAction(action);
-                return code.getCode();
-            }
-
             @Override
             public void ignore() {
                 throw new RuntimeException("Cannot call ignore within processAction()");
@@ -729,13 +716,16 @@ public class LoginActionsService {
         };
         provider.processAction(context);
         if (context.getStatus() == RequiredActionContext.Status.SUCCESS) {
-            event.clone().success();
+            event.success();
             // do both
             clientSession.removeRequiredAction(factory.getId());
             clientSession.getUserSession().getUser().removeRequiredAction(factory.getId());
-            event.event(EventType.LOGIN);
+            // redirect to a generic code URI so that browser refresh will work
-            return AuthenticationManager.nextActionAfterAuthentication(session, clientSession.getUserSession(), clientSession, clientConnection, request, uriInfo, event);
+            URI redirect = LoginActionsService.loginActionsBaseUrl(uriInfo)
-        }
+                    .path(LoginActionsService.REQUIRED_ACTION)
+                    .queryParam(OAuth2Constants.CODE, code).build(realm.getName());
+            return Response.status(302).location(redirect).build();
+       }
         if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) {
             return context.getChallenge();
         }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/ResetCredentialsTest.java

@@ -91,7 +91,7 @@ public class ResetCredentialsTest extends AbstractAccountManagementTest {
         
         log.info("navigating to " + url);
         driver.navigate().to(url);
-        assertCurrentUrlStartsWith(testRealmResetCredentialsPage);
+        //assertCurrentUrlStartsWith(testRealmResetCredentialsPage);
         testRealmResetCredentialsPage.updatePassword("newPassword");
         assertCurrentUrlStartsWith(testRealmAccountManagementPage);
         testRealmAccountManagementPage.signOut();

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/AbstractKerberosTest.java

@@ -175,6 +175,7 @@ public abstract class AbstractKerberosTest {
         events.clear();
         Response spnegoResponse = spnegoLogin("jduke", "theduke");
         Assert.assertEquals(302, spnegoResponse.getStatus());
+        String redirect = spnegoResponse.getLocation().toString();
         events.expectLogin()
                 .client("kerberos-app")
                 .user(keycloakRule.getUser("test", "jduke").getId())
@@ -244,6 +245,13 @@ public abstract class AbstractKerberosTest {
         spnegoSchemeFactory.setCredentials(username, password);
         Response response = client.target(kcLoginPageLocation).request().get();
         SpnegoAuthenticator.bypassChallengeJavascript = false;
+        if (response.getStatus() == 302) {
+            if (response.getLocation() == null) return response;
+            String uri = response.getLocation().toString();
+            if (uri.contains("login-actions/required-action")) {
+                response = client.target(uri).request().get();
+            }
+        }
         return response;
 
     }

testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java

@@ -153,6 +153,16 @@ public class LoginTotpTest {
         events.expectLogin().assertEvent();
     }
 
+    @Test
+    public void loginWithTotpCancel() throws Exception {
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+
+        loginTotpPage.assertCurrent();
+        loginTotpPage.cancel();
+        loginPage.assertCurrent();
+    }
+
     @Test
     public void loginWithTotpInvalidPassword() throws Exception {
         loginPage.open();

testsuite/integration/src/test/java/org/keycloak/testsuite/pages/LoginTotpPage.java

@@ -39,6 +39,9 @@ public class LoginTotpPage extends AbstractPage {
     @FindBy(css = "input[type=\"submit\"]")
     private WebElement submitButton;
 
+    @FindBy(id = "kc-cancel")
+    private WebElement cancelButton;
+
     @FindBy(className = "feedback-error")
     private WebElement loginErrorMessage;
 
@@ -49,6 +52,10 @@ public class LoginTotpPage extends AbstractPage {
         submitButton.click();
     }
 
+    public void cancel() {
+        cancelButton.click();
+    }
+
     public String getError() {
         return loginErrorMessage != null ? loginErrorMessage.getText() : null;
     }

testsuite/integration/src/test/java/org/keycloak/testsuite/perf/AccessTokenPerfTest.java

@@ -172,6 +172,14 @@ public class AccessTokenPerfTest {
             URI uri = null;
             Assert.assertEquals(302, response.getStatus());
             uri = response.getLocation();
+            if (response.getStatus() == 302) {
+                while (uri.toString().contains("login-actions/")) {
+                    response = client.target(uri).request().get();
+                    Assert.assertEquals(302, response.getStatus());
+                    uri = response.getLocation();
+                }
+            }
+
             for (String header : response.getHeaders().keySet()) {
                 for (Object value : response.getHeaders().get(header)) {
                     System.out.println(header + ": " + value);

testsuite/jetty/jetty81/src/test/java/org/keycloak/testsuite/Jetty8Test.java

@@ -85,9 +85,11 @@ public class Jetty8Test {
 
     @AfterClass
     public static void shutdownJetty() throws Exception {
-        server.stop();
+        try {
-        server.destroy();
+            server.stop();
-        Thread.sleep(1000);
+            server.destroy();
+            Thread.sleep(100);
+        } catch (Exception e) {}
     }
 
     @Rule

testsuite/jetty/jetty81/src/test/java/org/keycloak/testsuite/JettySamlTest.java

@@ -97,9 +97,11 @@ public class JettySamlTest {
 
     @AfterClass
     public static void shutdownJetty() throws Exception {
-        server.stop();
+        try {
-        server.destroy();
+            server.stop();
-        Thread.sleep(1000);
+            server.destroy();
+            Thread.sleep(100);
+        } catch (Exception e) {}
     }
 
     @Test

testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/Jetty9Test.java

@@ -85,9 +85,11 @@ public class Jetty9Test {
 
     @AfterClass
     public static void shutdownJetty() throws Exception {
-        server.stop();
+        try {
-        server.destroy();
+            server.stop();
-        Thread.sleep(1000);
+            server.destroy();
+            Thread.sleep(100);
+        } catch (Exception e) {}
     }
 
     @Rule

testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/JettySamlTest.java

@@ -96,9 +96,11 @@ public class JettySamlTest {
 
     @AfterClass
     public static void shutdownJetty() throws Exception {
-        server.stop();
+        try {
-        server.destroy();
+            server.stop();
-        Thread.sleep(1000);
+            server.destroy();
+            Thread.sleep(100);
+        } catch (Exception e) {}
     }
 
     @Test

testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/Jetty9Test.java

@@ -85,8 +85,11 @@ public class Jetty9Test {
 
     @AfterClass
     public static void shutdownJetty() throws Exception {
-        server.stop();
+        try {
-        server.destroy();
+            server.stop();
+            server.destroy();
+            Thread.sleep(100);
+        } catch (Exception e) {}
     }
 
     @Rule

testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/JettySamlTest.java

@@ -96,8 +96,11 @@ public class JettySamlTest {
 
     @AfterClass
     public static void shutdownJetty() throws Exception {
-        server.stop();
+        try {
-        server.destroy();
+            server.stop();
+            server.destroy();
+            Thread.sleep(100);
+        } catch (Exception e) {}
     }
 
     @Test