From bf87f13299a5c5f6a6a845b181f0f9be948efd1d Mon Sep 17 00:00:00 2001 From: Filip Hanik Date: Mon, 6 Jun 2016 16:03:08 -0600 Subject: [PATCH] Avoid duplicate attribute search. When using search-and-bind strategy, the user attributes are already returned in the first search. If the user happens to not have privileges to perform a search, the second search may fail. (user only has bind privileges) See https://github.com/cloudfoundry/uaa/issues/342 --- .../BindAuthenticatorTests.java | 5 ++++- .../authentication/BindAuthenticator.java | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java index 224ced6f390..59202e09730 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java @@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; + /** * Tests for {@link BindAuthenticator}. * @@ -90,7 +91,9 @@ public void testAuthenticationWithUserSearch() throws Exception { this.authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(uid={0})", getContextSource())); this.authenticator.afterPropertiesSet(); - this.authenticator.authenticate(this.bob); + DirContextOperations result = this.authenticator.authenticate(this.bob); + //ensure we are getting the same attributes back + assertThat(result.getStringAttribute("cn")).isEqualTo("Bob Hamilton"); // SEC-1444 this.authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(cn={0})", getContextSource())); diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java index 935dd6d4edb..63653fa0c9f 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java @@ -16,9 +16,6 @@ package org.springframework.security.ldap.authentication; -import javax.naming.directory.Attributes; -import javax.naming.directory.DirContext; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.ldap.NamingException; @@ -35,6 +32,9 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import javax.naming.directory.Attributes; +import javax.naming.directory.DirContext; + /** * An authenticator which binds as a user. * @@ -93,7 +93,8 @@ public DirContextOperations authenticate(Authentication authentication) { // with the returned DN. if (user == null && getUserSearch() != null) { DirContextOperations userFromSearch = getUserSearch().searchForUser(username); - user = bindWithDn(userFromSearch.getDn().toString(), username, password); + user = bindWithDn(userFromSearch.getDn().toString(), username, password, + userFromSearch.getAttributes()); } if (user == null) { @@ -106,6 +107,11 @@ public DirContextOperations authenticate(Authentication authentication) { private DirContextOperations bindWithDn(String userDnStr, String username, String password) { + return bindWithDn(userDnStr, username, password, null); + } + + private DirContextOperations bindWithDn(String userDnStr, String username, + String password, Attributes attrs) { BaseLdapPathContextSource ctxSource = (BaseLdapPathContextSource) getContextSource(); DistinguishedName userDn = new DistinguishedName(userDnStr); DistinguishedName fullDn = new DistinguishedName(userDn); @@ -121,8 +127,9 @@ private DirContextOperations bindWithDn(String userDnStr, String username, .extractControl(ctx); logger.debug("Retrieving attributes..."); - - Attributes attrs = ctx.getAttributes(userDn, getUserAttributes()); + if (attrs == null || attrs.size()==0) { + attrs = ctx.getAttributes(userDn, getUserAttributes()); + } DirContextAdapter result = new DirContextAdapter(attrs, userDn, ctxSource.getBaseLdapPath());