From 479e6bc86b5110945297074d94d10ddc709d1aa1 Mon Sep 17 00:00:00 2001 From: mposolda Date: Wed, 29 Nov 2023 13:03:49 +0100 Subject: [PATCH] Update Kerberos provider for user-profile closes #25074 Signed-off-by: mposolda --- .../kerberos/KerberosFederationProvider.java | 30 ++++++- .../storage/ldap/LDAPStorageProvider.java | 33 ++----- .../keycloak/userprofile/UserProfileUtil.java | 85 +++++++++++++++++++ .../AbstractUserProfileProvider.java | 2 - .../DeclarativeUserProfileProvider.java | 1 - .../java/org/keycloak/testsuite/Assert.java | 3 + .../kerberos/KerberosStandaloneTest.java | 55 +++++++++++- .../federation/ldap/LDAPUserProfileTest.java | 2 +- 8 files changed, 174 insertions(+), 37 deletions(-) create mode 100644 server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileUtil.java 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 cfdc58342e..5254bf7fe9 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,9 +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.HashMap; import java.util.Map; +import java.util.function.Predicate; import java.util.stream.Stream; import javax.security.auth.login.LoginException; @@ -57,7 +64,8 @@ public class KerberosFederationProvider implements UserStorageProvider, CredentialInputValidator, CredentialInputUpdater, CredentialAuthentication, - ImportedUserValidation { + ImportedUserValidation, + UserProfileDecorator { private static final Logger logger = Logger.getLogger(KerberosFederationProvider.class); public static final String KERBEROS_PRINCIPAL = KerberosConstants.KERBEROS_PRINCIPAL; @@ -292,4 +300,24 @@ public class KerberosFederationProvider implements UserStorageProvider, public String toString() { return "KerberosFederationProvider - " + model.getName(); } + + @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()); + }); + + 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()); + } } 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 2d745bdbe4..915d81cded 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 @@ -62,7 +62,6 @@ import org.keycloak.models.utils.ReadOnlyUserModelDelegate; import org.keycloak.policy.PasswordPolicyManagerProvider; import org.keycloak.policy.PolicyError; import org.keycloak.models.cache.UserCache; -import org.keycloak.representations.userprofile.config.UPConfig; import org.keycloak.storage.DatastoreProvider; import org.keycloak.storage.LegacyStoreManagers; import org.keycloak.storage.ReadOnlyException; @@ -89,14 +88,13 @@ import org.keycloak.storage.user.ImportedUserValidation; import org.keycloak.storage.user.UserLookupProvider; import org.keycloak.storage.user.UserQueryMethodsProvider; import org.keycloak.storage.user.UserRegistrationProvider; -import org.keycloak.userprofile.AbstractUserProfileProvider; import org.keycloak.userprofile.AttributeContext; import org.keycloak.userprofile.AttributeGroupMetadata; import org.keycloak.userprofile.AttributeMetadata; import org.keycloak.userprofile.UserProfileContext; import org.keycloak.userprofile.UserProfileDecorator; import org.keycloak.userprofile.UserProfileMetadata; -import org.keycloak.userprofile.UserProfileProvider; +import org.keycloak.userprofile.UserProfileUtil; import static org.keycloak.utils.StreamsUtil.paginatedStream; @@ -1027,23 +1025,12 @@ public class LDAPStorageProvider implements UserStorageProvider, metadataAttributes.add(KerberosConstants.KERBEROS_PRINCIPAL); } - AttributeGroupMetadata metadataGroup = lookupMetadataGroup(); + AttributeGroupMetadata metadataGroup = UserProfileUtil.lookupUserMetadataGroup(session); for (String attrName : metadataAttributes) { - // In case that attributes like LDAP_ID, KERBEROS_PRINCIPAL are explicitly defined on user profile, we can prefer defined configuration - if (!metadata.getAttribute(attrName).isEmpty()) { - logger.debugf("Ignore adding metadata attribute '%s' to user profile by LDAP provider '%s' as attribute is already defined on user profile.", attrName, getModel().getName()); - } else { - logger.debugf("Adding metadata attribute '%s' to user profile by LDAP provider '%s' for user profile context '%s'.", attrName, getModel().getName(), metadata.getContext().toString()); - AttributeMetadata attributeMetadata = metadata.addAttribute(attrName, guiOrder++, Collections.emptyList()) - .addWriteCondition(AttributeMetadata.ALWAYS_FALSE) // Not writable for anyone - .addReadCondition(onlyAdminCondition) // Read-only for administrators - .setRequired(AttributeMetadata.ALWAYS_FALSE); - - if (metadataGroup != null) { - attributeMetadata.setAttributeGroupMetadata(metadataGroup); - } - attributeMetadata.setSelector(ldapUsersSelector); + boolean attributeAdded = UserProfileUtil.addMetadataAttributeToUserProfile(attrName, metadata, metadataGroup, ldapUsersSelector, guiOrder++, getModel().getName()); + if (!attributeAdded) { + guiOrder--; } } @@ -1054,14 +1041,4 @@ public class LDAPStorageProvider implements UserStorageProvider, } } } - - private AttributeGroupMetadata lookupMetadataGroup() { - UserProfileProvider provider = session.getProvider(UserProfileProvider.class); - UPConfig config = provider.getConfiguration(); - return config.getGroups().stream() - .filter(upGroup -> AbstractUserProfileProvider.USER_METADATA_GROUP.equals(upGroup.getName())) - .map(upGroup -> new AttributeGroupMetadata(upGroup.getName(), upGroup.getDisplayHeader(), upGroup.getDisplayDescription(), upGroup.getAnnotations())) - .findAny() - .orElse(null); - } } 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 new file mode 100644 index 0000000000..cfa6e0f4bd --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileUtil.java @@ -0,0 +1,85 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.userprofile; + +import java.util.Collections; +import java.util.function.Predicate; + +import org.jboss.logging.Logger; +import org.keycloak.models.KeycloakSession; +import org.keycloak.representations.userprofile.config.UPConfig; + +/** + * @author Marek Posolda + */ +public class UserProfileUtil { + + private static final Logger logger = Logger.getLogger(UserProfileUtil.class); + + public static final String USER_METADATA_GROUP = "user-metadata"; + + /** + * Find the metadata group "user-metadata" + * + * @param session + * @return metadata group if exists, otherwise null + */ + public static AttributeGroupMetadata lookupUserMetadataGroup(KeycloakSession session) { + UserProfileProvider provider = session.getProvider(UserProfileProvider.class); + UPConfig config = provider.getConfiguration(); + return config.getGroups().stream() + .filter(upGroup -> USER_METADATA_GROUP.equals(upGroup.getName())) + .map(upGroup -> new AttributeGroupMetadata(upGroup.getName(), upGroup.getDisplayHeader(), upGroup.getDisplayDescription(), upGroup.getAnnotations())) + .findAny() + .orElse(null); + } + + /** + * Adds metadata attribute to the user-profile for users from specified userStorageProvider + * + * @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 + */ + 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 + 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() == UserProfileContext.USER_API; + AttributeMetadata attributeMetadata = metadata.addAttribute(attrName, guiOrder, Collections.emptyList()) + .addWriteCondition(AttributeMetadata.ALWAYS_FALSE) // Not writable for anyone + .addReadCondition(onlyAdminCondition) // Read-only for administrators + .setRequired(AttributeMetadata.ALWAYS_FALSE); + + if (metadataGroup != null) { + attributeMetadata.setAttributeGroupMetadata(metadataGroup); + } + attributeMetadata.setSelector(userFederationUsersSelector); + return true; + } + } +} diff --git a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java index d38f946e88..fe11825018 100644 --- a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java @@ -73,8 +73,6 @@ public abstract class AbstractUserProfileProvider public static final String CONFIG_READ_ONLY_ATTRIBUTES = "read-only-attributes"; public static final String MAX_EMAIL_LOCAL_PART_LENGTH = "max-email-local-part-length"; - public static final String USER_METADATA_GROUP = "user-metadata"; - private static boolean editUsernameCondition(AttributeContext c) { KeycloakSession session = c.getSession(); KeycloakContext context = session.getContext(); diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java index 32a49785d5..5b436476e1 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java @@ -52,7 +52,6 @@ import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.provider.ProviderConfigurationBuilder; import org.keycloak.services.messages.Messages; import org.keycloak.sessions.AuthenticationSessionModel; -import org.keycloak.storage.DatastoreProvider; import org.keycloak.userprofile.config.DeclarativeUserProfileModel; import org.keycloak.representations.userprofile.config.UPAttribute; import org.keycloak.representations.userprofile.config.UPAttributePermissions; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/Assert.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/Assert.java index edc13343db..d84f94de6b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/Assert.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/Assert.java @@ -28,6 +28,7 @@ import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserFederationProviderFactoryRepresentation; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.info.ThemeInfoRepresentation; @@ -95,6 +96,8 @@ public class Assert extends org.junit.Assert { return ((ClientScopeRepresentation) o1).getName(); } else if (o1 instanceof ThemeInfoRepresentation) { return ((ThemeInfoRepresentation) o1).getName(); + } else if (o1 instanceof UserProfileAttributeMetadata) { + return ((UserProfileAttributeMetadata) o1).getName(); } throw new IllegalArgumentException(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java index 72b815efd7..344a908121 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java @@ -22,29 +22,39 @@ import java.util.List; import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; -import org.junit.Assert; +import org.keycloak.testsuite.Assert; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; +import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.Profile; import org.keycloak.common.constants.KerberosConstants; import org.keycloak.federation.kerberos.CommonKerberosConfig; import org.keycloak.federation.kerberos.KerberosConfig; import org.keycloak.federation.kerberos.KerberosFederationProviderFactory; +import org.keycloak.models.UserModel; import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.storage.UserStorageProvider; import org.keycloak.testsuite.ActionURIUtils; import org.keycloak.testsuite.KerberosEmbeddedServer; import org.keycloak.testsuite.ProfileAssume; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected; +import org.keycloak.testsuite.forms.VerifyProfileTest; import org.keycloak.testsuite.util.KerberosRule; +import static org.keycloak.userprofile.UserProfileUtil.USER_METADATA_GROUP; + /** * Test for the KerberosFederationProvider (kerberos without LDAP integration) * * @author Marek Posolda */ +@EnableFeature(Profile.Feature.DECLARATIVE_USER_PROFILE) public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest { private static final String PROVIDER_CONFIG_LOCATION = "classpath:kerberos/kerberos-standalone-connection.properties"; @@ -90,7 +100,7 @@ public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest { // Switch updateProfileOnFirstLogin to on String parentId = testRealmResource().toRepresentation().getId(); List reps = testRealmResource().components().query(parentId, UserStorageProvider.class.getName()); - org.keycloak.testsuite.Assert.assertEquals(1, reps.size()); + Assert.assertEquals(1, reps.size()); ComponentRepresentation kerberosProvider = reps.get(0); kerberosProvider.getConfig().putSingle(KerberosConstants.UPDATE_PROFILE_FIRST_LOGIN, "true"); testRealmResource().components().component(kerberosProvider.getId()).update(kerberosProvider); @@ -124,7 +134,7 @@ public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest { public void noProvider() throws Exception { String parentId = testRealmResource().toRepresentation().getId(); List reps = testRealmResource().components().query(parentId, UserStorageProvider.class.getName()); - org.keycloak.testsuite.Assert.assertEquals(1, reps.size()); + Assert.assertEquals(1, reps.size()); ComponentRepresentation kerberosProvider = reps.get(0); testRealmResource().components().component(kerberosProvider.getId()).remove(); @@ -171,7 +181,7 @@ public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest { // Switch kerberos realm to "unavailable String parentId = testRealmResource().toRepresentation().getId(); List reps = testRealmResource().components().query(parentId, UserStorageProvider.class.getName()); - org.keycloak.testsuite.Assert.assertEquals(1, reps.size()); + Assert.assertEquals(1, reps.size()); ComponentRepresentation kerberosProvider = reps.get(0); kerberosProvider.getConfig().putSingle(KerberosConstants.KERBEROS_REALM, "unavailable"); testRealmResource().components().component(kerberosProvider.getId()).update(kerberosProvider); @@ -184,4 +194,41 @@ public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest { response.close(); } + @Test + public void testUserProfile() throws Exception { + RealmRepresentation realm = testRealmResource().toRepresentation(); + VerifyProfileTest.enableDynamicUserProfile(realm); + testRealmResource().update(realm); + + try { + assertSuccessfulSpnegoLogin("hnelson", "hnelson", "secret"); + + // User-profile data should be present (including KERBEROS_PRINCIPAL attribute) + UserResource johnResource = ApiUtil.findUserByUsernameId(testRealmResource(), "hnelson"); + UserRepresentation john = johnResource.toRepresentation(true); + Assert.assertNames(john.getUserProfileMetadata().getAttributes(), UserModel.FIRST_NAME, UserModel.LAST_NAME, UserModel.EMAIL, UserModel.USERNAME, KerberosConstants.KERBEROS_PRINCIPAL); + + // KERBEROS_PRINCIPAL attribute should be read-only and should be in "User metadata" group + UserProfileAttributeMetadata krbPrincipalAttribute = john.getUserProfileMetadata().getAttributeMetadata(KerberosConstants.KERBEROS_PRINCIPAL); + Assert.assertTrue(krbPrincipalAttribute.isReadOnly()); + Assert.assertEquals(USER_METADATA_GROUP, krbPrincipalAttribute.getGroup()); + + // Test Update profile + john.getRequiredActions().add(UserModel.RequiredAction.UPDATE_PROFILE.toString()); + johnResource.update(john); + + Response spnegoResponse = spnegoLogin("hnelson", "secret"); + Assert.assertEquals(200, spnegoResponse.getStatus()); + String responseText = spnegoResponse.readEntity(String.class); + Assert.assertTrue(responseText.contains("You need to update your user profile to activate your account.")); + Assert.assertFalse(responseText.contains("KERBEROS_PRINCIPAL")); + spnegoResponse.close(); + + john.getRequiredActions().remove(UserModel.RequiredAction.UPDATE_PROFILE.toString()); + johnResource.update(john); + } finally { + VerifyProfileTest.disableDynamicUserProfile(testRealmResource()); + } + } + } 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 7530bed7be..716ee47393 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 @@ -52,7 +52,7 @@ import org.keycloak.testsuite.util.LDAPTestUtils; import org.keycloak.userprofile.config.UPConfigUtils; import static org.keycloak.storage.UserStorageProviderModel.IMPORT_ENABLED; -import static org.keycloak.userprofile.AbstractUserProfileProvider.USER_METADATA_GROUP; +import static org.keycloak.userprofile.UserProfileUtil.USER_METADATA_GROUP; /** * @author Marek Posolda