diff --git a/docs/documentation/upgrading/topics/changes/changes-24_0_3.adoc b/docs/documentation/upgrading/topics/changes/changes-24_0_3.adoc new file mode 100644 index 0000000000..03b4fe3adb --- /dev/null +++ b/docs/documentation/upgrading/topics/changes/changes-24_0_3.adoc @@ -0,0 +1,17 @@ +ifeval::[{project_community}==true] += Changes to the `org.keycloak.userprofile.UserProfileDecorator` interface + +To properly support multiple user storage providers within a realm, the `org.keycloak.userprofile.UserProfileDecorator` +interface has changed. + +The `decorateUserProfile` method is no longer invoked when parsing the user profile configuration for the first time (and caching it), +but everytime a user is being managed through the user profile provider. As a result, the method changed its contract to: + +```java +List decorateUserProfile(String providerId, UserProfileMetadata metadata) +``` + +Differently than the previous contract and behavior, this method is only invoked for the user storage provider from where the user +was loaded from. + +endif::[] diff --git a/docs/documentation/upgrading/topics/changes/changes.adoc b/docs/documentation/upgrading/topics/changes/changes.adoc index c7a440ac02..c2739874cf 100644 --- a/docs/documentation/upgrading/topics/changes/changes.adoc +++ b/docs/documentation/upgrading/topics/changes/changes.adoc @@ -5,6 +5,10 @@ include::changes-25_0_0.adoc[leveloffset=3] +=== Migrating to 24.0.3 + +include::changes-24_0_3.adoc[leveloffset=3] + === Migrating to 24.0.2 include::changes-24_0_2.adoc[leveloffset=3] diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java index 137449a079..362945f69e 100755 --- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java +++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java @@ -42,16 +42,16 @@ import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.UserStorageProviderModel; import org.keycloak.storage.user.ImportedUserValidation; import org.keycloak.storage.user.UserLookupProvider; -import org.keycloak.userprofile.AttributeContext; import org.keycloak.userprofile.AttributeGroupMetadata; import org.keycloak.userprofile.AttributeMetadata; import org.keycloak.userprofile.UserProfileDecorator; import org.keycloak.userprofile.UserProfileMetadata; import org.keycloak.userprofile.UserProfileUtil; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; -import java.util.function.Predicate; import java.util.stream.Stream; import javax.security.auth.login.LoginException; @@ -302,22 +302,13 @@ public class KerberosFederationProvider implements UserStorageProvider, } @Override - public void decorateUserProfile(RealmModel realm, UserProfileMetadata metadata) { - Predicate kerberosUsersSelector = (attributeContext -> { - UserModel user = attributeContext.getUser(); - if (user == null) { - return false; - } - - return model.getId().equals(user.getFederationLink()); - }); - + public List decorateUserProfile(String providerId, UserProfileMetadata metadata) { int guiOrder = (int) metadata.getAttributes().stream() .map(AttributeMetadata::getName) .distinct() .count(); AttributeGroupMetadata metadataGroup = UserProfileUtil.lookupUserMetadataGroup(session); - UserProfileUtil.addMetadataAttributeToUserProfile(KerberosConstants.KERBEROS_PRINCIPAL, metadata, metadataGroup, kerberosUsersSelector, guiOrder++, model.getName()); + return Collections.singletonList(UserProfileUtil.createAttributeMetadata(KerberosConstants.KERBEROS_PRINCIPAL, metadata, metadataGroup, guiOrder++, model.getName())); } } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index b257ee9ef4..4d1f6ccb15 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -23,7 +23,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -1121,47 +1120,27 @@ public class LDAPStorageProvider implements UserStorageProvider, } @Override - public void decorateUserProfile(RealmModel realm, UserProfileMetadata metadata) { - Predicate ldapUsersSelector = (attributeContext -> { - UserModel user = attributeContext.getUser(); - if (user == null) { - return false; - } - - if (model.isImportEnabled()) { - return getModel().getId().equals(user.getFederationLink()); - } else { - return getModel().getId().equals(new StorageId(user.getId()).getProviderId()); - } - }); - - Predicate onlyAdminCondition = context -> metadata.getContext().isAdminContext(); - + public List decorateUserProfile(String providerId, UserProfileMetadata metadata) { int guiOrder = (int) metadata.getAttributes().stream() .map(AttributeMetadata::getName) .distinct() .count(); - + RealmModel realm = session.getContext().getRealm(); // 1 - get configured attributes from LDAP mappers and add them to the user profile (if they not already present) - Set attributes = new LinkedHashSet<>(); - realm.getComponentsStream(model.getId(), LDAPStorageMapper.class.getName()) + List attributes = realm.getComponentsStream(model.getId(), LDAPStorageMapper.class.getName()) .sorted(ldapMappersComparator.sortAsc()) - .forEachOrdered(mapperModel -> { + .flatMap(mapperModel -> { LDAPStorageMapper ldapMapper = mapperManager.getMapper(mapperModel); - attributes.addAll(ldapMapper.getUserAttributes()); - }); + return ldapMapper.getUserAttributes().stream(); + }).toList(); + + List metadatas = new ArrayList<>(); + for (String attrName : attributes) { - // In case that attributes from LDAP mappers are explicitly defined on user profile, we can prefer defined configuration - if (!metadata.getAttribute(attrName).isEmpty()) { - logger.debugf("Ignore adding attribute '%s' to user profile by LDAP provider '%s' as attribute is already defined on user profile.", attrName, getModel().getName()); - } else { - logger.debugf("Adding attribute '%s' to user profile by LDAP provider '%s' for user profile context '%s'.", attrName, getModel().getName(), metadata.getContext().toString()); - // Writable and readable only by administrators by default. Applied only for LDAP users - AttributeMetadata attributeMetadata = metadata.addAttribute(attrName, guiOrder++, Collections.emptyList()) - .addWriteCondition(onlyAdminCondition) - .addReadCondition(onlyAdminCondition) - .setRequired(AttributeMetadata.ALWAYS_FALSE); - attributeMetadata.setSelector(ldapUsersSelector); + AttributeMetadata attributeMetadata = UserProfileUtil.createAttributeMetadata(attrName, metadata, guiOrder++, getModel().getName()); + + if (attributeMetadata != null) { + metadatas.add(attributeMetadata); } } @@ -1174,17 +1153,20 @@ public class LDAPStorageProvider implements UserStorageProvider, AttributeGroupMetadata metadataGroup = UserProfileUtil.lookupUserMetadataGroup(session); for (String attrName : metadataAttributes) { - boolean attributeAdded = UserProfileUtil.addMetadataAttributeToUserProfile(attrName, metadata, metadataGroup, ldapUsersSelector, guiOrder++, getModel().getName()); - if (!attributeAdded) { + AttributeMetadata attributeAdded = UserProfileUtil.createAttributeMetadata(attrName, metadata, metadataGroup, guiOrder++, getModel().getName()); + if (attributeAdded == null) { guiOrder--; + } else { + metadatas.add(attributeAdded); } } // 3 - make all attributes read-only for LDAP users in case that LDAP itself is read-only if (getEditMode() == EditMode.READ_ONLY) { - for (AttributeMetadata attrMetadata : metadata.getAttributes()) { - attrMetadata.addWriteCondition(ldapUsersSelector.negate()); - } + Stream.concat(metadata.getAttributes().stream(), metadatas.stream()) + .forEach(attrMetadata -> attrMetadata.addWriteCondition(AttributeMetadata.ALWAYS_FALSE)); } + + return metadatas; } } diff --git a/federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java b/federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java index e56176b96a..cdc0bb48b2 100755 --- a/federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java +++ b/federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java @@ -33,16 +33,15 @@ import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.UserStorageProviderModel; import org.keycloak.storage.user.ImportedUserValidation; import org.keycloak.storage.user.UserLookupProvider; -import org.keycloak.userprofile.AttributeContext; import org.keycloak.userprofile.AttributeMetadata; import org.keycloak.userprofile.UserProfileDecorator; import org.keycloak.userprofile.UserProfileMetadata; +import org.keycloak.userprofile.UserProfileUtil; -import java.util.Collections; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Stream; /** @@ -221,38 +220,29 @@ public class SSSDFederationProvider implements UserStorageProvider, } @Override - public void decorateUserProfile(RealmModel realm, UserProfileMetadata metadata) { - // selector by sssd - Predicate sssdUsersSelector = attributeContext -> - attributeContext.getUser() != null && model.getId().equals(attributeContext.getUser().getFederationLink()); - - // condition to view only by admin - Predicate onlyAdminCondition = context -> metadata.getContext().isAdminContext(); - + public List decorateUserProfile(String providerId, UserProfileMetadata metadata) { // guiOrder if new attributes are needed int guiOrder = (int) metadata.getAttributes().stream() .map(AttributeMetadata::getName) .distinct() .count(); + List metadatas = new ArrayList<>(); + // firstName, lastName, username and email should be read-only for (String attrName : List.of(UserModel.FIRST_NAME, UserModel.LAST_NAME, UserModel.EMAIL, UserModel.USERNAME)) { List attrMetadatas = metadata.getAttribute(attrName); if (attrMetadatas.isEmpty()) { logger.debugf("Adding user profile attribute '%s' for sssd provider and context '%s'.", attrName, metadata.getContext()); - AttributeMetadata sssdAttrMetadata = metadata.addAttribute(attrName, guiOrder++, Collections.emptyList()) - .addWriteCondition(AttributeMetadata.ALWAYS_FALSE) - .addReadCondition(onlyAdminCondition) - .setRequired(AttributeMetadata.ALWAYS_FALSE); - sssdAttrMetadata.setSelector(sssdUsersSelector); + metadatas.add(UserProfileUtil.createAttributeMetadata(attrName, metadata, null, guiOrder++, model.getName())); } else { for (AttributeMetadata attrMetadata : attrMetadatas) { logger.debugf("Cloning attribute '%s' as read-only for sssd provider and context '%s'.", attrName, metadata.getContext()); - AttributeMetadata sssdAttrMetadata = metadata.addAttribute(attrMetadata.clone()) - .addWriteCondition(AttributeMetadata.ALWAYS_FALSE); - sssdAttrMetadata.setSelector(sssdUsersSelector); + metadatas.add(attrMetadata.clone().addWriteCondition(AttributeMetadata.ALWAYS_FALSE)); } } } + + return metadatas; } } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java index 91df0c7de9..4ad8ed3e43 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java @@ -63,6 +63,7 @@ import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.UserStorageProviderModel; import org.keycloak.storage.client.ClientStorageProvider; +import org.keycloak.userprofile.AttributeMetadata; import org.keycloak.userprofile.UserProfileDecorator; import org.keycloak.userprofile.UserProfileMetadata; @@ -950,9 +951,10 @@ public class UserCacheSession implements UserCache, OnCreateComponent, OnUpdateC } @Override - public void decorateUserProfile(RealmModel realm, UserProfileMetadata metadata) { + public List decorateUserProfile(String providerId, UserProfileMetadata metadata) { if (getDelegate() instanceof UserProfileDecorator) { - ((UserProfileDecorator) getDelegate()).decorateUserProfile(realm, metadata); + return ((UserProfileDecorator) getDelegate()).decorateUserProfile(providerId, metadata); } + return List.of(); } } diff --git a/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java b/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java index c6f21fcaa2..e3dcb4ebdc 100755 --- a/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java @@ -21,6 +21,8 @@ import static org.keycloak.models.utils.KeycloakModelUtils.runJobInTransaction; import static org.keycloak.utils.StreamsUtil.distinctByKey; import static org.keycloak.utils.StreamsUtil.paginatedStream; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -71,6 +73,7 @@ import org.keycloak.storage.user.UserLookupProvider; import org.keycloak.storage.user.UserQueryMethodsProvider; import org.keycloak.storage.user.UserQueryProvider; import org.keycloak.storage.user.UserRegistrationProvider; +import org.keycloak.userprofile.AttributeMetadata; import org.keycloak.userprofile.UserProfileDecorator; import org.keycloak.userprofile.UserProfileMetadata; @@ -857,10 +860,18 @@ public class UserStorageManager extends AbstractStorageManager decorateUserProfile(String providerId, UserProfileMetadata metadata) { + RealmModel realm = session.getContext().getRealm(); + UserStorageProviderModel providerModel = getStorageProviderModel(realm, providerId); + + if (providerModel != null) { + UserProfileDecorator decorator = getStorageProviderInstance(providerModel, UserProfileDecorator.class); + + if (decorator != null) { + return decorator.decorateUserProfile(providerId, metadata); + } } + + return Collections.emptyList(); } } diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java index 321411e342..f9cb740cda 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java @@ -30,6 +30,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -39,9 +40,11 @@ import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.UserProvider; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.userprofile.config.UPConfig; import org.keycloak.representations.userprofile.config.UPConfig.UnmanagedAttributePolicy; +import org.keycloak.storage.StorageId; import org.keycloak.utils.StringUtil; import org.keycloak.validate.ValidationContext; import org.keycloak.validate.ValidationError; @@ -84,7 +87,7 @@ public class DefaultAttributes extends HashMap> implements this.context = context; this.user = user; this.session = session; - this.metadataByAttribute = configureMetadata(profileMetadata.getAttributes()); + this.metadataByAttribute = configureMetadata(profileMetadata.getAttributes(), profileMetadata); this.upConfig = session.getProvider(UserProfileProvider.class).getConfiguration(); putAll(Collections.unmodifiableMap(normalizeAttributes(attributes))); } @@ -324,7 +327,7 @@ public class DefaultAttributes extends HashMap> implements return createAttributeContext(createAttribute(metadata.getName()), metadata); } - private Map configureMetadata(List attributes) { + private Map configureMetadata(List attributes, UserProfileMetadata profileMetadata) { Map metadatas = new HashMap<>(); for (AttributeMetadata metadata : attributes) { @@ -334,9 +337,35 @@ public class DefaultAttributes extends HashMap> implements } } + metadatas.putAll(getUserStorageProviderMetadata(profileMetadata)); + return metadatas; } + private Map getUserStorageProviderMetadata(UserProfileMetadata profileMetadata) { + if (user == null || (StorageId.isLocalStorage(user.getId()) && user.getFederationLink() == null)) { + // new user or not a user from a storage provider other than local + return Collections.emptyMap(); + } + + String providerId = user.getFederationLink(); + + if (providerId == null) { + providerId = StorageId.providerId(user.getId()); + } + + UserProvider userProvider = session.users(); + + if (userProvider instanceof UserProfileDecorator) { + // query the user provider from the source user storage provider for additional attribute metadata + UserProfileDecorator decorator = (UserProfileDecorator) userProvider; + return decorator.decorateUserProfile(providerId, profileMetadata).stream() + .collect(Collectors.toMap(AttributeMetadata::getName, Function.identity())); + } + + return Collections.emptyMap(); + } + private SimpleImmutableEntry> createAttribute(String name) { return new SimpleImmutableEntry>(name, null) { @Override diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileUtil.java b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileUtil.java index 0b716608d0..6c28ddd784 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileUtil.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileUtil.java @@ -47,6 +47,8 @@ public class UserProfileUtil { public static final String USER_METADATA_GROUP = "user-metadata"; + public static final Predicate ONLY_ADMIN_CONDITION = context -> context.getContext().isAdminContext(); + /** * Find the metadata group "user-metadata" * @@ -69,30 +71,38 @@ public class UserProfileUtil { * @param attrName attribute name * @param metadata user-profile metadata where attribute would be added * @param metadataGroup metadata group in user-profile - * @param userFederationUsersSelector used to recognize if user belongs to this user-storage provider or not * @param guiOrder guiOrder to where to put the attribute * @param storageProviderName storageProviderName (just for logging purposes) - * @return true if attribute was added. False otherwise + * @return the attribute metadata if attribute was created. False otherwise */ - public static boolean addMetadataAttributeToUserProfile(String attrName, UserProfileMetadata metadata, AttributeGroupMetadata metadataGroup, Predicate userFederationUsersSelector, int guiOrder, String storageProviderName) { - // In case that attributes like LDAP_ID, KERBEROS_PRINCIPAL are explicitly defined on user profile, we can prefer defined configuration + public static AttributeMetadata createAttributeMetadata(String attrName, UserProfileMetadata metadata, AttributeGroupMetadata metadataGroup, int guiOrder, String storageProviderName) { + return createAttributeMetadata(attrName, metadata, metadataGroup, ONLY_ADMIN_CONDITION, AttributeMetadata.ALWAYS_FALSE, guiOrder, storageProviderName); + } + + public static AttributeMetadata createAttributeMetadata(String attrName, UserProfileMetadata metadata, int guiOrder, String storageProviderName) { + return createAttributeMetadata(attrName, metadata, null, ONLY_ADMIN_CONDITION, ONLY_ADMIN_CONDITION, guiOrder, storageProviderName); + } + + private static AttributeMetadata createAttributeMetadata(String attrName, UserProfileMetadata metadata, AttributeGroupMetadata metadataGroup, Predicate readCondition, Predicate writeCondition, int guiOrder, String storageProviderName) { if (!metadata.getAttribute(attrName).isEmpty()) { logger.tracef("Ignore adding metadata attribute '%s' to user profile by user storage provider '%s' as attribute is already defined on user profile.", attrName, storageProviderName); - return false; } else { logger.tracef("Adding metadata attribute '%s' to user profile by user storage provider '%s' for user profile context '%s'.", attrName, storageProviderName, metadata.getContext().toString()); - Predicate onlyAdminCondition = context -> metadata.getContext().isAdminContext(); - AttributeMetadata attributeMetadata = metadata.addAttribute(attrName, guiOrder, Collections.emptyList()) - .addWriteCondition(AttributeMetadata.ALWAYS_FALSE) // Not writable for anyone - .addReadCondition(onlyAdminCondition) // Read-only for administrators + + AttributeMetadata attributeMetadata = new AttributeMetadata(attrName, guiOrder) + .setValidators(Collections.emptyList()) + .addWriteCondition(writeCondition) + .addReadCondition(readCondition) .setRequired(AttributeMetadata.ALWAYS_FALSE); if (metadataGroup != null) { attributeMetadata.setAttributeGroupMetadata(metadataGroup); } - attributeMetadata.setSelector(userFederationUsersSelector); - return true; + + return attributeMetadata; } + + return null; } /** diff --git a/server-spi/src/main/java/org/keycloak/userprofile/UserProfileDecorator.java b/server-spi/src/main/java/org/keycloak/userprofile/UserProfileDecorator.java index acf7b5508f..0675d769b7 100644 --- a/server-spi/src/main/java/org/keycloak/userprofile/UserProfileDecorator.java +++ b/server-spi/src/main/java/org/keycloak/userprofile/UserProfileDecorator.java @@ -19,18 +19,27 @@ package org.keycloak.userprofile; +import java.util.List; + import org.keycloak.models.RealmModel; /** + *

This interface allows user storage providers to customize the user profile configuration and its attributes for realm + * on a per-user storage provider basis. + * * @author Pedro Igor */ public interface UserProfileDecorator { /** - * Decorates user profile with additional metadata. For instance, metadata attributes, which are available just for your user-storage - * provider can be added there, so they are available just for the users coming from your provider + *

Decorates user profile with additional metadata. For instance, metadata attributes, which are available just for your user-storage + * provider can be added there, so they are available just for the users coming from your provider. * - * @param metadata to decorate + *

This method is invoked every time a user is being managed through a user profile provider. + * + * @param providerId the id of the user storage provider to which the user is associated with + * @param metadata the current {@link UserProfileMetadata} for the current realm + * @return a list of attribute metadata.The {@link AttributeMetadata} returned from this method overrides any other metadata already set in {@code metadata} for a given attribute. */ - void decorateUserProfile(RealmModel realm, UserProfileMetadata metadata); + List decorateUserProfile(String providerId, UserProfileMetadata metadata); } diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java index 78e4512438..65503a9fb6 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java @@ -176,13 +176,9 @@ public class DeclarativeUserProfileProvider implements UserProfileProvider { protected UserProfileMetadata configureUserProfile(UserProfileMetadata metadata, KeycloakSession session) { UserProfileContext context = metadata.getContext(); UserProfileMetadata decoratedMetadata = metadata.clone(); - RealmModel realm = session.getContext().getRealm(); - ComponentModel component = getComponentModel().orElse(null); if (component == null) { - // makes sure user providers can override metadata for any attribute - decorateUserProfileMetadataWithUserStorage(realm, decoratedMetadata); return decoratedMetadata; } @@ -411,23 +407,10 @@ public class DeclarativeUserProfileProvider implements UserProfileProvider { } } - if (session != null) { - // makes sure user providers can override metadata for any attribute - decorateUserProfileMetadataWithUserStorage(session.getContext().getRealm(), decoratedMetadata); - } - return decoratedMetadata; } - private void decorateUserProfileMetadataWithUserStorage(RealmModel realm, UserProfileMetadata userProfileMetadata) { - // makes sure user providers can override metadata for any attribute - UserProvider users = session.users(); - if (users instanceof UserProfileDecorator) { - ((UserProfileDecorator) users).decorateUserProfile(realm, userProfileMetadata); - } - } - private Map asHashMap(List groups) { return groups.stream().collect(Collectors.toMap(g -> g.getName(), g -> g)); } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java index b1adb22bcb..e9a62b1584 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java @@ -187,6 +187,14 @@ public class LDAPTestUtils { .orElse(null); } + public static ComponentModel getLdapProviderModel(RealmModel realm, String providerName) { + return realm.getComponentsStream(realm.getId(), UserStorageProvider.class.getName()) + .filter(component -> Objects.equals(component.getProviderId(), LDAPStorageProviderFactory.PROVIDER_NAME)) + .filter(component -> providerName == null || component.getName().equals(providerName)) + .findFirst() + .orElse(null); + } + public static LDAPStorageProvider getLdapProvider(KeycloakSession keycloakSession, ComponentModel ldapFedModel) { return (LDAPStorageProvider)keycloakSession.getProvider(UserStorageProvider.class, ldapFedModel); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java index 48298acc72..2699dc92f6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java @@ -124,6 +124,11 @@ public class LDAPAccountRestApiTest extends AbstractLDAPTest { List origLdapEntryDn = adminRestUserRep.getAttributes().get(LDAPConstants.LDAP_ENTRY_DN); Assert.assertNotNull(origLdapId.get(0)); Assert.assertNotNull(origLdapEntryDn.get(0)); + adminRestUserRep = testRealm().users().get(adminRestUserRep.getId()).toRepresentation(); + origLdapId = adminRestUserRep.getAttributes().get(LDAPConstants.LDAP_ID); + origLdapEntryDn = adminRestUserRep.getAttributes().get(LDAPConstants.LDAP_ENTRY_DN); + Assert.assertNotNull(origLdapId.get(0)); + Assert.assertNotNull(origLdapEntryDn.get(0)); // Trying to add KERBEROS_PRINCIPAL (Adding attribute, which was not yet present). Request does not fail, but attribute is not updated user.setFirstName("JohnUpdated"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestContext.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestContext.java index e724296e2d..a7f263fe2c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestContext.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestContext.java @@ -34,8 +34,12 @@ public class LDAPTestContext { private final LDAPStorageProvider ldapProvider; public static LDAPTestContext init(KeycloakSession session) { + return init(session, null); + } + + public static LDAPTestContext init(KeycloakSession session, String providerName) { RealmModel testRealm = session.realms().getRealmByName(AbstractLDAPTest.TEST_REALM_NAME); - ComponentModel ldapCompModel = LDAPTestUtils.getLdapProviderModel(testRealm); + ComponentModel ldapCompModel = LDAPTestUtils.getLdapProviderModel(testRealm, providerName); UserStorageProviderModel ldapModel = new UserStorageProviderModel(ldapCompModel); LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel); return new LDAPTestContext(testRealm, ldapModel, ldapProvider); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java index b99d2c2329..dd0ecf15d8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserProfileTest.java @@ -29,6 +29,8 @@ import org.junit.FixMethodOrder; import org.junit.Test; import org.junit.runners.MethodSorters; import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.component.ComponentModel; +import org.keycloak.component.PrioritizedComponentModel; import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -38,6 +40,8 @@ import org.keycloak.representations.userprofile.config.UPAttribute; import org.keycloak.representations.userprofile.config.UPAttributePermissions; import org.keycloak.representations.userprofile.config.UPConfig; import org.keycloak.storage.UserStorageProvider; +import org.keycloak.storage.UserStorageProviderModel; +import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.pages.LoginUpdateProfilePage; @@ -68,7 +72,7 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { @Override protected void afterImportTestRealm() { testingClient.server().run(session -> { - LDAPTestContext ctx = LDAPTestContext.init(session); + LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); RealmModel appRealm = ctx.getRealm(); UserModel user = LDAPTestUtils.addLocalUser(session, appRealm, "marykeycloak", "mary@test.com", "Password1"); @@ -218,7 +222,20 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { @Test public void testUserProfileWithoutImport() { setLDAPImportDisabled(); + UPConfig origConfig = testRealm().users().userProfile().getConfiguration(); try { + UPConfig config = testRealm().users().userProfile().getConfiguration(); + // Set postal code + UPAttribute postalCode = new UPAttribute(); + postalCode.setName("postal_code"); + postalCode.setDisplayName("Postal Code"); + + UPAttributePermissions permissions = new UPAttributePermissions(); + permissions.setView(Set.of(UPConfigUtils.ROLE_USER, UPConfigUtils.ROLE_ADMIN)); + permissions.setEdit(Set.of(UPConfigUtils.ROLE_USER, UPConfigUtils.ROLE_ADMIN)); + postalCode.setPermissions(permissions); + config.getAttributes().add(postalCode); + testRealm().users().userProfile().update(config); // Test local user is writable and has only attributes defined explicitly in user-profile // Test user profile of user johnkeycloak in admin API UserResource johnResource = ApiUtil.findUserByUsernameId(testRealm(), "johnkeycloak2"); @@ -229,12 +246,56 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { assertProfileAttributes(john, USER_METADATA_GROUP, true, LDAPConstants.LDAP_ID, LDAPConstants.LDAP_ENTRY_DN); } finally { setLDAPImportEnabled(); + testRealm().users().userProfile().update(origConfig); + } + } + + @Test + public void testMultipleLDAPProviders() { + testingClient.server().run(session -> { + RealmModel testRealm = session.realms().getRealmByName(AbstractLDAPTest.TEST_REALM_NAME); + ComponentModel ldapCompModel = LDAPTestUtils.getLdapProviderModel(testRealm); + UserStorageProviderModel ldapModel = new UserStorageProviderModel(ldapCompModel); + ldapModel.setId(null); + ldapModel.setParentId(null); + ldapModel.setName("other-ldap"); + ldapModel.put(LDAPConstants.USERS_DN, ldapModel.getConfig().getFirst(LDAPConstants.USERS_DN).replace("People", "OtherPeople")); + ldapCompModel.put(PrioritizedComponentModel.PRIORITY, "100"); + testRealm.addComponentModel(ldapModel); + LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel); + LDAPObject john = LDAPTestUtils.addLDAPUser(ldapProvider, testRealm, "anotherjohn", "AnotherJohn", "AnotherDoe", "anotherjohn@email.org", null, "1234"); + LDAPTestUtils.updateLDAPPassword(ldapProvider, john, "Password1"); + }); + + // the provider for this user does not have postal_code mapper + UserResource userResource = ApiUtil.findUserByUsernameId(testRealm(), "anotherjohn"); + UserRepresentation userRep = userResource.toRepresentation(true); + Assert.assertNull(userRep.getAttributes().get("postal_code")); + + // the provider for this user does have postal_code mapper + userResource = ApiUtil.findUserByUsernameId(testRealm(), "johnkeycloak"); + userRep = userResource.toRepresentation(true); + Assert.assertNotNull(userRep.getAttributes().get("postal_code")); + + setLDAPReadOnly(); + try { + // the second provider is not readonly + userResource = ApiUtil.findUserByUsernameId(testRealm(), "anotherjohn"); + userRep = userResource.toRepresentation(true); + assertProfileAttributes(userRep, null, false, "username", "email", "firstName", "lastName"); + + // the original provider is readonly + userResource = ApiUtil.findUserByUsernameId(testRealm(), "johnkeycloak"); + userRep = userResource.toRepresentation(true); + assertProfileAttributes(userRep, null, true, "username", "email", "firstName", "lastName", "postal_code"); + } finally { + setLDAPWritable(); } } private void setLDAPReadOnly() { testingClient.server().run(session -> { - LDAPTestContext ctx = LDAPTestContext.init(session); + LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); RealmModel appRealm = ctx.getRealm(); ctx.getLdapModel().getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.READ_ONLY.toString()); @@ -244,7 +305,7 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { private void setLDAPWritable() { testingClient.server().run(session -> { - LDAPTestContext ctx = LDAPTestContext.init(session); + LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); RealmModel appRealm = ctx.getRealm(); ctx.getLdapModel().getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.WRITABLE.toString()); @@ -254,7 +315,7 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { private void setLDAPImportDisabled() { testingClient.server().run(session -> { - LDAPTestContext ctx = LDAPTestContext.init(session); + LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); RealmModel appRealm = ctx.getRealm(); ctx.getLdapModel().getConfig().putSingle(IMPORT_ENABLED, "false"); @@ -264,7 +325,7 @@ public class LDAPUserProfileTest extends AbstractLDAPTest { private void setLDAPImportEnabled() { testingClient.server().run(session -> { - LDAPTestContext ctx = LDAPTestContext.init(session); + LDAPTestContext ctx = LDAPTestContext.init(session, "test-ldap"); RealmModel appRealm = ctx.getRealm(); ctx.getLdapModel().getConfig().putSingle(IMPORT_ENABLED, "true"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/ldap/users.ldif b/testsuite/integration-arquillian/tests/base/src/test/resources/ldap/users.ldif index 4df8b5e61a..388686add0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/ldap/users.ldif +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/ldap/users.ldif @@ -23,3 +23,8 @@ dn: ou=Groups,dc=keycloak,dc=org objectclass: top objectclass: organizationalUnit ou: Groups + +dn: ou=OtherPeople,dc=keycloak,dc=org +objectclass: top +objectclass: organizationalUnit +ou: People diff --git a/testsuite/integration-arquillian/tests/other/sssd/src/test/java/org/keycloak/testsuite/sssd/SSSDUserProfileTest.java b/testsuite/integration-arquillian/tests/other/sssd/src/test/java/org/keycloak/testsuite/sssd/SSSDUserProfileTest.java index 9d9ea49274..81965f78e2 100644 --- a/testsuite/integration-arquillian/tests/other/sssd/src/test/java/org/keycloak/testsuite/sssd/SSSDUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/other/sssd/src/test/java/org/keycloak/testsuite/sssd/SSSDUserProfileTest.java @@ -184,7 +184,8 @@ public class SSSDUserProfileTest extends AbstractBaseSSSDTest { String sssdId = getSssdProviderId(); UserResource userResource = ApiUtil.findUserByUsernameId(testRealm(), username); UserRepresentation user = userResource.toRepresentation(true); - assertUser(user, username, getEmail(username), getFirstName(username), getLastName(username), sssdId); + // first and last names are removed from the UP config (unmanaged) and are not available from the representation + assertUser(user, username, getEmail(username), null, null, sssdId); assertProfileAttributes(user, null, true, UserModel.USERNAME, UserModel.EMAIL, UserModel.FIRST_NAME, UserModel.LAST_NAME); assertProfileAttributes(user, null, false, "postal_code");