diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java index 0b8cd3d95b..b74c9353d2 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Details.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Details.java @@ -23,6 +23,7 @@ package org.keycloak.events; public interface Details { String PREF_PREVIOUS = "previous_"; String PREF_UPDATED = "updated_"; + String FIELDS_TO_UPDATE = "fields_to_update"; String CUSTOM_REQUIRED_ACTION="custom_required_action"; String CONTEXT = "context"; diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java index c0203f15f4..d9b8d9c76b 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/Constants.java +++ b/server-spi-private/src/main/java/org/keycloak/models/Constants.java @@ -91,6 +91,11 @@ public final class Constants { public static final String KC_ACTION_PARAMETER = "kc_action_parameter"; public static final String KC_ACTION_STATUS = "kc_action_status"; public static final String KC_ACTION_EXECUTING = "kc_action_executing"; + /** + * Auth session attribute whether an AIA is enforced, which means it cannot be cancelled. + *

Example use case: the action behind the AIA is also defined on the user (for example, UPDATE_PASSWORD).

+ */ + public static final String KC_ACTION_ENFORCED = "kc_action_enforced"; public static final int KC_ACTION_MAX_AGE = 300; public static final String IS_AIA_REQUEST = "IS_AIA_REQUEST"; diff --git a/server-spi/src/main/java/org/keycloak/models/UserModel.java b/server-spi/src/main/java/org/keycloak/models/UserModel.java index 60366542f8..209be4b104 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserModel.java +++ b/server-spi/src/main/java/org/keycloak/models/UserModel.java @@ -106,9 +106,9 @@ public interface UserModel extends RoleMapperModel { Map> getAttributes(); /** - * Obtains the names of required actions associated with the user. + * Obtains the aliases of required actions associated with the user. * - * @return a non-null {@link Stream} of required action names. + * @return a non-null {@link Stream} of required action aliases. */ Stream getRequiredActionsStream(); diff --git a/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionModel.java b/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionModel.java index 514f30516f..b7e453789c 100644 --- a/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionModel.java +++ b/server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionModel.java @@ -74,7 +74,7 @@ public interface AuthenticationSessionModel extends CommonClientSessionModel { void setAuthenticatedUser(UserModel user); /** - * Returns required actions that are attached to this client session. + * Returns required actions (aliases) that are attached to this client session. * @return {@code Set} Never returns {@code null}. */ Set getRequiredActions(); diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyUserProfile.java b/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyUserProfile.java index 58fefcf9b6..9a42198857 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyUserProfile.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyUserProfile.java @@ -27,6 +27,7 @@ import jakarta.ws.rs.core.MultivaluedMap; import org.keycloak.authentication.InitiatedActionSupport; import org.keycloak.authentication.RequiredActionContext; import org.keycloak.authentication.RequiredActionProvider; +import org.keycloak.events.Details; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.models.KeycloakSession; @@ -90,7 +91,7 @@ public class VerifyUserProfile extends UpdateProfile { EventBuilder event = context.getEvent().clone(); event.event(EventType.VERIFY_PROFILE); - event.detail("fields_to_update", collectFields(errors)); + event.detail(Details.FIELDS_TO_UPDATE, collectFields(errors)); event.success(); } } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index a863693559..4d21def535 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -543,7 +543,8 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { attributes.put("authenticatorConfigured", new AuthenticatorConfiguredMethod(realm, user, session)); } - if (authenticationSession != null && authenticationSession.getClientNote(Constants.KC_ACTION_EXECUTING) != null) { + if (authenticationSession != null && authenticationSession.getClientNote(Constants.KC_ACTION_EXECUTING) != null + && !Boolean.TRUE.toString().equals(authenticationSession.getClientNote(Constants.KC_ACTION_ENFORCED))) { attributes.put("isAppInitiatedAction", true); } } 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 0fad68f081..99de7e8d86 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -102,6 +102,8 @@ import java.net.URLDecoder; import java.net.URLEncoder; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -109,6 +111,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -1032,45 +1035,40 @@ public class AuthenticationManager { 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. + // Return null if action is not required. Or the alias of the requiredAction in case it is required. public static String nextRequiredAction(final KeycloakSession session, final AuthenticationSessionModel authSession, - final HttpRequest request, final EventBuilder event) { - final RealmModel realm = authSession.getRealm(); - final UserModel user = authSession.getAuthenticatedUser(); - final ClientModel client = authSession.getClient(); + final HttpRequest request, final EventBuilder event) { + final var realm = authSession.getRealm(); + final var user = authSession.getAuthenticatedUser(); evaluateRequiredActionTriggers(session, authSession, request, event, realm, user); - Optional reqAction = user.getRequiredActionsStream().findFirst(); - if (reqAction.isPresent()) { - return reqAction.get(); - } - if (!authSession.getRequiredActions().isEmpty()) { - return authSession.getRequiredActions().iterator().next(); - } - - String kcAction = authSession.getClientNote(Constants.KC_ACTION); - if (kcAction != null) { - return kcAction; + final var kcAction = authSession.getClientNote(Constants.KC_ACTION); + final var nextApplicableAction = + getFirstApplicableRequiredAction(realm, authSession, user, kcAction); + if (nextApplicableAction != null) { + return nextApplicableAction.getAlias(); } + final var client = authSession.getClient(); if (client.isConsentRequired() || isOAuth2DeviceVerificationFlow(authSession)) { UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession); // See if any clientScopes need to be approved on consent screen - List clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession); + List clientScopesToApprove = + getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession); if (!clientScopesToApprove.isEmpty()) { return CommonClientSessionModel.Action.OAUTH_GRANT.name(); } - String consentDetail = (grantedConsent != null) ? Details.CONSENT_VALUE_PERSISTED_CONSENT : Details.CONSENT_VALUE_NO_CONSENT_REQUIRED; + 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 null; - } @@ -1097,24 +1095,21 @@ public class AuthenticationManager { public static Response actionRequired(final KeycloakSession session, final AuthenticationSessionModel authSession, - final HttpRequest request, final EventBuilder event) { - final RealmModel realm = authSession.getRealm(); - final UserModel user = authSession.getAuthenticatedUser(); - final ClientModel client = authSession.getClient(); + final HttpRequest request, final EventBuilder event) { + final var realm = authSession.getRealm(); + final var user = authSession.getAuthenticatedUser(); evaluateRequiredActionTriggers(session, authSession, request, event, realm, user); - logger.debugv("processAccessCode: go to oauth page?: {0}", client.isConsentRequired()); - event.detail(Details.CODE_ID, authSession.getParentSession().getId()); - Stream requiredActions = user.getRequiredActionsStream(); - Response action = executionActions(session, authSession, request, event, realm, user, requiredActions); - if (action != null) return action; + final var actionResponse = executionActions(session, authSession, request, event, realm, user); + if (actionResponse != null) { + return actionResponse; + } - // executionActions() method should remove any duplicate actions that might be in the clientSession - action = executionActions(session, authSession, request, event, realm, user, authSession.getRequiredActions().stream()); - if (action != null) return action; + final var client = authSession.getClient(); + logger.debugv("processAccessCode: go to oauth page?: {0}", client.isConsentRequired()); // https://tools.ietf.org/html/draft-ietf-oauth-device-flow-15#section-5.4 // The spec says "The authorization server SHOULD display information about the device", @@ -1123,13 +1118,15 @@ public class AuthenticationManager { UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession); - List clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession); + List clientScopesToApprove = + getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession); // Skip grant screen if everything was already approved by this user if (clientScopesToApprove.size() > 0) { String execution = AuthenticatedClientSessionModel.Action.OAUTH_GRANT.name(); - ClientSessionCode accessCode = new ClientSessionCode<>(session, realm, authSession); + ClientSessionCode accessCode = + new ClientSessionCode<>(session, realm, authSession); accessCode.setAction(AuthenticatedClientSessionModel.Action.REQUIRED_ACTIONS.name()); authSession.setAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION, execution); @@ -1140,7 +1137,8 @@ public class AuthenticationManager { .setAccessRequest(clientScopesToApprove) .createOAuthGrant(); } else { - String consentDetail = (grantedConsent != null) ? Details.CONSENT_VALUE_PERSISTED_CONSENT : Details.CONSENT_VALUE_NO_CONSENT_REQUIRED; + String consentDetail = (grantedConsent != null) ? Details.CONSENT_VALUE_PERSISTED_CONSENT + : Details.CONSENT_VALUE_NO_CONSENT_REQUIRED; event.detail(Details.CONSENT, consentDetail); } } else { @@ -1220,26 +1218,14 @@ public class AuthenticationManager { protected static Response executionActions(KeycloakSession session, AuthenticationSessionModel authSession, - HttpRequest request, EventBuilder event, RealmModel realm, UserModel user, - Stream requiredActions) { + HttpRequest request, EventBuilder event, RealmModel realm, UserModel user) { + final var kcAction = authSession.getClientNote(Constants.KC_ACTION); + final var firstApplicableRequiredAction = + getFirstApplicableRequiredAction(realm, authSession, user, kcAction); - Optional response = sortRequiredActionsByPriority(realm, requiredActions) - .map(model -> executeAction(session, authSession, model, request, event, realm, user, false)) - .filter(Objects::nonNull).findFirst(); - if (response.isPresent()) - return response.get(); - - String kcAction = authSession.getClientNote(Constants.KC_ACTION); - if (kcAction != null) { - Optional requiredAction = realm.getRequiredActionProvidersStream() - .filter(m -> Objects.equals(m.getProviderId(), kcAction)) - .findFirst(); - if (requiredAction.isPresent()) { - return executeAction(session, authSession, requiredAction.get(), request, event, realm, user, true); - } - - logger.debugv("Requested action {0} not configured for realm", kcAction); - setKcActionStatus(kcAction, RequiredActionContext.KcActionStatus.ERROR, authSession); + if (firstApplicableRequiredAction != null) { + return executeAction(session, authSession, firstApplicableRequiredAction, request, event, realm, user, + kcAction != null); } return null; @@ -1304,17 +1290,81 @@ public class AuthenticationManager { return null; } - private static Stream sortRequiredActionsByPriority(RealmModel realm, Stream requiredActions) { - return requiredActions.map(action -> { - RequiredActionProviderModel model = realm.getRequiredActionProviderByAlias(action); - if (model == null) { - logger.warnv("Could not find configuration for Required Action {0}, did you forget to register it?", action); - } - return model; - }) + private static RequiredActionProviderModel getFirstApplicableRequiredAction(final RealmModel realm, + final AuthenticationSessionModel authSession, final UserModel user, final String kcAction) { + final var applicableRequiredActionsSorted = + getApplicableRequiredActionsSorted(realm, authSession, user, kcAction); + + final RequiredActionProviderModel firstApplicableRequiredAction; + if (applicableRequiredActionsSorted.isEmpty()) { + firstApplicableRequiredAction = null; + logger.debugv("Did not find applicable required action"); + } else { + firstApplicableRequiredAction = applicableRequiredActionsSorted.iterator().next(); + logger.debugv("first applicable required action: {0}", firstApplicableRequiredAction.getAlias()); + } + + return firstApplicableRequiredAction; + } + + private static List getApplicableRequiredActionsSorted(final RealmModel realm, + final AuthenticationSessionModel authSession, final UserModel user, final String kcActionAlias) { + final Set nonInitiatedActionAliases = new HashSet<>(); + nonInitiatedActionAliases.addAll(user.getRequiredActionsStream().toList()); + nonInitiatedActionAliases.addAll(authSession.getRequiredActions()); + + final var applicableNonInitiatedActions = nonInitiatedActionAliases.stream() + .map(alias -> getApplicableRequiredAction(realm, alias)) .filter(Objects::nonNull) - .filter(RequiredActionProviderModel::isEnabled) - .sorted(RequiredActionProviderModel.RequiredActionComparator.SINGLETON); + .collect(Collectors.toMap(RequiredActionProviderModel::getAlias, Function.identity())); + + RequiredActionProviderModel kcAction = null; + if (kcActionAlias != null) { + kcAction = getApplicableRequiredAction(realm, kcActionAlias); + if (kcAction == null) { + logger.debugv("Requested action {0} not configured for realm", kcActionAlias); + setKcActionStatus(kcActionAlias, RequiredActionContext.KcActionStatus.ERROR, authSession); + } else { + if (applicableNonInitiatedActions.containsKey(kcActionAlias)) { + setKcActionToEnforced(kcActionAlias, authSession); + } + } + } + + final Map applicableActions; + if (kcAction != null) { + applicableActions = new HashMap<>(applicableNonInitiatedActions); + applicableActions.put(kcAction.getAlias(), kcAction); + } else { + applicableActions = applicableNonInitiatedActions; + } + + final var applicableActionsSorted = applicableActions.values().stream() + .sorted(RequiredActionProviderModel.RequiredActionComparator.SINGLETON) + .toList(); + + if (logger.isDebugEnabled()) { + logger.debugv("applicable required actions (sorted): {0}", + applicableActionsSorted.stream().map(RequiredActionProviderModel::getAlias).toList()); + } + + return applicableActionsSorted; + } + + private static RequiredActionProviderModel getApplicableRequiredAction(final RealmModel realm, final String alias) { + final var model = realm.getRequiredActionProviderByAlias(alias); + if (model == null) { + logger.warnv( + "Could not find configuration for Required Action {0}, did you forget to register it?", + alias); + return null; + } + + if (!model.isEnabled()) { + return null; + } + + return model; } public static void evaluateRequiredActionTriggers(final KeycloakSession session, final AuthenticationSessionModel authSession, @@ -1524,6 +1574,12 @@ public class AuthenticationManager { } } + public static void setKcActionToEnforced(String executedProviderId, AuthenticationSessionModel authSession) { + if (executedProviderId.equals(authSession.getClientNote(Constants.KC_ACTION))) { + authSession.setClientNote(Constants.KC_ACTION_ENFORCED, Boolean.TRUE.toString()); + } + } + public static void logSuccess(KeycloakSession session, AuthenticationSessionModel authSession) { RealmModel realm = session.getContext().getRealm(); if (realm.isBruteForceProtected()) { 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 71fc06a9bb..55ee368a86 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -1190,7 +1190,8 @@ public class LoginActionsService { } private boolean isCancelAppInitiatedAction(String providerId, AuthenticationSessionModel authSession, RequiredActionContextResult context) { - if (providerId.equals(authSession.getClientNote(Constants.KC_ACTION_EXECUTING))) { + if (providerId.equals(authSession.getClientNote(Constants.KC_ACTION_EXECUTING)) + && !Boolean.TRUE.toString().equals(authSession.getClientNote(Constants.KC_ACTION_ENFORCED))) { MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); boolean userRequestedCancelAIA = formData.getFirst(CANCEL_AIA) != null; return userRequestedCancelAIA; diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java index 47e10d2ada..2c93dc0b12 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java @@ -24,11 +24,13 @@ import org.keycloak.admin.client.resource.GroupResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RoleResource; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.models.UserModel; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientScopeRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; +import org.keycloak.representations.idm.RequiredActionProviderRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -40,6 +42,7 @@ import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; import static org.keycloak.representations.idm.CredentialRepresentation.PASSWORD; @@ -163,7 +166,6 @@ public class ApiUtil { * Creates a user * @param realm * @param user - * @param password * @return ID of the new user */ public static String createUserWithAdminClient(RealmResource realm, UserRepresentation user) { @@ -276,4 +278,62 @@ public class ApiUtil { return null; } + /** + * Updates the order of required actions + * + * @param realmResource the realm + * @param requiredActionsInTargetOrder the required actions for which the order should be changed (order will be the + * order of this list) - can be a subset of the available required actions + * @see #updateRequiredActionsOrderByAlias(RealmResource, List) + */ + public static void updateRequiredActionsOrder(final RealmResource realmResource, + final List requiredActionsInTargetOrder) { + updateRequiredActionsOrderByAlias(realmResource, + requiredActionsInTargetOrder.stream().map(Enum::name).collect(Collectors.toList())); + } + + /** + * @see #updateRequiredActionsOrder(RealmResource, List) + */ + public static void updateRequiredActionsOrderByAlias(final RealmResource realmResource, + final List requiredActionsInTargetOrder) { + final var realmName = realmResource.toRepresentation().getRealm(); + final var initialRequiredActionsOrdered = realmResource.flows().getRequiredActions().stream() + .map(RequiredActionProviderRepresentation::getAlias).collect(Collectors.toList()); + log.infof("initial required actions order for realm '%s': %s", realmName, initialRequiredActionsOrdered); + log.infof("target order for realm '%s' (maybe partial): %s", realmName, requiredActionsInTargetOrder); + + final var requiredActionsToConfigureWithLowerPrio = new ArrayList<>(requiredActionsInTargetOrder); + for (final var requiredActionAlias : requiredActionsInTargetOrder) { + var allRequiredActionsOrdered = realmResource.flows().getRequiredActions().stream() + .map(RequiredActionProviderRepresentation::getAlias).collect(Collectors.toList()); + + requiredActionsToConfigureWithLowerPrio.remove(requiredActionAlias); + + final var currentIndex = allRequiredActionsOrdered.indexOf(requiredActionAlias); + if (currentIndex == -1) { + throw new IllegalStateException("Required action not found: " + requiredActionAlias); + } + + final var aliasOfCurrentlyFirstActionWithLowerTargetPrioOpt = allRequiredActionsOrdered.stream() + .filter(requiredActionsToConfigureWithLowerPrio::contains).findFirst(); + aliasOfCurrentlyFirstActionWithLowerTargetPrioOpt + .ifPresent(aliasOfCurrentlyFirstActionWithLowerTargetPrio -> { + final var indexOfCurrentlyFirstActionWithLowerTargetPrio = + allRequiredActionsOrdered.indexOf(aliasOfCurrentlyFirstActionWithLowerTargetPrio); + final var positionsToMoveCurrentActionUp = + Math.max(currentIndex - indexOfCurrentlyFirstActionWithLowerTargetPrio, 0); + if (positionsToMoveCurrentActionUp > 0) { + for (var i = 0; i < positionsToMoveCurrentActionUp; i++) { + realmResource.flows().raiseRequiredActionPriority(requiredActionAlias); + } + } + }); + } + + final var updatedRequiredActionsOrdered = realmResource.flows().getRequiredActions().stream() + .map(RequiredActionProviderRepresentation::getAlias).collect(Collectors.toList()); + log.infof("updated required actions order for realm '%s': %s", realmName, updatedRequiredActionsOrdered); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java index c7379cd5fe..fb7da92085 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java @@ -204,6 +204,10 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct doAIA(); changePasswordPage.assertCurrent(); + /* + * cancel should not be supported, because the action is not only application-initiated, but also required by + * Keycloak + */ assertFalse(changePasswordPage.isCancelDisplayed()); changePasswordPage.changePassword("new-password", "new-password"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java index e84860143b..830281d16c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java @@ -20,9 +20,11 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.keycloak.testsuite.actions.RequiredActionEmailVerificationTest.getEmailLink; import org.jboss.arquillian.graphene.page.Page; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -33,7 +35,6 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.admin.ApiUtil; @@ -41,34 +42,51 @@ import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.LoginConfigTotpPage; import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginPasswordResetPage; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; -import org.keycloak.testsuite.pages.LoginUpdateProfileEditUsernameAllowedPage; +import org.keycloak.testsuite.pages.LoginUpdateProfilePage; import org.keycloak.testsuite.pages.TermsAndConditionsPage; -import org.keycloak.testsuite.util.UserBuilder; +import org.keycloak.testsuite.pages.VerifyProfilePage; +import org.keycloak.testsuite.util.GreenMailRule; +import org.keycloak.testsuite.util.OAuthClient; + +import java.util.List; /** * @author Hiroyuki Wada */ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { - @Override - public void configureTestRealm(RealmRepresentation testRealm) { - } + private static final String EMAIL = "test-user@localhost"; + private static final String USERNAME = EMAIL; + private static final String PASSWORD = "password"; + private static final String NEW_EMAIL = "new@email.com"; + private static final String NEW_FIRST_NAME = "New first"; + private static final String NEW_LAST_NAME = "New last"; + private static final String NEW_PASSWORD = "new-password"; @Rule public AssertEvents events = new AssertEvents(this); + @Rule + public GreenMailRule greenMail = new GreenMailRule(); + @Page protected AppPage appPage; @Page protected LoginPage loginPage; + @Page + protected LoginPasswordResetPage resetPasswordPage; @Page protected LoginPasswordUpdatePage changePasswordPage; @Page - protected LoginUpdateProfileEditUsernameAllowedPage updateProfilePage; + protected LoginUpdateProfilePage updateProfilePage; + + @Page + protected VerifyProfilePage verifyProfilePage; @Page protected TermsAndConditionsPage termsPage; @@ -76,29 +94,41 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { @Page protected LoginConfigTotpPage totpPage; + private String testUserId; + @Before - public void setupRequiredActions() { - setRequiredActionEnabled("test", TermsAndConditions.PROVIDER_ID, true, false); + public void beforeEach() { + setRequiredActionEnabled(TEST_REALM_NAME, TermsAndConditions.PROVIDER_ID, true, false); - // Because of changing the password in test case, we need to re-create the user. - ApiUtil.removeUserByUsername(testRealm(), "test-user@localhost"); - UserRepresentation user = UserBuilder.create().enabled(true).username("test-user@localhost") - .email("test-user@localhost").build(); - String testUserId = ApiUtil.createUserAndResetPasswordWithAdminClient(testRealm(), user, "password"); + testUserId = ApiUtil.findUserByUsernameId(testRealm(), USERNAME).toRepresentation().getId(); + } - setRequiredActionEnabled("test", testUserId, RequiredAction.UPDATE_PASSWORD.name(), true); - setRequiredActionEnabled("test", testUserId, RequiredAction.UPDATE_PROFILE.name(), true); - setRequiredActionEnabled("test", testUserId, TermsAndConditions.PROVIDER_ID, true); + @Override + public void configureTestRealm(final RealmRepresentation testRealm) { + testRealm.setResetPasswordAllowed(true); + } + + @Override + protected boolean isImportAfterEachMethod() { + return true; + } + + @Override + protected boolean removeVerifyProfileAtImport() { + return false; } @Test - public void executeRequiredActionsWithDefaultPriority() throws Exception { + public void executeRequiredActionsWithDefaultPriority() { // Default priority is alphabetical order: // TermsAndConditions -> UpdatePassword -> UpdateProfile + enableRequiredActionForUser(RequiredAction.UPDATE_PASSWORD); + enableRequiredActionForUser(RequiredAction.UPDATE_PROFILE); + enableRequiredActionForUser(RequiredAction.TERMS_AND_CONDITIONS); // Login loginPage.open(); - loginPage.login("test-user@localhost", "password"); + loginPage.login(USERNAME, PASSWORD); // First, accept terms termsPage.assertCurrent(); @@ -108,50 +138,60 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { // Second, change password changePasswordPage.assertCurrent(); - changePasswordPage.changePassword("new-password", "new-password"); + changePasswordPage.changePassword(NEW_PASSWORD, NEW_PASSWORD); events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); // Finally, update profile updateProfilePage.assertCurrent(); - updateProfilePage.prepareUpdate().username("test-user@localhost").firstName("New first").lastName("New last").email("new@email.com").submit(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, "New first") - .detail(Details.UPDATED_LAST_NAME, "New last") - .detail(Details.PREVIOUS_EMAIL, "test-user@localhost") - .detail(Details.UPDATED_EMAIL, "new@email.com") + updateProfilePage.prepareUpdate().firstName(NEW_FIRST_NAME).lastName(NEW_LAST_NAME) + .email(NEW_EMAIL).submit(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, NEW_FIRST_NAME) + .detail(Details.UPDATED_LAST_NAME, NEW_LAST_NAME) + .detail(Details.PREVIOUS_EMAIL, EMAIL) + .detail(Details.UPDATED_EMAIL, NEW_EMAIL) .assertEvent(); // Logged in appPage.assertCurrent(); - Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); events.expectLogin().assertEvent(); } @Test - public void executeRequiredActionsWithCustomPriority() throws Exception { + public void executeRequiredActionsWithCustomPriority() { // Default priority is alphabetical order: // TermsAndConditions -> UpdatePassword -> UpdateProfile // After Changing the priority, the order will be: // UpdatePassword -> UpdateProfile -> TermsAndConditions - testRealm().flows().raiseRequiredActionPriority(UserModel.RequiredAction.UPDATE_PASSWORD.name()); - testRealm().flows().lowerRequiredActionPriority(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name()); + final var requiredActionsCustomOrdered = List.of( + RequiredAction.UPDATE_PASSWORD, + RequiredAction.UPDATE_PROFILE, + RequiredAction.TERMS_AND_CONDITIONS + ); + ApiUtil.updateRequiredActionsOrder(testRealm(), requiredActionsCustomOrdered); + + enableRequiredActionForUser(RequiredAction.UPDATE_PASSWORD); + enableRequiredActionForUser(RequiredAction.UPDATE_PROFILE); + enableRequiredActionForUser(RequiredAction.TERMS_AND_CONDITIONS); // Login loginPage.open(); - loginPage.login("test-user@localhost", "password"); + loginPage.login(USERNAME, PASSWORD); // First, change password changePasswordPage.assertCurrent(); - changePasswordPage.changePassword("new-password", "new-password"); + changePasswordPage.changePassword(NEW_PASSWORD, NEW_PASSWORD); events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); // Second, update profile updateProfilePage.assertCurrent(); - updateProfilePage.prepareUpdate().username("test-user@localhost").firstName("New first").lastName("New last").email("new@email.com").submit(); - events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, "New first") - .detail(Details.UPDATED_LAST_NAME, "New last") - .detail(Details.PREVIOUS_EMAIL, "test-user@localhost") - .detail(Details.UPDATED_EMAIL, "new@email.com") + updateProfilePage.prepareUpdate().firstName(NEW_FIRST_NAME).lastName(NEW_LAST_NAME) + .email(NEW_EMAIL).submit(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, NEW_FIRST_NAME) + .detail(Details.UPDATED_LAST_NAME, NEW_LAST_NAME) + .detail(Details.PREVIOUS_EMAIL, EMAIL) + .detail(Details.UPDATED_EMAIL, NEW_EMAIL) .assertEvent(); // Finally, accept terms @@ -160,32 +200,201 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { events.expectRequiredAction(EventType.CUSTOM_REQUIRED_ACTION).removeDetail(Details.REDIRECT_URI) .detail(Details.CUSTOM_REQUIRED_ACTION, TermsAndConditions.PROVIDER_ID).assertEvent(); - // Logined + // Logged in appPage.assertCurrent(); - Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + events.expectLogin().assertEvent(); + } + + @Test + public void executeRequiredActionsWithCustomPriorityAppliesSamePriorityToUserActionsAndKcActionParam() { + // Default priority is alphabetical order: + // TermsAndConditions -> UpdatePassword -> UpdateProfile + + // After Changing the priority, the order will be: + // UpdatePassword -> UpdateProfile -> TermsAndConditions + final var requiredActionsCustomOrdered = List.of( + RequiredAction.UPDATE_PASSWORD, + RequiredAction.UPDATE_PROFILE, + RequiredAction.TERMS_AND_CONDITIONS + ); + ApiUtil.updateRequiredActionsOrder(testRealm(), requiredActionsCustomOrdered); + + enableRequiredActionForUser(RequiredAction.UPDATE_PASSWORD); + // we don't enable UPDATE_PROFILE for the user, we set this as kc_action param instead + enableRequiredActionForUser(RequiredAction.TERMS_AND_CONDITIONS); + + // Login with kc_action=UPDATE_PROFILE + final var kcActionOauth = new OAuthClient(); + kcActionOauth.init(driver); + kcActionOauth.kcAction(RequiredAction.UPDATE_PROFILE.name()); + kcActionOauth.openLoginForm(); + loginPage.assertCurrent(TEST_REALM_NAME); + loginPage.login(USERNAME, PASSWORD); + + // First, change password + changePasswordPage.assertCurrent(); + changePasswordPage.changePassword(NEW_PASSWORD, NEW_PASSWORD); + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); + + // Second, update profile + updateProfilePage.assertCurrent(); + updateProfilePage.prepareUpdate().firstName(NEW_FIRST_NAME).lastName(NEW_LAST_NAME) + .email(NEW_EMAIL).submit(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, NEW_FIRST_NAME) + .detail(Details.UPDATED_LAST_NAME, NEW_LAST_NAME) + .detail(Details.PREVIOUS_EMAIL, EMAIL) + .detail(Details.UPDATED_EMAIL, NEW_EMAIL) + .assertEvent(); + + // Finally, accept terms + termsPage.assertCurrent(); + termsPage.acceptTerms(); + events.expectRequiredAction(EventType.CUSTOM_REQUIRED_ACTION).removeDetail(Details.REDIRECT_URI) + .detail(Details.CUSTOM_REQUIRED_ACTION, TermsAndConditions.PROVIDER_ID).assertEvent(); + + // Logged in + appPage.assertCurrent(); + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + events.expectLogin().assertEvent(); + } + + + @Test + public void executeLoginActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions() + throws Exception { + // Default priority is alphabetical order: + // TermsAndConditions -> UpdatePassword -> UpdateProfile + + // After Changing the priority, the order will be: + // UpdatePassword -> UpdateProfile -> TermsAndConditions + final var requiredActionsCustomOrdered = List.of( + RequiredAction.UPDATE_PASSWORD, + RequiredAction.UPDATE_PROFILE, + RequiredAction.TERMS_AND_CONDITIONS + ); + ApiUtil.updateRequiredActionsOrder(testRealm(), requiredActionsCustomOrdered); + + // NOTE: we don't configure UPDATE_PASSWORD on the user - it's set on the session by the reset-password flow + enableRequiredActionForUser(RequiredAction.UPDATE_PROFILE); + enableRequiredActionForUser(RequiredAction.TERMS_AND_CONDITIONS); + + // Get a password reset link + loginPage.open(); + loginPage.assertCurrent(TEST_REALM_NAME); + loginPage.resetPassword(); + + resetPasswordPage.assertCurrent(); + resetPasswordPage.changePassword(USERNAME); + events.expectRequiredAction(EventType.SEND_RESET_PASSWORD).assertEvent(); + loginPage.assertCurrent(); + assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage()); + + assertEquals(1, greenMail.getReceivedMessages().length); + final var message = greenMail.getLastReceivedMessage(); + final var resetUrl = getEmailLink(message); + assertNotNull(resetUrl); + driver.navigate().to(resetUrl); + + // First, change password + changePasswordPage.assertCurrent(); + changePasswordPage.changePassword(NEW_PASSWORD, NEW_PASSWORD); + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); + + // Second, update profile + updateProfilePage.assertCurrent(); + updateProfilePage.prepareUpdate().firstName(NEW_FIRST_NAME).lastName(NEW_LAST_NAME) + .email(NEW_EMAIL).submit(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).detail(Details.UPDATED_FIRST_NAME, NEW_FIRST_NAME) + .detail(Details.UPDATED_LAST_NAME, NEW_LAST_NAME) + .detail(Details.PREVIOUS_EMAIL, EMAIL) + .detail(Details.UPDATED_EMAIL, NEW_EMAIL) + .assertEvent(); + + // Finally, accept terms + termsPage.assertCurrent(); + termsPage.acceptTerms(); + events.expectRequiredAction(EventType.CUSTOM_REQUIRED_ACTION).removeDetail(Details.REDIRECT_URI) + .detail(Details.CUSTOM_REQUIRED_ACTION, TermsAndConditions.PROVIDER_ID).assertEvent(); + + // Logged in + appPage.assertCurrent(); + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + events.expectLogin().assertEvent(); + } + + @Test + public void executeRequiredActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions() { + // Default priority is alphabetical order: + // TermsAndConditions -> VerifyProfile + + // After Changing the priority, the order will be: + // VerifyProfile -> TermsAndConditions + final var requiredActionsCustomOrdered = List.of( + RequiredAction.VERIFY_PROFILE, + RequiredAction.TERMS_AND_CONDITIONS + ); + ApiUtil.updateRequiredActionsOrder(testRealm(), requiredActionsCustomOrdered); + + // make user profile invalid by setting lastName to empty + final var userResource = testRealm().users().get(testUserId); + final var user = userResource.toRepresentation(); + user.setLastName(""); + userResource.update(user); + + /* NOTE: we don't configure VERIFY_PROFILE on the user - it's set on the session because the profile is incomplete */ + enableRequiredActionForUser(RequiredAction.TERMS_AND_CONDITIONS); + + // Get a password reset link + loginPage.open(); + loginPage.assertCurrent(TEST_REALM_NAME); + loginPage.login(USERNAME, PASSWORD); + + // Second, complete the profile + verifyProfilePage.assertCurrent(); + events.expectRequiredAction(EventType.VERIFY_PROFILE) + .user(testUserId) + .detail(Details.FIELDS_TO_UPDATE, UserModel.LAST_NAME) + .assertEvent(); + + verifyProfilePage.update(NEW_FIRST_NAME, NEW_LAST_NAME); + events.expectRequiredAction(EventType.UPDATE_PROFILE) + .user(testUserId) + .detail(Details.UPDATED_FIRST_NAME, NEW_FIRST_NAME) + .detail(Details.UPDATED_LAST_NAME, NEW_LAST_NAME) + .assertEvent(); + + // Finally, accept terms + termsPage.assertCurrent(); + termsPage.acceptTerms(); + events.expectRequiredAction(EventType.CUSTOM_REQUIRED_ACTION).removeDetail(Details.REDIRECT_URI) + .detail(Details.CUSTOM_REQUIRED_ACTION, TermsAndConditions.PROVIDER_ID).assertEvent(); + + // Logged in + appPage.assertCurrent(); + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); events.expectLogin().assertEvent(); } @Test public void setupTotpAfterUpdatePassword() { - String testUserId = ApiUtil.findUserByUsername(testRealm(), "test-user@localhost").getId(); + enableRequiredActionForUser(RequiredAction.CONFIGURE_TOTP); + enableRequiredActionForUser(RequiredAction.UPDATE_PASSWORD); - setRequiredActionEnabled("test", testUserId, RequiredAction.CONFIGURE_TOTP.name(), true); - setRequiredActionEnabled("test", testUserId, RequiredAction.UPDATE_PASSWORD.name(), true); - setRequiredActionEnabled("test", testUserId, TermsAndConditions.PROVIDER_ID, false); - setRequiredActionEnabled("test", testUserId, RequiredAction.UPDATE_PROFILE.name(), false); - - // make UPDATE_PASSWORD on top - testRealm().flows().raiseRequiredActionPriority(UserModel.RequiredAction.UPDATE_PASSWORD.name()); - testRealm().flows().raiseRequiredActionPriority(UserModel.RequiredAction.UPDATE_PASSWORD.name()); + // move UPDATE_PASSWORD before top + final var requiredActionsCustomOrdered = List.of( + RequiredAction.UPDATE_PASSWORD, + RequiredAction.CONFIGURE_TOTP + ); + ApiUtil.updateRequiredActionsOrder(testRealm(), requiredActionsCustomOrdered); // Login loginPage.open(); - loginPage.login("test-user@localhost", "password"); + loginPage.login(USERNAME, PASSWORD); // change password changePasswordPage.assertCurrent(); - changePasswordPage.changePassword("new-password", "new-password"); + changePasswordPage.changePassword(NEW_PASSWORD, NEW_PASSWORD); events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); // CONFIGURE_TOTP @@ -200,10 +409,15 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret()), "userLabel"); events.expectRequiredAction(EventType.UPDATE_TOTP).assertEvent(); - // Logined + // Logged in appPage.assertCurrent(); assertThat(appPage.getRequestType(), is(RequestType.AUTH_RESPONSE)); events.expectLogin().assertEvent(); } + + private void enableRequiredActionForUser(final RequiredAction requiredAction) { + setRequiredActionEnabled(TEST_REALM_NAME, testUserId, requiredAction.name(), true); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java index 5c60803e26..9aa668e5e9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java @@ -350,7 +350,9 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { verifyProfilePage.assertCurrent(); //event when form is shown - events.expectRequiredAction(EventType.VERIFY_PROFILE).user(user5Id).detail("fields_to_update", "department").assertEvent(); + events.expectRequiredAction(EventType.VERIFY_PROFILE).user(user5Id) + .detail(Details.FIELDS_TO_UPDATE, "department") + .assertEvent(); verifyProfilePage.update("First", "Last", "Department"); //event after profile is updated diff --git a/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java b/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java index 0ebc9a9024..2d5afba650 100644 --- a/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java +++ b/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java @@ -227,11 +227,18 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest { webAuthnRegisterPage.assertCurrent(); webAuthnRegisterPage.clickRegister(); - webAuthnRegisterPage.registerWebAuthnCredential(PASSWORDLESS_LABEL); + webAuthnRegisterPage.registerWebAuthnCredential(WEBAUTHN_LABEL); webAuthnRegisterPage.assertCurrent(); + + events.expectRequiredAction(CUSTOM_REQUIRED_ACTION) + .user(userId) + .detail(Details.CUSTOM_REQUIRED_ACTION, WebAuthnRegisterFactory.PROVIDER_ID) + .detail(WebAuthnConstants.PUBKEY_CRED_LABEL_ATTR, WEBAUTHN_LABEL) + .assertEvent(); + webAuthnRegisterPage.clickRegister(); - webAuthnRegisterPage.registerWebAuthnCredential(WEBAUTHN_LABEL); + webAuthnRegisterPage.registerWebAuthnCredential(PASSWORDLESS_LABEL); appPage.assertCurrent(); @@ -241,12 +248,6 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest { .detail(WebAuthnConstants.PUBKEY_CRED_LABEL_ATTR, PASSWORDLESS_LABEL) .assertEvent(); - events.expectRequiredAction(CUSTOM_REQUIRED_ACTION) - .user(userId) - .detail(Details.CUSTOM_REQUIRED_ACTION, WebAuthnRegisterFactory.PROVIDER_ID) - .detail(WebAuthnConstants.PUBKEY_CRED_LABEL_ATTR, WEBAUTHN_LABEL) - .assertEvent(); - final String sessionID = events.expectLogin() .user(userId) .assertEvent()