diff --git a/common/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java b/common/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java index 2398b4ca530..f285e305dc7 100644 --- a/common/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java +++ b/common/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java @@ -27,8 +27,6 @@ import org.springframework.util.MultiValueMap; import java.util.Arrays; -import java.util.Collections; -import java.util.List; import java.util.Map; public class LdapLoginAuthenticationManager extends ExternalLoginAuthenticationManager { @@ -41,9 +39,6 @@ public void setProvisioning(IdentityProviderProvisioning provisioning) { this.provisioning = provisioning; } - public static final List ALREADY_MAPPED_ATTRS = - Collections.unmodifiableList(Arrays.asList("first_name", "family_name", "phone_number")); - @Override protected MultiValueMap getUserAttributes(UserDetails request) { MultiValueMap result = super.getUserAttributes(request); @@ -51,14 +46,14 @@ protected MultiValueMap getUserAttributes(UserDetails request) { IdentityProvider provider = provisioning.retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId()); if (request instanceof ExtendedLdapUserDetails) { ExtendedLdapUserDetails ldapDetails = ((ExtendedLdapUserDetails) request); - for (Map.Entry entry : provider.getConfigValue(LdapIdentityProviderDefinition.class).getAttributeMappings().entrySet()) { + LdapIdentityProviderDefinition ldapIdentityProviderDefinition = provider.getConfigValue(LdapIdentityProviderDefinition.class); + Map providerMappings = ldapIdentityProviderDefinition.getAttributeMappings(); + for (Map.Entry entry : providerMappings.entrySet()) { if (entry.getKey().startsWith(USER_ATTRIBUTE_PREFIX) && entry.getValue() != null) { String key = entry.getKey().substring(USER_ATTRIBUTE_PREFIX.length()); - if (! ALREADY_MAPPED_ATTRS.contains(key)) { - String[] values = ldapDetails.getAttribute((String) entry.getValue(), false); - if (values != null && values.length > 0) { - result.put(key, Arrays.asList(values)); - } + String[] values = ldapDetails.getAttribute((String) entry.getValue(), false); + if (values != null && values.length > 0) { + result.put(key, Arrays.asList(values)); } } } diff --git a/common/src/main/java/org/cloudfoundry/identity/uaa/ldap/LdapIdentityProviderDefinition.java b/common/src/main/java/org/cloudfoundry/identity/uaa/ldap/LdapIdentityProviderDefinition.java index 2ac0e3f7852..6e368d4e071 100644 --- a/common/src/main/java/org/cloudfoundry/identity/uaa/ldap/LdapIdentityProviderDefinition.java +++ b/common/src/main/java/org/cloudfoundry/identity/uaa/ldap/LdapIdentityProviderDefinition.java @@ -192,8 +192,10 @@ public static LdapIdentityProviderDefinition searchAndBindMapGroupToScopes( return definition; } + /** + * Load a LDAP definition from the Yaml config (IdentityProviderBootstrap) + */ public static LdapIdentityProviderDefinition fromConfig(Map ldapConfig) { - LdapIdentityProviderDefinition definition = new LdapIdentityProviderDefinition(); if (ldapConfig==null || ldapConfig.isEmpty()) { return definition; @@ -261,9 +263,13 @@ public static LdapIdentityProviderDefinition fromConfig(Map ldap definition.setAutoAddGroups((Boolean) ldapConfig.get(LDAP_GROUPS_AUTO_ADD)); definition.setGroupRoleAttribute((String) ldapConfig.get(LDAP_GROUPS_GROUP_ROLE_ATTRIBUTE)); } - final String LDAP_ATTR_MAP_PREFIX = "ldap."+ATTRIBUTE_MAPPINGS+"."; + + //if flat attributes are set in the properties + final String LDAP_ATTR_MAP_PREFIX = LDAP_ATTRIBUTE_MAPPINGS+"."; for (Map.Entry entry : ldapConfig.entrySet()) { - if (!LDAP_PROPERTY_NAMES.contains(entry.getKey()) && entry.getKey().startsWith(LDAP_ATTR_MAP_PREFIX+USER_ATTRIBUTE_PREFIX)) { + if (!LDAP_PROPERTY_NAMES.contains(entry.getKey()) && + entry.getKey().startsWith(LDAP_ATTR_MAP_PREFIX) && + entry.getValue() instanceof String) { definition.addAttributeMapping(entry.getKey().substring(LDAP_ATTR_MAP_PREFIX.length()), entry.getValue()); } } diff --git a/docs/UAA-LDAP.md b/docs/UAA-LDAP.md index 29cc3fd270c..027728178e8 100644 --- a/docs/UAA-LDAP.md +++ b/docs/UAA-LDAP.md @@ -704,7 +704,8 @@ ldap: first_name: givenname family_name: sn phone_number: telephonenumber - email: mail - user.attribute.employeeCostCenter: costCenter - user.attribute.terribleBosses: manager + user: + attribute: + employeeCostCenter: costCenter + terribleBosses: manager diff --git a/uaa/src/main/resources/ldap-integration.xml b/uaa/src/main/resources/ldap-integration.xml index 1875a8046ef..587599f11d9 100644 --- a/uaa/src/main/resources/ldap-integration.xml +++ b/uaa/src/main/resources/ldap-integration.xml @@ -48,9 +48,9 @@ - - - + + + diff --git a/uaa/src/main/resources/uaa.yml b/uaa/src/main/resources/uaa.yml index f9af3cb5bd2..2721627829a 100755 --- a/uaa/src/main/resources/uaa.yml +++ b/uaa/src/main/resources/uaa.yml @@ -95,8 +95,9 @@ # emailDomain: # - example.com # attributeMappings: -# given_name: firstName -# family_name: surname +# given_name: givenname +# family_name: sn +# phone_number: telephonenumber # user.attribute.employeeCostCenter: costCenter # user.attribute.terribleBosses: uaaManager diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java index 0d355f7b56f..7b91e7a6b15 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java @@ -80,6 +80,7 @@ import java.util.Set; import static org.cloudfoundry.identity.uaa.ExternalIdentityProviderDefinition.ATTRIBUTE_MAPPINGS; +import static org.cloudfoundry.identity.uaa.ldap.LdapIdentityProviderDefinition.LDAP_ATTRIBUTE_MAPPINGS; import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.CookieCsrfPostProcessor.cookieCsrf; import static org.hamcrest.Matchers.arrayContainingInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -241,10 +242,9 @@ public void testCustomUserAttributes() throws Exception { mockEnvironment.setProperty("ldap."+ATTRIBUTE_MAPPINGS+".user.attribute."+COST_CENTERS, COST_CENTER); //test to remap the user/person properties - mockEnvironment.setProperty("ldap."+ATTRIBUTE_MAPPINGS+".user.attribute."+FIRST_NAME, "sn"); - mockEnvironment.setProperty("ldap."+ATTRIBUTE_MAPPINGS+".user.attribute."+PHONE_NUMBER, "givenname"); - mockEnvironment.setProperty("ldap."+ATTRIBUTE_MAPPINGS+".user.attribute."+FAMILY_NAME, "telephonenumber"); - mockEnvironment.setProperty("ldap."+ATTRIBUTE_MAPPINGS+".user.attribute."+EMAIL, "mail"); + mockEnvironment.setProperty(LDAP_ATTRIBUTE_MAPPINGS+"."+FIRST_NAME, "sn"); + mockEnvironment.setProperty(LDAP_ATTRIBUTE_MAPPINGS+"."+PHONE_NUMBER, "givenname"); + mockEnvironment.setProperty(LDAP_ATTRIBUTE_MAPPINGS+"."+FAMILY_NAME, "telephonenumber"); setUp(); @@ -254,7 +254,7 @@ public void testCustomUserAttributes() throws Exception { UaaAuthentication authentication = (UaaAuthentication) ((SecurityContext) result.getRequest().getSession().getAttribute(SPRING_SECURITY_CONTEXT_KEY)).getAuthentication(); - assertEquals("Expected two user attributes", 3, authentication.getUserAttributes().size()); + assertEquals("Expected two user attributes", 2, authentication.getUserAttributes().size()); assertNotNull("Expected cost center attribute", authentication.getUserAttributes().get(COST_CENTERS)); assertEquals(DENVER_CO, authentication.getUserAttributes().getFirst(COST_CENTERS)); @@ -265,7 +265,6 @@ public void testCustomUserAttributes() throws Exception { assertEquals("8885550986", getFamilyName(username)); assertEquals("Marissa", getPhoneNumber(username)); assertEquals("Marissa9", getGivenName(username)); - assertThat(authentication.getUserAttributes().get(EMAIL), containsInAnyOrder("marissa9@test.com", "marissa9-custom@test.com")); } @Test