From 01be4032d84062baa8999098e6a0ae43b7adc586 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Tue, 30 Jan 2024 17:51:01 +0100 Subject: [PATCH] Enable verify-profile required action by default Closes #25985 Signed-off-by: rmartinc --- .../account-ui/test/realms/groups-realm.json | 3 + .../test/realms/user-profile-realm.json | 92 +++++++++++++++++++ .../models/utils/DefaultRequiredActions.java | 16 +++- .../services/managers/ApplianceBootstrap.java | 14 +++ .../resources/KeycloakApplication.java | 5 +- .../testsuite/AbstractKeycloakTest.java | 20 +++- .../testsuite/admin/PermissionsTest.java | 2 +- .../authentication/RequiredActionsTest.java | 9 +- .../testsuite/admin/realm/RealmTest.java | 2 +- .../testsuite/cli/admin/KcAdmSessionTest.java | 2 +- .../federation/storage/UserStorageTest.java | 2 +- .../testsuite/forms/VerifyProfileTest.java | 18 ++-- .../testsuite/oauth/RefreshTokenTest.java | 12 ++- 13 files changed, 171 insertions(+), 26 deletions(-) diff --git a/js/apps/account-ui/test/realms/groups-realm.json b/js/apps/account-ui/test/realms/groups-realm.json index 147429fd42..8f34028acd 100644 --- a/js/apps/account-ui/test/realms/groups-realm.json +++ b/js/apps/account-ui/test/realms/groups-realm.json @@ -12,6 +12,9 @@ "users": [ { "username": "jdoe", + "firstName": "John", + "lastName": "Doe", + "email": "jdoe@keycloak.org", "enabled": true, "realmRoles": [], "clientRoles": { diff --git a/js/apps/account-ui/test/realms/user-profile-realm.json b/js/apps/account-ui/test/realms/user-profile-realm.json index 7822a5d236..0733c07f1b 100644 --- a/js/apps/account-ui/test/realms/user-profile-realm.json +++ b/js/apps/account-ui/test/realms/user-profile-realm.json @@ -10,6 +10,98 @@ "attributes": { "userProfileEnabled": "true" }, + "requiredActions": [ + { + "alias": "CONFIGURE_TOTP", + "name": "Configure OTP", + "providerId": "CONFIGURE_TOTP", + "enabled": true, + "defaultAction": false, + "priority": 10, + "config": {} + }, + { + "alias": "TERMS_AND_CONDITIONS", + "name": "Terms and Conditions", + "providerId": "TERMS_AND_CONDITIONS", + "enabled": false, + "defaultAction": false, + "priority": 20, + "config": {} + }, + { + "alias": "UPDATE_PASSWORD", + "name": "Update Password", + "providerId": "UPDATE_PASSWORD", + "enabled": true, + "defaultAction": false, + "priority": 30, + "config": {} + }, + { + "alias": "UPDATE_PROFILE", + "name": "Update Profile", + "providerId": "UPDATE_PROFILE", + "enabled": true, + "defaultAction": false, + "priority": 40, + "config": {} + }, + { + "alias": "VERIFY_EMAIL", + "name": "Verify Email", + "providerId": "VERIFY_EMAIL", + "enabled": true, + "defaultAction": false, + "priority": 50, + "config": {} + }, + { + "alias": "delete_account", + "name": "Delete Account", + "providerId": "delete_account", + "enabled": false, + "defaultAction": false, + "priority": 60, + "config": {} + }, + { + "alias": "webauthn-register", + "name": "Webauthn Register", + "providerId": "webauthn-register", + "enabled": true, + "defaultAction": false, + "priority": 70, + "config": {} + }, + { + "alias": "webauthn-register-passwordless", + "name": "Webauthn Register Passwordless", + "providerId": "webauthn-register-passwordless", + "enabled": true, + "defaultAction": false, + "priority": 80, + "config": {} + }, + { + "alias": "VERIFY_PROFILE", + "name": "Verify Profile", + "providerId": "VERIFY_PROFILE", + "enabled": false, + "defaultAction": false, + "priority": 90, + "config": {} + }, + { + "alias": "update_user_locale", + "name": "Update User Locale", + "providerId": "update_user_locale", + "enabled": true, + "defaultAction": false, + "priority": 1000, + "config": {} + } + ], "clients": [ { "clientId": "security-admin-console-v2", 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 aa49046c37..3d81d69d36 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 @@ -81,7 +81,8 @@ public class DefaultRequiredActions { UPDATE_EMAIL(UserModel.RequiredAction.UPDATE_EMAIL.name(), DefaultRequiredActions::addUpdateEmailAction, () -> isFeatureEnabled(Profile.Feature.UPDATE_EMAIL)), CONFIGURE_RECOVERY_AUTHN_CODES(UserModel.RequiredAction.CONFIGURE_RECOVERY_AUTHN_CODES.name(), DefaultRequiredActions::addRecoveryAuthnCodesAction, () -> isFeatureEnabled(Profile.Feature.RECOVERY_CODES)), WEBAUTHN_REGISTER("webauthn-register", DefaultRequiredActions::addWebAuthnRegisterAction, () -> isFeatureEnabled(Profile.Feature.WEB_AUTHN)), - WEBAUTHN_PASSWORDLESS_REGISTER("webauthn-register-passwordless", DefaultRequiredActions::addWebAuthnPasswordlessRegisterAction, () -> isFeatureEnabled(Profile.Feature.WEB_AUTHN)); + WEBAUTHN_PASSWORDLESS_REGISTER("webauthn-register-passwordless", DefaultRequiredActions::addWebAuthnPasswordlessRegisterAction, () -> isFeatureEnabled(Profile.Feature.WEB_AUTHN)), + VERIFY_USER_PROFILE(UserModel.RequiredAction.VERIFY_PROFILE.name(), DefaultRequiredActions::addVerifyProfile); private final String alias; private final Consumer addAction; @@ -182,6 +183,19 @@ public class DefaultRequiredActions { } } + public static void addVerifyProfile(RealmModel realm) { + if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.VERIFY_PROFILE.name()) == null) { + RequiredActionProviderModel termsAndConditions = new RequiredActionProviderModel(); + termsAndConditions.setEnabled(true); + termsAndConditions.setAlias(UserModel.RequiredAction.VERIFY_PROFILE.name()); + termsAndConditions.setName("Verify Profile"); + termsAndConditions.setProviderId(UserModel.RequiredAction.VERIFY_PROFILE.name()); + termsAndConditions.setDefaultAction(false); + termsAndConditions.setPriority(90); + realm.addRequiredActionProvider(termsAndConditions); + } + } + public static void addDeleteAccountAction(RealmModel realm) { if (realm.getRequiredActionProviderByAlias("delete_account") == null) { RequiredActionProviderModel deleteAccount = new RequiredActionProviderModel(); diff --git a/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java b/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java index 10b6a924ce..0498b97339 100755 --- a/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java +++ b/services/src/main/java/org/keycloak/services/managers/ApplianceBootstrap.java @@ -28,7 +28,10 @@ import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.DefaultKeyProviders; import org.keycloak.representations.idm.CredentialRepresentation; +import org.keycloak.representations.userprofile.config.UPAttribute; +import org.keycloak.representations.userprofile.config.UPConfig; import org.keycloak.services.ServicesLogger; +import org.keycloak.userprofile.UserProfileProvider; /** * @author Bill Burke @@ -89,6 +92,17 @@ public class ApplianceBootstrap { session.getContext().setRealm(realm); DefaultKeyProviders.createProviders(realm); + // In master realm the UP config is more relaxed + // firstName, lastName and email are not required (all attributes except username) + UserProfileProvider UserProfileProvider = session.getProvider(UserProfileProvider.class); + UPConfig upConfig = UserProfileProvider.getConfiguration(); + for (UPAttribute attr : upConfig.getAttributes()) { + if (!UserModel.USERNAME.equals(attr.getName())) { + attr.setRequired(null); + } + } + UserProfileProvider.setConfiguration(upConfig); + return true; } diff --git a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java index 904c82a44c..195ba5d846 100644 --- a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java +++ b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java @@ -318,10 +318,7 @@ public class KeycloakApplication extends Application { if (users.getUserByUsername(realm, userRep.getUsername()) != null) { ServicesLogger.LOGGER.notCreatingExistingUser(userRep.getUsername()); } else { - UserModel user = users.addUser(realm, userRep.getUsername()); - user.setEnabled(userRep.isEnabled()); - RepresentationToModel.createCredentials(userRep, session, realm, user, false); - RepresentationToModel.createRoleMappings(userRep, user, realm); + UserModel user = RepresentationToModel.createUser(session, realm, userRep); ServicesLogger.LOGGER.addUserSuccess(userRep.getUsername(), realmRep.getRealm()); } }); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java index ff5f7435e3..fc2bbe463c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java @@ -37,7 +37,6 @@ import org.keycloak.admin.client.resource.UserResource; import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.common.util.Time; -import org.keycloak.models.cache.UserCache; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.representations.idm.ClientRepresentation; @@ -79,7 +78,6 @@ import java.util.Calendar; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Scanner; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; @@ -93,6 +91,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import org.keycloak.models.UserModel; import static org.keycloak.testsuite.admin.Users.setPasswordFor; import static org.keycloak.testsuite.auth.page.AuthRealm.MASTER; import static org.keycloak.testsuite.util.ServerURLs.AUTH_SERVER_HOST; @@ -473,6 +472,10 @@ public abstract class AbstractKeycloakTest { assertThat(adminClient.realms().findAll().size(), is(equalTo(1))); } + protected boolean removeVerifyProfileAtImport() { + // remove verify profile by default because most tests are not prepared + return true; + } public void importRealm(RealmRepresentation realm) { if (modifyRealmForSSL()) { @@ -511,6 +514,19 @@ public abstract class AbstractKeycloakTest { // expected when realm does not exist } adminClient.realms().create(realm); + + if (removeVerifyProfileAtImport()) { + try { + RequiredActionProviderRepresentation vpModel = adminClient.realm(realm.getRealm()).flows() + .getRequiredAction(UserModel.RequiredAction.VERIFY_PROFILE.name()); + vpModel.setEnabled(false); + vpModel.setDefaultAction(false); + adminClient.realm(realm.getRealm()).flows().updateRequiredAction( + UserModel.RequiredAction.VERIFY_PROFILE.name(), vpModel); + testingClient.testing().pollAdminEvent(); // remove the event + } catch (NotFoundException ignore) { + } + } } public void removeRealm(String realmName) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java index be8e872904..f959575816 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java @@ -191,7 +191,7 @@ public class PermissionsTest extends AbstractKeycloakTest { RealmRepresentation permissionRealm = testContext.getTestRealmReps().stream().filter(realm -> { return realm.getRealm().equals(REALM_NAME); }).findFirst().get(); - adminClient.realms().create(permissionRealm); + importRealm(permissionRealm); removeTestUsers(); createTestUsers(); 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 b5fd9e2451..ab9dc6ddb0 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 @@ -40,6 +40,12 @@ import java.util.Map; */ public class RequiredActionsTest extends AbstractAuthenticationTest { + @Override + protected boolean removeVerifyProfileAtImport() { + // do not remove verify profile action for this test + return false; + } + @Test public void testRequiredActions() { List result = authMgmtResource.getRequiredActions(); @@ -50,6 +56,7 @@ public class RequiredActionsTest extends AbstractAuthenticationTest { addRequiredAction(expected, "UPDATE_PASSWORD", "Update Password", true, false, null); addRequiredAction(expected, "UPDATE_PROFILE", "Update Profile", true, false, null); addRequiredAction(expected, "VERIFY_EMAIL", "Verify Email", true, false, null); + addRequiredAction(expected, "VERIFY_PROFILE", "Verify Profile", true, false, null); addRequiredAction(expected, "delete_account", "Delete Account", false, false, null); addRequiredAction(expected, "update_user_locale", "Update User Locale", true, false, null); addRequiredAction(expected, "webauthn-register", "Webauthn Register", true, false, null); @@ -84,7 +91,7 @@ public class RequiredActionsTest extends AbstractAuthenticationTest { // Dummy RequiredAction is not registered in the realm and WebAuthn actions List result = authMgmtResource.getUnregisteredRequiredActions(); - Assert.assertEquals(2, result.size()); + Assert.assertEquals(1, result.size()); RequiredActionProviderSimpleRepresentation action = result.stream().filter( a -> a.getProviderId().equals(DummyRequiredActionFactory.PROVIDER_ID) ).findFirst().get(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java index c43a1bcb39..9d2159b770 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java @@ -366,7 +366,7 @@ public class RealmTest extends AbstractAdminTest { RealmRepresentation realmRep = testContext.getTestRealmReps().stream().filter((RealmRepresentation realm) -> { return realm.getRealm().equals(REALM_NAME); }).findFirst().get(); - adminClient.realms().create(realmRep); + importRealm(realmRep); } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmSessionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmSessionTest.java index f080afdf65..68e3d3e36d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmSessionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmSessionTest.java @@ -43,7 +43,7 @@ public class KcAdmSessionTest extends AbstractAdmCliTest { Assert.assertTrue(exe.stderrLines().get(exe.stderrLines().size() - 1).startsWith("Created ")); // create user - exe = execute("create users --config '" + configFile.getName() + "' -r demorealm -s username=testuser -s enabled=true -i"); + exe = execute("create users --config '" + configFile.getName() + "' -r demorealm -s username=testuser -s firstName=testuser -s lastName=testuser -s email=testuser@keycloak.org -s enabled=true -i"); assertExitCodeAndStreamSizes(exe, 0, 1, 0); String userId = exe.stdoutLines().get(0); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java index 9257f57b65..0c544f8276 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java @@ -868,7 +868,7 @@ public class UserStorageTest extends AbstractAuthTest { // Re-create realm RealmRepresentation repOrig = testContext.getTestRealmReps().get(0); - adminClient.realms().create(repOrig); + importRealm(repOrig); } @Test 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 4e97a0d875..d66fd71b8e 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 @@ -49,7 +49,6 @@ import org.keycloak.models.UserModel; import org.keycloak.representations.idm.AdminEventRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.representations.idm.RequiredActionProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.userprofile.config.UPAttribute; import org.keycloak.representations.userprofile.config.UPAttributePermissions; @@ -113,6 +112,12 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { private static ClientRepresentation client_scope_default; private static ClientRepresentation client_scope_optional; + @Override + protected boolean removeVerifyProfileAtImport() { + // we need the verify profile action enabled as default + return false; + } + @Override public void configureTestRealm(RealmRepresentation testRealm) { UserRepresentation user = UserBuilder.create().id(UUID.randomUUID().toString()).username("login-test").email("login@test.com").enabled(true).password("password").build(); @@ -125,17 +130,6 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { RealmBuilder.edit(testRealm).user(user).user(user2).user(user3).user(user4).user(user5).user(user6).user(userWithoutEmail); - RequiredActionProviderRepresentation action = new RequiredActionProviderRepresentation(); - action.setAlias(UserModel.RequiredAction.VERIFY_PROFILE.name()); - action.setProviderId(UserModel.RequiredAction.VERIFY_PROFILE.name()); - action.setEnabled(true); - action.setDefaultAction(false); - action.setPriority(10); - - List actions = new ArrayList<>(); - actions.add(action); - testRealm.setRequiredActions(actions); - testRealm.setClientScopes(new ArrayList<>()); testRealm.getClientScopes().add(ClientScopeBuilder.create().name(SCOPE_DEPARTMENT).protocol("openid-connect").build()); testRealm.getClientScopes().add(ClientScopeBuilder.create().name("profile").protocol("openid-connect").build()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java index 699f49e124..cc76e63b87 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java @@ -364,9 +364,17 @@ public class RefreshTokenTest extends AbstractKeycloakTest { .build()); realmResource.users() - .create(UserBuilder.create().username("alice").password("alice").addRoles("offline_access").build()); + .create(UserBuilder.create().username("alice") + .firstName("alice") + .lastName("alice") + .email("alice@keycloak.org") + .password("alice").addRoles("offline_access").build()); realmResource.users() - .create(UserBuilder.create().username("bob").password("bob").addRoles("offline_access").build()); + .create(UserBuilder.create().username("bob") + .firstName("bob") + .lastName("bob") + .email("bob@keycloak.org") + .password("bob").addRoles("offline_access").build()); oauth.realm(realmName); oauth.clientId("public-client");