diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 17c02afe334..b3123cb0421 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/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()) - .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(); + Response challenge = authenticateOnly(); 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() { - // 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) { + public static Response redirectToRequiredActions(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(); - resetFlow(clientSession); + //logFailure(); + //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); - //return AuthenticationManager.nextActionAfterAuthentication(session, userSession, clientSession, connection, request, uriInfo, event); + if (isActionRequired()) { + 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 executions) { diff --git a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java index 9a3a8c221a4..6cc22f2ccde 100755 --- a/services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java +++ b/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 Bill Burke -* @version $Revision: 1 $ -*/ + * @author Bill Burke + * @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()) { - // redirect to a non-action URL so browser refresh works without reposting. - return processor.createSuccessRedirect(); - } else { - return processFlow(); - } - } - else return response; + processor.getClientSession().removeNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); + return processFlow(); + } else return response; } } throw new AuthenticationFlowException("action is not in current execution", AuthenticationFlowError.INTERNAL_ERROR); diff --git a/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java index 5ee8bbe748e..2913dcf920d 100755 --- a/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java +++ b/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); - - // redirect to no execution so browser refresh button works without reposting past data - return processor.createSuccessRedirect(); + processor.getClientSession().removeNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); + return null; } public URI getActionUrl(String executionId, String code) { diff --git a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java old mode 100644 new mode 100755 index befd364b782..9b11535301d --- a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java +++ b/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 (challenge == null) { - challenge2 = processor.attachSessionExecutionRequiredActions(); + if (processor.authenticateOnly() == null) { + processor.attachSession(); + } 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); } - - 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); - } + return processor.finishAuthentication(protocol); } else { try { RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index d034d753e83..5d045e4518e 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/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) { diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index eb3f8167a26..aa6d3fededa 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/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); } diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index 75dd87ee8b8..d015224f241 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/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"); - event.error(Errors.INVALID_CODE); - throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE)); + logger.debug("required action doesn't match current required action"); + clientSession.removeNote(AuthenticationManager.CURRENT_REQUIRED_ACTION); + 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) - .path(LoginActionsService.REQUIRED_ACTION) - .queryParam(OAuth2Constants.CODE, code).build(realm.getName()); - return Response.status(302).location(redirect).build(); + //return redirectToRequiredActions(code); + event.removeDetail(Details.CUSTOM_REQUIRED_ACTION); + initEvent(clientSession); + 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(); + } + }