Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow LDAP BindAuthenticator to skip attribute retrieval or retrieve using manager context #9

Closed
wants to merge 1 commit into from

2 participants

@alewando

This patch adds a couple of properties (retrieveUserAttributes and retrieveAttributesWithManagerContext) that provide more control over how BindAuthenticator retrieves user attributes. If retrieveUserAttributes is set to false no user attributes will be retrieved and if retrieveAttributesWithManagerContext is set to true, the attributes are retrieved with the manager context (contextSource property) as opposed to the context obtained from the user bind.
I have seen SEC-1510 , and humbly ask that the 'Won't Fix' resolution be reconsidered. I have run in to multiple LDAP server configurations that throw errors when users attempt to access their own attributes, rendering the BindAuthenticator unusable even if you aren't interested in those attribute values at all. The suggested workaround on the jira issue (reimplement AuthenticationProvider) seems heavy-handed, throwing away the baby with the bathwater. I can understand the desire to keep the complexity down, but the way this patch is implemented preserves the default functionality (get user attribs, using the user context). Only someone who wanted to would need to set the additional properties. I think this situation is common enough in security-conscious enterprises that BindAuthenticator should at least allow configuration to prevent it from retrieving user attributes at all.

@alewando alewando Add properties to BindAuthenticator allowing it to bypass user attrib…
…ute retrieval or retrieve the attributes using the manager context (instead of the user context). Existing behavior (retrieve attributes, using user context) is preserved as default.
dcd4aed
@rwinch
Owner

As @tekul mentioned on SEC-1510 adding additional complexity to the existing classes is not desirable. The reason is that LDAP has too many permutations to support all scenarios (especially in a single class). Instead, you are encouraged to extend AbstractLdapAuthenticator and implement this yourself. For this reason this will be closed without being merged.

@rwinch rwinch closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 8, 2012
  1. @alewando

    Add properties to BindAuthenticator allowing it to bypass user attrib…

    alewando authored
    …ute retrieval or retrieve the attributes using the manager context (instead of the user context). Existing behavior (retrieve attributes, using user context) is preserved as default.
This page is out of date. Refresh to see the latest.
View
16 ldap/src/integration-test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java
@@ -127,4 +127,20 @@ public void testUserDnPatternReturnsCorrectDn() {
authenticator.setUserDnPatterns(new String[] {"cn={0},ou=people"});
assertEquals("cn=Joe,ou=people", authenticator.getUserDns("Joe").get(0));
}
+
+ @Test
+ public void testRetrieveUserAttributes() {
+ authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people", "cn={0},ou=people"});
+ DirContextOperations user = authenticator.authenticate(new UsernamePasswordAuthenticationToken("mouse, jerry", "jerryspassword"));
+ assertEquals("Mouse", user.getStringAttribute("sn"));
+ }
+
+ @Test
+ public void testDoNotRetrieveUserAttributes() {
+ authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people", "cn={0},ou=people"});
+ authenticator.setRetrieveUserAttributes(false);
+ DirContextOperations user = authenticator.authenticate(new UsernamePasswordAuthenticationToken("mouse, jerry", "jerryspassword"));
+ assertNull(user.getStringAttribute("sn"));
+ }
+
}
View
58 ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java
@@ -36,7 +36,8 @@
/**
- * An authenticator which binds as a user.
+ * An authenticator which binds as a user, optionally retrieving user
+ * attributes.
*
* @author Luke Taylor
*
@@ -47,6 +48,10 @@
private static final Log logger = LogFactory.getLog(BindAuthenticator.class);
+ //~ Instance fields =====================================================================================
+ private boolean retrieveUserAttributes = true;
+ private boolean retrieveAttributesWithManagerContext = false;
+
//~ Constructors ===================================================================================================
/**
@@ -113,9 +118,7 @@ private DirContextOperations bindWithDn(String userDnStr, String username, Strin
// Check for password policy control
PasswordPolicyControl ppolicy = PasswordPolicyControlExtractor.extractControl(ctx);
- logger.debug("Retrieving attributes...");
-
- Attributes attrs = ctx.getAttributes(userDn, getUserAttributes());
+ Attributes attrs = retrieveUserAttribute(userDn, ctx);
DirContextAdapter result = new DirContextAdapter(attrs, userDn, ctxSource.getBaseLdapPath());
@@ -142,6 +145,25 @@ private DirContextOperations bindWithDn(String userDnStr, String username, Strin
return null;
}
+
+ /**
+ * Retrieves user attributes, using either the user's bind context or the manager context.
+ */
+ protected Attributes retrieveUserAttribute(DistinguishedName userDn,DirContext userContext) throws javax.naming.NamingException {
+ Attributes attrs = null;
+ if (retrieveUserAttributes) {
+ DirContext ctx = null;
+ if (retrieveAttributesWithManagerContext) {
+ logger.debug("Retrieving attributes using manager context...");
+ ctx = getContextSource().getReadOnlyContext();
+ } else {
+ logger.debug("Retrieving attributes using user context...");
+ ctx = userContext;
+ }
+ attrs = ctx.getAttributes(userDn, getUserAttributes());
+ }
+ return attrs;
+ }
/**
* Allows subclasses to inspect the exception thrown by an attempt to bind with a particular DN.
@@ -152,4 +174,32 @@ protected void handleBindException(String userDn, String username, Throwable cau
logger.debug("Failed to bind as " + userDn + ": " + cause);
}
}
+
+ public boolean isRetrieveUserAttributes() {
+ return retrieveUserAttributes;
+ }
+
+ /**
+ * If set to false, no user attributes will be retrieved after binding as
+ * the user. Default is true.
+ */
+ public void setRetrieveUserAttributes(boolean retrieveUserAttributes) {
+ this.retrieveUserAttributes = retrieveUserAttributes;
+ }
+
+ public boolean isRetrieveAttributesWithManagerContext() {
+ return retrieveAttributesWithManagerContext;
+ }
+
+ /**
+ * If set to true (the default), user attributes are retrieved using the
+ * {@link DirContextOperations} obtained when binding as the user. If set to
+ * false, the manager context (the {@link ContextSource} supplied as
+ * constructor argument) is used.
+ */
+ public void setRetrieveAttributesWithManagerContext(
+ boolean retrieveAttributesWithManagerContext) {
+ this.retrieveAttributesWithManagerContext = retrieveAttributesWithManagerContext;
+ }
+
}
Something went wrong with that request. Please try again.