From 7c0ca9aad2f552c63022010a1fc4bdbd96abab73 Mon Sep 17 00:00:00 2001 From: Hiroyuki Wada Date: Wed, 4 Jul 2018 21:37:03 +0900 Subject: [PATCH] KEYCLOAK-6313 Add required action's priority for customizing the execution order --- .../RequiredActionProviderRepresentation.java | 9 + .../AuthenticationManagementResource.java | 8 + .../org/keycloak/models/jpa/RealmAdapter.java | 4 + .../RequiredActionProviderEntity.java | 11 ++ .../META-INF/jpa-changelog-4.2.0.xml | 26 +++ .../META-INF/jpa-changelog-master.xml | 1 + .../migration/MigrationModelManager.java | 4 +- .../migration/migrators/MigrateTo4_2_0.java | 73 +++++++++ .../models/utils/DefaultRequiredActions.java | 5 + .../models/utils/RepresentationToModel.java | 1 + .../models/RequiredActionProviderModel.java | 19 +++ .../managers/AuthenticationManager.java | 29 +++- .../AuthenticationManagementResource.java | 77 +++++++++ .../testsuite/actions/ActionUtil.java | 4 + .../actions/RequiredActionPriorityTest.java | 154 ++++++++++++++++++ .../authentication/RequiredActionsTest.java | 5 + .../ShiftRequiredActionTest.java | 85 ++++++++++ .../migration/AbstractMigrationTest.java | 25 +++ .../testsuite/util/AdminEventPaths.java | 12 ++ .../admin/resources/js/controllers/realm.js | 16 +- .../theme/base/admin/resources/js/services.js | 14 ++ .../resources/partials/required-actions.html | 8 +- 22 files changed, 577 insertions(+), 13 deletions(-) create mode 100644 model/jpa/src/main/resources/META-INF/jpa-changelog-4.2.0.xml create mode 100644 server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_2_0.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ShiftRequiredActionTest.java diff --git a/core/src/main/java/org/keycloak/representations/idm/RequiredActionProviderRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/RequiredActionProviderRepresentation.java index cbec62a0de..4e1d76e40f 100755 --- a/core/src/main/java/org/keycloak/representations/idm/RequiredActionProviderRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/RequiredActionProviderRepresentation.java @@ -31,6 +31,7 @@ public class RequiredActionProviderRepresentation { private String providerId; private boolean enabled; private boolean defaultAction; + private int priority; private Map config = new HashMap(); @@ -80,6 +81,14 @@ public class RequiredActionProviderRepresentation { this.providerId = providerId; } + public int getPriority() { + return priority; + } + + public void setPriority(int priority) { + this.priority = priority; + } + public Map getConfig() { return config; } diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AuthenticationManagementResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AuthenticationManagementResource.java index a600ef38bf..bd89d78c9a 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AuthenticationManagementResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AuthenticationManagementResource.java @@ -164,6 +164,14 @@ public interface AuthenticationManagementResource { @DELETE void removeRequiredAction(@PathParam("alias") String alias); + @Path("required-actions/{alias}/raise-priority") + @POST + void raiseRequiredActionPriority(@PathParam("alias") String alias); + + @Path("required-actions/{alias}/lower-priority") + @POST + void lowerRequiredActionPriority(@PathParam("alias") String alias); + @Path("config-description/{providerId}") @GET @Produces(MediaType.APPLICATION_JSON) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index f006180e7b..fa29652496 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -1661,6 +1661,7 @@ public class RealmAdapter implements RealmModel, JpaModel { auth.setConfig(model.getConfig()); auth.setEnabled(model.isEnabled()); auth.setDefaultAction(model.isDefaultAction()); + auth.setPriority(model.getPriority()); realm.getRequiredActionProviders().add(auth); em.persist(auth); em.flush(); @@ -1691,6 +1692,7 @@ public class RealmAdapter implements RealmModel, JpaModel { model.setAlias(entity.getAlias()); model.setEnabled(entity.isEnabled()); model.setDefaultAction(entity.isDefaultAction()); + model.setPriority(entity.getPriority()); model.setName(entity.getName()); Map config = new HashMap<>(); if (entity.getConfig() != null) config.putAll(entity.getConfig()); @@ -1706,6 +1708,7 @@ public class RealmAdapter implements RealmModel, JpaModel { entity.setProviderId(model.getProviderId()); entity.setEnabled(model.isEnabled()); entity.setDefaultAction(model.isDefaultAction()); + entity.setPriority(model.getPriority()); entity.setName(model.getName()); if (entity.getConfig() == null) { entity.setConfig(model.getConfig()); @@ -1725,6 +1728,7 @@ public class RealmAdapter implements RealmModel, JpaModel { for (RequiredActionProviderEntity entity : entities) { actions.add(entityToModel(entity)); } + Collections.sort(actions, RequiredActionProviderModel.RequiredActionComparator.SINGLETON); return Collections.unmodifiableList(actions); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RequiredActionProviderEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RequiredActionProviderEntity.java index d8b43fb73e..02d6187382 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RequiredActionProviderEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RequiredActionProviderEntity.java @@ -66,6 +66,9 @@ public class RequiredActionProviderEntity { @Column(name="DEFAULT_ACTION") protected boolean defaultAction; + @Column(name="PRIORITY") + protected int priority; + @ElementCollection @MapKeyColumn(name="NAME") @Column(name="VALUE") @@ -136,6 +139,14 @@ public class RequiredActionProviderEntity { this.name = name; } + public int getPriority() { + return priority; + } + + public void setPriority(int priority) { + this.priority = priority; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-4.2.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-4.2.0.xml new file mode 100644 index 0000000000..be0202b69c --- /dev/null +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-4.2.0.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml index f087429d8f..7bf1d34b9d 100755 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml @@ -57,4 +57,5 @@ + diff --git a/server-spi-private/src/main/java/org/keycloak/migration/MigrationModelManager.java b/server-spi-private/src/main/java/org/keycloak/migration/MigrationModelManager.java index e68f694136..1ffc8fe1f3 100755 --- a/server-spi-private/src/main/java/org/keycloak/migration/MigrationModelManager.java +++ b/server-spi-private/src/main/java/org/keycloak/migration/MigrationModelManager.java @@ -39,6 +39,7 @@ import org.keycloak.migration.migrators.MigrateTo3_4_0; import org.keycloak.migration.migrators.MigrateTo3_4_1; import org.keycloak.migration.migrators.MigrateTo3_4_2; import org.keycloak.migration.migrators.MigrateTo4_0_0; +import org.keycloak.migration.migrators.MigrateTo4_2_0; import org.keycloak.migration.migrators.Migration; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -72,7 +73,8 @@ public class MigrationModelManager { new MigrateTo3_4_0(), new MigrateTo3_4_1(), new MigrateTo3_4_2(), - new MigrateTo4_0_0() + new MigrateTo4_0_0(), + new MigrateTo4_2_0() }; public static void migrate(KeycloakSession session) { diff --git a/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_2_0.java b/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_2_0.java new file mode 100644 index 0000000000..e14ca50cea --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/migration/migrators/MigrateTo4_2_0.java @@ -0,0 +1,73 @@ +/* + * Copyright 2018 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.migration.migrators; + +import static java.util.Comparator.comparing; + +import java.util.List; +import java.util.stream.Collectors; + +import org.jboss.logging.Logger; +import org.keycloak.migration.ModelVersion; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RequiredActionProviderModel; +import org.keycloak.representations.idm.RealmRepresentation; + +/** + * @author Hiroyuki Wada + */ +public class MigrateTo4_2_0 implements Migration { + + public static final ModelVersion VERSION = new ModelVersion("4.2.0"); + + private static final Logger LOG = Logger.getLogger(MigrateTo4_2_0.class); + + @Override + public ModelVersion getVersion() { + return VERSION; + } + + @Override + public void migrate(KeycloakSession session) { + session.realms().getRealms().stream().forEach(r -> { + migrateRealm(session, r, false); + }); + } + + @Override + public void migrateImport(KeycloakSession session, RealmModel realm, RealmRepresentation rep, boolean skipUserDependent) { + migrateRealm(session, realm, true); + } + + protected void migrateRealm(KeycloakSession session, RealmModel realm, boolean json) { + // Set default priority of required actions in alphabetical order + List actions = realm.getRequiredActionProviders().stream() + .sorted(comparing(RequiredActionProviderModel::getName)).collect(Collectors.toList()); + int priority = 10; + for (RequiredActionProviderModel model : actions) { + LOG.debugf("Setting priority '%d' for required action '%s' in realm '%s'", priority, model.getAlias(), + realm.getName()); + model.setPriority(priority); + priority += 10; + + // Save + realm.updateRequiredActionProvider(model); + } + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/DefaultRequiredActions.java b/server-spi-private/src/main/java/org/keycloak/models/utils/DefaultRequiredActions.java index db38b64850..19b802ce8a 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/DefaultRequiredActions.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/DefaultRequiredActions.java @@ -34,6 +34,7 @@ public class DefaultRequiredActions { verifyEmail.setName("Verify Email"); verifyEmail.setProviderId(UserModel.RequiredAction.VERIFY_EMAIL.name()); verifyEmail.setDefaultAction(false); + verifyEmail.setPriority(50); realm.addRequiredActionProvider(verifyEmail); } @@ -45,6 +46,7 @@ public class DefaultRequiredActions { updateProfile.setName("Update Profile"); updateProfile.setProviderId(UserModel.RequiredAction.UPDATE_PROFILE.name()); updateProfile.setDefaultAction(false); + updateProfile.setPriority(40); realm.addRequiredActionProvider(updateProfile); } @@ -55,6 +57,7 @@ public class DefaultRequiredActions { totp.setName("Configure OTP"); totp.setProviderId(UserModel.RequiredAction.CONFIGURE_TOTP.name()); totp.setDefaultAction(false); + totp.setPriority(10); realm.addRequiredActionProvider(totp); } @@ -65,6 +68,7 @@ public class DefaultRequiredActions { updatePassword.setName("Update Password"); updatePassword.setProviderId(UserModel.RequiredAction.UPDATE_PASSWORD.name()); updatePassword.setDefaultAction(false); + updatePassword.setPriority(30); realm.addRequiredActionProvider(updatePassword); } @@ -75,6 +79,7 @@ public class DefaultRequiredActions { termsAndConditions.setName("Terms and Conditions"); termsAndConditions.setProviderId("terms_and_conditions"); termsAndConditions.setDefaultAction(false); + termsAndConditions.setPriority(20); realm.addRequiredActionProvider(termsAndConditions); } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 9089fb2040..6be4b2c2a3 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -1862,6 +1862,7 @@ public class RepresentationToModel { public static RequiredActionProviderModel toModel(RequiredActionProviderRepresentation rep) { RequiredActionProviderModel model = new RequiredActionProviderModel(); model.setConfig(rep.getConfig()); + model.setPriority(rep.getPriority()); model.setDefaultAction(rep.isDefaultAction()); model.setEnabled(rep.isEnabled()); model.setProviderId(rep.getProviderId()); diff --git a/server-spi/src/main/java/org/keycloak/models/RequiredActionProviderModel.java b/server-spi/src/main/java/org/keycloak/models/RequiredActionProviderModel.java index 891186a8ce..e7a2f1982c 100755 --- a/server-spi/src/main/java/org/keycloak/models/RequiredActionProviderModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RequiredActionProviderModel.java @@ -18,6 +18,7 @@ package org.keycloak.models; import java.io.Serializable; +import java.util.Comparator; import java.util.HashMap; import java.util.Map; @@ -27,12 +28,22 @@ import java.util.Map; */ public class RequiredActionProviderModel implements Serializable { + public static class RequiredActionComparator implements Comparator { + public static final RequiredActionComparator SINGLETON = new RequiredActionComparator(); + + @Override + public int compare(RequiredActionProviderModel o1, RequiredActionProviderModel o2) { + return o1.priority - o2.priority; + } + } + private String id; private String alias; private String name; private String providerId; private boolean enabled; private boolean defaultAction; + private int priority; private Map config = new HashMap(); @@ -90,6 +101,14 @@ public class RequiredActionProviderModel implements Serializable { this.providerId = providerId; } + public int getPriority() { + return priority; + } + + public void setPriority(int priority) { + this.priority = priority; + } + public Map getConfig() { return config; } 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 60ae93f886..0db650cdc6 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -994,16 +994,10 @@ public class AuthenticationManager { protected static Response executionActions(KeycloakSession session, AuthenticationSessionModel authSession, HttpRequest request, EventBuilder event, RealmModel realm, UserModel user, Set requiredActions) { - for (String action : requiredActions) { - 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); - continue; - } - if (!model.isEnabled()) { - continue; - } + List sortedRequiredActions = sortRequiredActionsByPriority(realm, requiredActions); + + for (RequiredActionProviderModel model : sortedRequiredActions) { RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, model.getProviderId()); if (factory == null) { throw new RuntimeException("Unable to find factory for Required Action: " + model.getProviderId() + " did you forget to declare it in a META-INF/services file?"); @@ -1044,6 +1038,23 @@ public class AuthenticationManager { return null; } + private static List sortRequiredActionsByPriority(RealmModel realm, Set requiredActions) { + List actions = new ArrayList<>(); + for (String action : requiredActions) { + 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); + continue; + } + if (!model.isEnabled()) { + continue; + } + actions.add(model); + } + Collections.sort(actions, RequiredActionProviderModel.RequiredActionComparator.SINGLETON); + return actions; + } + public static void evaluateRequiredActionTriggers(final KeycloakSession session, final AuthenticationSessionModel authSession, final ClientConnection clientConnection, final HttpRequest request, final UriInfo uriInfo, final EventBuilder event, final RealmModel realm, final UserModel user) { // see if any required actions need triggering, i.e. an expired password diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java index 2874597186..86561f644e 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java @@ -72,6 +72,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; import static javax.ws.rs.core.Response.Status.NOT_FOUND; @@ -880,6 +881,7 @@ public class AuthenticationManagementResource { requiredAction.setName(name); requiredAction.setProviderId(providerId); requiredAction.setDefaultAction(false); + requiredAction.setPriority(getNextRequiredActionPriority()); requiredAction.setEnabled(true); requiredAction = realm.addRequiredActionProvider(requiredAction); @@ -887,7 +889,12 @@ public class AuthenticationManagementResource { adminEvent.operation(OperationType.CREATE).resource(ResourceType.REQUIRED_ACTION).resourcePath(uriInfo).representation(data).success(); } + private int getNextRequiredActionPriority() { + List actions = realm.getRequiredActionProviders(); + return actions.isEmpty() ? 0 : actions.get(actions.size() - 1).getPriority() + 1; + } + /** * Get required actions * @@ -913,6 +920,7 @@ public class AuthenticationManagementResource { rep.setAlias(model.getAlias()); rep.setName(model.getName()); rep.setDefaultAction(model.isDefaultAction()); + rep.setPriority(model.getPriority()); rep.setEnabled(model.isEnabled()); rep.setConfig(model.getConfig()); return rep; @@ -959,6 +967,7 @@ public class AuthenticationManagementResource { update.setAlias(rep.getAlias()); update.setProviderId(model.getProviderId()); update.setDefaultAction(rep.isDefaultAction()); + update.setPriority(rep.getPriority()); update.setEnabled(rep.isEnabled()); update.setConfig(rep.getConfig()); realm.updateRequiredActionProvider(update); @@ -984,6 +993,74 @@ public class AuthenticationManagementResource { adminEvent.operation(OperationType.DELETE).resource(ResourceType.REQUIRED_ACTION).resourcePath(uriInfo).success(); } + /** + * Raise required action's priority + * + * @param alias Alias of required action + */ + @Path("required-actions/{alias}/raise-priority") + @POST + @NoCache + public void raiseRequiredActionPriority(@PathParam("alias") String alias) { + auth.realm().requireManageRealm(); + + RequiredActionProviderModel model = realm.getRequiredActionProviderByAlias(alias); + if (model == null) { + throw new NotFoundException("Failed to find required action."); + } + + List actions = realm.getRequiredActionProviders(); + RequiredActionProviderModel previous = null; + for (RequiredActionProviderModel action : actions) { + if (action.getId().equals(model.getId())) { + break; + } + previous = action; + } + if (previous == null) return; + int tmp = previous.getPriority(); + previous.setPriority(model.getPriority()); + realm.updateRequiredActionProvider(previous); + model.setPriority(tmp); + realm.updateRequiredActionProvider(model); + + adminEvent.operation(OperationType.UPDATE).resource(ResourceType.REQUIRED_ACTION).resourcePath(uriInfo).success(); + } + + /** + * Lower required action's priority + * + * @param alias Alias of required action + */ + @Path("/required-actions/{alias}/lower-priority") + @POST + @NoCache + public void lowerRequiredActionPriority(@PathParam("alias") String alias) { + auth.realm().requireManageRealm(); + + RequiredActionProviderModel model = realm.getRequiredActionProviderByAlias(alias); + if (model == null) { + throw new NotFoundException("Failed to find required action."); + } + + List actions = realm.getRequiredActionProviders(); + int i = 0; + for (i = 0; i < actions.size(); i++) { + if (actions.get(i).getId().equals(model.getId())) { + break; + } + } + if (i + 1 >= actions.size()) return; + RequiredActionProviderModel next = actions.get(i + 1); + int tmp = model.getPriority(); + model.setPriority(next.getPriority()); + realm.updateRequiredActionProvider(model); + next.setPriority(tmp); + realm.updateRequiredActionProvider(next); + + adminEvent.operation(OperationType.UPDATE).resource(ResourceType.REQUIRED_ACTION).resourcePath(uriInfo).success(); + } + /** * Get authenticator provider's configuration description */ diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/ActionUtil.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/ActionUtil.java index a6a03ab1c7..8b4b4ef584 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/ActionUtil.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/ActionUtil.java @@ -25,6 +25,7 @@ import org.keycloak.testsuite.util.UserBuilder; import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; /** * @@ -52,12 +53,15 @@ public class ActionUtil { public static void addRequiredActionForRealm(RealmRepresentation testRealm, String providerId) { List requiredActions = testRealm.getRequiredActions(); if (requiredActions == null) requiredActions = new LinkedList(); + + RequiredActionProviderRepresentation last = requiredActions.get(requiredActions.size() - 1); RequiredActionProviderRepresentation action = new RequiredActionProviderRepresentation(); action.setAlias(providerId); action.setProviderId(providerId); action.setEnabled(true); action.setDefaultAction(true); + action.setPriority(last.getPriority() + 1); requiredActions.add(action); testRealm.setRequiredActions(requiredActions); 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 new file mode 100644 index 0000000000..8adc4115b9 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionPriorityTest.java @@ -0,0 +1,154 @@ +/* + * Copyright 2018 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.testsuite.actions; + +import org.jboss.arquillian.graphene.page.Page; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.keycloak.authentication.requiredactions.TermsAndConditions; +import org.keycloak.events.Details; +import org.keycloak.events.EventType; +import org.keycloak.models.UserModel; +import org.keycloak.models.UserModel.RequiredAction; +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; +import org.keycloak.testsuite.pages.AppPage; +import org.keycloak.testsuite.pages.AppPage.RequestType; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; +import org.keycloak.testsuite.pages.LoginUpdateProfileEditUsernameAllowedPage; +import org.keycloak.testsuite.pages.TermsAndConditionsPage; +import org.keycloak.testsuite.util.UserBuilder; + +/** + * @author Hiroyuki Wada + */ +public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest { + + @Override + public void configureTestRealm(RealmRepresentation testRealm) { + } + + @Rule + public AssertEvents events = new AssertEvents(this); + + @Page + protected AppPage appPage; + + @Page + protected LoginPage loginPage; + + @Page + protected LoginPasswordUpdatePage changePasswordPage; + + @Page + protected LoginUpdateProfileEditUsernameAllowedPage updateProfilePage; + + @Page + protected TermsAndConditionsPage termsPage; + + @Before + public void setupRequiredActions() { + setRequiredActionEnabled("test", 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"); + + setRequiredActionEnabled("test", testUserId, RequiredAction.UPDATE_PASSWORD.name(), true); + setRequiredActionEnabled("test", testUserId, RequiredAction.UPDATE_PROFILE.name(), true); + setRequiredActionEnabled("test", testUserId, TermsAndConditions.PROVIDER_ID, true); + } + + @Test + public void executeRequiredActionsWithDefaultPriority() throws Exception { + // Default priority is alphabetical order: + // TermsAndConditions -> UpdatePassword -> UpdateProfile + + // Login + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + // First, 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(); + + // Second, change password + changePasswordPage.assertCurrent(); + changePasswordPage.changePassword("new-password", "new-password"); + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); + + // Finally, update profile + updateProfilePage.assertCurrent(); + updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); + events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost") + .detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); + + // Logined + appPage.assertCurrent(); + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + events.expectLogin().assertEvent(); + } + + @Test + public void executeRequiredActionsWithCustomPriority() throws Exception { + // 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("terms_and_conditions"); + + // Login + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + // First, change password + changePasswordPage.assertCurrent(); + changePasswordPage.changePassword("new-password", "new-password"); + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); + + // Second, update profile + updateProfilePage.assertCurrent(); + updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost"); + events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost") + .detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); + events.expectRequiredAction(EventType.UPDATE_PROFILE).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(); + + // Logined + appPage.assertCurrent(); + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + events.expectLogin().assertEvent(); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/RequiredActionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/RequiredActionsTest.java index fb7256457f..0fe4a9b231 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/RequiredActionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/RequiredActionsTest.java @@ -72,6 +72,8 @@ public class RequiredActionsTest extends AbstractAuthenticationTest { @Test public void testCRUDRequiredAction() { + int lastPriority = authMgmtResource.getRequiredActions().get(authMgmtResource.getRequiredActions().size() - 1).getPriority(); + // Just Dummy RequiredAction is not registered in the realm List result = authMgmtResource.getUnregisteredRequiredActions(); Assert.assertEquals(1, result.size()); @@ -96,6 +98,9 @@ public class RequiredActionsTest extends AbstractAuthenticationTest { compareRequiredAction(rep, newRequiredAction(DummyRequiredActionFactory.PROVIDER_ID, "Dummy Action", true, false, Collections.emptyMap())); + // Confirm the registered priority - should be N + 1 + Assert.assertEquals(lastPriority + 1, rep.getPriority()); + // Update not-existent - should fail try { authMgmtResource.updateRequiredAction("not-existent", rep); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ShiftRequiredActionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ShiftRequiredActionTest.java new file mode 100644 index 0000000000..a7af497f29 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ShiftRequiredActionTest.java @@ -0,0 +1,85 @@ +/* + * Copyright 2018 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.admin.authentication; + +import java.util.List; + +import javax.ws.rs.NotFoundException; + +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.events.admin.OperationType; +import org.keycloak.events.admin.ResourceType; +import org.keycloak.representations.idm.RequiredActionProviderRepresentation; +import org.keycloak.testsuite.util.AdminEventPaths; + +/** + * @author Hiroyuki Wada + */ +public class ShiftRequiredActionTest extends AbstractAuthenticationTest { + + @Test + public void testShiftRequiredAction() { + + // get action + List actions = authMgmtResource.getRequiredActions(); + + RequiredActionProviderRepresentation last = actions.get(actions.size() - 1); + RequiredActionProviderRepresentation oneButLast = actions.get(actions.size() - 2); + + // Not possible to raisePriority of not-existent required action + try { + authMgmtResource.raisePriority("not-existent"); + Assert.fail("Not expected to raise priority of not existent required action"); + } catch (NotFoundException nfe) { + // Expected + } + + // shift last required action up + authMgmtResource.raiseRequiredActionPriority(last.getAlias()); + assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authRaiseRequiredActionPath(last.getAlias()), ResourceType.REQUIRED_ACTION); + + List actions2 = authMgmtResource.getRequiredActions(); + + RequiredActionProviderRepresentation last2 = actions2.get(actions.size() - 1); + RequiredActionProviderRepresentation oneButLast2 = actions2.get(actions.size() - 2); + + Assert.assertEquals("Required action shifted up - N", last.getAlias(), oneButLast2.getAlias()); + Assert.assertEquals("Required action up - N-1", oneButLast.getAlias(), last2.getAlias()); + + // Not possible to lowerPriority of not-existent required action + try { + authMgmtResource.lowerRequiredActionPriority("not-existent"); + Assert.fail("Not expected to raise priority of not existent required action"); + } catch (NotFoundException nfe) { + // Expected + } + + // shift one before last down + authMgmtResource.lowerRequiredActionPriority(oneButLast2.getAlias()); + assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authLowerRequiredActionPath(oneButLast2.getAlias()), ResourceType.REQUIRED_ACTION); + + actions2 = authMgmtResource.getRequiredActions(); + + last2 = actions2.get(actions.size() - 1); + oneButLast2 = actions2.get(actions.size() - 2); + + Assert.assertEquals("Required action shifted down - N", last.getAlias(), last2.getAlias()); + Assert.assertEquals("Required action shifted down - N-1", oneButLast.getAlias(), oneButLast2.getAlias()); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java index b2498d288e..1a1bd2d97d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java @@ -58,6 +58,7 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -198,6 +199,10 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { testOfflineScopeAddedToClient(); } + protected void testMigrationTo4_2_0() { + testRequiredActionsPriority(this.masterRealm, this.migrationRealm); + } + private void testCliConsoleScopeSize(RealmResource realm) { ClientRepresentation cli = realm.clients().findByClientId(Constants.ADMIN_CLI_CLIENT_ID).get(0); ClientRepresentation console = realm.clients().findByClientId(Constants.ADMIN_CONSOLE_CLIENT_ID).get(0); @@ -462,6 +467,25 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { } + private void testRequiredActionsPriority(RealmResource... realms) { + log.info("testing required action's priority"); + for (RealmResource realm : realms) { + List actions = realm.flows().getRequiredActions(); + + // Checking if the actions are in alphabetical order + List nameList = actions.stream().map(x -> x.getName()).collect(Collectors.toList()); + List sortedByName = nameList.stream().sorted().collect(Collectors.toList()); + assertArrayEquals(nameList.toArray(), sortedByName.toArray()); + + // Checking the priority + int priority = 10; + for (RequiredActionProviderRepresentation action : actions) { + assertEquals(priority, action.getPriority()); + priority += 10; + } + } + } + protected String getMigrationMode() { return System.getProperty("migration.mode"); } @@ -481,6 +505,7 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { protected void testMigrationTo4_x() { testMigrationTo4_0_0(); + testMigrationTo4_2_0(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/AdminEventPaths.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/AdminEventPaths.java index 0e727804f8..980465da52 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/AdminEventPaths.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/AdminEventPaths.java @@ -437,6 +437,18 @@ public class AdminEventPaths { return uri.toString(); } + public static String authRaiseRequiredActionPath(String requiredActionAlias) { + URI uri = UriBuilder.fromUri(authMgmtBasePath()).path(AuthenticationManagementResource.class, "raiseRequiredActionPriority") + .build(requiredActionAlias); + return uri.toString(); + } + + public static String authLowerRequiredActionPath(String requiredActionAlias) { + URI uri = UriBuilder.fromUri(authMgmtBasePath()).path(AuthenticationManagementResource.class, "lowerRequiredActionPriority") + .build(requiredActionAlias); + return uri.toString(); + } + // ATTACK DETECTION public static String attackDetectionClearBruteForceForUserPath(String username) { diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js index 44a74a555c..9bd365e724 100644 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js @@ -2284,7 +2284,7 @@ module.controller('AuthenticationFlowsCtrl', function($scope, $route, realm, flo module.controller('RequiredActionsCtrl', function($scope, realm, unregisteredRequiredActions, $modal, $route, - RegisterRequiredAction, RequiredActions, Notifications) { + RegisterRequiredAction, RequiredActions, RequiredActionRaisePriority, RequiredActionLowerPriority, Notifications) { console.log('RequiredActionsCtrl'); $scope.realm = realm; $scope.unregisteredRequiredActions = unregisteredRequiredActions; @@ -2306,6 +2306,20 @@ module.controller('RequiredActionsCtrl', function($scope, realm, unregisteredReq }); } + $scope.raisePriority = function(action) { + RequiredActionRaisePriority.save({realm: realm.realm, alias: action.alias}, function() { + Notifications.success("Required action's priority raised"); + setupRequiredActionsForm(); + }) + } + + $scope.lowerPriority = function(action) { + RequiredActionLowerPriority.save({realm: realm.realm, alias: action.alias}, function() { + Notifications.success("Required action's priority lowered"); + setupRequiredActionsForm(); + }) + } + $scope.register = function() { var controller = function($scope, $modalInstance) { $scope.unregisteredRequiredActions = unregisteredRequiredActions; diff --git a/themes/src/main/resources/theme/base/admin/resources/js/services.js b/themes/src/main/resources/theme/base/admin/resources/js/services.js index c80cc405f7..2517ae943e 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/services.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/services.js @@ -323,6 +323,20 @@ module.factory('RequiredActions', function($resource) { }); }); +module.factory('RequiredActionRaisePriority', function($resource) { + return $resource(authUrl + '/admin/realms/:realm/authentication/required-actions/:alias/raise-priority', { + realm : '@realm', + alias : '@alias' + }); +}); + +module.factory('RequiredActionLowerPriority', function($resource) { + return $resource(authUrl + '/admin/realms/:realm/authentication/required-actions/:alias/lower-priority', { + realm : '@realm', + alias : '@alias' + }); +}); + module.factory('UnregisteredRequiredActions', function($resource) { return $resource(authUrl + '/admin/realms/:realm/authentication/unregistered-required-actions', { realm : '@realm' diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/required-actions.html b/themes/src/main/resources/theme/base/admin/resources/partials/required-actions.html index 17dcc05685..ab16456c9e 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/required-actions.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/required-actions.html @@ -18,8 +18,12 @@ - - {{requiredAction.name}} + + + + + {{requiredAction.name}} +