Polish spring-security-ldap main code

Manually polish `spring-security-ldap` following the formatting
and checkstyle fixes.

Issue gh-8945

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;
 	}
 

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 tokenizer = new StringTokenizer(providerUrl);
-		StringTokenizer st = new StringTokenizer(providerUrl);
-
 		ArrayList<String> urls = new ArrayList<>();
-
 		// Work out rootDn from the first URL and check that the other URLs (if any) match
-		while (st.hasMoreTokens()) {
+		String rootDn = null;
-			String url = st.nextToken();
+		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 + "'");
-
+			Assert.isTrue(rootDn == null || rootDn.equals(urlRootDn),
-			if (this.rootDn == null) {
+					"Root DNs must be the same when using multiple URLs");
-				this.rootDn = urlRootDn;
+			rootDn = (rootDn != null) ? rootDn : urlRootDn;
-			}
-			else if (!this.rootDn.equals(urlRootDn)) {
-				throw new IllegalArgumentException("Root DNs must be the same when using multiple URLs");
-			}
 		}
-
 		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'
+				// Remove the pooling flag unless authenticating as the 'manager' user.
-				// 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<String> 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();
 
 	}

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();
-
+		return (raw.length() > 1) ? raw : "0" + raw;
-		if (raw.length() > 1) {
-			return raw;
-		}
-		else {
-			return "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 ch = value.charAt(i);
-			char c = value.charAt(i);
+			encodedValue.append((ch < FILTER_ESCAPE_TABLE.length) ? FILTER_ESCAPE_TABLE[ch] : ch);
-
-			if (c < FILTER_ESCAPE_TABLE.length) {
-				encodedValue.append(FILTER_ESCAPE_TABLE[c]);
-			}
-			else {
-				// default: add the char
-				encodedValue.append(c);
-			}
 		}
