Allow disabling shadow user creation at the time of ldap authentication

[#116487097] https://www.pivotaltracker.com/story/show/116487097 Signed-off-by: Jeanie Jung <jeaniejung@berkeley.edu>
cloudfoundry · Jul 5, 2016 · 7adcc94 · 7adcc94
1 parent 437df45
commit 7adcc94
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 41 deletions.
diff --git a/...java/org/cloudfoundry/identity/uaa/provider/AbstractXOAuthIdentityProviderDefinition.java b/...java/org/cloudfoundry/identity/uaa/provider/AbstractXOAuthIdentityProviderDefinition.java
@@ -30,7 +30,6 @@ public abstract class AbstractXOAuthIdentityProviderDefinition<T extends Abstrac
     private String relyingPartyId;
     private String relyingPartySecret;
     private List<String> scopes;
-    private boolean addShadowUserOnLogin = true;
     private String issuer;
 
     public URL getAuthUrl() {
@@ -123,15 +122,6 @@ public T setScopes(List<String> scopes) {
         return (T) this;
     }
 
-    public boolean isAddShadowUserOnLogin() {
-        return addShadowUserOnLogin;
-    }
-
-    public T setAddShadowUserOnLogin(boolean addShadowUserOnLogin) {
-        this.addShadowUserOnLogin = addShadowUserOnLogin;
-        return (T) this;
-    }
-
     public String getIssuer() {
         return issuer;
     }

diff --git a/.../main/java/org/cloudfoundry/identity/uaa/provider/ExternalIdentityProviderDefinition.java b/.../main/java/org/cloudfoundry/identity/uaa/provider/ExternalIdentityProviderDefinition.java
@@ -34,6 +34,7 @@ public class ExternalIdentityProviderDefinition extends AbstractIdentityProvider
 
     private List<String> externalGroupsWhitelist = new LinkedList<>();
     private Map<String, Object> attributeMappings = new HashMap<>();
+    private boolean addShadowUserOnLogin = true;
 
     public List<String> getExternalGroupsWhitelist() {
         return Collections.unmodifiableList(externalGroupsWhitelist);
@@ -67,6 +68,14 @@ public void addAttributeMapping(String key, Object value) {
         attributeMappings.put(key, value);
     }
 
+    public boolean isAddShadowUserOnLogin() {
+        return addShadowUserOnLogin;
+    }
+
+    public void setAddShadowUserOnLogin(boolean addShadowUserOnLogin) {
+        this.addShadowUserOnLogin = addShadowUserOnLogin;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/.../src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java b/.../src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java
@@ -56,7 +56,6 @@ public enum ExternalGroupMappingMode {
     private String socketFactoryClassName;
     private String linkText;
     private String iconUrl;
-    private boolean addShadowUserOnLogin = true;
     private ExternalGroupMappingMode groupMappingMode = ExternalGroupMappingMode.EXPLICITLY_MAPPED;
 
     public SamlIdentityProviderDefinition() {}
@@ -75,7 +74,7 @@ public SamlIdentityProviderDefinition clone() {
         def.setShowSamlLink(showSamlLink);
         def.setLinkText(linkText);
         def.setIconUrl(iconUrl);
-        def.setAddShadowUserOnLogin(addShadowUserOnLogin);
+        def.setAddShadowUserOnLogin(isAddShadowUserOnLogin());
         def.setEmailDomain(emailDomain);
         def.setExternalGroupsWhitelist(externalGroupsWhitelist);
         def.setAttributeMappings(attributeMappings);
@@ -242,15 +241,6 @@ public SamlIdentityProviderDefinition setZoneId(String zoneId) {
         return this;
     }
 
-    public boolean isAddShadowUserOnLogin() {
-        return addShadowUserOnLogin;
-    }
-
-    public SamlIdentityProviderDefinition setAddShadowUserOnLogin(boolean addShadowUserOnLogin) {
-        this.addShadowUserOnLogin = addShadowUserOnLogin;
-        return this;
-    }
-
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -285,7 +275,7 @@ public String toString() {
             ", linkText='" + linkText + '\'' +
             ", iconUrl='" + iconUrl + '\'' +
             ", zoneId='" + zoneId + '\'' +
-            ", addShadowUserOnLogin='" + addShadowUserOnLogin + '\'' +
+            ", addShadowUserOnLogin='" + isAddShadowUserOnLogin() + '\'' +
             '}';
     }
 

diff --git a/.../org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java b/.../org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java
@@ -15,7 +15,6 @@
 
 package org.cloudfoundry.identity.uaa.authentication.manager;
 
-import org.apache.commons.lang.StringUtils;
 import org.cloudfoundry.identity.uaa.provider.ldap.ExtendedLdapUserDetails;
 import org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.provider.ldap.extension.LdapAuthority;
@@ -126,4 +125,16 @@ protected boolean isAutoAddAuthorities() {
         return result!=null ? result.booleanValue() : true;
     }
 
+    @Override
+    protected boolean isAddNewShadowUser() {
+        Boolean result = true;
+        if (provisioning!=null) {
+            IdentityProvider provider = provisioning.retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId());
+            LdapIdentityProviderDefinition ldapIdentityProviderDefinition = ObjectUtils.castInstance(provider.getConfig(), LdapIdentityProviderDefinition.class);
+            if (ldapIdentityProviderDefinition!=null) {
+                result = ldapIdentityProviderDefinition.isAddShadowUserOnLogin();
+            }
+        }
+        return result!=null ? result.booleanValue() : true;
+    }
 }
diff --git a/...c/main/java/org/cloudfoundry/identity/uaa/provider/oauth/XOAuthAuthenticationManager.java b/...c/main/java/org/cloudfoundry/identity/uaa/provider/oauth/XOAuthAuthenticationManager.java
@@ -183,6 +183,7 @@ protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequ
 
     @Override
     protected boolean isAddNewShadowUser() {
+        if(!super.isAddNewShadowUser()) return false;
         IdentityProvider<AbstractXOAuthIdentityProviderDefinition> provider = providerProvisioning.retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId());
         return provider.getConfig().isAddShadowUserOnLogin();
     }

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/util/LdapUtils.java b/server/src/main/java/org/cloudfoundry/identity/uaa/util/LdapUtils.java
@@ -64,6 +64,7 @@ public static ConfigurableEnvironment getLdapConfigurationEnvironment(LdapIdenti
         setIfNotNull(LdapIdentityProviderDefinition.LDAP_GROUPS_SEARCH_SUBTREE, definition.isGroupSearchSubTree(), properties);
         setIfNotNull(LdapIdentityProviderDefinition.LDAP_PROFILE_FILE, definition.getLdapProfileFile(), properties);
         setIfNotNull(LdapIdentityProviderDefinition.LDAP_SSL_SKIPVERIFICATION, definition.isSkipSSLVerification(), properties);
+        setIfNotNull("ldap.addShadowUserOnLogin",  definition.isAddShadowUserOnLogin(), properties);
 
         MapPropertySource source = new NestedMapPropertySource("ldap", properties);
         return new LdapIdentityProviderDefinition.LdapConfigEnvironment(source);
@@ -100,6 +101,10 @@ public static LdapIdentityProviderDefinition fromConfig(Map<String, Object> ldap
             definition.setAttributeMappings((Map<String, Object>) ldapConfig.get(LdapIdentityProviderDefinition.LDAP_ATTRIBUTE_MAPPINGS));
         }
 
+        if (ldapConfig.get("ldap.addShadowUserOnLogin") != null) {
+            definition.setAddShadowUserOnLogin((boolean) ldapConfig.get("ldap.addShadowUserOnLogin"));
+        }
+
         definition.setLdapProfileFile((String) ldapConfig.get(LdapIdentityProviderDefinition.LDAP_PROFILE_FILE));
 
         final String profileFile = definition.getLdapProfileFile();

diff --git a/...udfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java b/...udfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java
@@ -3,6 +3,9 @@
 import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
 import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
 import org.cloudfoundry.identity.uaa.authentication.event.UserAuthenticationSuccessEvent;
+import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
+import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
+import org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.provider.ldap.ExtendedLdapUserDetails;
 import org.cloudfoundry.identity.uaa.provider.ldap.extension.ExtendedLdapUserImpl;
 import org.cloudfoundry.identity.uaa.user.Mailable;
@@ -24,12 +27,10 @@
 
 import java.util.HashMap;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.not;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
@@ -275,6 +276,35 @@ public void testAuthenticateLdapUserDetailsPrincipal() throws Exception {
         assertEquals(userId, uaaAuthentication.getPrincipal().getId());
     }
 
+    @Test
+    public void testShadowUserCreationDisabled() throws Exception {
+        String dn = "cn="+userName+",ou=Users,dc=test,dc=com";
+        String origin = "ldap";
+        LdapUserDetails ldapUserDetails = mock(LdapUserDetails.class);
+        mockUserDetails(ldapUserDetails);
+        when(ldapUserDetails.getDn()).thenReturn(dn);
+        manager = new LdapLoginAuthenticationManager() {
+            @Override
+            protected boolean isAddNewShadowUser() {
+                return false;
+            }
+        };
+
+        setupManager();
+        manager.setOrigin(origin);
+        when(uaaUserDatabase.retrieveUserByName(eq(userName), eq(origin))).thenReturn(null);
+        when(inputAuth.getPrincipal()).thenReturn(ldapUserDetails);
+
+        try {
+            manager.authenticate(inputAuth);
+            fail("Expected authentication to fail with an exception.");
+        } catch (IllegalStateException ex) {
+            assertThat(ex.getMessage(), containsString("user account must be pre-created"));
+        }
+
+        verify(applicationEventPublisher, times(0)).publishEvent(any());
+    }
+
     @Test
     public void testAuthenticateCreateUserWithLdapUserDetailsPrincipal() throws Exception {
         String dn = "cn="+userName+",ou=Users,dc=test,dc=com";

diff --git a/...cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManagerTests.java b/...cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManagerTests.java
@@ -41,10 +41,7 @@
 
 import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.USER_ATTRIBUTE_PREFIX;
 import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
@@ -184,6 +181,12 @@ public void testUserAuthenticated() throws Exception {
         verify(publisher, times(2)).publishEvent(Matchers.<ApplicationEvent>anyObject());
     }
 
+    @Test
+    public void shadowUserCreationDisabledWillNotAddShadowUser() throws Exception {
+        definition.setAddShadowUserOnLogin(false);
+        assertFalse(am.isAddNewShadowUser());
+    }
+
     @Test
     public void update_existingUser_if_attributes_different() throws Exception {
         ExtendedLdapUserImpl authDetails = getAuthDetails(LDAP_EMAIL, "MarissaChanged", "BloggsChanged", "8675309");

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/login/BootstrapTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/login/BootstrapTests.java
@@ -578,6 +578,7 @@ public void testBootstrappedIdps_and_ExcludedClaims_and_CorsConfig() throws Exce
         IdentityProvider<LdapIdentityProviderDefinition> ldapProvider =
             providerProvisioning.retrieveByOrigin(OriginKeys.LDAP, IdentityZone.getUaa().getId());
         assertNotNull(ldapProvider);
+        assertFalse(ldapProvider.getConfig().isAddShadowUserOnLogin());
         assertEquals("Test LDAP Provider Description", ldapProvider.getConfig().getProviderDescription());
 
         IdentityProvider<SamlIdentityProviderDefinition> samlProvider = providerProvisioning.retrieveByOrigin("okta-local", IdentityZone.getUaa().getId());

diff --git a/...test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsDocs.java b/...test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsDocs.java
@@ -28,11 +28,8 @@
 import org.cloudfoundry.identity.uaa.provider.UaaIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.provider.ldap.DynamicPasswordComparator;
 import org.cloudfoundry.identity.uaa.provider.saml.BootstrapSamlIdentityProviderConfiguratorTests;
-import org.cloudfoundry.identity.uaa.scim.jdbc.JdbcScimGroupProvisioning;
-import org.cloudfoundry.identity.uaa.scim.jdbc.JdbcScimUserProvisioning;
 import org.cloudfoundry.identity.uaa.test.SnippetUtils;
 import org.cloudfoundry.identity.uaa.test.TestClient;
-import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
@@ -43,19 +40,14 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.springframework.core.io.ClassPathResource;
-import org.springframework.core.io.Resource;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.restdocs.payload.FieldDescriptor;
 import org.springframework.restdocs.snippet.Snippet;
 import org.springframework.security.ldap.server.ApacheDsSSLContainer;
 import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
-import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.ResultActions;
-import org.springframework.web.context.support.XmlWebApplicationContext;
 
-import java.io.File;
 import java.net.URL;
 
 import static org.cloudfoundry.identity.uaa.constants.OriginKeys.LDAP;
@@ -105,6 +97,7 @@ public class IdentityProviderEndpointsDocs extends InjectedMockContextTest {
     private static final String GIVEN_NAME_DESC = "Map `given_name` to the attribute for given name in the provider assertion.";
 
     private static final FieldDescriptor ATTRIBUTE_MAPPING = fieldWithPath("config.attributeMappings").optional(null).type(STRING).description("Map external attribute to UAA recognized mappings.");
+    private static final FieldDescriptor ADD_SHADOW_USER = fieldWithPath("config.addShadowUserOnLogin").optional(true).description("Whether users should be allowed to authenticate from LDAP without having a user pre-populated in the users database");
     private static final FieldDescriptor GIVEN_NAME = fieldWithPath("config.attributeMappings.given_name").optional(null).type(STRING).description(GIVEN_NAME_DESC);
     private static final FieldDescriptor FAMILY_NAME = fieldWithPath("config.attributeMappings.family_name").optional(null).type(STRING).description(FAMILY_NAME_DESC);
     private static final FieldDescriptor EMAIL = fieldWithPath("config.attributeMappings.email").optional(null).type(STRING).description("Map `email` to the attribute for email in the provider assertion.");
@@ -134,7 +127,8 @@ public class IdentityProviderEndpointsDocs extends InjectedMockContextTest {
         NAME,
         PROVIDER_DESC,
         EMAIL_DOMAIN,
-        ACTIVE
+        ACTIVE,
+        ADD_SHADOW_USER
     };
 
     private static ApacheDsSSLContainer apacheDS;

diff --git a/...ava/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProviderTests.java b/...ava/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProviderTests.java
@@ -60,6 +60,7 @@
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.security.core.userdetails.UsernameNotFoundException;
 import org.springframework.security.saml.SAMLAuthenticationToken;
 import org.springframework.security.saml.SAMLConstants;
 import org.springframework.security.saml.SAMLCredential;
@@ -510,6 +511,33 @@ public void shadowAccount_createdWith_MappedUserAttributes() throws Exception {
         assertEquals("1234567890", user.getPhoneNumber());
     }
 
+    @Test
+    public void shadowAccountNotCreated_givenShadowAccountCreationDisabled() throws Exception {
+        Map<String,Object> attributeMappings = new HashMap<>();
+        attributeMappings.put("given_name", "firstName");
+        attributeMappings.put("family_name", "lastName");
+        attributeMappings.put("email", "emailAddress");
+        attributeMappings.put("phone_number", "phone");
+        providerDefinition.setAttributeMappings(attributeMappings);
+        providerDefinition.setAddShadowUserOnLogin(false);
+        provider.setConfig(providerDefinition);
+        providerProvisioning.update(provider);
+
+        try {
+            getAuthentication();
+            fail("Expected authentication to throw LoginSAMLException");
+        } catch (LoginSAMLException ex) {
+
+        }
+
+        try {
+            userDatabase.retrieveUserByName("marissa-saml", OriginKeys.SAML);
+            fail("Expected user not to exist in database");
+        } catch(UsernameNotFoundException ex) {
+
+        }
+    }
+
     @Test
     public void should_NotCreateShadowAccount_AndInstead_UpdateExistingUserUsername_if_userWithEmailExists() throws Exception {
         Map<String,Object> attributeMappings = new HashMap<>();

diff --git a/uaa/src/test/resources/test/bootstrap/uaa.yml b/uaa/src/test/resources/test/bootstrap/uaa.yml
@@ -8,6 +8,7 @@ ldap:
     searchBase: ''
     searchFilter: 'cn={0}'
   providerDescription: 'Test LDAP Provider Description'
+  addShadowUserOnLogin: false
 jwt:
   token:
     claims: