From 4f1985826cc9ad06f182360fc81829b65a69c9ea Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 31 Mar 2020 11:43:16 +0200 Subject: [PATCH] KEYCLOAK-12934 LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY user roles retrieve strategy role-ldap-mapper option should only be displayed if LDAP provider vendor is Active Directory --- .../group/GroupLDAPStorageMapperFactory.java | 21 ++- .../role/RoleLDAPStorageMapperFactory.java | 19 ++- .../client/resource/ComponentResource.java | 17 +++ .../testsuite/admin/UserStorageRestTest.java | 124 +++++++++++++----- 4 files changed, 139 insertions(+), 42 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java index 7153aad7c6..c2d814eaa0 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java @@ -55,7 +55,6 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact protected static final List MEMBERSHIP_TYPES = new LinkedList<>(); protected static final List MODES = new LinkedList<>(); protected static final List NO_IMPORT_MODES = new LinkedList<>(); - protected static final List ROLE_RETRIEVERS; // TODO: Merge with RoleLDAPFederationMapperFactory as there are lot of similar properties static { @@ -70,7 +69,6 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact } NO_IMPORT_MODES.add(LDAPGroupMapperMode.LDAP_ONLY.toString()); NO_IMPORT_MODES.add(LDAPGroupMapperMode.READ_ONLY.toString()); - ROLE_RETRIEVERS = new LinkedList<>(userGroupsStrategies.keySet()); List config = getProps(null); configProperties = config; @@ -81,12 +79,14 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact String mode = LDAPGroupMapperMode.LDAP_ONLY.toString(); String membershipUserAttribute = LDAPConstants.UID; boolean importEnabled = true; + boolean isActiveDirectory = false; if (parent != null) { LDAPConfig config = new LDAPConfig(parent.getConfig()); roleObjectClasses = config.isActiveDirectory() ? LDAPConstants.GROUP : LDAPConstants.GROUP_OF_NAMES; mode = config.getEditMode() == UserStorageProvider.EditMode.WRITABLE ? LDAPGroupMapperMode.LDAP_ONLY.toString() : LDAPGroupMapperMode.READ_ONLY.toString(); membershipUserAttribute = config.getUsernameLdapAttribute(); importEnabled = new UserStorageProviderModel(parent).isImportEnabled(); + isActiveDirectory = config.isActiveDirectory(); } ProviderConfigurationBuilder config = ProviderConfigurationBuilder.create() @@ -170,13 +170,22 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact .add(); } + + List groupRetrievers = new LinkedList<>(userGroupsStrategies.keySet()); + String groupRetrieversHelpText = "Specify how to retrieve groups of user. LOAD_GROUPS_BY_MEMBER_ATTRIBUTE means that roles of user will be retrieved by sending LDAP query to retrieve all groups where 'member' is our user. " + + "GET_GROUPS_FROM_USER_MEMBEROF_ATTRIBUTE means that groups of user will be retrieved from 'memberOf' attribute of our user. Or from the other attribute specified by 'Member-Of LDAP Attribute' . "; + if (isActiveDirectory) { + groupRetrieversHelpText = groupRetrieversHelpText + "LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY is applicable just in Active Directory and it means that groups of user will be retrieved recursively with usage of LDAP_MATCHING_RULE_IN_CHAIN Ldap extension."; + } else { + // Option should be available just for the Active Directory + groupRetrievers.remove(GroupMapperConfig.LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY); + } + config.property().name(GroupMapperConfig.USER_ROLES_RETRIEVE_STRATEGY) .label("User Groups Retrieve Strategy") - .helpText("Specify how to retrieve groups of user. LOAD_GROUPS_BY_MEMBER_ATTRIBUTE means that roles of user will be retrieved by sending LDAP query to retrieve all groups where 'member' is our user. " + - "GET_GROUPS_FROM_USER_MEMBEROF_ATTRIBUTE means that groups of user will be retrieved from 'memberOf' attribute of our user. Or from the other attribute specified by 'Member-Of LDAP Attribute' . " + - "LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY is applicable just in Active Directory and it means that groups of user will be retrieved recursively with usage of LDAP_MATCHING_RULE_IN_CHAIN Ldap extension.") + .helpText(groupRetrieversHelpText) .type(ProviderConfigProperty.LIST_TYPE) - .options(ROLE_RETRIEVERS) + .options(groupRetrievers) .defaultValue(GroupMapperConfig.LOAD_GROUPS_BY_MEMBER_ATTRIBUTE) .add() .property().name(GroupMapperConfig.MEMBEROF_LDAP_ATTRIBUTE) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java index 9bf2d45863..75142d7e08 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java @@ -54,7 +54,6 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto protected static final List MEMBERSHIP_TYPES = new LinkedList<>(); protected static final List MODES = new LinkedList<>(); protected static final List NO_IMPORT_MODES = new LinkedList<>(); - protected static final List roleRetrievers; static { userRolesStrategies.put(RoleMapperConfig.LOAD_ROLES_BY_MEMBER_ATTRIBUTE, new UserRolesRetrieveStrategy.LoadRolesByMember()); @@ -70,7 +69,6 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto } NO_IMPORT_MODES.add(LDAPGroupMapperMode.LDAP_ONLY.toString()); NO_IMPORT_MODES.add(LDAPGroupMapperMode.READ_ONLY.toString()); - roleRetrievers = new LinkedList<>(userRolesStrategies.keySet()); List config = getProps(null); configProperties = config; @@ -81,13 +79,14 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto String mode = LDAPGroupMapperMode.LDAP_ONLY.toString(); String membershipUserAttribute = LDAPConstants.UID; boolean importEnabled = true; + boolean isActiveDirectory = false; if (parent != null) { LDAPConfig config = new LDAPConfig(parent.getConfig()); roleObjectClasses = config.isActiveDirectory() ? LDAPConstants.GROUP : LDAPConstants.GROUP_OF_NAMES; mode = config.getEditMode() == UserStorageProvider.EditMode.WRITABLE ? LDAPGroupMapperMode.LDAP_ONLY.toString() : LDAPGroupMapperMode.READ_ONLY.toString(); membershipUserAttribute = config.getUsernameLdapAttribute(); importEnabled = new UserStorageProviderModel(parent).isImportEnabled(); - + isActiveDirectory = config.isActiveDirectory(); } ProviderConfigurationBuilder config = ProviderConfigurationBuilder.create() @@ -158,11 +157,19 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto } + List roleRetrievers = new LinkedList<>(userRolesStrategies.keySet()); + String roleRetrieveHelpText = "Specify how to retrieve roles of user. LOAD_ROLES_BY_MEMBER_ATTRIBUTE means that roles of user will be retrieved by sending LDAP query to retrieve all roles where 'member' is our user. " + + "GET_ROLES_FROM_USER_MEMBEROF_ATTRIBUTE means that roles of user will be retrieved from 'memberOf' attribute of our user. Or from the other attribute specified by 'Member-Of LDAP Attribute' . "; + if (isActiveDirectory) { + roleRetrieveHelpText = roleRetrieveHelpText + "LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY is applicable just in Active Directory and it means that roles of user will be retrieved recursively with usage of LDAP_MATCHING_RULE_IN_CHAIN Ldap extension."; + } else { + // Option should be available just for the Active Directory + roleRetrievers.remove(RoleMapperConfig.LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY); + } + config.property().name(RoleMapperConfig.USER_ROLES_RETRIEVE_STRATEGY) .label("User Roles Retrieve Strategy") - .helpText("Specify how to retrieve roles of user. LOAD_ROLES_BY_MEMBER_ATTRIBUTE means that roles of user will be retrieved by sending LDAP query to retrieve all roles where 'member' is our user. " + - "GET_ROLES_FROM_USER_MEMBEROF_ATTRIBUTE means that roles of user will be retrieved from 'memberOf' attribute of our user. Or from the other attribute specified by 'Member-Of LDAP Attribute' . " + - "LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY is applicable just in Active Directory and it means that roles of user will be retrieved recursively with usage of LDAP_MATCHING_RULE_IN_CHAIN Ldap extension.") + .helpText(roleRetrieveHelpText) .type(ProviderConfigProperty.LIST_TYPE) .options(roleRetrievers) .defaultValue(RoleMapperConfig.LOAD_ROLES_BY_MEMBER_ATTRIBUTE) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ComponentResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ComponentResource.java index e4a5bc3265..7fc040fd27 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ComponentResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ComponentResource.java @@ -16,12 +16,18 @@ */ package org.keycloak.admin.client.resource; +import java.util.List; + import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.ComponentTypeRepresentation; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; /** @@ -38,4 +44,15 @@ public interface ComponentResource { @DELETE void remove(); + + /** + * List of subcomponent types that are available to configure for a particular parent component. + * + * @param subtype fully qualified name of the java class of the provider, which is subtype of this component + * @return + */ + @GET + @Path("sub-component-types") + @Produces(MediaType.APPLICATION_JSON) + List getSubcomponentConfig(@QueryParam("type") String subtype); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java index c47736548c..b73f5262ae 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserStorageRestTest.java @@ -18,6 +18,7 @@ package org.keycloak.testsuite.admin; import org.junit.Test; +import org.keycloak.admin.client.resource.ComponentResource; import org.keycloak.common.constants.KerberosConstants; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.events.admin.OperationType; @@ -26,7 +27,15 @@ import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.LDAPConstants; import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.ComponentTypeRepresentation; +import org.keycloak.representations.idm.ConfigPropertyRepresentation; import org.keycloak.storage.UserStorageProvider; +import org.keycloak.storage.ldap.mappers.LDAPStorageMapper; +import org.keycloak.storage.ldap.mappers.membership.CommonLDAPGroupMapperConfig; +import org.keycloak.storage.ldap.mappers.membership.group.GroupLDAPStorageMapperFactory; +import org.keycloak.storage.ldap.mappers.membership.group.GroupMapperConfig; +import org.keycloak.storage.ldap.mappers.membership.role.RoleLDAPStorageMapperFactory; +import org.keycloak.storage.ldap.mappers.membership.role.RoleMapperConfig; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.authentication.AbstractAuthenticationTest; import org.keycloak.testsuite.util.AdminEventPaths; @@ -83,12 +92,7 @@ public class UserStorageRestTest extends AbstractAdminTest { Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.DISABLED.toString()); // create LDAP provider with kerberos - ComponentRepresentation ldapRep = new ComponentRepresentation(); - ldapRep.setName("ldap2"); - ldapRep.setProviderId("ldap"); - ldapRep.setProviderType(UserStorageProvider.class.getName()); - ldapRep.setConfig(new MultivaluedHashMap<>()); - ldapRep.getConfig().putSingle("priority", Integer.toString(2)); + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); ldapRep.getConfig().putSingle(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION, "true"); String id = createComponent(ldapRep); @@ -147,12 +151,7 @@ public class UserStorageRestTest extends AbstractAdminTest { assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.authUpdateExecutionPath("browser"), kerberosExecution, ResourceType.AUTH_EXECUTION); // create LDAP provider with kerberos - ComponentRepresentation ldapRep = new ComponentRepresentation(); - ldapRep.setName("ldap2"); - ldapRep.setProviderId("ldap"); - ldapRep.setProviderType(UserStorageProvider.class.getName()); - ldapRep.setConfig(new MultivaluedHashMap<>()); - ldapRep.getConfig().putSingle("priority", Integer.toString(2)); + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); ldapRep.getConfig().putSingle(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION, "true"); String id = createComponent(ldapRep); @@ -188,12 +187,7 @@ public class UserStorageRestTest extends AbstractAdminTest { Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.DISABLED.toString()); // create LDAP provider with kerberos - ComponentRepresentation ldapRep = new ComponentRepresentation(); - ldapRep.setName("ldap2"); - ldapRep.setProviderId("ldap"); - ldapRep.setProviderType(UserStorageProvider.class.getName()); - ldapRep.setConfig(new MultivaluedHashMap<>()); - ldapRep.getConfig().putSingle("priority", Integer.toString(2)); + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); ldapRep.getConfig().putSingle(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION, "true"); @@ -242,12 +236,7 @@ public class UserStorageRestTest extends AbstractAdminTest { public void testValidateAndCreateLdapProvider() { // Invalid filter - ComponentRepresentation ldapRep = new ComponentRepresentation(); - ldapRep.setName("ldap2"); - ldapRep.setProviderId("ldap"); - ldapRep.setProviderType(UserStorageProvider.class.getName()); - ldapRep.setConfig(new MultivaluedHashMap<>()); - ldapRep.getConfig().putSingle("priority", Integer.toString(2)); + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); ldapRep.getConfig().putSingle(LDAPConstants.CUSTOM_USER_SEARCH_FILTER, "dc=something"); Response resp = realm.components().add(ldapRep); @@ -297,12 +286,7 @@ public class UserStorageRestTest extends AbstractAdminTest { @Test public void testUpdateProvider() { - ComponentRepresentation ldapRep = new ComponentRepresentation(); - ldapRep.setName("ldap2"); - ldapRep.setProviderId("ldap"); - ldapRep.setProviderType(UserStorageProvider.class.getName()); - ldapRep.setConfig(new MultivaluedHashMap<>()); - ldapRep.getConfig().putSingle("priority", Integer.toString(2)); + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); ldapRep.getConfig().putSingle(LDAPConstants.BIND_DN, "cn=manager"); ldapRep.getConfig().putSingle(LDAPConstants.BIND_CREDENTIAL, "password"); String id = createComponent(ldapRep); @@ -345,8 +329,88 @@ public class UserStorageRestTest extends AbstractAdminTest { } + // KEYCLOAK-12934 + @Test + public void testLDAPMapperProviderConfigurationForVendorOther() { + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); + ldapRep.getConfig().putSingle(LDAPConstants.VENDOR, LDAPConstants.VENDOR_OTHER); + String ldapModelId = createComponent(ldapRep); + ComponentTypeRepresentation groupLDAPMapperType = findMapperTypeConfiguration(ldapModelId, GroupLDAPStorageMapperFactory.PROVIDER_ID); + ConfigPropertyRepresentation groupRetrieverConfigProperty = getUserRolesRetrieveStrategyConfigProperty(groupLDAPMapperType, CommonLDAPGroupMapperConfig.USER_ROLES_RETRIEVE_STRATEGY); + // LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY is expected to be present just for the active directory + List options = groupRetrieverConfigProperty.getOptions(); + Assert.assertNames(options, GroupMapperConfig.LOAD_GROUPS_BY_MEMBER_ATTRIBUTE, GroupMapperConfig.GET_GROUPS_FROM_USER_MEMBEROF_ATTRIBUTE); + Assert.assertFalse(groupRetrieverConfigProperty.getHelpText().contains("LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY")); + + ComponentTypeRepresentation roleLDAPMapperType = findMapperTypeConfiguration(ldapModelId, RoleLDAPStorageMapperFactory.PROVIDER_ID); + ConfigPropertyRepresentation roleRetrieverConfigProperty = getUserRolesRetrieveStrategyConfigProperty(roleLDAPMapperType, CommonLDAPGroupMapperConfig.USER_ROLES_RETRIEVE_STRATEGY); + + // LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY is expected to be present just for the active directory + options = roleRetrieverConfigProperty.getOptions(); + Assert.assertNames(options, RoleMapperConfig.LOAD_ROLES_BY_MEMBER_ATTRIBUTE, RoleMapperConfig.GET_ROLES_FROM_USER_MEMBEROF_ATTRIBUTE); + Assert.assertFalse(roleRetrieverConfigProperty.getHelpText().contains("LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY")); + + // Cleanup including mappers + removeComponent(ldapModelId); + } + + // KEYCLOAK-12934 + @Test + public void testLDAPMapperProviderConfigurationForVendorMSAD() { + ComponentRepresentation ldapRep = createBasicLDAPProviderRep(); + ldapRep.getConfig().putSingle(LDAPConstants.VENDOR, LDAPConstants.VENDOR_ACTIVE_DIRECTORY); + String ldapModelId = createComponent(ldapRep); + + ComponentTypeRepresentation groupLDAPMapperType = findMapperTypeConfiguration(ldapModelId, GroupLDAPStorageMapperFactory.PROVIDER_ID); + ConfigPropertyRepresentation groupRetrieverConfigProperty = getUserRolesRetrieveStrategyConfigProperty(groupLDAPMapperType, CommonLDAPGroupMapperConfig.USER_ROLES_RETRIEVE_STRATEGY); + + // LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY is expected to be present just for the active directory + List options = groupRetrieverConfigProperty.getOptions(); + Assert.assertNames(options, GroupMapperConfig.LOAD_GROUPS_BY_MEMBER_ATTRIBUTE, GroupMapperConfig.GET_GROUPS_FROM_USER_MEMBEROF_ATTRIBUTE, + GroupMapperConfig.LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY); + Assert.assertTrue(groupRetrieverConfigProperty.getHelpText().contains("LOAD_GROUPS_BY_MEMBER_ATTRIBUTE_RECURSIVELY")); + + ComponentTypeRepresentation roleLDAPMapperType = findMapperTypeConfiguration(ldapModelId, RoleLDAPStorageMapperFactory.PROVIDER_ID); + ConfigPropertyRepresentation roleRetrieverConfigProperty = getUserRolesRetrieveStrategyConfigProperty(roleLDAPMapperType, CommonLDAPGroupMapperConfig.USER_ROLES_RETRIEVE_STRATEGY); + + // LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY is expected to be present just for the active directory + options = roleRetrieverConfigProperty.getOptions(); + Assert.assertNames(options, RoleMapperConfig.LOAD_ROLES_BY_MEMBER_ATTRIBUTE, RoleMapperConfig.GET_ROLES_FROM_USER_MEMBEROF_ATTRIBUTE, + RoleMapperConfig.LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY); + Assert.assertTrue(roleRetrieverConfigProperty.getHelpText().contains("LOAD_ROLES_BY_MEMBER_ATTRIBUTE_RECURSIVELY")); + + // Cleanup including mappers + removeComponent(ldapModelId); + } + + private ComponentRepresentation createBasicLDAPProviderRep() { + ComponentRepresentation ldapRep = new ComponentRepresentation(); + ldapRep.setName("ldap2"); + ldapRep.setProviderId("ldap"); + ldapRep.setProviderType(UserStorageProvider.class.getName()); + ldapRep.setConfig(new MultivaluedHashMap<>()); + ldapRep.getConfig().putSingle("priority", Integer.toString(2)); + return ldapRep; + } + + private ComponentTypeRepresentation findMapperTypeConfiguration(String ldapModelId, String mapperProviderId) { + ComponentResource ldapProvider = realm.components().component(ldapModelId); + List componentTypes = ldapProvider.getSubcomponentConfig(LDAPStorageMapper.class.getName()); + + return componentTypes.stream() + .filter(componentType -> mapperProviderId.equals(componentType.getId())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Not able to find mapper with provider id: " + mapperProviderId)); + } + + private ConfigPropertyRepresentation getUserRolesRetrieveStrategyConfigProperty(ComponentTypeRepresentation componentType, String propertyName) { + return componentType.getProperties().stream() + .filter(configPropertyRep -> propertyName.equals(configPropertyRep.getName())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Not able to find config property with name: " + propertyName)); + } /* @Test