From 857b02be6378f8a26c3c809ac72683257197691a Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 20 Oct 2022 02:04:25 -0300 Subject: [PATCH] Allow managing the required settigs for the email attribute Closes #15026 --- .../userprofile/AttributeMetadata.java | 17 +++-- .../userprofile/UserProfileMetadata.java | 12 ++-- .../AbstractUserProfileProvider.java | 7 ++- .../DeclarativeUserProfileProvider.java | 53 ++++++++-------- .../keycloak/userprofile/config/UPConfig.java | 11 ++++ .../validator/BlankAttributeValidator.java | 8 +++ .../config/keycloak-default-user-profile.json | 9 +++ .../AppInitiatedActionUpdateProfileTest.java | 9 ++- .../RequiredActionUpdateProfileTest.java | 8 ++- .../BrokerLinkAndTokenExchangeTest.java | 4 +- .../testsuite/forms/RegisterTest.java | 5 +- .../forms/RegisterWithUserProfileTest.java | 62 ++++++++++++++++++- .../user/profile/UserProfileTest.java | 13 +++- .../admin/resources/js/controllers/realm.js | 14 ++++- .../partials/realm-user-profile.html | 6 +- 15 files changed, 190 insertions(+), 48 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/AttributeMetadata.java b/server-spi-private/src/main/java/org/keycloak/userprofile/AttributeMetadata.java index 565a8e527e..5eb3e276d6 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/AttributeMetadata.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/AttributeMetadata.java @@ -20,7 +20,6 @@ package org.keycloak.userprofile; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -47,7 +46,7 @@ public final class AttributeMetadata { private final Predicate selector; private final List> writeAllowed = new ArrayList<>(); /** Predicate to decide if attribute is required, it is handled as required if predicate is null */ - private final Predicate required; + private Predicate required; private final List> readAllowed = new ArrayList<>(); private List validators; private Map annotations; @@ -170,7 +169,7 @@ public final class AttributeMetadata { return validators; } - public AttributeMetadata addValidator(List validators) { + public AttributeMetadata addValidators(List validators) { if (this.validators == null) { this.validators = new ArrayList<>(); } @@ -202,7 +201,7 @@ public final class AttributeMetadata { // we clone validators list to allow adding or removing validators. Validators // itself are not cloned as we do not expect them to be reconfigured. if (validators != null) { - cloned.addValidator(validators); + cloned.addValidators(validators); } //we clone annotations map to allow adding to or removing from it if(annotations != null) { @@ -247,4 +246,14 @@ public final class AttributeMetadata { public int hashCode() { return attributeName.hashCode(); } + + public AttributeMetadata setRequired(Predicate required) { + this.required = required; + return this; + } + + public AttributeMetadata setValidators(List validators) { + this.validators = validators; + return this; + } } diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileMetadata.java b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileMetadata.java index 724e075bc4..5744861538 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileMetadata.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileMetadata.java @@ -61,19 +61,23 @@ public final class UserProfileMetadata implements Cloneable { } public AttributeMetadata addAttribute(String name, int guiOrder, Predicate writeAllowed, Predicate readAllowed, AttributeValidatorMetadata... validator) { - return addAttribute(new AttributeMetadata(name, guiOrder, ALWAYS_TRUE, writeAllowed, ALWAYS_TRUE, readAllowed).addValidator(Arrays.asList(validator))); + return addAttribute(new AttributeMetadata(name, guiOrder, ALWAYS_TRUE, writeAllowed, ALWAYS_TRUE, readAllowed).addValidators(Arrays.asList(validator))); } public AttributeMetadata addAttribute(String name, int guiOrder, Predicate writeAllowed, List validators) { - return addAttribute(new AttributeMetadata(name, guiOrder, ALWAYS_TRUE, writeAllowed, ALWAYS_TRUE, ALWAYS_TRUE).addValidator(validators)); + return addAttribute(new AttributeMetadata(name, guiOrder, ALWAYS_TRUE, writeAllowed, ALWAYS_TRUE, ALWAYS_TRUE).addValidators(validators)); + } + + public AttributeMetadata addAttribute(String name, int guiOrder, Predicate writeAllowed, Predicate required, List validators) { + return addAttribute(new AttributeMetadata(name, guiOrder, ALWAYS_TRUE, writeAllowed, required, ALWAYS_TRUE).addValidators(validators)); } public AttributeMetadata addAttribute(String name, int guiOrder, List validators) { - return addAttribute(new AttributeMetadata(name, guiOrder).addValidator(validators)); + return addAttribute(new AttributeMetadata(name, guiOrder).addValidators(validators)); } public AttributeMetadata addAttribute(String name, int guiOrder, List validator, Predicate selector, Predicate writeAllowed, Predicate required, Predicate readAllowed) { - return addAttribute(new AttributeMetadata(name, guiOrder, selector, writeAllowed, required, readAllowed).addValidator(validator)); + return addAttribute(new AttributeMetadata(name, guiOrder, selector, writeAllowed, required, readAllowed).addValidators(validator)); } /** diff --git a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java index 6dcc6aa446..c69e559bb0 100644 --- a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java @@ -298,7 +298,7 @@ public abstract class AbstractUserProfileProvider private UserProfileMetadata createRegistrationUserCreationProfile() { UserProfileMetadata metadata = new UserProfileMetadata(REGISTRATION_USER_CREATION); - metadata.addAttribute(UserModel.USERNAME, -2, new AttributeValidatorMetadata(RegistrationEmailAsUsernameUsernameValueValidator.ID), new AttributeValidatorMetadata(RegistrationUsernameExistsValidator.ID)); + metadata.addAttribute(UserModel.USERNAME, -2, new AttributeValidatorMetadata(RegistrationEmailAsUsernameUsernameValueValidator.ID), new AttributeValidatorMetadata(RegistrationUsernameExistsValidator.ID), new AttributeValidatorMetadata(UsernameHasValueValidator.ID)); metadata.addAttribute(UserModel.EMAIL, -1, new AttributeValidatorMetadata(RegistrationEmailAsUsernameEmailValueValidator.ID)); @@ -366,6 +366,11 @@ public abstract class AbstractUserProfileProvider private UserProfileMetadata createUserResourceValidation(Config.Scope config) { Pattern p = getRegexPatternString(config.getArray("admin-read-only-attributes")); UserProfileMetadata metadata = new UserProfileMetadata(USER_API); + + + metadata.addAttribute(UserModel.USERNAME, -2, new AttributeValidatorMetadata(UsernameHasValueValidator.ID)); + metadata.addAttribute(UserModel.EMAIL, -1, new AttributeValidatorMetadata(EmailValidator.ID, ValidatorConfig.builder().config(EmailValidator.IGNORE_EMPTY_VALUE, true).build())); + List readonlyValidators = new ArrayList<>(); if (p != null) { diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java index abd015db98..f13e447ffc 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java @@ -61,7 +61,6 @@ import org.keycloak.userprofile.validator.BlankAttributeValidator; import org.keycloak.userprofile.validator.ImmutableAttributeValidator; import org.keycloak.validate.AbstractSimpleValidator; import org.keycloak.validate.ValidatorConfig; -import org.keycloak.validate.validators.EmailValidator; /** * {@link UserProfileProvider} loading configuration from the changeable JSON file stored in component config. Parsed @@ -295,7 +294,7 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< } Predicate required = AttributeMetadata.ALWAYS_FALSE; - if (rc != null && !isUsernameOrEmailAttribute(attributeName)) { + if (rc != null) { if (rc.isAlways() || UPConfigUtils.isRoleForContext(context, rc.getRoles())) { required = AttributeMetadata.ALWAYS_TRUE; } else if (UPConfigUtils.canBeAuthFlowContext(context) && rc.getScopes() != null && !rc.getScopes().isEmpty()) { @@ -327,7 +326,7 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< Predicate selector = AttributeMetadata.ALWAYS_TRUE; UPAttributeSelector sc = attrConfig.getSelector(); - if (sc != null && !isUsernameOrEmailAttribute(attributeName) && UPConfigUtils.canBeAuthFlowContext(context) && sc.getScopes() != null && !sc.getScopes().isEmpty()) { + if (sc != null && !isBuiltInAttribute(attributeName) && UPConfigUtils.canBeAuthFlowContext(context) && sc.getScopes() != null && !sc.getScopes().isEmpty()) { // for contexts executed from auth flow and with configured scopes selector // we have to create correct predicate selector = (c) -> requestedScopePredicate(c, sc.getScopes()); @@ -337,48 +336,46 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< String attributeGroup = attrConfig.getGroup(); AttributeGroupMetadata groupMetadata = toAttributeGroupMeta(groupsByName.get(attributeGroup)); - if (isUsernameOrEmailAttribute(attributeName)) { + guiOrder++; + + if (isBuiltInAttribute(attributeName)) { // make sure username and email are writable if permissions are not set if (permissions == null || permissions.isEmpty()) { writeAllowed = AttributeMetadata.ALWAYS_TRUE; readAllowed = AttributeMetadata.ALWAYS_TRUE; } - List atts = decoratedMetadata.getAttribute(attributeName); + if (UserModel.USERNAME.equals(attributeName)) { + required = AttributeMetadata.ALWAYS_TRUE; + } // Add ImmutableAttributeValidator to ensure that attributes that are configured // as read-only are marked as such. // Skip this for username in realms with username = email to allow change of email // address on initial login with profile via idp - if (!realm.isRegistrationEmailAsUsername() || !UserModel.USERNAME.equals(attributeName)) { + if (!realm.isRegistrationEmailAsUsername() && UserModel.EMAIL.equals(attributeName)) { validators.add(new AttributeValidatorMetadata(ImmutableAttributeValidator.ID)); } - if (atts.isEmpty()) { - // attribute metadata doesn't exist so we have to add it. We keep it optional as Abstract base - // doesn't require it. - decoratedMetadata.addAttribute(attributeName, guiOrder++, writeAllowed, validators) - .addAnnotations(annotations) - .setAttributeDisplayName(attrConfig.getDisplayName()) - .setAttributeGroupMetadata(groupMetadata); - } else { - final int localGuiOrder = guiOrder++; - Predicate readAllowedFinal = readAllowed; - Predicate writeAllowedFinal = writeAllowed; + List existingMetadata = decoratedMetadata.getAttribute(attributeName); - // add configured validators and annotations to existing attribute metadata - atts.stream().forEach(c -> c.addValidator(validators) - .addAnnotations(annotations) - .setAttributeDisplayName(attrConfig.getDisplayName()) - .setGuiOrder(localGuiOrder) - .setAttributeGroupMetadata(groupMetadata) - .addReadCondition(readAllowedFinal) - .addWriteCondition(writeAllowedFinal)); + if (existingMetadata.isEmpty()) { + throw new IllegalStateException("Attribute " + attributeName + " not defined in the context."); + } + + for (AttributeMetadata metadata : existingMetadata) { + metadata.addAnnotations(annotations) + .setAttributeDisplayName(attrConfig.getDisplayName()) + .setGuiOrder(guiOrder) + .setAttributeGroupMetadata(groupMetadata) + .addReadCondition(readAllowed) + .addWriteCondition(writeAllowed) + .addValidators(validators) + .setRequired(required); } } else { - // always add validation for immutable/read-only attributes validators.add(new AttributeValidatorMetadata(ImmutableAttributeValidator.ID)); - decoratedMetadata.addAttribute(attributeName, guiOrder++, validators, selector, writeAllowed, required, readAllowed) + decoratedMetadata.addAttribute(attributeName, guiOrder, validators, selector, writeAllowed, required, readAllowed) .addAnnotations(annotations) .setAttributeDisplayName(attrConfig.getDisplayName()) .setAttributeGroupMetadata(groupMetadata); @@ -400,7 +397,7 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< return new AttributeGroupMetadata(group.getName(), group.getDisplayHeader(), group.getDisplayDescription(), group.getAnnotations()); } - private boolean isUsernameOrEmailAttribute(String attributeName) { + private boolean isBuiltInAttribute(String attributeName) { return UserModel.USERNAME.equals(attributeName) || UserModel.EMAIL.equals(attributeName); } diff --git a/services/src/main/java/org/keycloak/userprofile/config/UPConfig.java b/services/src/main/java/org/keycloak/userprofile/config/UPConfig.java index ca21d79a8f..a65b9c0d70 100644 --- a/services/src/main/java/org/keycloak/userprofile/config/UPConfig.java +++ b/services/src/main/java/org/keycloak/userprofile/config/UPConfig.java @@ -19,6 +19,7 @@ package org.keycloak.userprofile.config; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import com.fasterxml.jackson.annotation.JsonIgnore; /** * Configuration of the User Profile for one realm. @@ -70,6 +71,16 @@ public class UPConfig { return this; } + @JsonIgnore + public UPAttribute getAttribute(String name) { + for (UPAttribute attribute : getAttributes()) { + if (attribute.getName().equals(name)) { + return attribute; + } + } + return null; + } + @Override public String toString() { return "UPConfig [attributes=" + attributes + ", groups=" + groups + "]"; diff --git a/services/src/main/java/org/keycloak/userprofile/validator/BlankAttributeValidator.java b/services/src/main/java/org/keycloak/userprofile/validator/BlankAttributeValidator.java index 675fcfcba4..ed98e39fb6 100644 --- a/services/src/main/java/org/keycloak/userprofile/validator/BlankAttributeValidator.java +++ b/services/src/main/java/org/keycloak/userprofile/validator/BlankAttributeValidator.java @@ -19,6 +19,8 @@ package org.keycloak.userprofile.validator; import java.util.List; import org.keycloak.services.validation.Validation; +import org.keycloak.userprofile.AttributeContext; +import org.keycloak.userprofile.UserProfileAttributeValidationContext; import org.keycloak.validate.SimpleValidator; import org.keycloak.validate.ValidationContext; import org.keycloak.validate.ValidationError; @@ -56,6 +58,12 @@ public class BlankAttributeValidator implements SimpleValidator { return context; } + AttributeContext attributeContext = UserProfileAttributeValidationContext.from(context).getAttributeContext(); + + if (!attributeContext.getMetadata().isRequired(attributeContext)) { + return context; + } + String value = values.isEmpty() ? null: values.get(0); if ((failOnNull || value != null) && Validation.isBlank(value)) { diff --git a/services/src/main/resources/org/keycloak/userprofile/config/keycloak-default-user-profile.json b/services/src/main/resources/org/keycloak/userprofile/config/keycloak-default-user-profile.json index 30d33ed132..edd6888e69 100644 --- a/services/src/main/resources/org/keycloak/userprofile/config/keycloak-default-user-profile.json +++ b/services/src/main/resources/org/keycloak/userprofile/config/keycloak-default-user-profile.json @@ -3,6 +3,10 @@ { "name": "username", "displayName": "${username}", + "permissions": { + "view": ["admin", "user"], + "edit": ["admin", "user"] + }, "validations": { "length": { "min": 3, "max": 255 }, "username-prohibited-characters": {} @@ -11,6 +15,11 @@ { "name": "email", "displayName": "${email}", + "required": {"roles" : ["user"]}, + "permissions": { + "view": ["admin", "user"], + "edit": ["admin", "user"] + }, "validations": { "email" : {}, "length": { "max": 255 } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java index c0584d54cb..249cfaf180 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java @@ -16,6 +16,10 @@ */ package org.keycloak.testsuite.actions; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.containsString; + import org.hamcrest.Matchers; import org.jboss.arquillian.graphene.page.Page; import org.junit.Assert; @@ -258,7 +262,10 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct Assert.assertEquals("New last", updateProfilePage.getLastName()); Assert.assertEquals("", updateProfilePage.getEmail()); - Assert.assertEquals("Please specify email.", updateProfilePage.getInputErrors().getEmailError()); + assertThat(updateProfilePage.getInputErrors().getEmailError(), anyOf( + containsString("Please specify email"), + containsString("Please specify this field") + )); events.assertEmpty(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java index e9c3d3140c..db681e1adf 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java @@ -16,6 +16,9 @@ */ package org.keycloak.testsuite.actions; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertFalse; import java.util.Arrays; @@ -226,7 +229,10 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe Assert.assertEquals("New last", updateProfilePage.getLastName()); Assert.assertEquals("", updateProfilePage.getEmail()); - Assert.assertEquals("Please specify email.", updateProfilePage.getInputErrors().getEmailError()); + assertThat(updateProfilePage.getInputErrors().getEmailError(), anyOf( + containsString("Please specify email"), + containsString("Please specify this field") + )); events.assertEmpty(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java index f90490a71d..11dbd0a60e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java @@ -227,7 +227,7 @@ public class BrokerLinkAndTokenExchangeTest extends AbstractServletsAdapterTest user.setUsername(PARENT3_USERNAME); user.setFirstName("first name"); user.setLastName("last name"); - user.setEmail("email"); + user.setEmail("email@keycloak.org"); user.setEnabled(true); createUserAndResetPasswordWithAdminClient(realm, user, "password"); } @@ -749,7 +749,7 @@ public class BrokerLinkAndTokenExchangeTest extends AbstractServletsAdapterTest Assert.assertEquals(PARENT3_USERNAME, token.getPreferredUsername()); Assert.assertEquals("first name", token.getGivenName()); Assert.assertEquals("last name", token.getFamilyName()); - Assert.assertEquals("email", token.getEmail()); + Assert.assertEquals("email@keycloak.org", token.getEmail()); // cleanup remove the user childRealm.users().get(token.getSubject()).remove(); 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 4f0d71b037..37fc627996 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 @@ -52,6 +52,7 @@ import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.UserBuilder; import javax.mail.internet.MimeMessage; +import javax.ws.rs.core.Response; import java.io.IOException; import static org.hamcrest.MatcherAssert.assertThat; @@ -515,7 +516,9 @@ public class RegisterTest extends AbstractTestRealmKeycloakTest { assertTrue(registerPage.isCurrent()); assertEquals("Invalid password: must not be equal to the username.", registerPage.getInputPasswordErrors().getPasswordError()); - adminClient.realm("test").users().create(UserBuilder.create().username("registerUserNotUsername").build()); + try (Response response = adminClient.realm("test").users().create(UserBuilder.create().username("registerUserNotUsername").build())) { + assertEquals(Response.Status.CREATED.getStatusCode(), response.getStatus()); + } registerPage.register("firstName", "lastName", "registerUserNotUsername@email", "registerUserNotUsername", "registerUserNotUsername", "registerUserNotUsername"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterWithUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterWithUserProfileTest.java index 1fc1f4c1fd..20cff4b8a3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterWithUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterWithUserProfileTest.java @@ -18,6 +18,8 @@ package org.keycloak.testsuite.forms; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.keycloak.testsuite.forms.VerifyProfileTest.PERMISSIONS_ALL; @@ -59,7 +61,7 @@ public class RegisterWithUserProfileTest extends RegisterTest { private static ClientRepresentation client_scope_optional; public static String UP_CONFIG_BASIC_ATTRIBUTES = "{\"name\": \"username\"," + PERMISSIONS_ALL + ", \"required\": {}}," - + "{\"name\": \"email\"," + PERMISSIONS_ALL + ", \"required\": {}},"; + + "{\"name\": \"email\"," + PERMISSIONS_ALL + ", \"required\": {\"roles\" : [\"user\"]}},"; @Override public void configureTestRealm(RealmRepresentation testRealm) { @@ -602,6 +604,64 @@ public class RegisterWithUserProfileTest extends RegisterTest { assertEquals(null, user.firstAttribute(ATTRIBUTE_DEPARTMENT)); } + @Test + public void testEmailAsOptional() { + + setUserProfileConfiguration("{\"attributes\": [" + + "{\"name\": \"firstName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + + "{\"name\": \"lastName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + + "{\"name\": \"email\"," + PERMISSIONS_ALL + "}" + + "]}"); + + loginPage.open(); + loginPage.clickRegister(); + registerPage.assertCurrent(); + + registerPage.register("firstName", "lastName", null, "registerWithoutEmail", "password", "password"); + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + } + + @Test + public void testEmailRequired() { + + setUserProfileConfiguration("{\"attributes\": [" + + "{\"name\": \"firstName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + + "{\"name\": \"lastName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + + "{\"name\": \"email\"," + PERMISSIONS_ALL + ", \"required\": {}}" + + "]}"); + + loginPage.open(); + loginPage.clickRegister(); + registerPage.assertCurrent(); + + registerPage.register("firstName", "lastName", null, "registerWithoutEmail", "password", "password"); + registerPage.assertCurrent(); + assertThat(registerPage.getInputAccountErrors().getEmailError(), anyOf( + containsString("Please specify email"), + containsString("Please specify this field") + )); + + } + + @Test + public void testEmailRequiredForUser() { + + setUserProfileConfiguration("{\"attributes\": [" + + "{\"name\": \"firstName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + + "{\"name\": \"lastName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + + "{\"name\": \"email\"," + PERMISSIONS_ALL + ", \"required\": {\"roles\" : [\"user\"]}}" + + "]}"); + + loginPage.open(); + loginPage.clickRegister(); + registerPage.assertCurrent(); + + registerPage.register("firstName", "lastName", null, "registerWithoutEmail", "password", "password"); + assertThat(registerPage.getInputAccountErrors().getEmailError(), anyOf( + containsString("Please specify email"), + containsString("Please specify this field") + )); + } private void assertUserRegistered(String userId, String username, String email, String firstName, String lastName) { events.expectLogin().detail("username", username.toLowerCase()).user(userId).assertEvent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java index 0084cb98f6..e16eb384b9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java @@ -184,7 +184,7 @@ public class UserProfileTest extends AbstractUserProfileTest { getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testAttributeValidation); } - private static void failValidationWhenEmptyAttributes(KeycloakSession session) { + private static void failValidationWhenEmptyAttributes(KeycloakSession session) throws IOException { Map attributes = new HashMap<>(); UserProfileProvider provider = session.getProvider(UserProfileProvider.class); provider.setConfiguration(null); @@ -227,6 +227,14 @@ public class UserProfileTest extends AbstractUserProfileTest { realm.setRegistrationEmailAsUsername(false); } + UPConfig config = JsonSerialization.readValue(provider.getConfiguration(), UPConfig.class); + + UPAttribute email = config.getAttribute("email"); + + email.setRequired(null); + + provider.setConfiguration(JsonSerialization.writeValueAsString(config)); + attributes.clear(); attributes.put(UserModel.USERNAME, "profile-user"); attributes.put(UserModel.FIRST_NAME, "Joe"); @@ -438,6 +446,7 @@ public class UserProfileTest extends AbstractUserProfileTest { String userName = org.keycloak.models.utils.KeycloakModelUtils.generateId(); attributes.put(UserModel.USERNAME, userName); + attributes.put(UserModel.EMAIL, "user@keycloak.org"); attributes.put(UserModel.FIRST_NAME, "Joe"); attributes.put(UserModel.LAST_NAME, "Doe"); attributes.put("address", "fixed-address"); @@ -457,6 +466,7 @@ public class UserProfileTest extends AbstractUserProfileTest { Map attributesUpdatedOldValues = new HashMap<>(); attributesUpdatedOldValues.put(UserModel.FIRST_NAME, "Joe"); attributesUpdatedOldValues.put(UserModel.LAST_NAME, "Doe"); + attributesUpdatedOldValues.put(UserModel.EMAIL, "user@keycloak.org"); profile.update((attributeName, userModel, oldValue) -> { assertTrue(attributesUpdated.add(attributeName)); @@ -856,6 +866,7 @@ public class UserProfileTest extends AbstractUserProfileTest { provider.setConfiguration(null); attributes.put(UserModel.USERNAME, "user"); + attributes.put(UserModel.EMAIL, "user@keycloak.org"); attributes.put(UserModel.FIRST_NAME, "Joe"); attributes.put(UserModel.LAST_NAME, "Doe"); 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 064b7d616f..d683e4028a 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 @@ -1634,7 +1634,19 @@ module.controller('RealmUserProfileCtrl', function($scope, Realm, realm, clientS return attributeName != "username" && attributeName != "email"; }; - $scope.guiOrderUp = function(index) { + $scope.showRequiredSettings = function(attributeName) { + if (attributeName == "username") { + return false; + } + + if (attributeName == "email" && realm.registrationEmailAsUsername) { + return false; + } + + return true; + }; + + $scope.guiOrderUp = function(index) { $scope.moveAttribute(index, index - 1); }; diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/realm-user-profile.html b/themes/src/main/resources/theme/base/admin/resources/partials/realm-user-profile.html index df56951743..842369cc28 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/realm-user-profile.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/realm-user-profile.html @@ -152,7 +152,7 @@ -
+
{{:: 'user.profile.attribute.required.tooltip' | translate}}
@@ -160,14 +160,14 @@ on-text="{{:: 'onText' | translate}}" off-text="{{:: 'offText' | translate}}"/>
-
+
{{:: 'user.profile.attribute.required.roles.tooltip' | translate}}
-
+
{{:: 'user.profile.attribute.required.scopes.tooltip' | translate}}