From 554ef627fbe537887d4133f75339b88de66ca84a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 31 Jul 2020 12:05:12 -0700 Subject: [PATCH] Polish spring-security-ldap main code Manually polish `spring-security-ldap` following the formatting and checkstyle fixes. Issue gh-8945 --- .../ldap/DefaultLdapUsernameToDnMapper.java | 2 - .../DefaultSpringSecurityContextSource.java | 36 ++---- .../security/ldap/LdapEncoder.java | 96 ++++---------- .../security/ldap/LdapUtils.java | 27 +--- .../ldap/SpringSecurityLdapTemplate.java | 117 ++++++------------ .../AbstractLdapAuthenticationProvider.java | 18 +-- .../AbstractLdapAuthenticator.java | 4 - .../authentication/BindAuthenticator.java | 24 +--- .../LdapAuthenticationProvider.java | 14 +-- .../ldap/authentication/LdapEncoder.java | 96 ++++---------- .../PasswordComparisonAuthenticator.java | 14 +-- .../SpringSecurityAuthenticationSource.java | 17 +-- ...veDirectoryLdapAuthenticationProvider.java | 59 +++------ .../PasswordPolicyAwareContextSource.java | 38 ++---- .../ldap/ppolicy/PasswordPolicyControl.java | 4 +- .../PasswordPolicyControlExtractor.java | 2 - .../ppolicy/PasswordPolicyControlFactory.java | 1 - .../ppolicy/PasswordPolicyErrorStatus.java | 32 ++--- .../PasswordPolicyResponseControl.java | 84 ------------- .../search/FilterBasedLdapUserSearch.java | 23 +--- .../ldap/server/ApacheDSContainer.java | 92 ++++---------- .../ldap/server/UnboundIdContainer.java | 5 - .../DefaultLdapAuthoritiesPopulator.java | 32 +---- .../ldap/userdetails/InetOrgPerson.java | 2 +- .../InetOrgPersonContextMapper.java | 3 - .../ldap/userdetails/LdapAuthority.java | 32 ++--- .../ldap/userdetails/LdapUserDetailsImpl.java | 8 -- .../userdetails/LdapUserDetailsManager.java | 67 ++-------- .../userdetails/LdapUserDetailsMapper.java | 24 +--- .../userdetails/LdapUserDetailsService.java | 1 - .../NestedLdapAuthoritiesPopulator.java | 31 ++--- .../security/ldap/userdetails/Person.java | 9 +- .../ldap/userdetails/PersonContextMapper.java | 4 - 33 files changed, 241 insertions(+), 777 deletions(-) diff --git a/ldap/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java b/ldap/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java index 7cb9753c20..e4c0ac0e08 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java +++ b/ldap/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java @@ -47,9 +47,7 @@ public class DefaultLdapUsernameToDnMapper implements LdapUsernameToDnMapper { @Override public DistinguishedName buildDn(String username) { DistinguishedName dn = new DistinguishedName(this.userDnBase); - dn.add(this.usernameAttribute, username); - return dn; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java b/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java index 6307c3d92a..f9c6293712 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java +++ b/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java @@ -51,8 +51,6 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { protected final Log logger = LogFactory.getLog(getClass()); - private String rootDn; - /** * Create and initialize an instance which will connect to the supplied LDAP URL. If * you want to use more than one server for fail-over, rather use the @@ -62,44 +60,36 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { */ public DefaultSpringSecurityContextSource(String providerUrl) { Assert.hasLength(providerUrl, "An LDAP connection URL must be supplied."); - - StringTokenizer st = new StringTokenizer(providerUrl); - + StringTokenizer tokenizer = new StringTokenizer(providerUrl); ArrayList urls = new ArrayList<>(); - // Work out rootDn from the first URL and check that the other URLs (if any) match - while (st.hasMoreTokens()) { - String url = st.nextToken(); + String rootDn = null; + while (tokenizer.hasMoreTokens()) { + String url = tokenizer.nextToken(); String urlRootDn = LdapUtils.parseRootDnFromUrl(url); - urls.add(url.substring(0, url.lastIndexOf(urlRootDn))); - this.logger.info(" URL '" + url + "', root DN is '" + urlRootDn + "'"); - - if (this.rootDn == null) { - this.rootDn = urlRootDn; - } - else if (!this.rootDn.equals(urlRootDn)) { - throw new IllegalArgumentException("Root DNs must be the same when using multiple URLs"); - } + Assert.isTrue(rootDn == null || rootDn.equals(urlRootDn), + "Root DNs must be the same when using multiple URLs"); + rootDn = (rootDn != null) ? rootDn : urlRootDn; } - setUrls(urls.toArray(new String[0])); - setBase(this.rootDn); + setBase(rootDn); setPooled(true); setAuthenticationStrategy(new SimpleDirContextAuthenticationStrategy() { + @Override @SuppressWarnings("rawtypes") public void setupEnvironment(Hashtable env, String dn, String password) { super.setupEnvironment(env, dn, password); - // Remove the pooling flag unless we are authenticating as the 'manager' - // user. + // Remove the pooling flag unless authenticating as the 'manager' user. if (!DefaultSpringSecurityContextSource.this.userDn.equals(dn) && env.containsKey(SUN_LDAP_POOLING_FLAG)) { DefaultSpringSecurityContextSource.this.logger.debug("Removing pooling flag for user " + dn); env.remove(SUN_LDAP_POOLING_FLAG); } } + }); } @@ -146,16 +136,13 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { private static String buildProviderUrl(List urls, String baseDn) { Assert.notNull(baseDn, "The Base DN for the LDAP server must not be null."); Assert.notEmpty(urls, "At least one LDAP server URL must be provided."); - String trimmedBaseDn = baseDn.trim(); StringBuilder providerUrl = new StringBuilder(); - for (String serverUrl : urls) { String trimmedUrl = serverUrl.trim(); if ("".equals(trimmedUrl)) { continue; } - providerUrl.append(trimmedUrl); if (!trimmedUrl.endsWith("/")) { providerUrl.append("/"); @@ -163,7 +150,6 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { providerUrl.append(trimmedBaseDn); providerUrl.append(" "); } - return providerUrl.toString(); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/LdapEncoder.java b/ldap/src/main/java/org/springframework/security/ldap/LdapEncoder.java index 5727f668af..a3911aa180 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/LdapEncoder.java +++ b/ldap/src/main/java/org/springframework/security/ldap/LdapEncoder.java @@ -34,18 +34,11 @@ final class LdapEncoder { private static final int HEX = 16; private static String[] NAME_ESCAPE_TABLE = new String[96]; - - private static String[] FILTER_ESCAPE_TABLE = new String['\\' + 1]; - static { - - // Name encoding table ------------------------------------- - // all below 0x20 (control chars) for (char c = 0; c < ' '; c++) { NAME_ESCAPE_TABLE[c] = "\\" + toTwoCharHex(c); } - NAME_ESCAPE_TABLE['#'] = "\\#"; NAME_ESCAPE_TABLE[','] = "\\,"; NAME_ESCAPE_TABLE[';'] = "\\;"; @@ -55,21 +48,21 @@ final class LdapEncoder { NAME_ESCAPE_TABLE['>'] = "\\>"; NAME_ESCAPE_TABLE['\"'] = "\\\""; NAME_ESCAPE_TABLE['\\'] = "\\\\"; + } - // Filter encoding table ------------------------------------- + private static String[] FILTER_ESCAPE_TABLE = new String['\\' + 1]; + static { // fill with char itself for (char c = 0; c < FILTER_ESCAPE_TABLE.length; c++) { FILTER_ESCAPE_TABLE[c] = String.valueOf(c); } - // escapes (RFC2254) FILTER_ESCAPE_TABLE['*'] = "\\2a"; FILTER_ESCAPE_TABLE['('] = "\\28"; FILTER_ESCAPE_TABLE[')'] = "\\29"; FILTER_ESCAPE_TABLE['\\'] = "\\5c"; FILTER_ESCAPE_TABLE[0] = "\\00"; - } /** @@ -79,15 +72,8 @@ final class LdapEncoder { } protected static String toTwoCharHex(char c) { - String raw = Integer.toHexString(c).toUpperCase(); - - if (raw.length() > 1) { - return raw; - } - else { - return "0" + raw; - } + return (raw.length() > 1) ? raw : "0" + raw; } /** @@ -96,29 +82,15 @@ final class LdapEncoder { * @return a properly escaped representation of the supplied value. */ static String filterEncode(String value) { - if (value == null) { return null; } - - // make buffer roomy StringBuilder encodedValue = new StringBuilder(value.length() * 2); - int length = value.length(); - for (int i = 0; i < length; i++) { - - char c = value.charAt(i); - - if (c < FILTER_ESCAPE_TABLE.length) { - encodedValue.append(FILTER_ESCAPE_TABLE[c]); - } - else { - // default: add the char - encodedValue.append(c); - } + char ch = value.charAt(i); + encodedValue.append((ch < FILTER_ESCAPE_TABLE.length) ? FILTER_ESCAPE_TABLE[ch] : ch); } - return encodedValue.toString(); } @@ -141,43 +113,31 @@ final class LdapEncoder { * @return The escaped value. */ static String nameEncode(String value) { - if (value == null) { return null; } - - // make buffer roomy StringBuilder encodedValue = new StringBuilder(value.length() * 2); - int length = value.length(); int last = length - 1; - for (int i = 0; i < length; i++) { - char c = value.charAt(i); - // space first or last if (c == ' ' && (i == 0 || i == last)) { encodedValue.append("\\ "); continue; } - + // check in table for escapes if (c < NAME_ESCAPE_TABLE.length) { - // check in table for escapes String esc = NAME_ESCAPE_TABLE[c]; - if (esc != null) { encodedValue.append(esc); continue; } } - // default: add the char encodedValue.append(c); } - return encodedValue.toString(); - } /** @@ -188,43 +148,32 @@ final class LdapEncoder { * @throws BadLdapGrammarException */ static String nameDecode(String value) throws BadLdapGrammarException { - if (value == null) { return null; } - - // make buffer same size StringBuilder decoded = new StringBuilder(value.length()); - int i = 0; while (i < value.length()) { char currentChar = value.charAt(i); if (currentChar == '\\') { + // Ending with a single backslash is not allowed if (value.length() <= i + 1) { - // Ending with a single backslash is not allowed throw new BadLdapGrammarException("Unexpected end of value " + "unterminated '\\'"); } + char nextChar = value.charAt(i + 1); + if (isNormalBackslashEscape(nextChar)) { + decoded.append(nextChar); + i += 2; + } else { - char nextChar = value.charAt(i + 1); - if (nextChar == ',' || nextChar == '=' || nextChar == '+' || nextChar == '<' || nextChar == '>' - || nextChar == '#' || nextChar == ';' || nextChar == '\\' || nextChar == '\"' - || nextChar == ' ') { - // Normal backslash escape - decoded.append(nextChar); - i += 2; - } - else { - if (value.length() <= i + 2) { - throw new BadLdapGrammarException( - "Unexpected end of value " + "expected special or hex, found '" + nextChar + "'"); - } - else { - // This should be a hex value - String hexString = "" + nextChar + value.charAt(i + 2); - decoded.append((char) Integer.parseInt(hexString, HEX)); - i += 3; - } + if (value.length() <= i + 2) { + throw new BadLdapGrammarException( + "Unexpected end of value " + "expected special or hex, found '" + nextChar + "'"); } + // This should be a hex value + String hexString = "" + nextChar + value.charAt(i + 2); + decoded.append((char) Integer.parseInt(hexString, HEX)); + i += 3; } } else { @@ -238,4 +187,9 @@ final class LdapEncoder { } + private static boolean isNormalBackslashEscape(char nextChar) { + return nextChar == ',' || nextChar == '=' || nextChar == '+' || nextChar == '<' || nextChar == '>' + || nextChar == '#' || nextChar == ';' || nextChar == '\\' || nextChar == '\"' || nextChar == ' '; + } + } diff --git a/ldap/src/main/java/org/springframework/security/ldap/LdapUtils.java b/ldap/src/main/java/org/springframework/security/ldap/LdapUtils.java index 6bcb430b03..a222bf48a8 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/LdapUtils.java +++ b/ldap/src/main/java/org/springframework/security/ldap/LdapUtils.java @@ -47,7 +47,6 @@ public final class LdapUtils { if (ctx instanceof DirContextAdapter) { return; } - try { if (ctx != null) { ctx.close(); @@ -81,24 +80,17 @@ public final class LdapUtils { * @throws NamingException any exceptions thrown by the context are propagated. */ public static String getRelativeName(String fullDn, Context baseCtx) throws NamingException { - String baseDn = baseCtx.getNameInNamespace(); - if (baseDn.length() == 0) { return fullDn; } - DistinguishedName base = new DistinguishedName(baseDn); DistinguishedName full = new DistinguishedName(fullDn); - if (base.equals(full)) { return ""; } - Assert.isTrue(full.startsWith(base), "Full DN does not start with base DN"); - full.removeFirst(base); - return full.toString(); } @@ -108,28 +100,22 @@ public final class LdapUtils { */ public static DistinguishedName getFullDn(DistinguishedName dn, Context baseCtx) throws NamingException { DistinguishedName baseDn = new DistinguishedName(baseCtx.getNameInNamespace()); - if (dn.contains(baseDn)) { return dn; } - baseDn.append(dn); - return baseDn; } public static String convertPasswordToString(Object passObj) { Assert.notNull(passObj, "Password object to convert must not be null"); - if (passObj instanceof byte[]) { return Utf8.decode((byte[]) passObj); } - else if (passObj instanceof String) { + if (passObj instanceof String) { return (String) passObj; } - else { - throw new IllegalArgumentException("Password object was not a String or byte array."); - } + throw new IllegalArgumentException("Password object was not a String or byte array."); } /** @@ -143,9 +129,7 @@ public final class LdapUtils { */ public static String parseRootDnFromUrl(String url) { Assert.hasLength(url, "url must have length"); - String urlRootDn; - if (url.startsWith("ldap:") || url.startsWith("ldaps:")) { URI uri = parseLdapUrl(url); urlRootDn = uri.getRawPath(); @@ -154,11 +138,9 @@ public final class LdapUtils { // Assume it's an embedded server urlRootDn = url; } - if (urlRootDn.startsWith("/")) { urlRootDn = urlRootDn.substring(1); } - return urlRootDn; } @@ -173,14 +155,11 @@ public final class LdapUtils { private static URI parseLdapUrl(String url) { Assert.hasLength(url, "url must have length"); - try { return new URI(url); } catch (URISyntaxException ex) { - IllegalArgumentException iae = new IllegalArgumentException("Unable to parse url: " + url); - iae.initCause(ex); - throw iae; + throw new IllegalArgumentException("Unable to parse url: " + url, ex); } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java b/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java index ce0b69e61b..08c281b1e9 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java +++ b/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java @@ -37,6 +37,7 @@ import javax.naming.directory.SearchResult; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.ldap.core.ContextExecutor; import org.springframework.ldap.core.ContextMapper; @@ -46,6 +47,7 @@ import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DistinguishedName; import org.springframework.ldap.core.LdapTemplate; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * Extension of Spring LDAP's LdapTemplate class which adds extra functionality required @@ -76,7 +78,6 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { public SpringSecurityLdapTemplate(ContextSource contextSource) { Assert.notNull(contextSource, "ContextSource cannot be null"); setContextSource(contextSource); - this.searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE); } @@ -88,31 +89,18 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { * @param value the value to be checked against the directory value * @return true if the supplied value matches that in the directory */ - public boolean compare(final String dn, final String attributeName, final Object value) { - final String comparisonFilter = "(" + attributeName + "={0})"; - - class LdapCompareCallback implements ContextExecutor { - - @Override - public Object executeWithContext(DirContext ctx) throws NamingException { - SearchControls ctls = new SearchControls(); - ctls.setReturningAttributes(NO_ATTRS); - ctls.setSearchScope(SearchControls.OBJECT_SCOPE); - - NamingEnumeration results = ctx.search(dn, comparisonFilter, new Object[] { value }, - ctls); - - Boolean match = results.hasMore(); - LdapUtils.closeEnumeration(results); - - return match; - } - - } - - Boolean matches = (Boolean) executeReadOnly(new LdapCompareCallback()); - - return matches; + public boolean compare(String dn, String attributeName, Object value) { + String comparisonFilter = "(" + attributeName + "={0})"; + return executeReadOnly((ctx) -> { + SearchControls searchControls = new SearchControls(); + searchControls.setReturningAttributes(NO_ATTRS); + searchControls.setSearchScope(SearchControls.OBJECT_SCOPE); + Object[] params = new Object[] { value }; + NamingEnumeration results = ctx.search(dn, comparisonFilter, params, searchControls); + Boolean match = results.hasMore(); + LdapUtils.closeEnumeration(results); + return match; + }); } /** @@ -123,12 +111,8 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { * @return the object created by the mapper */ public DirContextOperations retrieveEntry(final String dn, final String[] attributesToRetrieve) { - return (DirContextOperations) executeReadOnly((ContextExecutor) (ctx) -> { Attributes attrs = ctx.getAttributes(dn, attributesToRetrieve); - - // Object object = ctx.lookup(LdapUtils.getRelativeName(dn, ctx)); - return new DirContextAdapter(attrs, new DistinguishedName(dn), new DistinguishedName(ctx.getNameInNamespace())); }); @@ -174,27 +158,23 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { * entries. The attribute name is the key for each set of values. In addition each map * contains the DN as a String with the key predefined key {@link #DN_KEY}. */ - public Set>> searchForMultipleAttributeValues(final String base, final String filter, - final Object[] params, final String[] attributeNames) { + public Set>> searchForMultipleAttributeValues(String base, String filter, Object[] params, + String[] attributeNames) { // Escape the params acording to RFC2254 Object[] encodedParams = new String[params.length]; - for (int i = 0; i < params.length; i++) { encodedParams[i] = LdapEncoder.filterEncode(params[i].toString()); } - String formattedFilter = MessageFormat.format(filter, encodedParams); - logger.debug("Using filter: " + formattedFilter); - - final HashSet>> set = new HashSet<>(); - + logger.debug(LogMessage.format("Using filter: %s", formattedFilter)); + HashSet>> result = new HashSet<>(); ContextMapper roleMapper = (ctx) -> { DirContextAdapter adapter = (DirContextAdapter) ctx; Map> record = new HashMap<>(); - if (attributeNames == null || attributeNames.length == 0) { + if (ObjectUtils.isEmpty(attributeNames)) { try { - for (NamingEnumeration ae = adapter.getAttributes().getAll(); ae.hasMore();) { - Attribute attr = (Attribute) ae.next(); + for (NamingEnumeration enumeration = adapter.getAttributes().getAll(); enumeration.hasMore();) { + Attribute attr = (Attribute) enumeration.next(); extractStringAttributeValues(adapter, record, attr.getID()); } } @@ -208,17 +188,14 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { } } record.put(DN_KEY, Arrays.asList(getAdapterDN(adapter))); - set.add(record); + result.add(record); return null; }; - SearchControls ctls = new SearchControls(); ctls.setSearchScope(this.searchControls.getSearchScope()); ctls.setReturningAttributes((attributeNames != null && attributeNames.length > 0) ? attributeNames : null); - search(base, formattedFilter, ctls, roleMapper); - - return set; + return result; } /** @@ -246,27 +223,23 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { String attributeName) { Object[] values = adapter.getObjectAttributes(attributeName); if (values == null || values.length == 0) { - if (logger.isDebugEnabled()) { - logger.debug("No attribute value found for '" + attributeName + "'"); - } + logger.debug(LogMessage.format("No attribute value found for '%s'", attributeName)); return; } - List svalues = new ArrayList<>(); - for (Object o : values) { - if (o != null) { - if (String.class.isAssignableFrom(o.getClass())) { - svalues.add((String) o); + List stringValues = new ArrayList<>(); + for (Object value : values) { + if (value != null) { + if (String.class.isAssignableFrom(value.getClass())) { + stringValues.add((String) value); } else { - if (logger.isDebugEnabled()) { - logger.debug("Attribute:" + attributeName + " contains a non string value of type[" - + o.getClass() + "]"); - } - svalues.add(o.toString()); + logger.debug(LogMessage.format("Attribute:%s contains a non string value of type[%s]", + attributeName, value.getClass())); + stringValues.add(value.toString()); } } } - record.put(attributeName, svalues); + record.put(attributeName, stringValues); } /** @@ -283,8 +256,7 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { * @throws IncorrectResultSizeDataAccessException if no results are found or the * search returns more than one result. */ - public DirContextOperations searchForSingleEntry(final String base, final String filter, final Object[] params) { - + public DirContextOperations searchForSingleEntry(String base, String filter, Object[] params) { return (DirContextOperations) executeReadOnly((ContextExecutor) (ctx) -> searchForSingleEntryInternal(ctx, this.searchControls, base, filter, params)); } @@ -298,22 +270,15 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { final DistinguishedName searchBaseDn = new DistinguishedName(base); final NamingEnumeration resultsEnum = ctx.search(searchBaseDn, filter, params, buildControls(searchControls)); - - if (logger.isDebugEnabled()) { - logger.debug("Searching for entry under DN '" + ctxBaseDn + "', base = '" + searchBaseDn + "', filter = '" - + filter + "'"); - } - + logger.debug(LogMessage.format("Searching for entry under DN '%s', base = '%s', filter = '%s'", ctxBaseDn, + searchBaseDn, filter)); Set results = new HashSet<>(); try { while (resultsEnum.hasMore()) { SearchResult searchResult = resultsEnum.next(); DirContextAdapter dca = (DirContextAdapter) searchResult.getObject(); Assert.notNull(dca, "No object returned by search, DirContext is not correctly configured"); - - if (logger.isDebugEnabled()) { - logger.debug("Found DN: " + dca.getDn()); - } + logger.debug(LogMessage.format("Found DN: %s", dca.getDn())); results.add(dca); } } @@ -321,15 +286,9 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { LdapUtils.closeEnumeration(resultsEnum); logger.info("Ignoring PartialResultException"); } - - if (results.size() == 0) { - throw new IncorrectResultSizeDataAccessException(1, 0); - } - - if (results.size() > 1) { + if (results.size() != 1) { throw new IncorrectResultSizeDataAccessException(1, results.size()); } - return results.iterator().next(); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticationProvider.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticationProvider.java index c8cc25f645..69dd517de6 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticationProvider.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticationProvider.java @@ -24,6 +24,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.context.MessageSource; import org.springframework.context.MessageSourceAware; import org.springframework.context.support.MessageSourceAccessor; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.core.DirContextOperations; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.authentication.BadCredentialsException; @@ -64,33 +65,22 @@ public abstract class AbstractLdapAuthenticationProvider implements Authenticati Assert.isInstanceOf(UsernamePasswordAuthenticationToken.class, authentication, () -> this.messages.getMessage("LdapAuthenticationProvider.onlySupports", "Only UsernamePasswordAuthenticationToken is supported")); - - final UsernamePasswordAuthenticationToken userToken = (UsernamePasswordAuthenticationToken) authentication; - + UsernamePasswordAuthenticationToken userToken = (UsernamePasswordAuthenticationToken) authentication; String username = userToken.getName(); String password = (String) authentication.getCredentials(); - - if (this.logger.isDebugEnabled()) { - this.logger.debug("Processing authentication request for user: " + username); - } - + this.logger.debug(LogMessage.format("Processing authentication request for user: %s", username)); if (!StringUtils.hasLength(username)) { throw new BadCredentialsException( this.messages.getMessage("LdapAuthenticationProvider.emptyUsername", "Empty Username")); } - if (!StringUtils.hasLength(password)) { throw new BadCredentialsException( this.messages.getMessage("AbstractLdapAuthenticationProvider.emptyPassword", "Empty Password")); } - Assert.notNull(password, "Null password was supplied in authentication token"); - DirContextOperations userData = doAuthentication(userToken); - UserDetails user = this.userDetailsContextMapper.mapUserFromContext(userData, authentication.getName(), loadUserAuthorities(userData, authentication.getName(), (String) authentication.getCredentials())); - return createSuccessfulAuthentication(userToken, user); } @@ -111,11 +101,9 @@ public abstract class AbstractLdapAuthenticationProvider implements Authenticati UserDetails user) { Object password = this.useAuthenticationRequestCredentials ? authentication.getCredentials() : user.getPassword(); - UsernamePasswordAuthenticationToken result = new UsernamePasswordAuthenticationToken(user, password, this.authoritiesMapper.mapAuthorities(user.getAuthorities())); result.setDetails(authentication.getDetails()); - return result; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticator.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticator.java index e3f5148c8b..346aa1ca4d 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticator.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticator.java @@ -91,16 +91,13 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator, In if (this.userDnFormat == null) { return Collections.emptyList(); } - List userDns = new ArrayList<>(this.userDnFormat.length); String[] args = new String[] { LdapEncoder.nameEncode(username) }; - synchronized (this.userDnFormat) { for (MessageFormat formatter : this.userDnFormat) { userDns.add(formatter.format(args)); } } - return userDns; } @@ -134,7 +131,6 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator, In Assert.notNull(dnPattern, "The array of DN patterns cannot be set to null"); // this.userDnPattern = dnPattern; this.userDnFormat = new MessageFormat[dnPattern.length]; - for (int i = 0; i < dnPattern.length; i++) { this.userDnFormat[i] = new MessageFormat(dnPattern[i]); } 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 ccac1f3822..1c4fa66eff 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 @@ -22,6 +22,7 @@ import javax.naming.directory.DirContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.NamingException; import org.springframework.ldap.core.DirContextAdapter; import org.springframework.ldap.core.DirContextOperations; @@ -51,7 +52,6 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { * provided. * @param contextSource the BaseLdapPathContextSource instance against which bind * operations will be performed. - * */ public BindAuthenticator(BaseLdapPathContextSource contextSource) { super(contextSource); @@ -62,37 +62,30 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { DirContextOperations user = null; Assert.isInstanceOf(UsernamePasswordAuthenticationToken.class, authentication, "Can only process UsernamePasswordAuthenticationToken objects"); - String username = authentication.getName(); String password = (String) authentication.getCredentials(); - if (!StringUtils.hasLength(password)) { - logger.debug("Rejecting empty password for user " + username); + logger.debug(LogMessage.format("Rejecting empty password for user %s", username)); throw new BadCredentialsException( this.messages.getMessage("BindAuthenticator.emptyPassword", "Empty Password")); } - // If DN patterns are configured, try authenticating with them directly for (String dn : getUserDns(username)) { user = bindWithDn(dn, username, password); - if (user != null) { break; } } - // Otherwise use the configured search object to find the user and authenticate // with the returned DN. if (user == null && getUserSearch() != null) { DirContextOperations userFromSearch = getUserSearch().searchForUser(username); user = bindWithDn(userFromSearch.getDn().toString(), username, password, userFromSearch.getAttributes()); } - if (user == null) { throw new BadCredentialsException( this.messages.getMessage("BindAuthenticator.badCredentials", "Bad credentials")); } - return user; } @@ -105,26 +98,20 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { DistinguishedName userDn = new DistinguishedName(userDnStr); DistinguishedName fullDn = new DistinguishedName(userDn); fullDn.prepend(ctxSource.getBaseLdapPath()); - - logger.debug("Attempting to bind as " + fullDn); - + logger.debug(LogMessage.format("Attempting to bind as %s", fullDn)); DirContext ctx = null; try { ctx = getContextSource().getContext(fullDn.toString(), password); // Check for password policy control PasswordPolicyControl ppolicy = PasswordPolicyControlExtractor.extractControl(ctx); - logger.debug("Retrieving attributes..."); if (attrs == null || attrs.size() == 0) { attrs = ctx.getAttributes(userDn, getUserAttributes()); } - DirContextAdapter result = new DirContextAdapter(attrs, userDn, ctxSource.getBaseLdapPath()); - if (ppolicy != null) { result.setAttributeValue(ppolicy.getID(), ppolicy); } - return result; } catch (NamingException ex) { @@ -145,7 +132,6 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { finally { LdapUtils.closeContext(ctx); } - return null; } @@ -155,9 +141,7 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { * logger. */ protected void handleBindException(String userDn, String username, Throwable cause) { - if (logger.isDebugEnabled()) { - logger.debug("Failed to bind as " + userDn + ": " + cause); - } + logger.debug(LogMessage.format("Failed to bind as %s: %s", userDn, cause)); } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java index 6cf5d300b2..9b4573a0db 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java @@ -173,23 +173,21 @@ public class LdapAuthenticationProvider extends AbstractLdapAuthenticationProvid try { return getAuthenticator().authenticate(authentication); } - catch (PasswordPolicyException ppe) { + catch (PasswordPolicyException ex) { // The only reason a ppolicy exception can occur during a bind is that the // account is locked. throw new LockedException( - this.messages.getMessage(ppe.getStatus().getErrorCode(), ppe.getStatus().getDefaultMessage())); + this.messages.getMessage(ex.getStatus().getErrorCode(), ex.getStatus().getDefaultMessage())); } - catch (UsernameNotFoundException notFound) { + catch (UsernameNotFoundException ex) { if (this.hideUserNotFoundExceptions) { throw new BadCredentialsException( this.messages.getMessage("LdapAuthenticationProvider.badCredentials", "Bad credentials")); } - else { - throw notFound; - } + throw ex; } - catch (NamingException ldapAccessFailure) { - throw new InternalAuthenticationServiceException(ldapAccessFailure.getMessage(), ldapAccessFailure); + catch (NamingException ex) { + throw new InternalAuthenticationServiceException(ex.getMessage(), ex); } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapEncoder.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapEncoder.java index ae6d6c67c7..f79f4843ae 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapEncoder.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapEncoder.java @@ -34,18 +34,11 @@ final class LdapEncoder { private static final int HEX = 16; private static String[] NAME_ESCAPE_TABLE = new String[96]; - - private static String[] FILTER_ESCAPE_TABLE = new String['\\' + 1]; - static { - - // Name encoding table ------------------------------------- - // all below 0x20 (control chars) for (char c = 0; c < ' '; c++) { NAME_ESCAPE_TABLE[c] = "\\" + toTwoCharHex(c); } - NAME_ESCAPE_TABLE['#'] = "\\#"; NAME_ESCAPE_TABLE[','] = "\\,"; NAME_ESCAPE_TABLE[';'] = "\\;"; @@ -55,21 +48,21 @@ final class LdapEncoder { NAME_ESCAPE_TABLE['>'] = "\\>"; NAME_ESCAPE_TABLE['\"'] = "\\\""; NAME_ESCAPE_TABLE['\\'] = "\\\\"; + } - // Filter encoding table ------------------------------------- + private static String[] FILTER_ESCAPE_TABLE = new String['\\' + 1]; + static { // fill with char itself for (char c = 0; c < FILTER_ESCAPE_TABLE.length; c++) { FILTER_ESCAPE_TABLE[c] = String.valueOf(c); } - // escapes (RFC2254) FILTER_ESCAPE_TABLE['*'] = "\\2a"; FILTER_ESCAPE_TABLE['('] = "\\28"; FILTER_ESCAPE_TABLE[')'] = "\\29"; FILTER_ESCAPE_TABLE['\\'] = "\\5c"; FILTER_ESCAPE_TABLE[0] = "\\00"; - } /** @@ -79,15 +72,8 @@ final class LdapEncoder { } protected static String toTwoCharHex(char c) { - String raw = Integer.toHexString(c).toUpperCase(); - - if (raw.length() > 1) { - return raw; - } - else { - return "0" + raw; - } + return (raw.length() > 1) ? raw : "0" + raw; } /** @@ -96,29 +82,15 @@ final class LdapEncoder { * @return a properly escaped representation of the supplied value. */ static String filterEncode(String value) { - if (value == null) { return null; } - - // make buffer roomy StringBuilder encodedValue = new StringBuilder(value.length() * 2); - int length = value.length(); - for (int i = 0; i < length; i++) { - - char c = value.charAt(i); - - if (c < FILTER_ESCAPE_TABLE.length) { - encodedValue.append(FILTER_ESCAPE_TABLE[c]); - } - else { - // default: add the char - encodedValue.append(c); - } + char ch = value.charAt(i); + encodedValue.append((ch < FILTER_ESCAPE_TABLE.length) ? FILTER_ESCAPE_TABLE[ch] : ch); } - return encodedValue.toString(); } @@ -141,43 +113,31 @@ final class LdapEncoder { * @return The escaped value. */ static String nameEncode(String value) { - if (value == null) { return null; } - - // make buffer roomy StringBuilder encodedValue = new StringBuilder(value.length() * 2); - int length = value.length(); int last = length - 1; - for (int i = 0; i < length; i++) { - char c = value.charAt(i); - // space first or last if (c == ' ' && (i == 0 || i == last)) { encodedValue.append("\\ "); continue; } - + // check in table for escapes if (c < NAME_ESCAPE_TABLE.length) { - // check in table for escapes String esc = NAME_ESCAPE_TABLE[c]; - if (esc != null) { encodedValue.append(esc); continue; } } - // default: add the char encodedValue.append(c); } - return encodedValue.toString(); - } /** @@ -188,43 +148,32 @@ final class LdapEncoder { * @throws BadLdapGrammarException */ static String nameDecode(String value) throws BadLdapGrammarException { - if (value == null) { return null; } - - // make buffer same size StringBuilder decoded = new StringBuilder(value.length()); - int i = 0; while (i < value.length()) { char currentChar = value.charAt(i); if (currentChar == '\\') { + // Ending with a single backslash is not allowed if (value.length() <= i + 1) { - // Ending with a single backslash is not allowed throw new BadLdapGrammarException("Unexpected end of value " + "unterminated '\\'"); } + char nextChar = value.charAt(i + 1); + if (isNormalBackslashEscape(nextChar)) { + decoded.append(nextChar); + i += 2; + } else { - char nextChar = value.charAt(i + 1); - if (nextChar == ',' || nextChar == '=' || nextChar == '+' || nextChar == '<' || nextChar == '>' - || nextChar == '#' || nextChar == ';' || nextChar == '\\' || nextChar == '\"' - || nextChar == ' ') { - // Normal backslash escape - decoded.append(nextChar); - i += 2; - } - else { - if (value.length() <= i + 2) { - throw new BadLdapGrammarException( - "Unexpected end of value " + "expected special or hex, found '" + nextChar + "'"); - } - else { - // This should be a hex value - String hexString = "" + nextChar + value.charAt(i + 2); - decoded.append((char) Integer.parseInt(hexString, HEX)); - i += 3; - } + if (value.length() <= i + 2) { + throw new BadLdapGrammarException( + "Unexpected end of value " + "expected special or hex, found '" + nextChar + "'"); } + // This should be a hex value + String hexString = "" + nextChar + value.charAt(i + 2); + decoded.append((char) Integer.parseInt(hexString, HEX)); + i += 3; } } else { @@ -238,4 +187,9 @@ final class LdapEncoder { } + private static boolean isNormalBackslashEscape(char nextChar) { + return nextChar == ',' || nextChar == '=' || nextChar == '+' || nextChar == '<' || nextChar == '>' + || nextChar == '#' || nextChar == ';' || nextChar == '\\' || nextChar == '\"' || nextChar == ' '; + } + } diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java index 790b513f7e..a64b8b0368 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java @@ -19,6 +19,7 @@ package org.springframework.security.ldap.authentication; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.NameNotFoundException; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.support.BaseLdapPathContextSource; @@ -66,13 +67,10 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic Assert.isInstanceOf(UsernamePasswordAuthenticationToken.class, authentication, "Can only process UsernamePasswordAuthenticationToken objects"); // locate the user and check the password - DirContextOperations user = null; String username = authentication.getName(); String password = (String) authentication.getCredentials(); - SpringSecurityLdapTemplate ldapTemplate = new SpringSecurityLdapTemplate(getContextSource()); - for (String userDn : getUserDns(username)) { try { user = ldapTemplate.retrieveEntry(userDn, getUserAttributes()); @@ -83,24 +81,20 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic break; } } - if (user == null && getUserSearch() != null) { user = getUserSearch().searchForUser(username); } - if (user == null) { throw new UsernameNotFoundException("User not found: " + username); } - if (logger.isDebugEnabled()) { - logger.debug("Performing LDAP compare of password attribute '" + this.passwordAttributeName + "' for user '" - + user.getDn() + "'"); + logger.debug(LogMessage.format("Performing LDAP compare of password attribute '%s' for user '%s'", + this.passwordAttributeName, user.getDn())); } - if (this.usePasswordAttrCompare && isPasswordAttrCompare(user, password)) { return user; } - else if (isLdapPasswordCompare(user, ldapTemplate, password)) { + if (isLdapPasswordCompare(user, ldapTemplate, password)) { return user; } throw new BadCredentialsException( diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/SpringSecurityAuthenticationSource.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/SpringSecurityAuthenticationSource.java index abafcb7e8b..14584623fc 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/SpringSecurityAuthenticationSource.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/SpringSecurityAuthenticationSource.java @@ -47,28 +47,21 @@ public class SpringSecurityAuthenticationSource implements AuthenticationSource @Override public String getPrincipal() { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { log.warn("No Authentication object set in SecurityContext - returning empty String as Principal"); return ""; } - Object principal = authentication.getPrincipal(); - if (principal instanceof LdapUserDetails) { LdapUserDetails details = (LdapUserDetails) principal; return details.getDn(); } - else if (authentication instanceof AnonymousAuthenticationToken) { - if (log.isDebugEnabled()) { - log.debug("Anonymous Authentication, returning empty String as Principal"); - } + if (authentication instanceof AnonymousAuthenticationToken) { + log.debug("Anonymous Authentication, returning empty String as Principal"); return ""; } - else { - throw new IllegalArgumentException( - "The principal property of the authentication object" + "needs to be an LdapUserDetails."); - } + throw new IllegalArgumentException( + "The principal property of the authentication object" + "needs to be an LdapUserDetails."); } /** @@ -77,12 +70,10 @@ public class SpringSecurityAuthenticationSource implements AuthenticationSource @Override public String getCredentials() { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { log.warn("No Authentication object set in SecurityContext - returning empty String as Credentials"); return ""; } - return (String) authentication.getCredentials(); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java index 40b82b5d01..381b3c3179 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Hashtable; +import java.util.List; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -34,6 +35,7 @@ import javax.naming.directory.DirContext; import javax.naming.directory.SearchControls; import javax.naming.ldap.InitialLdapContext; +import org.springframework.core.log.LogMessage; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.ldap.CommunicationException; import org.springframework.ldap.core.DirContextOperations; @@ -161,7 +163,6 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda String username = auth.getName(); String password = (String) auth.getCredentials(); DirContext ctx = null; - try { ctx = bindAsUser(username, password); return searchForUser(ctx, username); @@ -186,30 +187,23 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda protected Collection loadUserAuthorities(DirContextOperations userData, String username, String password) { String[] groups = userData.getStringAttributes("memberOf"); - if (groups == null) { this.logger.debug("No values for 'memberOf' attribute."); - return AuthorityUtils.NO_AUTHORITIES; } - if (this.logger.isDebugEnabled()) { this.logger.debug("'memberOf' attribute values: " + Arrays.asList(groups)); } - - ArrayList authorities = new ArrayList<>(groups.length); - + List authorities = new ArrayList<>(groups.length); for (String group : groups) { authorities.add(new SimpleGrantedAuthority(new DistinguishedName(group).removeLast().getValue())); } - return authorities; } private DirContext bindAsUser(String username, String password) { // TODO. add DNS lookup based on domain final String bindUrl = this.url; - Hashtable env = new Hashtable<>(); env.put(Context.SECURITY_AUTHENTICATION, "simple"); String bindPrincipal = createBindPrincipal(username); @@ -219,7 +213,6 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); env.put(Context.OBJECT_FACTORIES, DefaultDirObjectFactory.class.getName()); env.putAll(this.contextEnvironmentProperties); - try { return this.contextFactory.createContext(env); } @@ -228,28 +221,20 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda handleBindException(bindPrincipal, ex); throw badCredentials(ex); } - else { - throw LdapUtils.convertLdapException(ex); - } + throw LdapUtils.convertLdapException(ex); } } private void handleBindException(String bindPrincipal, NamingException exception) { - if (this.logger.isDebugEnabled()) { - this.logger.debug("Authentication for " + bindPrincipal + " failed:" + exception); - } - + this.logger.debug(LogMessage.format("Authentication for %s failed:%s", bindPrincipal, exception)); handleResolveObj(exception); - int subErrorCode = parseSubErrorCode(exception.getMessage()); - if (subErrorCode <= 0) { this.logger.debug("Failed to locate AD-specific sub-error code in message"); return; } - - this.logger.info("Active Directory authentication failed: " + subCodeToLogMessage(subErrorCode)); - + this.logger.info( + LogMessage.of(() -> "Active Directory authentication failed: " + subCodeToLogMessage(subErrorCode))); if (this.convertSubErrorCodesToExceptions) { raiseExceptionForErrorCode(subErrorCode, exception); } @@ -264,12 +249,10 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda } private int parseSubErrorCode(String message) { - Matcher m = SUB_ERROR_CODE.matcher(message); - - if (m.matches()) { - return Integer.parseInt(m.group(1), 16); + Matcher matcher = SUB_ERROR_CODE.matcher(message); + if (matcher.matches()) { + return Integer.parseInt(matcher.group(1), 16); } - return -1; } @@ -313,7 +296,6 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda case ACCOUNT_LOCKED: return "Account locked"; } - return "Unknown (error code " + Integer.toHexString(code) + ")"; } @@ -334,7 +316,6 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda private DirContextOperations searchForUser(DirContext context, String username) throws NamingException { SearchControls searchControls = new SearchControls(); searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE); - String bindPrincipal = createBindPrincipal(username); String searchRoot = (this.rootDn != null) ? this.rootDn : searchRootFromPrincipal(bindPrincipal); @@ -342,45 +323,40 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context, searchControls, searchRoot, this.searchFilter, new Object[] { bindPrincipal, username }); } - catch (CommunicationException ldapCommunicationException) { - throw badLdapConnection(ldapCommunicationException); + catch (CommunicationException ex) { + throw badLdapConnection(ex); } - catch (IncorrectResultSizeDataAccessException incorrectResults) { - // Search should never return multiple results if properly configured - just - // rethrow - if (incorrectResults.getActualSize() != 0) { - throw incorrectResults; + catch (IncorrectResultSizeDataAccessException ex) { + // Search should never return multiple results if properly configured - + if (ex.getActualSize() != 0) { + throw ex; } // If we found no results, then the username/password did not match UsernameNotFoundException userNameNotFoundException = new UsernameNotFoundException( - "User " + username + " not found in directory.", incorrectResults); + "User " + username + " not found in directory.", ex); throw badCredentials(userNameNotFoundException); } } private String searchRootFromPrincipal(String bindPrincipal) { int atChar = bindPrincipal.lastIndexOf('@'); - if (atChar < 0) { this.logger.debug("User principal '" + bindPrincipal + "' does not contain the domain, and no domain has been configured"); throw badCredentials(); } - return rootDnFromDomain(bindPrincipal.substring(atChar + 1, bindPrincipal.length())); } private String rootDnFromDomain(String domain) { String[] tokens = StringUtils.tokenizeToStringArray(domain, "."); StringBuilder root = new StringBuilder(); - for (String token : tokens) { if (root.length() > 0) { root.append(','); } root.append("dc=").append(token); } - return root.toString(); } @@ -388,7 +364,6 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda if (this.domain == null || username.toLowerCase().endsWith(this.domain)) { return username; } - return username + "@" + this.domain; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyAwareContextSource.java b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyAwareContextSource.java index c364899d77..6fb79ffd4f 100755 --- a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyAwareContextSource.java +++ b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyAwareContextSource.java @@ -23,6 +23,7 @@ import javax.naming.directory.DirContext; import javax.naming.ldap.Control; import javax.naming.ldap.LdapContext; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.support.LdapUtils; import org.springframework.security.ldap.DefaultSpringSecurityContextSource; @@ -49,45 +50,30 @@ public class PasswordPolicyAwareContextSource extends DefaultSpringSecurityConte if (principal.equals(this.userDn)) { return super.getContext(principal, credentials); } - - final boolean debug = this.logger.isDebugEnabled(); - - if (debug) { - this.logger.debug("Binding as '" + this.userDn + "', prior to reconnect as user '" + principal + "'"); - } - + this.logger + .debug(LogMessage.format("Binding as '%s', prior to reconnect as user '%s'", this.userDn, principal)); // First bind as manager user before rebinding as the specific principal. LdapContext ctx = (LdapContext) super.getContext(this.userDn, this.password); - Control[] rctls = { new PasswordPolicyControl(false) }; - try { ctx.addToEnvironment(Context.SECURITY_PRINCIPAL, principal); ctx.addToEnvironment(Context.SECURITY_CREDENTIALS, credentials); ctx.reconnect(rctls); } - catch (javax.naming.NamingException ne) { + catch (javax.naming.NamingException ex) { PasswordPolicyResponseControl ctrl = PasswordPolicyControlExtractor.extractControl(ctx); - if (debug) { - this.logger.debug("Failed to obtain context", ne); + if (this.logger.isDebugEnabled()) { + this.logger.debug("Failed to obtain context", ex); this.logger.debug("Password policy response: " + ctrl); } - LdapUtils.closeContext(ctx); - - if (ctrl != null) { - if (ctrl.isLocked()) { - throw new PasswordPolicyException(ctrl.getErrorStatus()); - } + if (ctrl != null && ctrl.isLocked()) { + throw new PasswordPolicyException(ctrl.getErrorStatus()); } - - throw LdapUtils.convertLdapException(ne); + throw LdapUtils.convertLdapException(ex); } - - if (debug) { - this.logger.debug("PPolicy control returned: " + PasswordPolicyControlExtractor.extractControl(ctx)); - } - + this.logger.debug( + LogMessage.of(() -> "PPolicy control returned: " + PasswordPolicyControlExtractor.extractControl(ctx))); return ctx; } @@ -95,9 +81,7 @@ public class PasswordPolicyAwareContextSource extends DefaultSpringSecurityConte @SuppressWarnings("unchecked") protected Hashtable getAuthenticatedEnv(String principal, String credentials) { Hashtable env = super.getAuthenticatedEnv(principal, credentials); - env.put(LdapContext.CONTROL_FACTORIES, PasswordPolicyControlFactory.class.getName()); - return env; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControl.java b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControl.java index 36b5832c94..84eb48cdf9 100755 --- a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControl.java +++ b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControl.java @@ -32,7 +32,9 @@ import javax.naming.ldap.Control; */ public class PasswordPolicyControl implements Control { - /** OID of the Password Policy Control */ + /** + * OID of the Password Policy Control + */ public static final String OID = "1.3.6.1.4.1.42.2.27.8.5.1"; private final boolean critical; diff --git a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlExtractor.java b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlExtractor.java index 6c6341bfd5..79f007e408 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlExtractor.java +++ b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlExtractor.java @@ -45,13 +45,11 @@ public final class PasswordPolicyControlExtractor { catch (javax.naming.NamingException ex) { logger.error("Failed to obtain response controls", ex); } - for (int i = 0; ctrls != null && i < ctrls.length; i++) { if (ctrls[i] instanceof PasswordPolicyResponseControl) { return (PasswordPolicyResponseControl) ctrls[i]; } } - return null; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlFactory.java b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlFactory.java index 84780ae9f5..0bb3e274a2 100755 --- a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlFactory.java +++ b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyControlFactory.java @@ -39,7 +39,6 @@ public class PasswordPolicyControlFactory extends ControlFactory { if (ctl.getID().equals(PasswordPolicyControl.OID)) { return new PasswordPolicyResponseControl(ctl.getEncodedValue()); } - return null; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyErrorStatus.java b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyErrorStatus.java index 0b134f3daf..b81bc34eab 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyErrorStatus.java +++ b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyErrorStatus.java @@ -41,20 +41,24 @@ package org.springframework.security.ldap.ppolicy; */ public enum PasswordPolicyErrorStatus { - PASSWORD_EXPIRED("ppolicy.expired", "Your password has expired"), ACCOUNT_LOCKED("ppolicy.locked", - "Account is locked"), CHANGE_AFTER_RESET("ppolicy.change.after.reset", - "Your password must be changed after being reset"), PASSWORD_MOD_NOT_ALLOWED( - "ppolicy.mod.not.allowed", - "Password cannot be changed"), MUST_SUPPLY_OLD_PASSWORD("ppolicy.must.supply.old.password", - "The old password must be supplied"), INSUFFICIENT_PASSWORD_QUALITY( - "ppolicy.insufficient.password.quality", - "The supplied password is of insufficient quality"), PASSWORD_TOO_SHORT( - "ppolicy.password.too.short", - "The supplied password is too short"), PASSWORD_TOO_YOUNG( - "ppolicy.password.too.young", - "Your password was changed too recently to be changed again"), PASSWORD_IN_HISTORY( - "ppolicy.password.in.history", - "The supplied password has already been used"); + PASSWORD_EXPIRED("ppolicy.expired", "Your password has expired"), + + ACCOUNT_LOCKED("ppolicy.locked", "Account is locked"), + + CHANGE_AFTER_RESET("ppolicy.change.after.reset", "Your password must be changed after being reset"), + + PASSWORD_MOD_NOT_ALLOWED("ppolicy.mod.not.allowed", "Password cannot be changed"), + + MUST_SUPPLY_OLD_PASSWORD("ppolicy.must.supply.old.password", "The old password must be supplied"), + + INSUFFICIENT_PASSWORD_QUALITY("ppolicy.insufficient.password.quality", + "The supplied password is of insufficient quality"), + + PASSWORD_TOO_SHORT("ppolicy.password.too.short", "The supplied password is too short"), + + PASSWORD_TOO_YOUNG("ppolicy.password.too.young", "Your password was changed too recently to be changed again"), + + PASSWORD_IN_HISTORY("ppolicy.password.in.history", "The supplied password has already been used"); private final String errorCode; diff --git a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyResponseControl.java b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyResponseControl.java index da45f0db7f..bb1c8b0898 100755 --- a/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyResponseControl.java +++ b/ldap/src/main/java/org/springframework/security/ldap/ppolicy/PasswordPolicyResponseControl.java @@ -77,10 +77,7 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { */ public PasswordPolicyResponseControl(byte[] encodedValue) { this.encodedValue = encodedValue; - - // PPolicyDecoder decoder = new JLdapDecoder(); PPolicyDecoder decoder = new NetscapeDecoder(); - try { decoder.decode(); } @@ -162,23 +159,18 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { @Override public String toString() { StringBuilder sb = new StringBuilder("PasswordPolicyResponseControl"); - if (hasError()) { sb.append(", error: ").append(this.errorStatus.getDefaultMessage()); } - if (this.graceLoginsRemaining != Integer.MAX_VALUE) { sb.append(", warning: ").append(this.graceLoginsRemaining).append(" grace logins remain"); } - if (this.timeBeforeExpiration != Integer.MAX_VALUE) { sb.append(", warning: time before expiration is ").append(this.timeBeforeExpiration); } - if (!hasError() && !hasWarning()) { sb.append(" (no error, no warning)"); } - return sb.toString(); } @@ -198,24 +190,17 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { int[] bread = { 0 }; BERSequence seq = (BERSequence) BERElement.getElement(new SpecificTagDecoder(), new ByteArrayInputStream(PasswordPolicyResponseControl.this.encodedValue), bread); - int size = seq.size(); - if (logger.isDebugEnabled()) { logger.debug("PasswordPolicyResponse, ASN.1 sequence has " + size + " elements"); } - for (int i = 0; i < seq.size(); i++) { BERTag elt = (BERTag) seq.elementAt(i); - int tag = elt.getTag() & 0x1F; - if (tag == 0) { BERChoice warning = (BERChoice) elt.getValue(); - BERTag content = (BERTag) warning.getValue(); int value = ((BERInteger) content.getValue()).getValue(); - if ((content.getTag() & 0x1F) == 0) { PasswordPolicyResponseControl.this.timeBeforeExpiration = value; } @@ -241,19 +226,15 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { boolean[] implicit) throws IOException { tag &= 0x1F; implicit[0] = false; - if (tag == 0) { // Either the choice or the time before expiry within it if (this.inChoice == null) { setInChoice(true); - // Read the choice length from the stream (ignored) BERElement.readLengthOctets(stream, bytesRead); - int[] componentLength = new int[1]; BERElement choice = new BERChoice(decoder, stream, componentLength); bytesRead[0] += componentLength[0]; - // inChoice = null; return choice; } @@ -267,7 +248,6 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { if (this.inChoice == null) { // The enumeration setInChoice(false); - return new BEREnumerated(stream, bytesRead); } else { @@ -277,7 +257,6 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { } } } - throw new DataRetrievalFailureException("Unexpected tag " + tag); } @@ -289,67 +268,4 @@ public class PasswordPolicyResponseControl extends PasswordPolicyControl { } - /** Decoder based on the OpenLDAP/Novell JLDAP library */ - - // private class JLdapDecoder implements PPolicyDecoder { - // - // public void decode() throws IOException { - // - // LBERDecoder decoder = new LBERDecoder(); - // - // ASN1Sequence seq = (ASN1Sequence)decoder.decode(encodedValue); - // - // if(seq == null) { - // - // } - // - // int size = seq.size(); - // - // if(logger.isDebugEnabled()) { - // logger.debug("PasswordPolicyResponse, ASN.1 sequence has " + - // size + " elements"); - // } - // - // for(int i=0; i < size; i++) { - // - // ASN1Tagged taggedObject = (ASN1Tagged)seq.get(i); - // - // int tag = taggedObject.getIdentifier().getTag(); - // - // ASN1OctetString value = (ASN1OctetString)taggedObject.taggedValue(); - // byte[] content = value.byteValue(); - // - // if(tag == 0) { - // parseWarning(content, decoder); - // - // } else if(tag == 1) { - // // Error: set the code to the value - // errorCode = content[0]; - // } - // } - // } - // - // private void parseWarning(byte[] content, LBERDecoder decoder) { - // // It's the warning (choice). Parse the number and set either the - // // expiry time or number of logins remaining. - // ASN1Tagged taggedObject = (ASN1Tagged)decoder.decode(content); - // int contentTag = taggedObject.getIdentifier().getTag(); - // content = ((ASN1OctetString)taggedObject.taggedValue()).byteValue(); - // int number; - // - // try { - // number = ((Long)decoder.decodeNumeric(new ByteArrayInputStream(content), - // content.length)).intValue(); - // } catch(IOException ex) { - // throw new LdapDataAccessException("Failed to parse number ", ex); - // } - // - // if(contentTag == 0) { - // timeBeforeExpiration = number; - // } else if (contentTag == 1) { - // graceLoginsRemaining = number; - // } - // } - // } - } diff --git a/ldap/src/main/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearch.java b/ldap/src/main/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearch.java index 6475ae7546..86c6b4e8e7 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearch.java +++ b/ldap/src/main/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearch.java @@ -21,6 +21,7 @@ import javax.naming.directory.SearchControls; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.DirContextOperations; @@ -73,13 +74,10 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { Assert.notNull(contextSource, "contextSource must not be null"); Assert.notNull(searchFilter, "searchFilter must not be null."); Assert.notNull(searchBase, "searchBase must not be null (an empty string is acceptable)."); - this.searchFilter = searchFilter; this.contextSource = contextSource; this.searchBase = searchBase; - setSearchSubtree(true); - if (searchBase.length() == 0) { logger.info( "SearchBase not set. Searches will be performed from the root: " + contextSource.getBaseLdapPath()); @@ -95,26 +93,18 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { */ @Override public DirContextOperations searchForUser(String username) { - if (logger.isDebugEnabled()) { - logger.debug("Searching for user '" + username + "', with user search " + this); - } - + logger.debug(LogMessage.of(() -> "Searching for user '" + username + "', with user search " + this)); SpringSecurityLdapTemplate template = new SpringSecurityLdapTemplate(this.contextSource); - template.setSearchControls(this.searchControls); - try { - return template.searchForSingleEntry(this.searchBase, this.searchFilter, new String[] { username }); - } - catch (IncorrectResultSizeDataAccessException notFound) { - if (notFound.getActualSize() == 0) { + catch (IncorrectResultSizeDataAccessException ex) { + if (ex.getActualSize() == 0) { throw new UsernameNotFoundException("User " + username + " not found in directory."); } - // Search should never return multiple results if properly configured, so just - // rethrow - throw notFound; + // Search should never return multiple results if properly configured + throw ex; } } @@ -161,7 +151,6 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { @Override public String toString() { StringBuilder sb = new StringBuilder(); - sb.append("[ searchFilter: '").append(this.searchFilter).append("', "); sb.append("searchBase: '").append(this.searchBase).append("'"); sb.append(", scope: ").append( diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/ApacheDSContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/ApacheDSContainer.java index 917ca80ffd..eb1eb79093 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/ApacheDSContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/ApacheDSContainer.java @@ -113,22 +113,12 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life this.ldifResources = ldifs; this.service = new DefaultDirectoryService(); List list = new ArrayList<>(); - list.add(new NormalizationInterceptor()); list.add(new AuthenticationInterceptor()); list.add(new ReferralInterceptor()); - // list.add( new AciAuthorizationInterceptor() ); - // list.add( new DefaultAuthorizationInterceptor() ); list.add(new ExceptionInterceptor()); - // list.add( new ChangeLogInterceptor() ); list.add(new OperationalAttributeInterceptor()); - // list.add( new SchemaInterceptor() ); list.add(new SubentryInterceptor()); - // list.add( new CollectiveAttributeInterceptor() ); - // list.add( new EventInterceptor() ); - // list.add( new TriggerInterceptor() ); - // list.add( new JournalInterceptor() ); - this.service.setInterceptors(list); this.partition = new JdbmPartition(); this.partition.setId("rootPartition"); @@ -145,21 +135,16 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life public void afterPropertiesSet() throws Exception { if (this.workingDir == null) { String apacheWorkDir = System.getProperty("apacheDSWorkDir"); - if (apacheWorkDir == null) { apacheWorkDir = createTempDirectory("apacheds-spring-security-"); } - setWorkingDirectory(new File(apacheWorkDir)); } - if (this.ldapOverSslEnabled && this.keyStoreFile == null) { - throw new IllegalArgumentException("When LdapOverSsl is enabled, the keyStoreFile property must be set."); - } - + Assert.isTrue(!this.ldapOverSslEnabled || this.keyStoreFile != null, + "When LdapOverSsl is enabled, the keyStoreFile property must be set."); this.server = new LdapServer(); this.server.setDirectoryService(this.service); // AbstractLdapIntegrationTests assume IPv4, so we specify the same here - this.transport = new TcpTransport(this.port); if (this.ldapOverSslEnabled) { this.transport.setEnableSSL(true); @@ -182,18 +167,13 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life public void setWorkingDirectory(File workingDir) { Assert.notNull(workingDir, "workingDir cannot be null"); - this.logger.info("Setting working directory for LDAP_PROVIDER: " + workingDir.getAbsolutePath()); - - if (workingDir.exists()) { - throw new IllegalArgumentException("The specified working directory '" + workingDir.getAbsolutePath() - + "' already exists. Another directory service instance may be using it or it may be from a " - + " previous unclean shutdown. Please confirm and delete it or configure a different " - + "working directory"); - } - + Assert.isTrue(!workingDir.exists(), + "The specified working directory '" + workingDir.getAbsolutePath() + + "' already exists. Another directory service instance may be using it or it may be from a " + + " previous unclean shutdown. Please confirm and delete it or configure a different " + + "working directory"); this.workingDir = workingDir; - this.service.setWorkingDirectory(workingDir); } @@ -250,11 +230,7 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life if (isRunning()) { return; } - - if (this.service.isStarted()) { - throw new IllegalStateException("DirectoryService is already running."); - } - + Assert.state(!this.service.isStarted(), "DirectoryService is already running."); this.logger.info("Starting directory server..."); try { this.service.startup(); @@ -263,7 +239,6 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life catch (Exception ex) { throw new RuntimeException("Server startup failed", ex); } - try { this.service.getAdminSession().lookup(this.partition.getSuffixDn()); } @@ -273,13 +248,10 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life catch (Exception ex) { this.logger.error("Lookup failed", ex); } - SocketAcceptor socketAcceptor = this.server.getSocketAcceptor(this.transport); InetSocketAddress localAddress = socketAcceptor.getLocalAddress(); this.localPort = localAddress.getPort(); - this.running = true; - try { importLdifs(); } @@ -308,7 +280,6 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life if (!isRunning()) { return; } - this.logger.info("Shutting down directory server ..."); try { this.server.stop(); @@ -318,9 +289,7 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life this.logger.error("Shutdown failed", ex); return; } - this.running = false; - if (this.workingDir.exists()) { this.logger.info("Deleting working directory " + this.workingDir.getAbsolutePath()); deleteDir(this.workingDir); @@ -329,43 +298,31 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life private void importLdifs() throws Exception { // Import any ldif files - Resource[] ldifs; - - if (this.ctxt == null) { - // Not running within an app context - ldifs = new PathMatchingResourcePatternResolver().getResources(this.ldifResources); - } - else { - ldifs = this.ctxt.getResources(this.ldifResources); - } - + Resource[] ldifs = (this.ctxt != null) ? this.ctxt.getResources(this.ldifResources) + : new PathMatchingResourcePatternResolver().getResources(this.ldifResources); // Note that we can't just import using the ServerContext returned // from starting Apache DS, apparently because of the long-running issue // DIRSERVER-169. // We need a standard context. // DirContext dirContext = contextSource.getReadWriteContext(); - if (ldifs == null || ldifs.length == 0) { return; } + Assert.isTrue(ldifs.length == 1, () -> "More than one LDIF resource found with the supplied pattern:" + + this.ldifResources + " Got " + Arrays.toString(ldifs)); + String ldifFile = getLdifFile(ldifs); + this.logger.info("Loading LDIF file: " + ldifFile); + LdifFileLoader loader = new LdifFileLoader(this.service.getAdminSession(), new File(ldifFile), null, + getClass().getClassLoader()); + loader.execute(); + } - if (ldifs.length == 1) { - String ldifFile; - - try { - ldifFile = ldifs[0].getFile().getAbsolutePath(); - } - catch (IOException ex) { - ldifFile = ldifs[0].getURI().toString(); - } - this.logger.info("Loading LDIF file: " + ldifFile); - LdifFileLoader loader = new LdifFileLoader(this.service.getAdminSession(), new File(ldifFile), null, - getClass().getClassLoader()); - loader.execute(); + private String getLdifFile(Resource[] ldifs) throws IOException { + try { + return ldifs[0].getFile().getAbsolutePath(); } - else { - throw new IllegalArgumentException("More than one LDIF resource found with the supplied pattern:" - + this.ldifResources + " Got " + Arrays.toString(ldifs)); + catch (IOException ex) { + return ldifs[0].getURI().toString(); } } @@ -373,7 +330,6 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life String parentTempDir = System.getProperty("java.io.tmpdir"); String fileNamePrefix = prefix + System.nanoTime(); String fileName = fileNamePrefix; - for (int i = 0; i < 1000; i++) { File tempDir = new File(parentTempDir, fileName); if (!tempDir.exists()) { @@ -381,7 +337,6 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life } fileName = fileNamePrefix + "~" + i; } - throw new IOException( "Failed to create a temporary directory for file at " + new File(parentTempDir, fileNamePrefix)); } @@ -396,7 +351,6 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life } } } - return dir.delete(); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java index 3a50a0f838..269b8adae1 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java +++ b/ldap/src/main/java/org/springframework/security/ldap/server/UnboundIdContainer.java @@ -85,20 +85,16 @@ public class UnboundIdContainer implements InitializingBean, DisposableBean, Lif if (isRunning()) { return; } - try { InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig(this.defaultPartitionSuffix); config.addAdditionalBindCredentials("uid=admin,ou=system", "secret"); - config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("LDAP", this.port)); config.setEnforceSingleStructuralObjectClass(false); config.setEnforceAttributeSyntaxCompliance(true); - DN dn = new DN(this.defaultPartitionSuffix); Entry entry = new Entry(dn); entry.addAttribute("objectClass", "top", "domain", "extensibleObject"); entry.addAttribute("dc", dn.getRDN().getAttributeValues()[0]); - InMemoryDirectoryServer directoryServer = new InMemoryDirectoryServer(config); directoryServer.add(entry); importLdif(directoryServer); @@ -110,7 +106,6 @@ public class UnboundIdContainer implements InitializingBean, DisposableBean, Lif catch (LDAPException ex) { throw new RuntimeException("Server startup failed", ex); } - } private void importLdif(InMemoryDirectoryServer directoryServer) { diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java index 2fd023c839..8ef21554c4 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java @@ -29,6 +29,7 @@ import javax.naming.directory.SearchControls; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.LdapTemplate; @@ -161,21 +162,17 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator this.ldapTemplate = new SpringSecurityLdapTemplate(contextSource); getLdapTemplate().setSearchControls(getSearchControls()); this.groupSearchBase = groupSearchBase; - if (groupSearchBase == null) { logger.info("groupSearchBase is null. No group search will be performed."); } else if (groupSearchBase.length() == 0) { logger.info("groupSearchBase is empty. Searches will be performed from the context source base"); } - this.authorityMapper = (record) -> { String role = record.get(this.groupRoleAttribute).get(0); - if (this.convertToUpperCase) { role = role.toUpperCase(); } - return new SimpleGrantedAuthority(this.rolePrefix + role); }; } @@ -202,26 +199,17 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator @Override public final Collection getGrantedAuthorities(DirContextOperations user, String username) { String userDn = user.getNameInNamespace(); - - if (logger.isDebugEnabled()) { - logger.debug("Getting authorities for user " + userDn); - } - + logger.debug(LogMessage.format("Getting authorities for user %s", userDn)); Set roles = getGroupMembershipRoles(userDn, username); - Set extraRoles = getAdditionalRoles(user, username); - if (extraRoles != null) { roles.addAll(extraRoles); } - if (this.defaultRole != null) { roles.add(this.defaultRole); } - List result = new ArrayList<>(roles.size()); result.addAll(roles); - return result; } @@ -229,26 +217,16 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator if (getGroupSearchBase() == null) { return new HashSet<>(); } - Set authorities = new HashSet<>(); - - if (logger.isDebugEnabled()) { - logger.debug("Searching for roles for user '" + username + "', DN = " + "'" + userDn + "', with filter " - + this.groupSearchFilter + " in search base '" + getGroupSearchBase() + "'"); - } - + logger.debug(LogMessage.of(() -> "Searching for roles for user '" + username + "', DN = " + "'" + userDn + + "', with filter " + this.groupSearchFilter + " in search base '" + getGroupSearchBase() + "'")); Set>> userRoles = getLdapTemplate().searchForMultipleAttributeValues( getGroupSearchBase(), this.groupSearchFilter, new String[] { userDn, username }, new String[] { this.groupRoleAttribute }); - - if (logger.isDebugEnabled()) { - logger.debug("Roles from search: " + userRoles); - } - + logger.debug(LogMessage.of(() -> "Roles from search: " + userRoles)); for (Map> role : userRoles) { authorities.add(this.authorityMapper.apply(role)); } - return authorities; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPerson.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPerson.java index 3ef3427b09..2c70b4f425 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPerson.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPerson.java @@ -27,7 +27,7 @@ import org.springframework.security.core.SpringSecurityCoreVersion; *

* The username will be mapped from the uid attribute by default. * - * @author Luke + * @author Luke Taylor */ public class InetOrgPerson extends Person { diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPersonContextMapper.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPersonContextMapper.java index 95866535aa..a5770becd8 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPersonContextMapper.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPersonContextMapper.java @@ -33,10 +33,8 @@ public class InetOrgPersonContextMapper implements UserDetailsContextMapper { public UserDetails mapUserFromContext(DirContextOperations ctx, String username, Collection authorities) { InetOrgPerson.Essence p = new InetOrgPerson.Essence(ctx); - p.setUsername(username); p.setAuthorities(authorities); - return p.createUserDetails(); } @@ -44,7 +42,6 @@ public class InetOrgPersonContextMapper implements UserDetailsContextMapper { @Override public void mapUserToContext(UserDetails user, DirContextAdapter ctx) { Assert.isInstanceOf(InetOrgPerson.class, user, "UserDetails must be an InetOrgPerson instance"); - InetOrgPerson p = (InetOrgPerson) user; p.populateContext(ctx); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapAuthority.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapAuthority.java index e60b6c8f2c..9e89a5d631 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapAuthority.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapAuthority.java @@ -55,7 +55,6 @@ public class LdapAuthority implements GrantedAuthority { public LdapAuthority(String role, String dn, Map> attributes) { Assert.notNull(role, "role can not be null"); Assert.notNull(dn, "dn can not be null"); - this.role = role; this.dn = dn; this.attributes = attributes; @@ -87,10 +86,7 @@ public class LdapAuthority implements GrantedAuthority { if (this.attributes != null) { result = this.attributes.get(name); } - if (result == null) { - result = Collections.emptyList(); - } - return result; + return (result != null) ? result : Collections.emptyList(); } /** @@ -100,17 +96,9 @@ public class LdapAuthority implements GrantedAuthority { */ public String getFirstAttributeValue(String name) { List result = getAttributeValues(name); - if (result.isEmpty()) { - return null; - } - else { - return result.get(0); - } + return (!result.isEmpty()) ? result.get(0) : null; } - /** - * {@inheritDoc} - */ @Override public String getAuthority() { return this.role; @@ -118,23 +106,21 @@ public class LdapAuthority implements GrantedAuthority { /** * Compares the LdapAuthority based on {@link #getAuthority()} and {@link #getDn()} - * values {@inheritDoc} + * values. */ @Override - public boolean equals(Object o) { - if (this == o) { + public boolean equals(Object obj) { + if (this == obj) { return true; } - if (!(o instanceof LdapAuthority)) { + if (!(obj instanceof LdapAuthority)) { return false; } - - LdapAuthority that = (LdapAuthority) o; - - if (!this.dn.equals(that.dn)) { + LdapAuthority other = (LdapAuthority) obj; + if (!this.dn.equals(other.dn)) { return false; } - return this.role.equals(that.role); + return this.role.equals(other.role); } @Override diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java index 8985e401af..29a7323e3d 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsImpl.java @@ -154,11 +154,9 @@ public class LdapUserDetailsImpl implements LdapUserDetails, PasswordPolicyData sb.append("AccountNonExpired: ").append(this.accountNonExpired).append("; "); sb.append("CredentialsNonExpired: ").append(this.credentialsNonExpired).append("; "); sb.append("AccountNonLocked: ").append(this.accountNonLocked).append("; "); - if (this.getAuthorities() != null && !this.getAuthorities().isEmpty()) { sb.append("Granted Authorities: "); boolean first = true; - for (Object authority : this.getAuthorities()) { if (first) { first = false; @@ -166,14 +164,12 @@ public class LdapUserDetailsImpl implements LdapUserDetails, PasswordPolicyData else { sb.append(", "); } - sb.append(authority.toString()); } } else { sb.append("Not granted any authorities"); } - return sb.toString(); } @@ -231,13 +227,9 @@ public class LdapUserDetailsImpl implements LdapUserDetails, PasswordPolicyData Assert.notNull(this.instance, "Essence can only be used to create a single instance"); Assert.notNull(this.instance.username, "username must not be null"); Assert.notNull(this.instance.getDn(), "Distinguished name must not be null"); - this.instance.authorities = Collections.unmodifiableList(this.mutableAuthorities); - LdapUserDetails newInstance = this.instance; - this.instance = null; - return newInstance; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java index b2faf060bc..b2baff26a3 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java @@ -40,6 +40,7 @@ import javax.naming.ldap.LdapContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.core.AttributesMapper; import org.springframework.ldap.core.AttributesMapperCallbackHandler; import org.springframework.ldap.core.ContextExecutor; @@ -116,12 +117,9 @@ public class LdapUserDetailsManager implements UserDetailsManager { /** Default context mapper used to create a set of roles from a list of attributes */ private AttributesMapper roleMapper = (attributes) -> { Attribute roleAttr = attributes.get(this.groupRoleAttributeName); - NamingEnumeration ne = roleAttr.getAll(); - // assert ne.hasMore(); Object group = ne.next(); String role = group.toString(); - return new SimpleGrantedAuthority(this.rolePrefix + role.toUpperCase()); }; @@ -137,11 +135,8 @@ public class LdapUserDetailsManager implements UserDetailsManager { public UserDetails loadUserByUsername(String username) { DistinguishedName dn = this.usernameMapper.buildDn(username); List authorities = getUserAuthorities(dn, username); - - this.logger.debug("Loading user '" + username + "' with DN '" + dn + "'"); - + this.logger.debug(LogMessage.format("Loading user '%s' with DN '%s'", username, dn)); DirContextAdapter userCtx = loadUserAsContext(dn, username); - return this.userDetailsMapper.mapUserFromContext(userCtx, username, authorities); } @@ -151,8 +146,8 @@ public class LdapUserDetailsManager implements UserDetailsManager { Attributes attrs = ctx.getAttributes(dn, this.attributesToRetrieve); return new DirContextAdapter(attrs, LdapUtils.getFullDn(dn, ctx)); } - catch (NameNotFoundException notFound) { - throw new UsernameNotFoundException("User " + username + " not found", notFound); + catch (NameNotFoundException ex) { + throw new UsernameNotFoundException("User " + username + " not found", ex); } }); } @@ -187,13 +182,9 @@ public class LdapUserDetailsManager implements UserDetailsManager { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); Assert.notNull(authentication, "No authentication object found in security context. Can't change current user's password!"); - String username = authentication.getName(); - - this.logger.debug("Changing password for user '" + username); - + this.logger.debug(LogMessage.format("Changing password for user '%s'", username)); DistinguishedName userDn = this.usernameMapper.buildDn(username); - if (this.usePasswordModifyExtensionOperation) { changePasswordUsingExtensionOperation(userDn, oldPassword, newPassword); } @@ -214,13 +205,10 @@ public class LdapUserDetailsManager implements UserDetailsManager { DistinguishedName fullDn = LdapUtils.getFullDn(dn, ctx); SearchControls ctrls = new SearchControls(); ctrls.setReturningAttributes(new String[] { this.groupRoleAttributeName }); - return ctx.search(this.groupSearchBase, this.groupSearchFilter, new String[] { fullDn.toUrl(), username }, ctrls); }; - AttributesMapperCallbackHandler roleCollector = new AttributesMapperCallbackHandler(this.roleMapper); - this.template.search(se, roleCollector); return roleCollector.getList(); } @@ -230,38 +218,28 @@ public class LdapUserDetailsManager implements UserDetailsManager { DirContextAdapter ctx = new DirContextAdapter(); copyToContext(user, ctx); DistinguishedName dn = this.usernameMapper.buildDn(user.getUsername()); - - this.logger.debug("Creating new user '" + user.getUsername() + "' with DN '" + dn + "'"); - + this.logger.debug(LogMessage.format("Creating new user '%s' with DN '%s'", user.getUsername(), dn)); this.template.bind(dn, ctx, null); - - // Check for any existing authorities which might be set for this DN and remove - // them + // Check for any existing authorities which might be set for this + // DN and remove them List authorities = getUserAuthorities(dn, user.getUsername()); - if (authorities.size() > 0) { removeAuthorities(dn, authorities); } - addAuthorities(dn, user.getAuthorities()); } @Override public void updateUser(UserDetails user) { DistinguishedName dn = this.usernameMapper.buildDn(user.getUsername()); - - this.logger.debug("Updating user '" + user.getUsername() + "' with DN '" + dn + "'"); - + this.logger.debug(LogMessage.format("Updating new user '%s' with DN '%s'", user.getUsername(), dn)); List authorities = getUserAuthorities(dn, user.getUsername()); - DirContextAdapter ctx = loadUserAsContext(dn, user.getUsername()); ctx.setUpdateMode(true); copyToContext(user, ctx); - // Remove the objectclass attribute from the list of mods (if present). List mods = new LinkedList<>(Arrays.asList(ctx.getModificationItems())); ListIterator modIt = mods.listIterator(); - while (modIt.hasNext()) { ModificationItem mod = modIt.next(); Attribute a = mod.getAttribute(); @@ -269,9 +247,7 @@ public class LdapUserDetailsManager implements UserDetailsManager { modIt.remove(); } } - this.template.modifyAttributes(dn, mods.toArray(new ModificationItem[0])); - // template.rebind(dn, ctx, null); // Remove the old authorities and replace them with the new one removeAuthorities(dn, authorities); @@ -288,7 +264,6 @@ public class LdapUserDetailsManager implements UserDetailsManager { @Override public boolean userExists(String username) { DistinguishedName dn = this.usernameMapper.buildDn(username); - try { Object obj = this.template.lookup(dn); if (obj instanceof Context) { @@ -309,7 +284,6 @@ public class LdapUserDetailsManager implements UserDetailsManager { protected DistinguishedName buildGroupDn(String group) { DistinguishedName dn = new DistinguishedName(this.groupSearchBase); dn.add(this.groupRoleAttributeName, group.toLowerCase()); - return dn; } @@ -333,7 +307,6 @@ public class LdapUserDetailsManager implements UserDetailsManager { DistinguishedName fullDn = LdapUtils.getFullDn(userDn, ctx); ModificationItem addGroup = new ModificationItem(modType, new BasicAttribute(this.groupMemberAttributeName, fullDn.toUrl())); - ctx.modifyAttributes(buildGroupDn(group), new ModificationItem[] { addGroup }); } return null; @@ -342,11 +315,9 @@ public class LdapUserDetailsManager implements UserDetailsManager { private String convertAuthorityToGroup(GrantedAuthority authority) { String group = authority.getAuthority(); - if (group.startsWith(this.rolePrefix)) { group = group.substring(this.rolePrefix.length()); } - return group; } @@ -419,15 +390,12 @@ public class LdapUserDetailsManager implements UserDetailsManager { private void changePasswordUsingAttributeModification(DistinguishedName userDn, String oldPassword, String newPassword) { - - final ModificationItem[] passwordChange = new ModificationItem[] { new ModificationItem( - DirContext.REPLACE_ATTRIBUTE, new BasicAttribute(this.passwordAttributeName, newPassword)) }; - + ModificationItem[] passwordChange = new ModificationItem[] { new ModificationItem(DirContext.REPLACE_ATTRIBUTE, + new BasicAttribute(this.passwordAttributeName, newPassword)) }; if (oldPassword == null) { this.template.modifyAttributes(userDn, passwordChange); return; } - this.template.executeReadWrite((dirCtx) -> { LdapContext ctx = (LdapContext) dirCtx; ctx.removeFromEnvironment("com.sun.jndi.ldap.connect.pool"); @@ -440,23 +408,17 @@ public class LdapUserDetailsManager implements UserDetailsManager { catch (javax.naming.AuthenticationException ex) { throw new BadCredentialsException("Authentication for password change failed."); } - ctx.modifyAttributes(userDn, passwordChange); - return null; }); - } private void changePasswordUsingExtensionOperation(DistinguishedName userDn, String oldPassword, String newPassword) { - this.template.executeReadWrite((dirCtx) -> { LdapContext ctx = (LdapContext) dirCtx; - String userIdentity = LdapUtils.getFullDn(userDn, ctx).encode(); PasswordModifyRequest request = new PasswordModifyRequest(userIdentity, oldPassword, newPassword); - try { return ctx.extendedOperation(request); } @@ -493,19 +455,15 @@ public class LdapUserDetailsManager implements UserDetailsManager { PasswordModifyRequest(String userIdentity, String oldPassword, String newPassword) { ByteArrayOutputStream elements = new ByteArrayOutputStream(); - if (userIdentity != null) { berEncode(USER_IDENTITY_OCTET_TYPE, userIdentity.getBytes(), elements); } - if (oldPassword != null) { berEncode(OLD_PASSWORD_OCTET_TYPE, oldPassword.getBytes(), elements); } - if (newPassword != null) { berEncode(NEW_PASSWORD_OCTET_TYPE, newPassword.getBytes(), elements); } - berEncode(SEQUENCE_TYPE, elements.toByteArray(), this.value); } @@ -532,9 +490,7 @@ public class LdapUserDetailsManager implements UserDetailsManager { */ private void berEncode(byte type, byte[] src, ByteArrayOutputStream dest) { int length = src.length; - dest.write(type); - if (length < 128) { dest.write(length); } @@ -560,7 +516,6 @@ public class LdapUserDetailsManager implements UserDetailsManager { dest.write((byte) ((length >> 8) & 0xFF)); dest.write((byte) (length & 0xFF)); } - try { dest.write(src); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsMapper.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsMapper.java index ba974652da..56e724a075 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsMapper.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsMapper.java @@ -21,6 +21,7 @@ import java.util.Collection; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.core.DirContextAdapter; import org.springframework.ldap.core.DirContextOperations; import org.springframework.security.core.GrantedAuthority; @@ -53,56 +54,41 @@ public class LdapUserDetailsMapper implements UserDetailsContextMapper { public UserDetails mapUserFromContext(DirContextOperations ctx, String username, Collection authorities) { String dn = ctx.getNameInNamespace(); - - this.logger.debug("Mapping user details from context with DN: " + dn); - + this.logger.debug(LogMessage.format("Mapping user details from context with DN: %s", dn)); LdapUserDetailsImpl.Essence essence = new LdapUserDetailsImpl.Essence(); essence.setDn(dn); - Object passwordValue = ctx.getObjectAttribute(this.passwordAttributeName); - if (passwordValue != null) { essence.setPassword(mapPassword(passwordValue)); } - essence.setUsername(username); - // Map the roles for (int i = 0; (this.roleAttributes != null) && (i < this.roleAttributes.length); i++) { String[] rolesForAttribute = ctx.getStringAttributes(this.roleAttributes[i]); - if (rolesForAttribute == null) { - this.logger.debug("Couldn't read role attribute '" + this.roleAttributes[i] + "' for user " + dn); + this.logger.debug( + LogMessage.format("Couldn't read role attribute '%s' for user $s", this.roleAttributes[i], dn)); continue; } - for (String role : rolesForAttribute) { GrantedAuthority authority = createAuthority(role); - if (authority != null) { essence.addAuthority(authority); } } } - // Add the supplied authorities - for (GrantedAuthority authority : authorities) { essence.addAuthority(authority); } - // Check for PPolicy data - PasswordPolicyResponseControl ppolicy = (PasswordPolicyResponseControl) ctx .getObjectAttribute(PasswordPolicyControl.OID); - if (ppolicy != null) { essence.setTimeBeforeExpiration(ppolicy.getTimeBeforeExpiration()); essence.setGraceLoginsRemaining(ppolicy.getGraceLoginsRemaining()); } - return essence.createUserDetails(); - } @Override @@ -118,12 +104,10 @@ public class LdapUserDetailsMapper implements UserDetailsContextMapper { * @return a String representation of the password. */ protected String mapPassword(Object passwordValue) { - if (!(passwordValue instanceof String)) { // Assume it's binary passwordValue = new String((byte[]) passwordValue); } - return (String) passwordValue; } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsService.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsService.java index bc5ee9fc03..94a592c68e 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsService.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsService.java @@ -57,7 +57,6 @@ public class LdapUserDetailsService implements UserDetailsService { @Override public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { DirContextOperations userData = this.userSearch.searchForUser(username); - return this.userDetailsMapper.mapUserFromContext(userData, username, this.authoritiesPopulator.getGrantedAuthorities(userData, username)); } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulator.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulator.java index 0507597bf0..33d55d7c5b 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulator.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/NestedLdapAuthoritiesPopulator.java @@ -24,6 +24,7 @@ import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.ldap.core.ContextSource; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.ldap.SpringSecurityLdapTemplate; @@ -144,19 +145,13 @@ public class NestedLdapAuthoritiesPopulator extends DefaultLdapAuthoritiesPopula super(contextSource, groupSearchBase); } - /** - * {@inheritDoc} - */ @Override public Set getGroupMembershipRoles(String userDn, String username) { if (getGroupSearchBase() == null) { return new HashSet<>(); } - Set authorities = new HashSet<>(); - performNestedSearch(userDn, username, authorities, getMaxSearchDepth()); - return authorities; } @@ -171,34 +166,23 @@ public class NestedLdapAuthoritiesPopulator extends DefaultLdapAuthoritiesPopula private void performNestedSearch(String userDn, String username, Set authorities, int depth) { if (depth == 0) { // back out of recursion - if (logger.isDebugEnabled()) { - logger.debug("Search aborted, max depth reached," + " for roles for user '" + username + "', DN = " - + "'" + userDn + "', with filter " + getGroupSearchFilter() + " in search base '" - + getGroupSearchBase() + "'"); - } + logger.debug(LogMessage.of(() -> "Search aborted, max depth reached," + " for roles for user '" + username + + "', DN = " + "'" + userDn + "', with filter " + getGroupSearchFilter() + " in search base '" + + getGroupSearchBase() + "'")); return; } - - if (logger.isDebugEnabled()) { - logger.debug("Searching for roles for user '" + username + "', DN = " + "'" + userDn + "', with filter " - + getGroupSearchFilter() + " in search base '" + getGroupSearchBase() + "'"); - } - + logger.debug(LogMessage.of(() -> "Searching for roles for user '" + username + "', DN = " + "'" + userDn + + "', with filter " + getGroupSearchFilter() + " in search base '" + getGroupSearchBase() + "'")); if (getAttributeNames() == null) { setAttributeNames(new HashSet<>()); } if (StringUtils.hasText(getGroupRoleAttribute()) && !getAttributeNames().contains(getGroupRoleAttribute())) { getAttributeNames().add(getGroupRoleAttribute()); } - Set>> userRoles = getLdapTemplate().searchForMultipleAttributeValues( getGroupSearchBase(), getGroupSearchFilter(), new String[] { userDn, username }, getAttributeNames().toArray(new String[0])); - - if (logger.isDebugEnabled()) { - logger.debug("Roles from search: " + userRoles); - } - + logger.debug(LogMessage.format("Roles from search: %s", userRoles)); for (Map> record : userRoles) { boolean circular = false; String dn = record.get(SpringSecurityLdapTemplate.DN_KEY).get(0); @@ -220,7 +204,6 @@ public class NestedLdapAuthoritiesPopulator extends DefaultLdapAuthoritiesPopula if (!circular) { performNestedSearch(dn, roleName, authorities, (depth - 1)); } - } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/Person.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/Person.java index 0fd4b8d9d9..5dbc580cf8 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/Person.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/Person.java @@ -76,7 +76,6 @@ public class Person extends LdapUserDetailsImpl { adapter.setAttributeValues("cn", getCn()); adapter.setAttributeValue("description", getDescription()); adapter.setAttributeValue("telephoneNumber", getTelephoneNumber()); - if (getPassword() != null) { adapter.setAttributeValue("userPassword", getPassword()); } @@ -95,11 +94,9 @@ public class Person extends LdapUserDetailsImpl { setSn(ctx.getStringAttribute("sn")); setDescription(ctx.getStringAttribute("description")); setTelephoneNumber(ctx.getStringAttribute("telephoneNumber")); - Object passo = ctx.getObjectAttribute("userPassword"); - - if (passo != null) { - String password = LdapUtils.convertPasswordToString(passo); - setPassword(password); + Object password = ctx.getObjectAttribute("userPassword"); + if (password != null) { + setPassword(LdapUtils.convertPasswordToString(password)); } } diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/PersonContextMapper.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/PersonContextMapper.java index b4fc788d1d..3662112afa 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/PersonContextMapper.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/PersonContextMapper.java @@ -33,18 +33,14 @@ public class PersonContextMapper implements UserDetailsContextMapper { public UserDetails mapUserFromContext(DirContextOperations ctx, String username, Collection authorities) { Person.Essence p = new Person.Essence(ctx); - p.setUsername(username); p.setAuthorities(authorities); - return p.createUserDetails(); - } @Override public void mapUserToContext(UserDetails user, DirContextAdapter ctx) { Assert.isInstanceOf(Person.class, user, "UserDetails must be a Person instance"); - Person p = (Person) user; p.populateContext(ctx); }