From 90818fc53a1e994404db13519ca54f133d310958 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 5 Oct 2023 10:08:45 -0300 Subject: [PATCH] Avoid creating the component when there is no component and configuration is not provided Closes #20970 Co-authored-by: Pedro Igor --- .../DeclarativeUserProfileProvider.java | 142 +++++++++--------- .../ImmutableAttributeValidator.java | 22 ++- .../keycloak/testsuite/admin/UserTest.java | 13 +- .../testsuite/admin/realm/RealmTest.java | 19 +++ .../user/profile/AbstractUserProfileTest.java | 37 ++++- .../user/profile/CustomUserProfileTest.java | 26 ++-- .../user/profile/UserProfileTest.java | 125 +++++---------- 7 files changed, 208 insertions(+), 176 deletions(-) diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java index 7d357f71a97..3d1da0bfe87 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java @@ -27,10 +27,13 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -103,6 +106,7 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< } private String defaultRawConfig; + private static final Map DEFAULT_METADATA = Collections.synchronizedMap(new HashMap<>()); public DeclarativeUserProfileProvider() { defaultRawConfig = UPConfigUtils.readDefaultConfig(); @@ -154,16 +158,21 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< return decoratedMetadata; } - ComponentModel model = getComponentModelOrCreate(session); - Map metadataMap = model.getNote(PARSED_CONFIG_COMPONENT_KEY); + ComponentModel component = getComponentModel().orElse(null); + + if (component == null) { + return DEFAULT_METADATA.computeIfAbsent(context, (c) -> decorateUserProfileForCache(decoratedMetadata, getParsedConfig(defaultRawConfig))); + } + + Map metadataMap = component.getNote(PARSED_CONFIG_COMPONENT_KEY); // not cached, create a note with cache if (metadataMap == null) { metadataMap = new ConcurrentHashMap<>(); - model.setNote(PARSED_CONFIG_COMPONENT_KEY, metadataMap); + component.setNote(PARSED_CONFIG_COMPONENT_KEY, metadataMap); } - return metadataMap.computeIfAbsent(context, (c) -> decorateUserProfileForCache(decoratedMetadata, model)); + return metadataMap.computeIfAbsent(context, createUserDefinedProfileDecorator(session, decoratedMetadata, component)); } @Override @@ -203,40 +212,52 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< return defaultRawConfig; } - String cfg = getConfigJsonFromComponentModel(getComponentModel()); + Optional component = getComponentModel(); - if (isBlank(cfg)) { - return defaultRawConfig; + if (component.isPresent()) { + String cfg = getConfigJsonFromComponentModel(component.get()); + + if (isBlank(cfg)) { + return defaultRawConfig; + } + + return cfg; } - return cfg; + return defaultRawConfig; } @Override public void setConfiguration(String configuration) { - ComponentModel component = getComponentModel(); + RealmModel realm = session.getContext().getRealm(); + Optional optionalComponent = realm.getComponentsStream(realm.getId(), UserProfileProvider.class.getName()).findAny(); + + if (isBlank(configuration) && !optionalComponent.isPresent()) { + return; + } + + ComponentModel component = optionalComponent.isPresent() ? optionalComponent.get() : createComponentModel(); removeConfigJsonFromComponentModel(component); - RealmModel realm = session.getContext().getRealm(); - - if (!isBlank(configuration)) { - // store new parts - List parts = UPConfigUtils.getChunks(configuration, 3800); - MultivaluedHashMap config = component.getConfig(); - - config.putSingle(UP_PIECES_COUNT_COMPONENT_CONFIG_KEY, "" + parts.size()); - - int i = 0; - - for (String part : parts) { - config.putSingle(UP_PIECE_COMPONENT_CONFIG_KEY_BASE + (i++), part); - } - - realm.updateComponent(component); - } else { + if (isBlank(configuration)) { realm.removeComponent(component); + return; } + + // store new parts + List parts = UPConfigUtils.getChunks(configuration, 3800); + MultivaluedHashMap config = component.getConfig(); + + config.putSingle(UP_PIECES_COUNT_COMPONENT_CONFIG_KEY, "" + parts.size()); + + int i = 0; + + for (String part : parts) { + config.putSingle(UP_PIECE_COMPONENT_CONFIG_KEY_BASE + (i++), part); + } + + realm.updateComponent(component); } @Override @@ -255,22 +276,18 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< return PROVIDER_PRIORITY; } - public ComponentModel getComponentModel() { - return getComponentModelOrCreate(session); + private Optional getComponentModel() { + RealmModel realm = session.getContext().getRealm(); + return realm.getComponentsStream(realm.getId(), UserProfileProvider.class.getName()).findAny(); } /** * Decorate basic metadata provided from {@link AbstractUserProfileProvider} based on 'per realm' configuration. * This method is called for each {@link UserProfileContext} in each realm, and metadata are cached then and this * method is called again only if configuration changes. - * - * @param decoratedMetadata base to be decorated based on configuration loaded from component model - * @param model component model to get "per realm" configuration from - * @return decorated metadata */ - protected UserProfileMetadata decorateUserProfileForCache(UserProfileMetadata decoratedMetadata, ComponentModel model) { + protected UserProfileMetadata decorateUserProfileForCache(UserProfileMetadata decoratedMetadata, UPConfig parsedConfig) { UserProfileContext context = decoratedMetadata.getContext(); - UPConfig parsedConfig = getParsedConfig(model); // do not change config for REGISTRATION_USER_CREATION context, everything important is covered thanks to REGISTRATION_PROFILE // do not change config for UPDATE_EMAIL context, validations are already set and do not need including anything else from the configuration @@ -282,7 +299,6 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< } Map groupsByName = asHashMap(parsedConfig.getGroups()); - RealmModel realm = session.getContext().getRealm(); int guiOrder = 0; for (UPAttribute attrConfig : parsedConfig.getAttributes()) { @@ -346,6 +362,8 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< guiOrder++; + validators.add(new AttributeValidatorMetadata(ImmutableAttributeValidator.ID)); + if (isBuiltInAttribute(attributeName)) { // make sure username and email are writable if permissions are not set if (permissions == null || permissions.isEmpty()) { @@ -381,14 +399,6 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< } } - // 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.EMAIL.equals(attributeName)) { - validators.add(new AttributeValidatorMetadata(ImmutableAttributeValidator.ID)); - } - List existingMetadata = decoratedMetadata.getAttribute(attributeName); if (existingMetadata.isEmpty()) { @@ -406,7 +416,6 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< .setRequired(required); } } else { - validators.add(new AttributeValidatorMetadata(ImmutableAttributeValidator.ID)); decoratedMetadata.addAttribute(attributeName, guiOrder, validators, selector, writeAllowed, required, readAllowed) .addAnnotations(annotations) .setAttributeDisplayName(attrConfig.getDisplayName()) @@ -440,24 +449,11 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< /** * Get parsed config file configured in model. Default one used if not configured. - * - * @param model to take config from - * @return parsed configuration */ - protected UPConfig getParsedConfig(ComponentModel model) { - String rawConfig = getConfigJsonFromComponentModel(model); - + protected UPConfig getParsedConfig(String rawConfig) { if (!isBlank(rawConfig)) { try { - UPConfig upc = parseConfig(rawConfig); - - //validate configuration to catch things like changed/removed validators etc, and warn early and clearly about this problem - List errors = UPConfigUtils.validate(session, upc); - if (!errors.isEmpty()) { - throw new RuntimeException("UserProfile configuration for realm '" + session.getContext().getRealm().getName() + "' is invalid: " + errors.toString()); - } - return upc; - + return parseConfig(rawConfig); } catch (IOException e) { throw new RuntimeException("UserProfile configuration for realm '" + session.getContext().getRealm().getName() + "' is invalid:" + e.getMessage(), e); } @@ -470,23 +466,13 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< return readConfig(new ByteArrayInputStream(rawConfig.getBytes("UTF-8"))); } - /** - * Get component to store our "per realm" configuration into. - * - * @param session to be used, and take realm from - * @return component - */ - private ComponentModel getComponentModelOrCreate(KeycloakSession session) { - RealmModel realm = session.getContext().getRealm(); - return realm.getComponentsStream(realm.getId(), UserProfileProvider.class.getName()).findAny().orElseGet(() -> realm.addComponentModel(createComponentModel())); - } - /** * Create the component model to store configuration * @return component model */ protected ComponentModel createComponentModel() { - return new DeclarativeUserProfileModel(getId()); + RealmModel realm = session.getContext().getRealm(); + return realm.addComponentModel(new DeclarativeUserProfileModel(getId())); } /** @@ -538,4 +524,18 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider< public boolean isEnabled(RealmModel realm) { return isDeclarativeConfigurationEnabled && realm.getAttribute(REALM_USER_PROFILE_ENABLED, false); } + + private Function createUserDefinedProfileDecorator(KeycloakSession session, UserProfileMetadata decoratedMetadata, ComponentModel component) { + return (c) -> { + UPConfig parsedConfig = getParsedConfig(getConfigJsonFromComponentModel(component)); + + //validate configuration to catch things like changed/removed validators etc, and warn early and clearly about this problem + List errors = UPConfigUtils.validate(session, parsedConfig); + if (!errors.isEmpty()) { + throw new RuntimeException("UserProfile configuration for realm '" + session.getContext().getRealm().getName() + "' is invalid: " + errors.toString()); + } + + return decorateUserProfileForCache(decoratedMetadata, parsedConfig); + }; + } } diff --git a/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java b/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java index 11e24610fbd..3b588adbff1 100644 --- a/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java +++ b/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java @@ -16,6 +16,7 @@ */ package org.keycloak.userprofile.validator; +import static org.keycloak.common.util.CollectionUtil.collectionEquals; import static org.keycloak.validate.Validators.notBlankValidator; import java.util.List; @@ -23,8 +24,10 @@ import java.util.Objects; import java.util.stream.Collectors; import org.keycloak.common.util.CollectionUtil; +import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.userprofile.AttributeContext; +import org.keycloak.userprofile.AttributeValidatorMetadata; import org.keycloak.userprofile.UserProfileAttributeValidationContext; import org.keycloak.validate.SimpleValidator; import org.keycloak.validate.ValidationContext; @@ -61,10 +64,27 @@ public class ImmutableAttributeValidator implements SimpleValidator { List currentValue = user.getAttributeStream(inputHint).filter(Objects::nonNull).collect(Collectors.toList()); List values = (List) input; - if (!CollectionUtil.collectionEquals(currentValue, values) && isReadOnly(attributeContext)) { + if (!collectionEquals(currentValue, values) && isReadOnly(attributeContext)) { if (currentValue.isEmpty() && !notBlankValidator().validate(values).isValid()) { return context; } + + RealmModel realm = ac.getSession().getContext().getRealm(); + + if (realm.isRegistrationEmailAsUsername()) { + String attributeName = attributeContext.getMetadata().getName(); + + if (UserModel.EMAIL.equals(attributeName)) { + return context; + } + + List email = attributeContext.getAttributes().getValues(UserModel.EMAIL); + + if (UserModel.USERNAME.equals(attributeName) && collectionEquals(values, email)) { + return context; + } + } + context.addError(new ValidationError(ID, inputHint, DEFAULT_ERROR_MESSAGE)); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 1583225968c..01561c93c82 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -2442,12 +2442,23 @@ public class UserTest extends AbstractAdminTest { @Test public void updateUserWithNewUsernameNotPossible() { + RealmRepresentation realmRep = realm.toRepresentation(); + assertFalse(realmRep.isEditUsernameAllowed()); String id = createUser(); UserResource user = realm.users().get(id); UserRepresentation userRep = user.toRepresentation(); userRep.setUsername("user11"); - updateUser(user, userRep); + + try { + updateUser(user, userRep); + if (isDeclarativeUserProfile()) { + fail("Should fail because realm does not allow edit username"); + } + } catch (BadRequestException expected) { + ErrorRepresentation error = expected.getResponse().readEntity(ErrorRepresentation.class); + assertEquals("Attribute username is read only.", error.getErrorMessage()); + } userRep = realm.users().get(id).toRepresentation(); assertEquals("user1", userRep.getUsername()); 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 1ef4a7bc685..883feced26f 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 @@ -70,6 +70,7 @@ import org.keycloak.testsuite.util.OAuthClient.AccessTokenResponse; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.utils.tls.TLSUtils; +import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.util.JsonSerialization; import jakarta.ws.rs.BadRequestException; @@ -88,6 +89,7 @@ import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertEquals; @@ -945,4 +947,21 @@ public class RealmTest extends AbstractAdminTest { assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.clientResourcePath(clientDbId), client, ResourceType.CLIENT); } + @Test + public void testNoUserProfileProviderComponentUponRealmChange() { + String realmName = "new-realm"; + RealmRepresentation rep = new RealmRepresentation(); + rep.setRealm(realmName); + + adminClient.realms().create(rep); + getCleanup().addCleanup(() -> adminClient.realms().realm(realmName).remove()); + + assertThat(adminClient.realm(realmName).components().query(null, UserProfileProvider.class.getName()), empty()); + + rep.setDisplayName("displayName"); + adminClient.realm(realmName).update(rep); + + // this used to return non-empty collection + assertThat(adminClient.realm(realmName).components().query(null, UserProfileProvider.class.getName()), empty()); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java index f6d528757ce..845ce23ee36 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java @@ -25,9 +25,12 @@ import java.io.IOException; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Optional; import java.util.Set; +import org.junit.Before; import org.keycloak.common.Profile; +import org.keycloak.component.ComponentModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -37,10 +40,11 @@ import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; -import org.keycloak.userprofile.DeclarativeUserProfileProvider; +import org.keycloak.testsuite.forms.VerifyProfileTest; import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.config.UPAttribute; import org.keycloak.userprofile.config.UPConfig; +import org.keycloak.userprofile.config.UPConfigUtils; import org.keycloak.util.JsonSerialization; /** @@ -63,12 +67,27 @@ public abstract class AbstractUserProfileTest extends AbstractTestRealmKeycloakT session.getContext().setAuthenticationSession(createAuthenticationSession(realm.getClientByClientId(clientId), requestedScopes)); } - protected static DeclarativeUserProfileProvider getDynamicUserProfileProvider(KeycloakSession session) { - UserProfileProvider provider = session.getProvider(UserProfileProvider.class); + protected static Optional setAndGetDefaultConfiguration(KeycloakSession session) { + setDefaultConfiguration(session); + return getComponentModel(session); + } - provider.setConfiguration(null); + protected static Optional getComponentModel(KeycloakSession session) { + RealmModel realm = session.getContext().getRealm(); + return realm.getComponentsStream(realm.getId(), UserProfileProvider.class.getName()).findAny(); + } - return (DeclarativeUserProfileProvider) provider; + protected static void setDefaultConfiguration(KeycloakSession session) { + setConfiguration(session, UPConfigUtils.readDefaultConfig()); + } + + protected static void setConfiguration(KeycloakSession session, String config) { + UserProfileProvider provider = getUserProfileProvider(session); + provider.setConfiguration(config); + } + + protected static UserProfileProvider getUserProfileProvider(KeycloakSession session) { + return session.getProvider(UserProfileProvider.class); } /** @@ -272,4 +291,12 @@ public abstract class AbstractUserProfileTest extends AbstractTestRealmKeycloakT } testRealm.getAttributes().put(REALM_USER_PROFILE_ENABLED, Boolean.TRUE.toString()); } + + @Before + public void resetConfigBeforeTest() { + VerifyProfileTest.disableDynamicUserProfile(testRealm()); + RealmRepresentation realm = testRealm().toRepresentation(); + VerifyProfileTest.enableDynamicUserProfile(realm); + testRealm().update(realm); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/CustomUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/CustomUserProfileTest.java index bed0106930d..0fdcd176449 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/CustomUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/CustomUserProfileTest.java @@ -20,7 +20,6 @@ package org.keycloak.testsuite.user.profile; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -35,10 +34,13 @@ import org.keycloak.testsuite.runonserver.RunOnServer; import org.keycloak.userprofile.DeclarativeUserProfileProvider; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileContext; +import org.keycloak.userprofile.UserProfileProvider; +import org.keycloak.userprofile.config.UPConfigUtils; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.Optional; /** * @author Jörg Matysiak @@ -52,10 +54,13 @@ public class CustomUserProfileTest extends AbstractUserProfileTest { } private static void testCustomUserProfileProviderIsActive(KeycloakSession session) { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); assertEquals(CustomUserProfileProvider.class.getName(), provider.getClass().getName()); assertTrue(provider instanceof CustomUserProfileProvider); - assertEquals("custom-user-profile", provider.getComponentModel().getProviderId()); + provider.setConfiguration(UPConfigUtils.readDefaultConfig()); + Optional component = getComponentModel(session); + assertTrue(component.isPresent()); + assertEquals("custom-user-profile", component.get().getProviderId()); } @Test @@ -64,7 +69,7 @@ public class CustomUserProfileTest extends AbstractUserProfileTest { } private static void testInvalidConfiguration(KeycloakSession session) { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); try { provider.setConfiguration("{\"validateConfigAttribute\": true}"); @@ -80,19 +85,16 @@ public class CustomUserProfileTest extends AbstractUserProfileTest { } private static void testConfigurationChunks(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); String newConfig = generateLargeProfileConfig(); provider.setConfiguration(newConfig); - component = provider.getComponentModel(); + Optional component = getComponentModel(session); + assertTrue(component.isPresent()); // assert config is persisted in 2 pieces - Assert.assertEquals("2", component.get(DeclarativeUserProfileProvider.UP_PIECES_COUNT_COMPONENT_CONFIG_KEY)); + Assert.assertEquals("2", component.get().get(DeclarativeUserProfileProvider.UP_PIECES_COUNT_COMPONENT_CONFIG_KEY)); // assert config is returned correctly Assert.assertEquals(newConfig, provider.getConfiguration()); } @@ -104,7 +106,7 @@ public class CustomUserProfileTest extends AbstractUserProfileTest { } private static void testDefaultConfig(KeycloakSession session) { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); // reset configuration to default provider.setConfiguration(null); 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 c794e84b059..0b17a46348e 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 @@ -39,6 +39,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.Consumer; @@ -121,7 +122,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put(UserModel.USERNAME, "profiled-user"); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [{\"name\": \"address\", \"required\": {}, \"permissions\": {\"edit\": [\"user\"]}}]}"); @@ -157,7 +158,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put(UserModel.USERNAME, "profiled-user"); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [{\"name\": \"business.address\", \"required\": {\"scopes\": [\"customer\"]}, \"permissions\": {\"edit\": [\"user\"]}}]}"); @@ -281,7 +282,7 @@ public class UserProfileTest extends AbstractUserProfileTest { private static void testValidateComplianceWithUserProfile(KeycloakSession session) throws IOException { RealmModel realm = session.getContext().getRealm(); UserModel user = session.users().addUser(realm, "profiled-user"); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -325,7 +326,7 @@ public class UserProfileTest extends AbstractUserProfileTest { private static void testGetProfileAttributes(KeycloakSession session) { RealmModel realm = session.getContext().getRealm(); UserModel user = session.users().addUser(realm, org.keycloak.models.utils.KeycloakModelUtils.generateId()); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [{\"name\": \"address\", \"required\": {}, \"permissions\": {\"edit\": [\"user\"]}}]}"); @@ -367,7 +368,7 @@ public class UserProfileTest extends AbstractUserProfileTest { private static void testGetProfileAttributeGroups(KeycloakSession session) { RealmModel realm = session.getContext().getRealm(); UserModel user = session.users().addUser(realm, org.keycloak.models.utils.KeycloakModelUtils.generateId()); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); String configuration = "{\n" + " \"attributes\": [\n" + @@ -426,7 +427,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testCreateAndUpdateUser(KeycloakSession session) throws IOException { - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = JsonSerialization.readValue(provider.getConfiguration(), UPConfig.class); UPAttribute attribute = new UPAttribute(); @@ -510,7 +511,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put("address", Arrays.asList("fixed-address")); attributes.put("department", Arrays.asList("sales")); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [{\"name\": \"department\", \"permissions\": {\"edit\": [\"admin\"]}}]}"); @@ -559,7 +560,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId()); attributes.put(UserModel.EMAIL, "readonly@foo.bar"); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); // configure email r/o for user provider.setConfiguration("{\"attributes\": [{\"name\": \"email\", \"permissions\": {\"edit\": [ \"admin\"]}}]}"); @@ -601,7 +602,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId()); attributes.put(UserModel.EMAIL, "canchange@foo.bar"); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); // configure email r/w for user provider.setConfiguration("{\"attributes\": [{\"name\": \"email\", \"permissions\": {\"edit\": [ \"user\", \"admin\"]}}]}"); @@ -640,7 +641,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put("department", Arrays.asList("sales")); attributes.put("phone", Arrays.asList("fixed-phone")); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [{\"name\": \"department\", \"permissions\": {\"edit\": [\"admin\"]}}," + "{\"name\": \"phone\", \"permissions\": {\"edit\": [\"admin\"]}}," @@ -707,8 +708,10 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testComponentModelId(KeycloakSession session) { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - assertEquals("declarative-user-profile", provider.getComponentModel().getProviderId()); + setDefaultConfiguration(session); + Optional component = getComponentModel(session); + assertTrue(component.isPresent()); + assertEquals("declarative-user-profile", component.get().getProviderId()); } @Test @@ -717,7 +720,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testInvalidConfiguration(KeycloakSession session) { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); try { provider.setConfiguration("{\"validateConfigAttribute\": true}"); @@ -734,15 +737,14 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testConfigurationChunks(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - + ComponentModel component = setAndGetDefaultConfiguration(session).orElse(null); assertNotNull(component); String newConfig = generateLargeProfileConfig(); - provider.setConfiguration(newConfig); + UserProfileProvider provider = getUserProfileProvider(session); - component = provider.getComponentModel(); + provider.setConfiguration(newConfig); + component = getComponentModel(session).orElse(null); // assert config is persisted in 2 pieces Assert.assertEquals("2", component.get(DeclarativeUserProfileProvider.UP_PIECES_COUNT_COMPONENT_CONFIG_KEY)); @@ -756,17 +758,8 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testResetConfiguration(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - - provider.setConfiguration(null); - - Assert.assertNull(provider.getComponentModel().get(DeclarativeUserProfileProvider.UP_PIECES_COUNT_COMPONENT_CONFIG_KEY)); - - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - - Assert.assertTrue(component.getConfig().isEmpty()); + setConfiguration(session, null); + assertFalse(getComponentModel(session).isPresent()); } @Test @@ -775,7 +768,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testDefaultConfig(KeycloakSession session) { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); // reset configuration to default provider.setConfiguration(null); @@ -825,11 +818,6 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testCustomValidationForUsername(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -843,6 +831,7 @@ public class UserProfileTest extends AbstractUserProfileTest { config.addAttribute(attribute); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration(JsonSerialization.writeValueAsString(config)); Map attributes = new HashMap<>(); @@ -883,7 +872,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testRemoveDefaultValidationFromUsername(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); // reset configuration to default provider.setConfiguration(null); @@ -926,11 +915,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testOptionalAttributes(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); attribute.setName(UserModel.FIRST_NAME); @@ -985,11 +970,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testCustomAttributeRequired(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1052,11 +1033,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testCustomAttributeOptional(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1105,11 +1082,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testRequiredIfUser(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1172,11 +1145,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testRequiredIfAdmin(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1227,11 +1196,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testNoValidationsIfUserReadOnly(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1275,11 +1240,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testNoValidationsIfAdminReadOnly(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1320,7 +1281,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testIgnoreReadOnlyAttribute(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute firstName = new UPAttribute(); @@ -1395,7 +1356,7 @@ public class UserProfileTest extends AbstractUserProfileTest { maria.setAttribute(LDAPConstants.LDAP_ID, List.of("1")); - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); Map> attributes = new HashMap<>(); attributes.put(LDAPConstants.LDAP_ID, List.of("2")); @@ -1416,11 +1377,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testRequiredByClientScope(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1509,11 +1466,7 @@ public class UserProfileTest extends AbstractUserProfileTest { private static void testConfigurationInvalidScope(KeycloakSession session) throws IOException { RealmModel realm = session.getContext().getRealm(); - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); - ComponentModel component = provider.getComponentModel(); - - assertNotNull(component); - + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = new UPConfig(); UPAttribute attribute = new UPAttribute(); @@ -1544,7 +1497,7 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testUsernameAndEmailPermissionNotSetIfEmpty(KeycloakSession session) throws IOException { - DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = JsonSerialization.readValue(provider.getConfiguration(), UPConfig.class); for (UPAttribute attribute : config.getAttributes()) { @@ -1585,7 +1538,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put("test-attribute", Arrays.asList("Test Value")); attributes.put("foo", Arrays.asList("foo")); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [" + "{\"name\": \"test-attribute\", \"permissions\": {\"edit\": [\"admin\", \"user\"]}}," @@ -1668,7 +1621,7 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put(UserModel.FIRST_NAME, List.of("")); attributes.put("test-attribute", List.of("")); - UserProfileProvider provider = getDynamicUserProfileProvider(session); + UserProfileProvider provider = getUserProfileProvider(session); provider.setConfiguration("{\"attributes\": [" + "{\"name\": \"test-attribute\", \"permissions\": {\"edit\": [\"admin\", \"user\"]}},"