From 1bd6aca6297141517de001db9c4d8e4be9f2425f Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Tue, 24 Oct 2023 20:19:33 +0200 Subject: [PATCH] Remove RegistrationProfile class and handle migration (#24215) closes #24182 Co-authored-by: andymunro <48995441+andymunro@users.noreply.github.com> --- .../server_development/topics/auth-spi.adoc | 103 ++++++++------ .../topics/keycloak/changes-23_0_0.adoc | 8 +- .../migration/migrators/MigrateTo23_0_0.java | 26 +++- .../forms/RegistrationProfile.java | 134 ------------------ ....keycloak.authentication.FormActionFactory | 1 - .../admin/authentication/ProvidersTest.java | 2 - .../testsuite/forms/RegisterTest.java | 1 - .../migration/AbstractMigrationTest.java | 12 ++ .../webauthn/testrealm-webauthn.json | 7 - 9 files changed, 105 insertions(+), 189 deletions(-) delete mode 100755 services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java diff --git a/docs/documentation/server_development/topics/auth-spi.adoc b/docs/documentation/server_development/topics/auth-spi.adoc index 068b7e775b..3b4ca14df0 100644 --- a/docs/documentation/server_development/topics/auth-spi.adoc +++ b/docs/documentation/server_development/topics/auth-spi.adoc @@ -1032,51 +1032,43 @@ Let's also look at the user profile plugin that is used to validate email addres @Override public void validate(ValidationContext context) { MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); - List errors = new ArrayList<>(); + context.getEvent().detail(Details.REGISTER_METHOD, "form"); - String eventError = Errors.INVALID_REGISTRATION; + UserProfile profile = getOrCreateUserProfile(context, formData); - if (Validation.isBlank(formData.getFirst((RegistrationPage.FIELD_FIRST_NAME)))) { - errors.add(new FormMessage(RegistrationPage.FIELD_FIRST_NAME, Messages.MISSING_FIRST_NAME)); - } + try { + profile.validate(); + } catch (ValidationException pve) { + List errors = Validation.getFormErrorsFromValidation(pve.getErrors()); - if (Validation.isBlank(formData.getFirst((RegistrationPage.FIELD_LAST_NAME)))) { - errors.add(new FormMessage(RegistrationPage.FIELD_LAST_NAME, Messages.MISSING_LAST_NAME)); - } + if (pve.hasError(Messages.EMAIL_EXISTS, Messages.INVALID_EMAIL)) { + context.getEvent().detail(Details.EMAIL, profile.getAttributes().getFirstValue(UserModel.EMAIL)); + } - String email = formData.getFirst(Validation.FIELD_EMAIL); - if (Validation.isBlank(email)) { - errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL)); - } else if (!Validation.isEmailValid(email)) { - formData.remove(Validation.FIELD_EMAIL); - errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.INVALID_EMAIL)); - } + if (pve.hasError(Messages.EMAIL_EXISTS)) { + context.error(Errors.EMAIL_IN_USE); + } else if (pve.hasError(Messages.USERNAME_EXISTS)) { + context.error(Errors.USERNAME_IN_USE); + } else { + context.error(Errors.INVALID_REGISTRATION); + } - if (context.getSession().users().getUserByEmail(email, context.getRealm()) != null) { - formData.remove(Validation.FIELD_EMAIL); - errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.EMAIL_EXISTS)); - } - - if (errors.size() > 0) { context.validationError(formData, errors); return; - - } else { - context.success(); } + context.success(); } ---- -As you can see, this validate() method of user profile processing makes sure that the email, first, and last name are filled in the form. -It also makes sure that email is in the right format. -If any of these validations fail, an error message is queued up for rendering. -Any fields in error are removed from the form data. -Error messages are represented by the FormMessage class. -The first parameter of the constructor of this class takes the HTML element id. -The input in error will be highlighted when the form is re-rendered. -The second parameter is a message reference id. -This id must correspond to a property in one of the localized message bundle files. -in the theme. +As you can see, this validate() method of user profile processing makes sure that the email and all other attributes are filled in the form. +It delegates to User Profile SPI, which makes sure that email is in the right format and does all other validations. +If any of these validations fail, an error message is queued up for rendering. It would contain the message for every field where the validation failed. + +NOTE: As you can see, the user profile makes sure that registration form contains all the needed user profile fields. User profile also makes sure that correct validations +are used, attributes are correctly grouped on the page. There is a correct type used for each field (such as if a user needs to choose from predefined values), fields +are "conditionally" rendered just for some scopes (Progressive profiling) and others. So usually you will not need to implement new `FormAction` or registration fields, but +you can just properly configure user-profile to reflect this. For more details, see link:{adminguide_link}#user-profile[User Profile documentation]. +In general, new FormAction might be useful for instance if you want to add new credentials to the registration form (such as ReCaptcha support as mentioned here) rather than new user profile fields. After all validations have been processed then, the form flow then invokes the FormAction.success() method. For recaptcha this is a no-op, so we won't go over it. @@ -1087,17 +1079,44 @@ For user profile processing, this method fills in values in the registered user. @Override public void success(FormContext context) { - UserModel user = context.getUser(); + checkNotOtherUserAuthenticating(context); + MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); - user.setFirstName(formData.getFirst(RegistrationPage.FIELD_FIRST_NAME)); - user.setLastName(formData.getFirst(RegistrationPage.FIELD_LAST_NAME)); - user.setEmail(formData.getFirst(RegistrationPage.FIELD_EMAIL)); + + String email = formData.getFirst(UserModel.EMAIL); + String username = formData.getFirst(UserModel.USERNAME); + + if (context.getRealm().isRegistrationEmailAsUsername()) { + username = email; + } + + context.getEvent().detail(Details.USERNAME, username) + .detail(Details.REGISTER_METHOD, "form") + .detail(Details.EMAIL, email); + + UserProfile profile = getOrCreateUserProfile(context, formData); + UserModel user = profile.create(); + + user.setEnabled(true); + + // This means that following actions can retrieve user from the context by context.getUser() method + context.setUser(user); } ---- -Pretty simple implementation. -The UserModel of the newly registered user is obtained from the FormContext. -The appropriate methods are called to initialize UserModel data. +The new user is created and the UserModel of the newly registered user is added to the FormContext. +The appropriate methods are called to initialize UserModel data. In your own FormAction, you can possibly obtain user by using something like: +[source,java] +---- + + @Override + public void success(FormContext context) { + UserModel user = context.getUser(); + if (user != null) { + // Do something useful with the user here ... + } + } +---- Finally, you are also required to define a FormActionFactory class. This class is implemented similarly to AuthenticatorFactory, so we won't go over it. @@ -1112,7 +1131,7 @@ For example: [source] ---- -org.keycloak.authentication.forms.RegistrationProfile +org.keycloak.authentication.forms.RegistrationUserCreation org.keycloak.authentication.forms.RegistrationRecaptcha ---- diff --git a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc index 8620c51c19..cbc4ea218a 100644 --- a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc +++ b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc @@ -100,4 +100,10 @@ Use a single escape: ``` bin/kc.sh start --db postgres --db-username keycloak --db-url "jdbc:postgresql://localhost:5432/keycloak?ssl=false&connectTimeout=30" --db-password keycloak --hostname localhost -``` \ No newline at end of file +``` + += Removed RegistrationProfile form action + +The form action `RegistrationProfile` (displayed in the UI of authentication flows as `Profile Validation`) was removed from the codebase and also from all authentication flows. By default, it was in +the built-in registration flow of every realm. The validation of user attributes as well as creation of the user including all that user's attributes is handled by `RegistrationUserCreation` form action and +hence `RegistrationProfile` is not needed anymore. There is usually no further action needed in relation to this change, unless you used `RegistrationProfile` class in your own providers. diff --git a/model/legacy-private/src/main/java/org/keycloak/migration/migrators/MigrateTo23_0_0.java b/model/legacy-private/src/main/java/org/keycloak/migration/migrators/MigrateTo23_0_0.java index ef642d0858..113c5b761e 100644 --- a/model/legacy-private/src/main/java/org/keycloak/migration/migrators/MigrateTo23_0_0.java +++ b/model/legacy-private/src/main/java/org/keycloak/migration/migrators/MigrateTo23_0_0.java @@ -20,6 +20,9 @@ package org.keycloak.migration.migrators; import java.util.Optional; + +import org.jboss.logging.Logger; +import org.keycloak.authentication.AuthenticationFlow; import org.keycloak.component.ComponentModel; import org.keycloak.migration.ModelVersion; import org.keycloak.models.KeycloakSession; @@ -29,6 +32,8 @@ import org.keycloak.userprofile.UserProfileProvider; public class MigrateTo23_0_0 implements Migration { + private static final Logger LOG = Logger.getLogger(MigrateTo23_0_0.class); + public static final ModelVersion VERSION = new ModelVersion("23.0.0"); private static final String USER_PROFILE_ENABLED_PROP = "userProfileEnabled"; @@ -38,12 +43,17 @@ public class MigrateTo23_0_0 implements Migration { @Override public void migrate(KeycloakSession session) { - session.realms().getRealmsStream().forEach(this::updateUserProfileConfig); + session.realms().getRealmsStream().forEach(this::migrateRealm); } @Override public void migrateImport(KeycloakSession session, RealmModel realm, RealmRepresentation rep, boolean skipUserDependent) { + migrateRealm(realm); + } + + private void migrateRealm(RealmModel realm) { updateUserProfileConfig(realm); + removeRegistrationProfileFormExecution(realm); } private void updateUserProfileConfig(RealmModel realm) { @@ -74,6 +84,20 @@ public class MigrateTo23_0_0 implements Migration { } } + private void removeRegistrationProfileFormExecution(RealmModel realm) { + realm.getAuthenticationFlowsStream() + .filter(flow -> AuthenticationFlow.FORM_FLOW.equals(flow.getProviderId())) + .forEach(registrationFlow -> { + realm.getAuthenticationExecutionsStream(registrationFlow.getId()) + .filter(authExecution -> "registration-profile-action".equals(authExecution.getAuthenticator())) + .forEach(registrationProfileExecution -> { + realm.removeAuthenticatorExecution(registrationProfileExecution); + LOG.debugf("Removed 'registration-profile-action' form action from authentication flow '%s' in the realm '%s'.", registrationFlow.getAlias(), realm.getName()); + }); + }); + + } + @Override public ModelVersion getVersion() { return VERSION; diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java deleted file mode 100755 index dee5744be0..0000000000 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java +++ /dev/null @@ -1,134 +0,0 @@ -/* - * Copyright 2016 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.authentication.forms; - -import org.keycloak.Config; -import org.keycloak.authentication.FormAction; -import org.keycloak.authentication.FormActionFactory; -import org.keycloak.authentication.FormContext; -import org.keycloak.forms.login.LoginFormsProvider; -import org.keycloak.models.AuthenticationExecutionModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.RealmModel; -import org.keycloak.models.UserModel; -import org.keycloak.provider.ProviderConfigProperty; -import java.util.List; - -/** - * TODO: This class not needed. Remove it entirely (will need some migration) - * - * @author Bill Burke - * @version $Revision: 1 $ - */ -public class RegistrationProfile implements FormAction, FormActionFactory { - public static final String PROVIDER_ID = "registration-profile-action"; - - @Override - public String getHelpText() { - return "Validates email, first name, and last name attributes and stores them in user data."; - } - - @Override - public List getConfigProperties() { - return null; - } - - @Override - public void validate(org.keycloak.authentication.ValidationContext context) { - context.success(); - } - - @Override - public void success(FormContext context) { - } - - @Override - public void buildPage(FormContext context, LoginFormsProvider form) { - // complete - } - - @Override - public boolean requiresUser() { - return false; - } - - @Override - public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) { - return true; - } - - @Override - public void setRequiredActions(KeycloakSession session, RealmModel realm, UserModel user) { - - } - - @Override - public boolean isUserSetupAllowed() { - return false; - } - - - @Override - public void close() { - - } - - @Override - public String getDisplayType() { - return "Profile Validation"; - } - - @Override - public String getReferenceCategory() { - return null; - } - - @Override - public boolean isConfigurable() { - return false; - } - - private static AuthenticationExecutionModel.Requirement[] REQUIREMENT_CHOICES = { - AuthenticationExecutionModel.Requirement.REQUIRED, - AuthenticationExecutionModel.Requirement.DISABLED - }; - @Override - public AuthenticationExecutionModel.Requirement[] getRequirementChoices() { - return REQUIREMENT_CHOICES; - } - @Override - public FormAction create(KeycloakSession session) { - return this; - } - - @Override - public void init(Config.Scope config) { - - } - - @Override - public void postInit(KeycloakSessionFactory factory) { - - } - - @Override - public String getId() { - return PROVIDER_ID; - } -} diff --git a/services/src/main/resources/META-INF/services/org.keycloak.authentication.FormActionFactory b/services/src/main/resources/META-INF/services/org.keycloak.authentication.FormActionFactory index 5287a11c1f..24db202c5d 100755 --- a/services/src/main/resources/META-INF/services/org.keycloak.authentication.FormActionFactory +++ b/services/src/main/resources/META-INF/services/org.keycloak.authentication.FormActionFactory @@ -16,7 +16,6 @@ # org.keycloak.authentication.forms.RegistrationPassword -org.keycloak.authentication.forms.RegistrationProfile org.keycloak.authentication.forms.RegistrationUserCreation org.keycloak.authentication.forms.RegistrationRecaptcha org.keycloak.authentication.forms.RegistrationTermsAndConditions diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ProvidersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ProvidersTest.java index d43f0403f6..fff1c85e03 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ProvidersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ProvidersTest.java @@ -61,8 +61,6 @@ public class ProvidersTest extends AbstractAuthenticationTest { List> result = authMgmtResource.getFormActionProviders(); List> expected = new LinkedList<>(); - addProviderInfo(expected, "registration-profile-action", "Profile Validation", - "Validates email, first name, and last name attributes and stores them in user data."); addProviderInfo(expected, "registration-recaptcha-action", "Recaptcha", "Adds Google Recaptcha button. Recaptchas verify that the entity that is registering is a human. " + "This can only be used on the internet and must be configured after you add it."); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java index ca32ab4886..6d75bb0d4f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java @@ -25,7 +25,6 @@ import org.junit.Test; import org.keycloak.authentication.AuthenticationFlow; import org.keycloak.authentication.authenticators.browser.CookieAuthenticatorFactory; import org.keycloak.authentication.forms.RegistrationPassword; -import org.keycloak.authentication.forms.RegistrationProfile; import org.keycloak.authentication.forms.RegistrationRecaptcha; import org.keycloak.authentication.forms.RegistrationTermsAndConditions; import org.keycloak.authentication.forms.RegistrationUserCreation; 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 a3cebfc4fb..593fa10888 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 @@ -81,6 +81,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; import java.util.regex.Matcher; @@ -193,6 +194,16 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { assertThat(component.getConfig().getList(UP_COMPONENT_CONFIG_KEY), not(empty())); } + protected void testRegistrationProfileFormActionRemoved(RealmResource realm) { + AuthenticationFlowRepresentation registrationFlow = realm.flows().getFlows().stream() + .filter(flowRep -> DefaultAuthenticationFlows.REGISTRATION_FLOW.equals(flowRep.getAlias())) + .findFirst().orElseThrow(() -> new NoSuchElementException("No registration flow in realm " + realm.toRepresentation().getRealm())); + + Assert.assertFalse(realm.flows().getExecutions(registrationFlow.getAlias()) + .stream() + .anyMatch(execution -> "registration-profile-action".equals(execution.getProviderId()))); + } + /** * @see org.keycloak.migration.migrators.MigrateTo2_0_0 */ @@ -382,6 +393,7 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { */ protected void testMigrationTo23_0_0(boolean testUserProfileMigration) { if (testUserProfileMigration) testUserProfile(migrationRealm2); + testRegistrationProfileFormActionRemoved(migrationRealm2); } protected void testDeleteAccount(RealmResource realm) { diff --git a/testsuite/integration-arquillian/tests/other/webauthn/src/test/resources/webauthn/testrealm-webauthn.json b/testsuite/integration-arquillian/tests/other/webauthn/src/test/resources/webauthn/testrealm-webauthn.json index ce17266c34..3cf2b0556a 100644 --- a/testsuite/integration-arquillian/tests/other/webauthn/src/test/resources/webauthn/testrealm-webauthn.json +++ b/testsuite/integration-arquillian/tests/other/webauthn/src/test/resources/webauthn/testrealm-webauthn.json @@ -986,13 +986,6 @@ "userSetupAllowed": false, "authenticatorFlow": false }, - { - "authenticator": "registration-profile-action", - "requirement": "REQUIRED", - "priority": 40, - "userSetupAllowed": false, - "authenticatorFlow": false - }, { "authenticator": "registration-password-action", "requirement": "REQUIRED",