From 10ba70c9725a1ae8a7ad40cb56ad2632f969806c Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 29 Jan 2024 10:46:57 +0100 Subject: [PATCH] Possibility to email being not required closes #26552 Signed-off-by: mposolda Co-authored-by: Jon Koops --- .../attribute/AttributeGeneralSettings.tsx | 4 + .../DeclarativeUserProfileProvider.java | 27 ++--- .../arquillian/ModelTestExecutor.java | 3 +- .../arquillian/annotation/ModelTest.java | 5 + .../user/profile/UserProfileTest.java | 110 ++++++++++++++++++ 5 files changed, 135 insertions(+), 14 deletions(-) diff --git a/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx b/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx index 577550f438..c3a2164e4b 100644 --- a/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx +++ b/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx @@ -243,6 +243,10 @@ export const AttributeGeneralSettings = () => { /> )} + + )} + {attributeName !== "username" && ( + <> () { - @Override - public boolean test(AttributeContext context) { - UserModel user = context.getUser(); + Predicate requiredFromConfig = required; + required = new Predicate() { + @Override + public boolean test(AttributeContext context) { + UserModel user = context.getUser(); - if (user != null && user.getServiceAccountClientLink() != null) { - return false; - } - - RealmModel realm = context.getSession().getContext().getRealm(); - return realm.isRegistrationEmailAsUsername(); + if (user != null && user.getServiceAccountClientLink() != null) { + return false; } - }; - } + + if (requiredFromConfig.test(context)) return true; + + RealmModel realm = context.getSession().getContext().getRealm(); + return realm.isRegistrationEmailAsUsername(); + } + }; } List existingMetadata = decoratedMetadata.getAttribute(attributeName); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/ModelTestExecutor.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/ModelTestExecutor.java index 2819296306..b6b6cbc7bc 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/ModelTestExecutor.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/ModelTestExecutor.java @@ -53,7 +53,8 @@ public class ModelTestExecutor extends LocalTestExecuter { // Model test - wrap the call inside the TestContext ctx = testContext.get(); KeycloakTestingClient testingClient = ctx.getTestingClient(); - testingClient.server().runModelTest(testMethod.getDeclaringClass().getName(), testMethod.getName()); + String realmName = annotation.realmName(); + testingClient.server(realmName).runModelTest(testMethod.getDeclaringClass().getName(), testMethod.getName()); result.setStatus(TestResult.Status.PASSED); } catch (Throwable e) { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/annotation/ModelTest.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/annotation/ModelTest.java index 673c98ae9c..5285e86633 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/annotation/ModelTest.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/annotation/ModelTest.java @@ -33,4 +33,9 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME; @Retention(RUNTIME) @Target({ElementType.METHOD}) // TODO: Maybe ElementClass.TYPE too? That way it will be possible to add the annotation on the the test class and not need to add on all the test methods inside the class public @interface ModelTest { + + /** + * Name of the realm to be set on the KeycloakContext when the test is executed. Defaults to "master" for backwards compatibility + */ + String realmName() default "master"; } 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 4d9de6e67b..7800befae0 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 @@ -1257,6 +1257,116 @@ public class UserProfileTest extends AbstractUserProfileTest { } + @Test + @ModelTest(realmName=TEST_REALM_NAME) + public void testEmailRequired(KeycloakSession session) { + RealmModel realm = session.getContext().getRealm(); + + Map attributes = new HashMap<>(); + attributes.put(UserModel.USERNAME, "james"); + attributes.put(UserModel.FIRST_NAME, "James"); + attributes.put(UserModel.LAST_NAME, "Doe"); + UserProfile profile; + + // Email required for users by default, but not for admins + UserProfileProvider provider = getUserProfileProvider(session); + UPConfig config = parseDefaultConfig(); + provider.setConfiguration(config); + UPAttribute emailOrigConfig = config.getAttribute(UserModel.EMAIL); + Assert.assertEquals(emailOrigConfig.getRequired().getRoles(), Set.of(ROLE_USER)); // Should be required only for users by default + + try { + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for users"); + } catch (ValidationException ve) { + // expected + } + try { + profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + } catch (ValidationException ve) { + Assert.fail("Should not be here as email is NOT required for administrators"); + } + + // Test email required in config, registrationEmailAsUsername = false : Email should be required + config.addOrReplaceAttribute(new UPAttribute(UserModel.EMAIL, new UPAttributePermissions(Set.of(), Set.of(ROLE_ADMIN, ROLE_USER)), new UPAttributeRequired(Set.of(ROLE_ADMIN, ROLE_USER), Set.of()))); + provider.setConfiguration(config); + + try { + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for users"); + } catch (ValidationException ve) { + // expected + } + try { + profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for administrators"); + } catch (ValidationException ve) { + // expected + } + + // Test email required in config, registrationEmailAsUsername = true : Email should be required + try { + realm.setRegistrationEmailAsUsername(true); + try { + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for users"); + } catch (ValidationException ve) { + // expected + } + try { + profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for administrators"); + } catch (ValidationException ve) { + // expected + } + } finally { + realm.setRegistrationEmailAsUsername(false); + } + + // Test email NOT required in config, registrationEmailAsUsername = true : Email should be required + config.addOrReplaceAttribute(new UPAttribute(UserModel.EMAIL, new UPAttributePermissions(Set.of(), Set.of(ROLE_ADMIN, ROLE_USER)), null)); + provider.setConfiguration(config); + try { + realm.setRegistrationEmailAsUsername(true); + try { + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for users"); + } catch (ValidationException ve) { + // expected + } + try { + profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + Assert.fail("Should not be here as email is required for administrators"); + } catch (ValidationException ve) { + // expected + } + } finally { + realm.setRegistrationEmailAsUsername(false); + } + + // Test email NOT required in config, registrationEmailAsUsername = false : Email should NOT be required + try { + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + } catch (ValidationException ve) { + Assert.fail("Should not be here as email is required for users"); + } + try { + profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + } catch (ValidationException ve) { + Assert.fail("Should not be here as email is required for administrators"); + } + } + @Test public void testNoValidationsIfUserReadOnly() { getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testNoValidationsIfUserReadOnly);