-
 		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 (value.length() <= i + 2) {
-					if (nextChar == ',' || nextChar == '=' || nextChar == '+' || nextChar == '<' || nextChar == '>'
+						throw new BadLdapGrammarException(
-							|| nextChar == '#' || nextChar == ';' || nextChar == '\\' || nextChar == '\"'
+								"Unexpected end of value " + "expected special or hex, found '" + 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;
-						}
 					}
+					// 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 == ' ';
+	}
+
 }

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);
+			throw new IllegalArgumentException("Unable to parse url: " + url, ex);
-			iae.initCause(ex);
-			throw iae;
 		}
 	}
 

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) {
+	public boolean compare(String dn, String attributeName, Object value) {
-		final String comparisonFilter = "(" + attributeName + "={0})";
+		String comparisonFilter = "(" + attributeName + "={0})";
-
+		return executeReadOnly((ctx) -> {
-		class LdapCompareCallback implements ContextExecutor {
+			SearchControls searchControls = new SearchControls();
-
+			searchControls.setReturningAttributes(NO_ATTRS);
-			@Override
+			searchControls.setSearchScope(SearchControls.OBJECT_SCOPE);
-			public Object executeWithContext(DirContext ctx) throws NamingException {
+			Object[] params = new Object[] { value };
-				SearchControls ctls = new SearchControls();
+			NamingEnumeration<SearchResult> results = ctx.search(dn, comparisonFilter, params, searchControls);
-				ctls.setReturningAttributes(NO_ATTRS);
+			Boolean match = results.hasMore();
-				ctls.setSearchScope(SearchControls.OBJECT_SCOPE);
+			LdapUtils.closeEnumeration(results);
-
+			return match;
-				NamingEnumeration<SearchResult> 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;
 	}
 
 	/**
@@ -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<Map<String, List<String>>> searchForMultipleAttributeValues(final String base, final String filter,
+	public Set<Map<String, List<String>>> searchForMultipleAttributeValues(String base, String filter, Object[] params,
-			final Object[] params, final String[] attributeNames) {
+			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);
+		logger.debug(LogMessage.format("Using filter: %s", formattedFilter));
-
+		HashSet<Map<String, List<String>>> result = new HashSet<>();
-		final HashSet<Map<String, List<String>>> set = new HashSet<>();
-
 		ContextMapper roleMapper = (ctx) -> {
 			DirContextAdapter adapter = (DirContextAdapter) ctx;
 			Map<String, List<String>> record = new HashMap<>();
-			if (attributeNames == null || attributeNames.length == 0) {
+			if (ObjectUtils.isEmpty(attributeNames)) {
 				try {
-					for (NamingEnumeration ae = adapter.getAttributes().getAll(); ae.hasMore();) {
+					for (NamingEnumeration enumeration = adapter.getAttributes().getAll(); enumeration.hasMore();) {
-						Attribute attr = (Attribute) ae.next();
+						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 result;
-		return set;
 	}
 
 	/**
@@ -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(LogMessage.format("No attribute value found for '%s'", attributeName));
-				logger.debug("No attribute value found for '" + attributeName + "'");
-			}
 			return;
 		}
-		List<String> svalues = new ArrayList<>();
+		List<String> stringValues = new ArrayList<>();
-		for (Object o : values) {
+		for (Object value : values) {
-			if (o != null) {
+			if (value != null) {
-				if (String.class.isAssignableFrom(o.getClass())) {
+				if (String.class.isAssignableFrom(value.getClass())) {
-					svalues.add((String) o);
+					stringValues.add((String) value);
 				}
 				else {
-					if (logger.isDebugEnabled()) {
+					logger.debug(LogMessage.format("Attribute:%s contains a non string value of type[%s]",
-						logger.debug("Attribute:" + attributeName + " contains a non string value of type["
+							attributeName, value.getClass()));
-								+ o.getClass() + "]");
+					stringValues.add(value.toString());
-					}
-					svalues.add(o.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<SearchResult> resultsEnum = ctx.search(searchBaseDn, filter, params,
 				buildControls(searchControls));
-
+		logger.debug(LogMessage.format("Searching for entry under DN '%s', base = '%s', filter = '%s'", ctxBaseDn,
-		if (logger.isDebugEnabled()) {
+				searchBaseDn, filter));
-			logger.debug("Searching for entry under DN '" + ctxBaseDn + "', base = '" + searchBaseDn + "', filter = '"
-					+ filter + "'");
-		}
-
 		Set<DirContextOperations> 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");
-
+				logger.debug(LogMessage.format("Found DN: %s", dca.getDn()));
-				if (logger.isDebugEnabled()) {
-					logger.debug("Found DN: " + dca.getDn());
-				}
 				results.add(dca);
 			}
 		}
@@ -321,15 +286,9 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
 			LdapUtils.closeEnumeration(resultsEnum);
 			logger.info("Ignoring PartialResultException");
 		}
-
+		if (results.size() != 1) {
-		if (results.size() == 0) {
-			throw new IncorrectResultSizeDataAccessException(1, 0);
-		}
-
-		if (results.size() > 1) {
 			throw new IncorrectResultSizeDataAccessException(1, results.size());
 		}
-
 		return results.iterator().next();
 	}
 

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"));
-
+		UsernamePasswordAuthenticationToken userToken = (UsernamePasswordAuthenticationToken) authentication;
-		final UsernamePasswordAuthenticationToken userToken = (UsernamePasswordAuthenticationToken) authentication;
-
 		String username = userToken.getName();
 		String password = (String) authentication.getCredentials();
-
+		this.logger.debug(LogMessage.format("Processing authentication request for user: %s", username));
-		if (this.logger.isDebugEnabled()) {
-			this.logger.debug("Processing authentication request for user: " + 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;
 	}
 

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<String> 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]);
 		}

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(LogMessage.format("Attempting to bind as %s", fullDn));
-		logger.debug("Attempting to bind as " + 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(LogMessage.format("Failed to bind as %s: %s", userDn, cause));
-			logger.debug("Failed to bind as " + userDn + ": " + cause);
-		}
 	}
 
 }

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 ex;
-				throw notFound;
-			}
 		}
-		catch (NamingException ldapAccessFailure) {
+		catch (NamingException ex) {
-			throw new InternalAuthenticationServiceException(ldapAccessFailure.getMessage(), ldapAccessFailure);
+			throw new InternalAuthenticationServiceException(ex.getMessage(), ex);
 		}
 	}
 

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();
-
+		return (raw.length() > 1) ? raw : "0" + raw;
-		if (raw.length() > 1) {
-			return raw;
-		}
-		else {
-			return "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 ch = value.charAt(i);
-			char c = value.charAt(i);
+			encodedValue.append((ch < FILTER_ESCAPE_TABLE.length) ? FILTER_ESCAPE_TABLE[ch] : ch);
-
-			if (c < FILTER_ESCAPE_TABLE.length) {
-				encodedValue.append(FILTER_ESCAPE_TABLE[c]);
-			}
-			else {
-				// default: add the char
-				encodedValue.append(c);
-			}
 		}
-
 		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 (value.length() <= i + 2) {
-					if (nextChar == ',' || nextChar == '=' || nextChar == '+' || nextChar == '<' || nextChar == '>'
+						throw new BadLdapGrammarException(
-							|| nextChar == '#' || nextChar == ';' || nextChar == '\\' || nextChar == '\"'
+								"Unexpected end of value " + "expected special or hex, found '" + 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;
-						}
 					}
+					// 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 == ' ';
+	}
+
 }

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 '"
+			logger.debug(LogMessage.format("Performing LDAP compare of password attribute '%s' for user '%s'",
-					+ user.getDn() + "'");
+					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(

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 (authentication instanceof AnonymousAuthenticationToken) {
-			if (log.isDebugEnabled()) {
+			log.debug("Anonymous Authentication, returning empty String as Principal");
-				log.debug("Anonymous Authentication, returning empty String as Principal");
-			}
 			return "";
 		}
-		else {
+		throw new IllegalArgumentException(
-			throw new IllegalArgumentException(
+				"The principal property of the authentication object" + "needs to be an LdapUserDetails.");
-					"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();
 	}
 

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<? extends GrantedAuthority> 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));
 		}
-
+		List<GrantedAuthority> authorities = new ArrayList<>(groups.length);
-		ArrayList<GrantedAuthority> 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<String, Object> 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(LogMessage.format("Authentication for %s failed:%s", bindPrincipal, exception));
-			this.logger.debug("Authentication for " + bindPrincipal + " failed:" + 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(
-		this.logger.info("Active Directory authentication failed: " + subCodeToLogMessage(subErrorCode));
+				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);
+		Matcher matcher = SUB_ERROR_CODE.matcher(message);
-
+		if (matcher.matches()) {
-		if (m.matches()) {
+			return Integer.parseInt(matcher.group(1), 16);
-			return Integer.parseInt(m.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) {
+		catch (CommunicationException ex) {
-			throw badLdapConnection(ldapCommunicationException);
+			throw badLdapConnection(ex);
 		}
-		catch (IncorrectResultSizeDataAccessException incorrectResults) {
+		catch (IncorrectResultSizeDataAccessException ex) {
-			// Search should never return multiple results if properly configured - just
+			// Search should never return multiple results if properly configured -
-			// rethrow
+			if (ex.getActualSize() != 0) {
-			if (incorrectResults.getActualSize() != 0) {
+				throw ex;
-				throw incorrectResults;
 			}
 			// 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;
 	}
 

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);
 		}
-
+		this.logger
-		final boolean debug = this.logger.isDebugEnabled();
+				.debug(LogMessage.format("Binding as '%s', prior to reconnect as user '%s'", this.userDn, principal));
-
-		if (debug) {
-			this.logger.debug("Binding as '" + this.userDn + "', prior to reconnect as user '" + 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) {
+			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Failed to obtain context", ne);
+				this.logger.debug("Failed to obtain context", ex);
 				this.logger.debug("Password policy response: " + ctrl);
 			}
-
 			LdapUtils.closeContext(ctx);
-
+			if (ctrl != null && ctrl.isLocked()) {
-			if (ctrl != null) {
+				throw new PasswordPolicyException(ctrl.getErrorStatus());
-				if (ctrl.isLocked()) {
-					throw new PasswordPolicyException(ctrl.getErrorStatus());
-				}
 			}
-
+			throw LdapUtils.convertLdapException(ex);
-			throw LdapUtils.convertLdapException(ne);
 		}
-
+		this.logger.debug(
-		if (debug) {
+				LogMessage.of(() -> "PPolicy control returned: " + PasswordPolicyControlExtractor.extractControl(ctx)));
-			this.logger.debug("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;
 	}
 

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;

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;
 	}
 

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;
 	}
 

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",
+	PASSWORD_EXPIRED("ppolicy.expired", "Your password has expired"),
-			"Account is locked"), CHANGE_AFTER_RESET("ppolicy.change.after.reset",
+
-					"Your password must be changed after being reset"), PASSWORD_MOD_NOT_ALLOWED(
+	ACCOUNT_LOCKED("ppolicy.locked", "Account is locked"),
-							"ppolicy.mod.not.allowed",
+
-							"Password cannot be changed"), MUST_SUPPLY_OLD_PASSWORD("ppolicy.must.supply.old.password",
+	CHANGE_AFTER_RESET("ppolicy.change.after.reset", "Your password must be changed after being reset"),
-									"The old password must be supplied"), INSUFFICIENT_PASSWORD_QUALITY(
+
-											"ppolicy.insufficient.password.quality",
+	PASSWORD_MOD_NOT_ALLOWED("ppolicy.mod.not.allowed", "Password cannot be changed"),
-											"The supplied password is of insufficient quality"), PASSWORD_TOO_SHORT(
+
-													"ppolicy.password.too.short",
+	MUST_SUPPLY_OLD_PASSWORD("ppolicy.must.supply.old.password", "The old password must be supplied"),
-													"The supplied password is too short"), PASSWORD_TOO_YOUNG(
+
-															"ppolicy.password.too.young",
+	INSUFFICIENT_PASSWORD_QUALITY("ppolicy.insufficient.password.quality",
-															"Your password was changed too recently to be changed again"), PASSWORD_IN_HISTORY(
+			"The supplied password is of insufficient quality"),
-																	"ppolicy.password.in.history",
+
-																	"The supplied password has already been used");
+	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;
 

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;
-	// }
-	// }
-	// }
-
 }

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(LogMessage.of(() -> "Searching for user '" + username + "', with user search " + this));
-			logger.debug("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) {
+		catch (IncorrectResultSizeDataAccessException ex) {
-			if (notFound.getActualSize() == 0) {
+			if (ex.getActualSize() == 0) {
 				throw new UsernameNotFoundException("User " + username + " not found in directory.");
 			}
-			// Search should never return multiple results if properly configured, so just
+			// Search should never return multiple results if properly configured
-			// rethrow
+			throw ex;
-			throw notFound;
 		}
 	}
 
@@ -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(

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<Interceptor> 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) {
+		Assert.isTrue(!this.ldapOverSslEnabled || this.keyStoreFile != null,
-			throw new IllegalArgumentException("When LdapOverSsl is enabled, the keyStoreFile property must be set.");
+				"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());
-
+		Assert.isTrue(!workingDir.exists(),
-		if (workingDir.exists()) {
+				"The specified working directory '" + workingDir.getAbsolutePath()
-			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 "
-					+ "' 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 "
-					+ " previous unclean shutdown. Please confirm and delete it or configure a different "
+						+ "working directory");
-					+ "working directory");
-		}
-
 		this.workingDir = workingDir;
-
 		this.service.setWorkingDirectory(workingDir);
 	}
 
@@ -250,11 +230,7 @@ public class ApacheDSContainer implements InitializingBean, DisposableBean, Life
 		if (isRunning()) {
 			return;
 		}
-
+		Assert.state(!this.service.isStarted(), "DirectoryService is already running.");
-		if (this.service.isStarted()) {
-			throw new IllegalStateException("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;
+		Resource[] ldifs = (this.ctxt != null) ? this.ctxt.getResources(this.ldifResources)
-
+				: new PathMatchingResourcePatternResolver().getResources(this.ldifResources);
-		if (this.ctxt == null) {
-			// Not running within an app context
-			ldifs = new PathMatchingResourcePatternResolver().getResources(this.ldifResources);
-		}
-		else {
-			ldifs = this.ctxt.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) {
+	private String getLdifFile(Resource[] ldifs) throws IOException {
-			String ldifFile;
+		try {
-
+			return ldifs[0].getFile().getAbsolutePath();
-			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();
 		}
-		else {
+		catch (IOException ex) {
-			throw new IllegalArgumentException("More than one LDIF resource found with the supplied pattern:"
+			return ldifs[0].getURI().toString();
-					+ this.ldifResources + " Got " + Arrays.toString(ldifs));
 		}
 	}
 
@@ -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();
 	}
 

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) {

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<GrantedAuthority> getGrantedAuthorities(DirContextOperations user, String username) {
 		String userDn = user.getNameInNamespace();
-
+		logger.debug(LogMessage.format("Getting authorities for user %s", userDn));
-		if (logger.isDebugEnabled()) {
-			logger.debug("Getting authorities for user " + userDn);
-		}
-
 		Set<GrantedAuthority> roles = getGroupMembershipRoles(userDn, username);
-
 		Set<GrantedAuthority> extraRoles = getAdditionalRoles(user, username);
-
 		if (extraRoles != null) {
 			roles.addAll(extraRoles);
 		}
-
 		if (this.defaultRole != null) {
 			roles.add(this.defaultRole);
 		}
-
 		List<GrantedAuthority> 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<GrantedAuthority> authorities = new HashSet<>();
-
+		logger.debug(LogMessage.of(() -> "Searching for roles for user '" + username + "', DN = " + "'" + userDn
-		if (logger.isDebugEnabled()) {
+				+ "', with filter " + this.groupSearchFilter + " in search base '" + getGroupSearchBase() + "'"));
-			logger.debug("Searching for roles for user '" + username + "', DN = " + "'" + userDn + "', with filter "
-					+ this.groupSearchFilter + " in search base '" + getGroupSearchBase() + "'");
-		}
-
 		Set<Map<String, List<String>>> userRoles = getLdapTemplate().searchForMultipleAttributeValues(
 				getGroupSearchBase(), this.groupSearchFilter, new String[] { userDn, username },
 				new String[] { this.groupRoleAttribute });
-
+		logger.debug(LogMessage.of(() -> "Roles from search: " + userRoles));
-		if (logger.isDebugEnabled()) {
-			logger.debug("Roles from search: " + userRoles);
-		}
-
 		for (Map<String, List<String>> role : userRoles) {
 			authorities.add(this.authorityMapper.apply(role));
 		}
-
 		return authorities;
 	}
 

ldap/src/main/java/org/springframework/security/ldap/userdetails/InetOrgPerson.java

@@ -27,7 +27,7 @@ import org.springframework.security.core.SpringSecurityCoreVersion;
  * <p>
  * The username will be mapped from the <tt>uid</tt> attribute by default.
  *
- * @author Luke
+ * @author Luke Taylor
  */
 public class InetOrgPerson extends Person {
 

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<? extends GrantedAuthority> 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);
 	}

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<String, List<String>> 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) {
+		return (result != null) ? result : Collections.emptyList();
-			result = Collections.emptyList();
-		}
-		return result;
 	}
 
 	/**
@@ -100,17 +96,9 @@ public class LdapAuthority implements GrantedAuthority {
 	 */
 	public String getFirstAttributeValue(String name) {
 		List<String> result = getAttributeValues(name);
-		if (result.isEmpty()) {
+		return (!result.isEmpty()) ? result.get(0) : null;
-			return null;
-		}
-		else {
-			return result.get(0);
-		}
 	}
 
-	/**
-	 * {@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) {
+	public boolean equals(Object obj) {
-		if (this == o) {
+		if (this == obj) {
 			return true;
 		}
-		if (!(o instanceof LdapAuthority)) {
+		if (!(obj instanceof LdapAuthority)) {
 			return false;
 		}
-
+		LdapAuthority other = (LdapAuthority) obj;
-		LdapAuthority that = (LdapAuthority) o;
+		if (!this.dn.equals(other.dn)) {
-
-		if (!this.dn.equals(that.dn)) {
 			return false;
 		}
-		return this.role.equals(that.role);
+		return this.role.equals(other.role);
 	}
 
 	@Override

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;
 		}
 

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<GrantedAuthority> authorities = getUserAuthorities(dn, username);
-
+		this.logger.debug(LogMessage.format("Loading user '%s' with DN '%s'", username, dn));
-		this.logger.debug("Loading user '" + username + "' with DN '" + 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) {
+			catch (NameNotFoundException ex) {
-				throw new UsernameNotFoundException("User " + username + " not found", notFound);
+				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(LogMessage.format("Changing password for user '%s'", username));
-		this.logger.debug("Changing password for user '" + 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(LogMessage.format("Creating new user '%s' with DN '%s'", user.getUsername(), dn));
-		this.logger.debug("Creating new user '" + user.getUsername() + "' with DN '" + dn + "'");
-
 		this.template.bind(dn, ctx, null);
-
+		// Check for any existing authorities which might be set for this
-		// Check for any existing authorities which might be set for this DN and remove
+		// DN and remove them
-		// them
 		List<GrantedAuthority> 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(LogMessage.format("Updating new user '%s' with DN '%s'", user.getUsername(), dn));
-		this.logger.debug("Updating user '" + user.getUsername() + "' with DN '" + dn + "'");
-
 		List<GrantedAuthority> 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<ModificationItem> mods = new LinkedList<>(Arrays.asList(ctx.getModificationItems()));
 		ListIterator<ModificationItem> 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) {
-
+		ModificationItem[] passwordChange = new ModificationItem[] { new ModificationItem(DirContext.REPLACE_ATTRIBUTE,
-		final ModificationItem[] passwordChange = new ModificationItem[] { new ModificationItem(
+				new BasicAttribute(this.passwordAttributeName, newPassword)) };
-				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);
 			}

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<? extends GrantedAuthority> authorities) {
 		String dn = ctx.getNameInNamespace();
-
+		this.logger.debug(LogMessage.format("Mapping user details from context with DN: %s", dn));
-		this.logger.debug("Mapping user details from context with DN: " + 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;
 
 	}

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));
 	}

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<GrantedAuthority> getGroupMembershipRoles(String userDn, String username) {
 		if (getGroupSearchBase() == null) {
 			return new HashSet<>();
 		}
-
 		Set<GrantedAuthority> 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<GrantedAuthority> authorities, int depth) {
 		if (depth == 0) {
 			// back out of recursion
-			if (logger.isDebugEnabled()) {
+			logger.debug(LogMessage.of(() -> "Search aborted, max depth reached," + " for roles for user '" + username
-				logger.debug("Search aborted, max depth reached," + " for roles for user '" + username + "', DN = "
+					+ "', DN = " + "'" + userDn + "', with filter " + getGroupSearchFilter() + " in search base '"
-						+ "'" + userDn + "', with filter " + getGroupSearchFilter() + " in search base '"
+					+ getGroupSearchBase() + "'"));
-						+ getGroupSearchBase() + "'");
-			}
 			return;
 		}
-
+		logger.debug(LogMessage.of(() -> "Searching for roles for user '" + username + "', DN = " + "'" + userDn
-		if (logger.isDebugEnabled()) {
+				+ "', with filter " + getGroupSearchFilter() + " in search base '" + getGroupSearchBase() + "'"));
-			logger.debug("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<Map<String, List<String>>> userRoles = getLdapTemplate().searchForMultipleAttributeValues(
 				getGroupSearchBase(), getGroupSearchFilter(), new String[] { userDn, username },
 				getAttributeNames().toArray(new String[0]));
-
+		logger.debug(LogMessage.format("Roles from search: %s", userRoles));
-		if (logger.isDebugEnabled()) {
-			logger.debug("Roles from search: " + userRoles);
-		}
-
 		for (Map<String, List<String>> 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));
 			}
-
 		}
 	}
 

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");
+			Object password = ctx.getObjectAttribute("userPassword");
-
+			if (password != null) {
-			if (passo != null) {
+				setPassword(LdapUtils.convertPasswordToString(password));
-				String password = LdapUtils.convertPasswordToString(passo);
-				setPassword(password);
 			}
 		}
 

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<? extends GrantedAuthority> 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);
 	}