浏览代码

Polish spring-security-core main code

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

Issue gh-8945
Phillip Webb 5 年之前
父节点
当前提交
771ef0dadc
共有 100 个文件被更改,包括 680 次插入1226 次删除
  1. 0 4
      core/src/main/java/org/springframework/security/access/SecurityConfig.java
  2. 5 6
      core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java
  3. 0 4
      core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java
  4. 2 8
      core/src/main/java/org/springframework/security/access/annotation/SecuredAnnotationSecurityMetadataSource.java
  5. 5 7
      core/src/main/java/org/springframework/security/access/event/AuthenticationCredentialsNotFoundEvent.java
  6. 6 8
      core/src/main/java/org/springframework/security/access/event/AuthorizationFailureEvent.java
  7. 4 7
      core/src/main/java/org/springframework/security/access/event/AuthorizedEvent.java
  8. 29 29
      core/src/main/java/org/springframework/security/access/event/LoggerListener.java
  9. 4 5
      core/src/main/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandler.java
  10. 5 4
      core/src/main/java/org/springframework/security/access/expression/DenyAllPermissionEvaluator.java
  11. 6 7
      core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java
  12. 68 96
      core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java
  13. 4 14
      core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPostInvocationAdvice.java
  14. 12 27
      core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPreInvocationAdvice.java
  15. 0 4
      core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java
  16. 0 2
      core/src/main/java/org/springframework/security/access/expression/method/PreInvocationExpressionAttribute.java
  17. 13 29
      core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java
  18. 6 15
      core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java
  19. 53 101
      core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java
  20. 7 16
      core/src/main/java/org/springframework/security/access/intercept/AfterInvocationProviderManager.java
  21. 10 20
      core/src/main/java/org/springframework/security/access/intercept/MethodInvocationPrivilegeEvaluator.java
  22. 2 5
      core/src/main/java/org/springframework/security/access/intercept/RunAsImplAuthenticationProvider.java
  23. 0 4
      core/src/main/java/org/springframework/security/access/intercept/RunAsManagerImpl.java
  24. 0 1
      core/src/main/java/org/springframework/security/access/intercept/RunAsUserToken.java
  25. 0 1
      core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityInterceptor.java
  26. 4 7
      core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityMetadataSourceAdvisor.java
  27. 0 2
      core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptor.java
  28. 3 8
      core/src/main/java/org/springframework/security/access/intercept/aspectj/MethodInvocationAdapter.java
  29. 0 2
      core/src/main/java/org/springframework/security/access/method/AbstractFallbackMethodSecurityMetadataSource.java
  30. 0 2
      core/src/main/java/org/springframework/security/access/method/AbstractMethodSecurityMetadataSource.java
  31. 4 11
      core/src/main/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSource.java
  32. 27 42
      core/src/main/java/org/springframework/security/access/method/MapBasedMethodSecurityMetadataSource.java
  33. 4 7
      core/src/main/java/org/springframework/security/access/prepost/PostInvocationAdviceProvider.java
  34. 2 10
      core/src/main/java/org/springframework/security/access/prepost/PreInvocationAuthorizationAdviceVoter.java
  35. 4 12
      core/src/main/java/org/springframework/security/access/prepost/PrePostAdviceReactiveMethodInterceptor.java
  36. 7 21
      core/src/main/java/org/springframework/security/access/prepost/PrePostAnnotationSecurityMetadataSource.java
  37. 2 4
      core/src/main/java/org/springframework/security/access/vote/AbstractAccessDecisionManager.java
  38. 2 7
      core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java
  39. 3 11
      core/src/main/java/org/springframework/security/access/vote/AffirmativeBased.java
  40. 0 5
      core/src/main/java/org/springframework/security/access/vote/AuthenticatedVoter.java
  41. 5 18
      core/src/main/java/org/springframework/security/access/vote/ConsensusBased.java
  42. 0 3
      core/src/main/java/org/springframework/security/access/vote/RoleVoter.java
  43. 3 14
      core/src/main/java/org/springframework/security/access/vote/UnanimousBased.java
  44. 3 28
      core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java
  45. 28 23
      core/src/main/java/org/springframework/security/authentication/AbstractUserDetailsReactiveAuthenticationManager.java
  46. 0 3
      core/src/main/java/org/springframework/security/authentication/AccountStatusUserDetailsChecker.java
  47. 0 2
      core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationProvider.java
  48. 2 13
      core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java
  49. 0 2
      core/src/main/java/org/springframework/security/authentication/AuthenticationTrustResolverImpl.java
  50. 0 4
      core/src/main/java/org/springframework/security/authentication/CachingUserDetailsService.java
  51. 0 3
      core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java
  52. 6 22
      core/src/main/java/org/springframework/security/authentication/ProviderManager.java
  53. 11 8
      core/src/main/java/org/springframework/security/authentication/ReactiveAuthenticationManagerAdapter.java
  54. 0 2
      core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationProvider.java
  55. 2 9
      core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java
  56. 3 5
      core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java
  57. 19 39
      core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java
  58. 0 4
      core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java
  59. 15 15
      core/src/main/java/org/springframework/security/authentication/event/LoggerListener.java
  60. 38 45
      core/src/main/java/org/springframework/security/authentication/jaas/AbstractJaasAuthenticationProvider.java
  61. 2 11
      core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java
  62. 8 12
      core/src/main/java/org/springframework/security/authentication/jaas/JaasNameCallbackHandler.java
  63. 1 2
      core/src/main/java/org/springframework/security/authentication/jaas/JaasPasswordCallbackHandler.java
  64. 8 21
      core/src/main/java/org/springframework/security/authentication/jaas/SecurityContextLoginModule.java
  65. 2 3
      core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationManagerImpl.java
  66. 0 1
      core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProvider.java
  67. 5 1
      core/src/main/java/org/springframework/security/authorization/AuthenticatedReactiveAuthorizationManager.java
  68. 3 5
      core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java
  69. 2 2
      core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationManager.java
  70. 0 1
      core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextCallable.java
  71. 1 2
      core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextExecutor.java
  72. 3 6
      core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextExecutorService.java
  73. 0 1
      core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnable.java
  74. 4 8
      core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextScheduledExecutorService.java
  75. 2 4
      core/src/main/java/org/springframework/security/converter/RsaKeyConverters.java
  76. 5 10
      core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java
  77. 4 5
      core/src/main/java/org/springframework/security/core/authority/AuthorityUtils.java
  78. 0 2
      core/src/main/java/org/springframework/security/core/authority/SimpleGrantedAuthority.java
  79. 11 14
      core/src/main/java/org/springframework/security/core/authority/mapping/MapBasedAttributes2GrantedAuthoritiesMapper.java
  80. 0 4
      core/src/main/java/org/springframework/security/core/authority/mapping/SimpleAuthorityMapper.java
  81. 0 1
      core/src/main/java/org/springframework/security/core/context/GlobalSecurityContextHolderStrategy.java
  82. 0 2
      core/src/main/java/org/springframework/security/core/context/InheritableThreadLocalSecurityContextHolderStrategy.java
  83. 10 2
      core/src/main/java/org/springframework/security/core/context/ReactiveSecurityContextHolder.java
  84. 25 27
      core/src/main/java/org/springframework/security/core/context/SecurityContextHolder.java
  85. 6 15
      core/src/main/java/org/springframework/security/core/context/SecurityContextImpl.java
  86. 0 2
      core/src/main/java/org/springframework/security/core/context/ThreadLocalSecurityContextHolderStrategy.java
  87. 6 6
      core/src/main/java/org/springframework/security/core/parameters/AnnotationParameterNameDiscoverer.java
  88. 0 7
      core/src/main/java/org/springframework/security/core/parameters/DefaultSecurityParameterNameDiscoverer.java
  89. 9 36
      core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java
  90. 6 11
      core/src/main/java/org/springframework/security/core/token/KeyBasedPersistenceTokenService.java
  91. 4 7
      core/src/main/java/org/springframework/security/core/token/SecureRandomFactoryBean.java
  92. 9 4
      core/src/main/java/org/springframework/security/core/userdetails/MapReactiveUserDetailsService.java
  93. 9 21
      core/src/main/java/org/springframework/security/core/userdetails/User.java
  94. 5 20
      core/src/main/java/org/springframework/security/core/userdetails/cache/EhCacheBasedUserCache.java
  95. 5 18
      core/src/main/java/org/springframework/security/core/userdetails/cache/SpringCacheBasedUserCache.java
  96. 0 15
      core/src/main/java/org/springframework/security/core/userdetails/jdbc/JdbcDaoImpl.java
  97. 21 31
      core/src/main/java/org/springframework/security/core/userdetails/memory/UserAttributeEditor.java
  98. 22 15
      core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java
  99. 14 9
      core/src/main/java/org/springframework/security/jackson2/UserDeserializer.java
  100. 30 25
      core/src/main/java/org/springframework/security/jackson2/UsernamePasswordAuthenticationTokenDeserializer.java

+ 0 - 4
core/src/main/java/org/springframework/security/access/SecurityConfig.java

@@ -40,10 +40,8 @@ public class SecurityConfig implements ConfigAttribute {
 	public boolean equals(Object obj) {
 		if (obj instanceof ConfigAttribute) {
 			ConfigAttribute attr = (ConfigAttribute) obj;
-
 			return this.attrib.equals(attr.getAttribute());
 		}
-
 		return false;
 	}
 
@@ -69,11 +67,9 @@ public class SecurityConfig implements ConfigAttribute {
 	public static List<ConfigAttribute> createList(String... attributeNames) {
 		Assert.notNull(attributeNames, "You must supply an array of attribute names");
 		List<ConfigAttribute> attributes = new ArrayList<>(attributeNames.length);
-
 		for (String attribute : attributeNames) {
 			attributes.add(new SecurityConfig(attribute.trim()));
 		}
-
 		return attributes;
 	}
 

+ 5 - 6
core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java

@@ -76,18 +76,17 @@ public class Jsr250MethodSecurityMetadataSource extends AbstractFallbackMethodSe
 			return null;
 		}
 		List<ConfigAttribute> attributes = new ArrayList<>();
-
-		for (Annotation a : annotations) {
-			if (a instanceof DenyAll) {
+		for (Annotation annotation : annotations) {
+			if (annotation instanceof DenyAll) {
 				attributes.add(Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE);
 				return attributes;
 			}
-			if (a instanceof PermitAll) {
+			if (annotation instanceof PermitAll) {
 				attributes.add(Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE);
 				return attributes;
 			}
-			if (a instanceof RolesAllowed) {
-				RolesAllowed ra = (RolesAllowed) a;
+			if (annotation instanceof RolesAllowed) {
+				RolesAllowed ra = (RolesAllowed) annotation;
 
 				for (String allowed : ra.value()) {
 					String defaultedAllowed = getRoleWithDefaultPrefix(allowed);

+ 0 - 4
core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java

@@ -65,16 +65,13 @@ public class Jsr250Voter implements AccessDecisionVoter<Object> {
 	@Override
 	public int vote(Authentication authentication, Object object, Collection<ConfigAttribute> definition) {
 		boolean jsr250AttributeFound = false;
-
 		for (ConfigAttribute attribute : definition) {
 			if (Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE.equals(attribute)) {
 				return ACCESS_GRANTED;
 			}
-
 			if (Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE.equals(attribute)) {
 				return ACCESS_DENIED;
 			}
-
 			if (supports(attribute)) {
 				jsr250AttributeFound = true;
 				// Attempt to find a matching granted authority
@@ -85,7 +82,6 @@ public class Jsr250Voter implements AccessDecisionVoter<Object> {
 				}
 			}
 		}
-
 		return jsr250AttributeFound ? ACCESS_DENIED : ACCESS_ABSTAIN;
 	}
 

+ 2 - 8
core/src/main/java/org/springframework/security/access/annotation/SecuredAnnotationSecurityMetadataSource.java

@@ -74,12 +74,8 @@ public class SecuredAnnotationSecurityMetadataSource extends AbstractFallbackMet
 		return null;
 	}
 
-	private Collection<ConfigAttribute> processAnnotation(Annotation a) {
-		if (a == null) {
-			return null;
-		}
-
-		return this.annotationExtractor.extractAttributes(a);
+	private Collection<ConfigAttribute> processAnnotation(Annotation annotation) {
+		return (annotation != null) ? this.annotationExtractor.extractAttributes(annotation) : null;
 	}
 
 	static class SecuredAnnotationMetadataExtractor implements AnnotationMetadataExtractor<Secured> {
@@ -88,11 +84,9 @@ public class SecuredAnnotationSecurityMetadataSource extends AbstractFallbackMet
 		public Collection<ConfigAttribute> extractAttributes(Secured secured) {
 			String[] attributeTokens = secured.value();
 			List<ConfigAttribute> attributes = new ArrayList<>(attributeTokens.length);
-
 			for (String token : attributeTokens) {
 				attributes.add(new SecurityConfig(token));
 			}
-
 			return attributes;
 		}
 

+ 5 - 7
core/src/main/java/org/springframework/security/access/event/AuthenticationCredentialsNotFoundEvent.java

@@ -20,6 +20,7 @@ import java.util.Collection;
 
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
+import org.springframework.util.Assert;
 
 /**
  * Indicates a secure object invocation failed because the <code>Authentication</code>
@@ -29,9 +30,9 @@ import org.springframework.security.authentication.AuthenticationCredentialsNotF
  */
 public class AuthenticationCredentialsNotFoundEvent extends AbstractAuthorizationEvent {
 
-	private AuthenticationCredentialsNotFoundException credentialsNotFoundException;
+	private final AuthenticationCredentialsNotFoundException credentialsNotFoundException;
 
-	private Collection<ConfigAttribute> configAttribs;
+	private final Collection<ConfigAttribute> configAttribs;
 
 	/**
 	 * Construct the event.
@@ -44,11 +45,8 @@ public class AuthenticationCredentialsNotFoundEvent extends AbstractAuthorizatio
 	public AuthenticationCredentialsNotFoundEvent(Object secureObject, Collection<ConfigAttribute> attributes,
 			AuthenticationCredentialsNotFoundException credentialsNotFoundException) {
 		super(secureObject);
-
-		if ((attributes == null) || (credentialsNotFoundException == null)) {
-			throw new IllegalArgumentException("All parameters are required and cannot be null");
-		}
-
+		Assert.isTrue(attributes != null && credentialsNotFoundException != null,
+				"All parameters are required and cannot be null");
 		this.configAttribs = attributes;
 		this.credentialsNotFoundException = credentialsNotFoundException;
 	}

+ 6 - 8
core/src/main/java/org/springframework/security/access/event/AuthorizationFailureEvent.java

@@ -21,6 +21,7 @@ import java.util.Collection;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.core.Authentication;
+import org.springframework.util.Assert;
 
 /**
  * Indicates a secure object invocation failed because the principal could not be
@@ -36,11 +37,11 @@ import org.springframework.security.core.Authentication;
  */
 public class AuthorizationFailureEvent extends AbstractAuthorizationEvent {
 
-	private AccessDeniedException accessDeniedException;
+	private final AccessDeniedException accessDeniedException;
 
-	private Authentication authentication;
+	private final Authentication authentication;
 
-	private Collection<ConfigAttribute> configAttributes;
+	private final Collection<ConfigAttribute> configAttributes;
 
 	/**
 	 * Construct the event.
@@ -54,11 +55,8 @@ public class AuthorizationFailureEvent extends AbstractAuthorizationEvent {
 	public AuthorizationFailureEvent(Object secureObject, Collection<ConfigAttribute> attributes,
 			Authentication authentication, AccessDeniedException accessDeniedException) {
 		super(secureObject);
-
-		if ((attributes == null) || (authentication == null) || (accessDeniedException == null)) {
-			throw new IllegalArgumentException("All parameters are required and cannot be null");
-		}
-
+		Assert.isTrue(attributes != null && authentication != null && accessDeniedException != null,
+				"All parameters are required and cannot be null");
 		this.configAttributes = attributes;
 		this.authentication = authentication;
 		this.accessDeniedException = accessDeniedException;

+ 4 - 7
core/src/main/java/org/springframework/security/access/event/AuthorizedEvent.java

@@ -20,6 +20,7 @@ import java.util.Collection;
 
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.core.Authentication;
+import org.springframework.util.Assert;
 
 /**
  * Event indicating a secure object was invoked successfully.
@@ -31,9 +32,9 @@ import org.springframework.security.core.Authentication;
  */
 public class AuthorizedEvent extends AbstractAuthorizationEvent {
 
-	private Authentication authentication;
+	private final Authentication authentication;
 
-	private Collection<ConfigAttribute> configAttributes;
+	private final Collection<ConfigAttribute> configAttributes;
 
 	/**
 	 * Construct the event.
@@ -44,11 +45,7 @@ public class AuthorizedEvent extends AbstractAuthorizationEvent {
 	 */
 	public AuthorizedEvent(Object secureObject, Collection<ConfigAttribute> attributes, Authentication authentication) {
 		super(secureObject);
-
-		if ((attributes == null) || (authentication == null)) {
-			throw new IllegalArgumentException("All parameters are required and cannot be null");
-		}
-
+		Assert.isTrue(attributes != null && authentication != null, "All parameters are required and cannot be null");
 		this.configAttributes = attributes;
 		this.authentication = authentication;
 	}

+ 29 - 29
core/src/main/java/org/springframework/security/access/event/LoggerListener.java

@@ -20,6 +20,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.context.ApplicationListener;
+import org.springframework.core.log.LogMessage;
 
 /**
  * Outputs interceptor-related application events to Commons Logging.
@@ -37,42 +38,41 @@ public class LoggerListener implements ApplicationListener<AbstractAuthorization
 	@Override
 	public void onApplicationEvent(AbstractAuthorizationEvent event) {
 		if (event instanceof AuthenticationCredentialsNotFoundEvent) {
-			AuthenticationCredentialsNotFoundEvent authEvent = (AuthenticationCredentialsNotFoundEvent) event;
-
-			if (logger.isWarnEnabled()) {
-				logger.warn("Security interception failed due to: " + authEvent.getCredentialsNotFoundException()
-						+ "; secure object: " + authEvent.getSource() + "; configuration attributes: "
-						+ authEvent.getConfigAttributes());
-			}
+			onAuthenticationCredentialsNotFoundEvent((AuthenticationCredentialsNotFoundEvent) event);
 		}
-
 		if (event instanceof AuthorizationFailureEvent) {
-			AuthorizationFailureEvent authEvent = (AuthorizationFailureEvent) event;
-
-			if (logger.isWarnEnabled()) {
-				logger.warn("Security authorization failed due to: " + authEvent.getAccessDeniedException()
-						+ "; authenticated principal: " + authEvent.getAuthentication() + "; secure object: "
-						+ authEvent.getSource() + "; configuration attributes: " + authEvent.getConfigAttributes());
-			}
+			onAuthorizationFailureEvent((AuthorizationFailureEvent) event);
 		}
-
 		if (event instanceof AuthorizedEvent) {
-			AuthorizedEvent authEvent = (AuthorizedEvent) event;
-
-			if (logger.isInfoEnabled()) {
-				logger.info("Security authorized for authenticated principal: " + authEvent.getAuthentication()
-						+ "; secure object: " + authEvent.getSource() + "; configuration attributes: "
-						+ authEvent.getConfigAttributes());
-			}
+			onAuthorizedEvent((AuthorizedEvent) event);
 		}
-
 		if (event instanceof PublicInvocationEvent) {
-			PublicInvocationEvent authEvent = (PublicInvocationEvent) event;
-
-			if (logger.isInfoEnabled()) {
-				logger.info("Security interception not required for public secure object: " + authEvent.getSource());
-			}
+			onPublicInvocationEvent((PublicInvocationEvent) event);
 		}
 	}
 
+	private void onAuthenticationCredentialsNotFoundEvent(AuthenticationCredentialsNotFoundEvent authEvent) {
+		logger.warn(LogMessage.format(
+				"Security interception failed due to: %s; secure object: %s; configuration attributes: %s",
+				authEvent.getCredentialsNotFoundException(), authEvent.getSource(), authEvent.getConfigAttributes()));
+	}
+
+	private void onPublicInvocationEvent(PublicInvocationEvent event) {
+		logger.info(LogMessage.format("Security interception not required for public secure object: %s",
+				event.getSource()));
+	}
+
+	private void onAuthorizedEvent(AuthorizedEvent authEvent) {
+		logger.info(LogMessage.format(
+				"Security authorized for authenticated principal: %s; secure object: %s; configuration attributes: %s",
+				authEvent.getAuthentication(), authEvent.getSource(), authEvent.getConfigAttributes()));
+	}
+
+	private void onAuthorizationFailureEvent(AuthorizationFailureEvent authEvent) {
+		logger.warn(LogMessage.format(
+				"Security authorization failed due to: %s; authenticated principal: %s; secure object: %s; configuration attributes: %s",
+				authEvent.getAccessDeniedException(), authEvent.getAuthentication(), authEvent.getSource(),
+				authEvent.getConfigAttributes()));
+	}
+
 }

+ 4 - 5
core/src/main/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandler.java

@@ -42,7 +42,7 @@ public abstract class AbstractSecurityExpressionHandler<T>
 
 	private ExpressionParser expressionParser = new SpelExpressionParser();
 
-	private BeanResolver br;
+	private BeanResolver beanResolver;
 
 	private RoleHierarchy roleHierarchy;
 
@@ -70,9 +70,8 @@ public abstract class AbstractSecurityExpressionHandler<T>
 	public final EvaluationContext createEvaluationContext(Authentication authentication, T invocation) {
 		SecurityExpressionOperations root = createSecurityExpressionRoot(authentication, invocation);
 		StandardEvaluationContext ctx = createEvaluationContextInternal(authentication, invocation);
-		ctx.setBeanResolver(this.br);
+		ctx.setBeanResolver(this.beanResolver);
 		ctx.setRootObject(root);
-
 		return ctx;
 	}
 
@@ -96,7 +95,7 @@ public abstract class AbstractSecurityExpressionHandler<T>
 	 * invocation type.
 	 * @param authentication the current authentication object
 	 * @param invocation the invocation (filter, method, channel)
-	 * @return the object wh
+	 * @return the object
 	 */
 	protected abstract SecurityExpressionOperations createSecurityExpressionRoot(Authentication authentication,
 			T invocation);
@@ -119,7 +118,7 @@ public abstract class AbstractSecurityExpressionHandler<T>
 
 	@Override
 	public void setApplicationContext(ApplicationContext applicationContext) {
-		this.br = new BeanFactoryResolver(applicationContext);
+		this.beanResolver = new BeanFactoryResolver(applicationContext);
 	}
 
 }

+ 5 - 4
core/src/main/java/org/springframework/security/access/expression/DenyAllPermissionEvaluator.java

@@ -21,6 +21,7 @@ import java.io.Serializable;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.PermissionEvaluator;
 import org.springframework.security.core.Authentication;
 
@@ -40,8 +41,8 @@ public class DenyAllPermissionEvaluator implements PermissionEvaluator {
 	 */
 	@Override
 	public boolean hasPermission(Authentication authentication, Object target, Object permission) {
-		this.logger.warn(
-				"Denying user " + authentication.getName() + " permission '" + permission + "' on object " + target);
+		this.logger.warn(LogMessage.format("Denying user %s permission '%s' on object %s", authentication.getName(),
+				permission, target));
 		return false;
 	}
 
@@ -51,8 +52,8 @@ public class DenyAllPermissionEvaluator implements PermissionEvaluator {
 	@Override
 	public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType,
 			Object permission) {
-		this.logger.warn("Denying user " + authentication.getName() + " permission '" + permission
-				+ "' on object with Id '" + targetId);
+		this.logger.warn(LogMessage.format("Denying user %s permission '%s' on object with Id %s",
+				authentication.getName(), permission, targetId));
 		return false;
 	}
 

+ 6 - 7
core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java

@@ -45,10 +45,14 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
 
 	private String defaultRolePrefix = "ROLE_";
 
-	/** Allows "permitAll" expression */
+	/**
+	 * Allows "permitAll" expression
+	 */
 	public final boolean permitAll = true;
 
-	/** Allows "denyAll" expression */
+	/**
+	 * Allows "denyAll" expression
+	 */
 	public final boolean denyAll = false;
 
 	private PermissionEvaluator permissionEvaluator;
@@ -96,14 +100,12 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
 
 	private boolean hasAnyAuthorityName(String prefix, String... roles) {
 		Set<String> roleSet = getAuthoritySet();
-
 		for (String role : roles) {
 			String defaultedRole = getRoleWithDefaultPrefix(prefix, role);
 			if (roleSet.contains(defaultedRole)) {
 				return true;
 			}
 		}
-
 		return false;
 	}
 
@@ -180,14 +182,11 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
 	private Set<String> getAuthoritySet() {
 		if (this.roles == null) {
 			Collection<? extends GrantedAuthority> userAuthorities = this.authentication.getAuthorities();
-
 			if (this.roleHierarchy != null) {
 				userAuthorities = this.roleHierarchy.getReachableGrantedAuthorities(userAuthorities);
 			}
-
 			this.roles = AuthorityUtils.authorityListToSet(userAuthorities);
 		}
-
 		return this.roles;
 	}
 

+ 68 - 96
core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java

@@ -30,6 +30,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.core.ParameterNameDiscoverer;
+import org.springframework.core.log.LogMessage;
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.Expression;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -88,7 +89,6 @@ public class DefaultMethodSecurityExpressionHandler extends AbstractSecurityExpr
 		root.setTrustResolver(getTrustResolver());
 		root.setRoleHierarchy(getRoleHierarchy());
 		root.setDefaultRolePrefix(getDefaultRolePrefix());
-
 		return root;
 	}
 
@@ -101,117 +101,89 @@ public class DefaultMethodSecurityExpressionHandler extends AbstractSecurityExpr
 	 * {@code true}. For an array, a new array instance will be returned.
 	 */
 	@Override
-	@SuppressWarnings("unchecked")
 	public Object filter(Object filterTarget, Expression filterExpression, EvaluationContext ctx) {
 		MethodSecurityExpressionOperations rootObject = (MethodSecurityExpressionOperations) ctx.getRootObject()
 				.getValue();
-		final boolean debug = this.logger.isDebugEnabled();
-		List retainList;
-
-		if (debug) {
-			this.logger.debug("Filtering with expression: " + filterExpression.getExpressionString());
-		}
-
+		this.logger.debug(LogMessage.format("Filtering with expression: %s", filterExpression.getExpressionString()));
 		if (filterTarget instanceof Collection) {
-			Collection collection = (Collection) filterTarget;
-			retainList = new ArrayList(collection.size());
-
-			if (debug) {
-				this.logger.debug("Filtering collection with " + collection.size() + " elements");
-			}
-
-			if (this.permissionCacheOptimizer != null) {
-				this.permissionCacheOptimizer.cachePermissionsFor(rootObject.getAuthentication(), collection);
-			}
-
-			for (Object filterObject : (Collection) filterTarget) {
-				rootObject.setFilterObject(filterObject);
-
-				if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
-					retainList.add(filterObject);
-				}
-			}
-
-			if (debug) {
-				this.logger.debug("Retaining elements: " + retainList);
-			}
-
-			collection.clear();
-			collection.addAll(retainList);
-
-			return filterTarget;
+			return filterCollection((Collection<?>) filterTarget, filterExpression, ctx, rootObject);
 		}
-
 		if (filterTarget.getClass().isArray()) {
-			Object[] array = (Object[]) filterTarget;
-			retainList = new ArrayList(array.length);
-
-			if (debug) {
-				this.logger.debug("Filtering array with " + array.length + " elements");
-			}
-
-			if (this.permissionCacheOptimizer != null) {
-				this.permissionCacheOptimizer.cachePermissionsFor(rootObject.getAuthentication(), Arrays.asList(array));
-			}
-
-			for (Object o : array) {
-				rootObject.setFilterObject(o);
-
-				if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
-					retainList.add(o);
-				}
-			}
-
-			if (debug) {
-				this.logger.debug("Retaining elements: " + retainList);
-			}
-
-			Object[] filtered = (Object[]) Array.newInstance(filterTarget.getClass().getComponentType(),
-					retainList.size());
-			for (int i = 0; i < retainList.size(); i++) {
-				filtered[i] = retainList.get(i);
-			}
-
-			return filtered;
+			return filterArray((Object[]) filterTarget, filterExpression, ctx, rootObject);
 		}
-
 		if (filterTarget instanceof Map) {
-			final Map<?, ?> map = (Map<?, ?>) filterTarget;
-			final Map retainMap = new LinkedHashMap(map.size());
-
-			if (debug) {
-				this.logger.debug("Filtering map with " + map.size() + " elements");
-			}
-
-			for (Map.Entry<?, ?> filterObject : map.entrySet()) {
-				rootObject.setFilterObject(filterObject);
+			return filterMap((Map<?, ?>) filterTarget, filterExpression, ctx, rootObject);
+		}
+		if (filterTarget instanceof Stream) {
+			return filterStream((Stream<?>) filterTarget, filterExpression, ctx, rootObject);
+		}
+		throw new IllegalArgumentException(
+				"Filter target must be a collection, array, map or stream type, but was " + filterTarget);
+	}
 
-				if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
-					retainMap.put(filterObject.getKey(), filterObject.getValue());
-				}
+	private <T> Object filterCollection(Collection<T> filterTarget, Expression filterExpression, EvaluationContext ctx,
+			MethodSecurityExpressionOperations rootObject) {
+		this.logger.debug(LogMessage.format("Filtering collection with %s elements", filterTarget.size()));
+		List<T> retain = new ArrayList<>(filterTarget.size());
+		if (this.permissionCacheOptimizer != null) {
+			this.permissionCacheOptimizer.cachePermissionsFor(rootObject.getAuthentication(), filterTarget);
+		}
+		for (T filterObject : filterTarget) {
+			rootObject.setFilterObject(filterObject);
+			if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
+				retain.add(filterObject);
 			}
+		}
+		this.logger.debug(LogMessage.format("Retaining elements: %s", retain));
+		filterTarget.clear();
+		filterTarget.addAll(retain);
+		return filterTarget;
+	}
 
-			if (debug) {
-				this.logger.debug("Retaining elements: " + retainMap);
+	private Object filterArray(Object[] filterTarget, Expression filterExpression, EvaluationContext ctx,
+			MethodSecurityExpressionOperations rootObject) {
+		List<Object> retain = new ArrayList<>(filterTarget.length);
+		this.logger.debug(LogMessage.format("Filtering array with %s elements", filterTarget.length));
+		if (this.permissionCacheOptimizer != null) {
+			this.permissionCacheOptimizer.cachePermissionsFor(rootObject.getAuthentication(),
+					Arrays.asList(filterTarget));
+		}
+		for (Object filterObject : filterTarget) {
+			rootObject.setFilterObject(filterObject);
+			if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
+				retain.add(filterObject);
 			}
-
-			map.clear();
-			map.putAll(retainMap);
-
-			return filterTarget;
 		}
+		this.logger.debug(LogMessage.format("Retaining elements: %s", retain));
+		Object[] filtered = (Object[]) Array.newInstance(filterTarget.getClass().getComponentType(), retain.size());
+		for (int i = 0; i < retain.size(); i++) {
+			filtered[i] = retain.get(i);
+		}
+		return filtered;
+	}
 
-		if (filterTarget instanceof Stream) {
-			final Stream<?> original = (Stream<?>) filterTarget;
-
-			return original.filter((filterObject) -> {
-				rootObject.setFilterObject(filterObject);
-				return ExpressionUtils.evaluateAsBoolean(filterExpression, ctx);
-			}).onClose(original::close);
+	private <K, V> Object filterMap(final Map<K, V> filterTarget, Expression filterExpression, EvaluationContext ctx,
+			MethodSecurityExpressionOperations rootObject) {
+		Map<K, V> retain = new LinkedHashMap<>(filterTarget.size());
+		this.logger.debug(LogMessage.format("Filtering map with %s elements", filterTarget.size()));
+		for (Map.Entry<K, V> filterObject : filterTarget.entrySet()) {
+			rootObject.setFilterObject(filterObject);
+			if (ExpressionUtils.evaluateAsBoolean(filterExpression, ctx)) {
+				retain.put(filterObject.getKey(), filterObject.getValue());
+			}
 		}
+		this.logger.debug(LogMessage.format("Retaining elements: %s", retain));
+		filterTarget.clear();
+		filterTarget.putAll(retain);
+		return filterTarget;
+	}
 
-		throw new IllegalArgumentException(
-				"Filter target must be a collection, array, map or stream type, but was " + filterTarget);
+	private Object filterStream(final Stream<?> filterTarget, Expression filterExpression, EvaluationContext ctx,
+			MethodSecurityExpressionOperations rootObject) {
+		return filterTarget.filter((filterObject) -> {
+			rootObject.setFilterObject(filterObject);
+			return ExpressionUtils.evaluateAsBoolean(filterExpression, ctx);
+		}).onClose(filterTarget::close);
 	}
 
 	/**

+ 4 - 14
core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPostInvocationAdvice.java

@@ -20,6 +20,7 @@ import org.aopalliance.intercept.MethodInvocation;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.Expression;
 import org.springframework.security.access.AccessDeniedException;
@@ -49,31 +50,20 @@ public class ExpressionBasedPostInvocationAdvice implements PostInvocationAuthor
 		EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication, mi);
 		Expression postFilter = pia.getFilterExpression();
 		Expression postAuthorize = pia.getAuthorizeExpression();
-
 		if (postFilter != null) {
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Applying PostFilter expression " + postFilter);
-			}
-
+			this.logger.debug(LogMessage.format("Applying PostFilter expression %s", postFilter));
 			if (returnedObject != null) {
 				returnedObject = this.expressionHandler.filter(returnedObject, postFilter, ctx);
 			}
 			else {
-				if (this.logger.isDebugEnabled()) {
-					this.logger.debug("Return object is null, filtering will be skipped");
-				}
+				this.logger.debug("Return object is null, filtering will be skipped");
 			}
 		}
-
 		this.expressionHandler.setReturnObject(returnedObject, ctx);
-
 		if (postAuthorize != null && !ExpressionUtils.evaluateAsBoolean(postAuthorize, ctx)) {
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("PostAuthorize expression rejected access");
-			}
+			this.logger.debug("PostAuthorize expression rejected access");
 			throw new AccessDeniedException("Access is denied");
 		}
-
 		return returnedObject;
 	}
 

+ 12 - 27
core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedPreInvocationAdvice.java

@@ -26,6 +26,7 @@ import org.springframework.security.access.expression.ExpressionUtils;
 import org.springframework.security.access.prepost.PreInvocationAttribute;
 import org.springframework.security.access.prepost.PreInvocationAuthorizationAdvice;
 import org.springframework.security.core.Authentication;
+import org.springframework.util.Assert;
 
 /**
  * Method pre-invocation handling based on expressions.
@@ -43,50 +44,34 @@ public class ExpressionBasedPreInvocationAdvice implements PreInvocationAuthoriz
 		EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication, mi);
 		Expression preFilter = preAttr.getFilterExpression();
 		Expression preAuthorize = preAttr.getAuthorizeExpression();
-
 		if (preFilter != null) {
 			Object filterTarget = findFilterTarget(preAttr.getFilterTarget(), ctx, mi);
-
 			this.expressionHandler.filter(filterTarget, preFilter, ctx);
 		}
-
-		if (preAuthorize == null) {
-			return true;
-		}
-
-		return ExpressionUtils.evaluateAsBoolean(preAuthorize, ctx);
+		return (preAuthorize != null) ? ExpressionUtils.evaluateAsBoolean(preAuthorize, ctx) : true;
 	}
 
-	private Object findFilterTarget(String filterTargetName, EvaluationContext ctx, MethodInvocation mi) {
+	private Object findFilterTarget(String filterTargetName, EvaluationContext ctx, MethodInvocation invocation) {
 		Object filterTarget = null;
-
 		if (filterTargetName.length() > 0) {
 			filterTarget = ctx.lookupVariable(filterTargetName);
-			if (filterTarget == null) {
-				throw new IllegalArgumentException(
-						"Filter target was null, or no argument with name " + filterTargetName + " found in method");
-			}
+			Assert.notNull(filterTarget,
+					() -> "Filter target was null, or no argument with name " + filterTargetName + " found in method");
 		}
-		else if (mi.getArguments().length == 1) {
-			Object arg = mi.getArguments()[0];
+		else if (invocation.getArguments().length == 1) {
+			Object arg = invocation.getArguments()[0];
 			if (arg.getClass().isArray() || arg instanceof Collection<?>) {
 				filterTarget = arg;
 			}
-			if (filterTarget == null) {
-				throw new IllegalArgumentException("A PreFilter expression was set but the method argument type"
-						+ arg.getClass() + " is not filterable");
-			}
+			Assert.notNull(filterTarget, () -> "A PreFilter expression was set but the method argument type"
+					+ arg.getClass() + " is not filterable");
 		}
-		else if (mi.getArguments().length > 1) {
+		else if (invocation.getArguments().length > 1) {
 			throw new IllegalArgumentException(
 					"Unable to determine the method argument for filtering. Specify the filter target.");
 		}
-
-		if (filterTarget.getClass().isArray()) {
-			throw new IllegalArgumentException(
-					"Pre-filtering on array types is not supported. " + "Using a Collection will solve this problem");
-		}
-
+		Assert.isTrue(!filterTarget.getClass().isArray(),
+				"Pre-filtering on array types is not supported. Using a Collection will solve this problem");
 		return filterTarget;
 	}
 

+ 0 - 4
core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java

@@ -19,8 +19,6 @@ package org.springframework.security.access.expression.method;
 import java.lang.reflect.Method;
 
 import org.aopalliance.intercept.MethodInvocation;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 
 import org.springframework.aop.framework.AopProxyUtils;
 import org.springframework.aop.support.AopUtils;
@@ -40,8 +38,6 @@ import org.springframework.security.core.parameters.DefaultSecurityParameterName
  */
 class MethodSecurityEvaluationContext extends MethodBasedEvaluationContext {
 
-	private static final Log logger = LogFactory.getLog(MethodSecurityEvaluationContext.class);
-
 	/**
 	 * Intended for testing. Don't use in practice as it creates a new parameter resolver
 	 * for each instance. Use the constructor which takes the resolver, as an argument

+ 0 - 2
core/src/main/java/org/springframework/security/access/expression/method/PreInvocationExpressionAttribute.java

@@ -32,14 +32,12 @@ class PreInvocationExpressionAttribute extends AbstractExpressionBasedMethodConf
 	PreInvocationExpressionAttribute(String filterExpression, String filterTarget, String authorizeExpression)
 			throws ParseException {
 		super(filterExpression, authorizeExpression);
-
 		this.filterTarget = filterTarget;
 	}
 
 	PreInvocationExpressionAttribute(Expression filterExpression, String filterTarget, Expression authorizeExpression)
 			throws ParseException {
 		super(filterExpression, authorizeExpression);
-
 		this.filterTarget = filterTarget;
 	}
 

+ 13 - 29
core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java

@@ -20,13 +20,13 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 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.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
@@ -109,11 +109,8 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 	 */
 	public void setHierarchy(String roleHierarchyStringRepresentation) {
 		this.roleHierarchyStringRepresentation = roleHierarchyStringRepresentation;
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("setHierarchy() - The following role hierarchy was set: " + roleHierarchyStringRepresentation);
-		}
-
+		logger.debug(LogMessage.format("setHierarchy() - The following role hierarchy was set: %s",
+				roleHierarchyStringRepresentation));
 		buildRolesReachableInOneStepMap();
 		buildRolesReachableInOneOrMoreStepsMap();
 	}
@@ -124,10 +121,8 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 		if (authorities == null || authorities.isEmpty()) {
 			return AuthorityUtils.NO_AUTHORITIES;
 		}
-
 		Set<GrantedAuthority> reachableRoles = new HashSet<>();
 		Set<String> processedNames = new HashSet<>();
-
 		for (GrantedAuthority authority : authorities) {
 			// Do not process authorities without string representation
 			if (authority.getAuthority() == null) {
@@ -151,16 +146,10 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 				}
 			}
 		}
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("getReachableGrantedAuthorities() - From the roles " + authorities + " one can reach "
-					+ reachableRoles + " in zero or more steps.");
-		}
-
-		List<GrantedAuthority> reachableRoleList = new ArrayList<>(reachableRoles.size());
-		reachableRoleList.addAll(reachableRoles);
-
-		return reachableRoleList;
+		logger.debug(LogMessage.format(
+				"getReachableGrantedAuthorities() - From the roles %s one can reach %s in zero or more steps.",
+				authorities, reachableRoles));
+		return new ArrayList<>(reachableRoles);
 	}
 
 	/**
@@ -172,11 +161,9 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 		for (String line : this.roleHierarchyStringRepresentation.split("\n")) {
 			// Split on > and trim excessive whitespace
 			String[] roles = line.trim().split("\\s+>\\s+");
-
 			for (int i = 1; i < roles.length; i++) {
 				String higherRole = roles[i - 1];
 				GrantedAuthority lowerRole = new SimpleGrantedAuthority(roles[i]);
-
 				Set<GrantedAuthority> rolesReachableInOneStepSet;
 				if (!this.rolesReachableInOneStepMap.containsKey(higherRole)) {
 					rolesReachableInOneStepSet = new HashSet<>();
@@ -186,11 +173,9 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 					rolesReachableInOneStepSet = this.rolesReachableInOneStepMap.get(higherRole);
 				}
 				rolesReachableInOneStepSet.add(lowerRole);
-
-				if (logger.isDebugEnabled()) {
-					logger.debug("buildRolesReachableInOneStepMap() - From role " + higherRole + " one can reach role "
-							+ lowerRole + " in one step.");
-				}
+				logger.debug(LogMessage.format(
+						"buildRolesReachableInOneStepMap() - From role %s one can reach role %s in one step.",
+						higherRole, lowerRole));
 			}
 		}
 	}
@@ -207,7 +192,6 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 		for (String roleName : this.rolesReachableInOneStepMap.keySet()) {
 			Set<GrantedAuthority> rolesToVisitSet = new HashSet<>(this.rolesReachableInOneStepMap.get(roleName));
 			Set<GrantedAuthority> visitedRolesSet = new HashSet<>();
-
 			while (!rolesToVisitSet.isEmpty()) {
 				// take a role from the rolesToVisit set
 				GrantedAuthority lowerRole = rolesToVisitSet.iterator().next();
@@ -222,9 +206,9 @@ public class RoleHierarchyImpl implements RoleHierarchy {
 				rolesToVisitSet.addAll(this.rolesReachableInOneStepMap.get(lowerRole.getAuthority()));
 			}
 			this.rolesReachableInOneOrMoreStepsMap.put(roleName, visitedRolesSet);
-
-			logger.debug("buildRolesReachableInOneOrMoreStepsMap() - From role " + roleName + " one can reach "
-					+ visitedRolesSet + " in one or more steps.");
+			logger.debug(LogMessage.format(
+					"buildRolesReachableInOneOrMoreStepsMap() - From role %s one can reach %s in one or more steps.",
+					roleName, visitedRolesSet));
 		}
 
 	}

+ 6 - 15
core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java

@@ -43,28 +43,19 @@ public final class RoleHierarchyUtils {
 	 * @return a string representation of a role hierarchy
 	 * @throws IllegalArgumentException if roleHierarchyMap is null or empty or if a role
 	 * name is null or empty or if an implied role name(s) is null or empty
-	 *
 	 */
 	public static String roleHierarchyFromMap(Map<String, List<String>> roleHierarchyMap) {
 		Assert.notEmpty(roleHierarchyMap, "roleHierarchyMap cannot be empty");
-
-		StringWriter roleHierarchyBuffer = new StringWriter();
-		PrintWriter roleHierarchyWriter = new PrintWriter(roleHierarchyBuffer);
-
-		for (Map.Entry<String, List<String>> roleHierarchyEntry : roleHierarchyMap.entrySet()) {
-			String role = roleHierarchyEntry.getKey();
-			List<String> impliedRoles = roleHierarchyEntry.getValue();
-
+		StringWriter result = new StringWriter();
+		PrintWriter writer = new PrintWriter(result);
+		roleHierarchyMap.forEach((role, impliedRoles) -> {
 			Assert.hasLength(role, "role name must be supplied");
 			Assert.notEmpty(impliedRoles, "implied role name(s) cannot be empty");
-
 			for (String impliedRole : impliedRoles) {
-				String roleMapping = role + " > " + impliedRole;
-				roleHierarchyWriter.println(roleMapping);
+				writer.println(role + " > " + impliedRole);
 			}
-		}
-
-		return roleHierarchyBuffer.toString();
+		});
+		return result.toString();
 	}
 
 }

+ 53 - 101
core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java

@@ -30,6 +30,7 @@ import org.springframework.context.ApplicationEventPublisherAware;
 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.security.access.AccessDecisionManager;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
@@ -47,6 +48,7 @@ import org.springframework.security.core.SpringSecurityMessageSource;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 
 /**
  * Abstract class that implements security interception for secure objects.
@@ -141,120 +143,91 @@ public abstract class AbstractSecurityInterceptor
 				() -> "RunAsManager does not support secure object class: " + getSecureObjectClass());
 		Assert.isTrue(this.accessDecisionManager.supports(getSecureObjectClass()),
 				() -> "AccessDecisionManager does not support secure object class: " + getSecureObjectClass());
-
 		if (this.afterInvocationManager != null) {
 			Assert.isTrue(this.afterInvocationManager.supports(getSecureObjectClass()),
 					() -> "AfterInvocationManager does not support secure object class: " + getSecureObjectClass());
 		}
-
 		if (this.validateConfigAttributes) {
 			Collection<ConfigAttribute> attributeDefs = this.obtainSecurityMetadataSource().getAllConfigAttributes();
-
 			if (attributeDefs == null) {
-				this.logger.warn(
-						"Could not validate configuration attributes as the SecurityMetadataSource did not return "
-								+ "any attributes from getAllConfigAttributes()");
+				this.logger.warn("Could not validate configuration attributes as the "
+						+ "SecurityMetadataSource did not return any attributes from getAllConfigAttributes()");
 				return;
 			}
+			validateAttributeDefs(attributeDefs);
+		}
+	}
 
-			Set<ConfigAttribute> unsupportedAttrs = new HashSet<>();
-
-			for (ConfigAttribute attr : attributeDefs) {
-				if (!this.runAsManager.supports(attr) && !this.accessDecisionManager.supports(attr)
-						&& ((this.afterInvocationManager == null) || !this.afterInvocationManager.supports(attr))) {
-					unsupportedAttrs.add(attr);
-				}
-			}
-
-			if (unsupportedAttrs.size() != 0) {
-				throw new IllegalArgumentException("Unsupported configuration attributes: " + unsupportedAttrs);
+	private void validateAttributeDefs(Collection<ConfigAttribute> attributeDefs) {
+		Set<ConfigAttribute> unsupportedAttrs = new HashSet<>();
+		for (ConfigAttribute attr : attributeDefs) {
+			if (!this.runAsManager.supports(attr) && !this.accessDecisionManager.supports(attr)
+					&& ((this.afterInvocationManager == null) || !this.afterInvocationManager.supports(attr))) {
+				unsupportedAttrs.add(attr);
 			}
-
-			this.logger.debug("Validated configuration attributes");
 		}
+		if (unsupportedAttrs.size() != 0) {
+			throw new IllegalArgumentException("Unsupported configuration attributes: " + unsupportedAttrs);
+		}
+		this.logger.debug("Validated configuration attributes");
 	}
 
 	protected InterceptorStatusToken beforeInvocation(Object object) {
 		Assert.notNull(object, "Object was null");
-		final boolean debug = this.logger.isDebugEnabled();
-
 		if (!getSecureObjectClass().isAssignableFrom(object.getClass())) {
 			throw new IllegalArgumentException("Security invocation attempted for object " + object.getClass().getName()
 					+ " but AbstractSecurityInterceptor only configured to support secure objects of type: "
 					+ getSecureObjectClass());
 		}
-
 		Collection<ConfigAttribute> attributes = this.obtainSecurityMetadataSource().getAttributes(object);
-
-		if (attributes == null || attributes.isEmpty()) {
-			if (this.rejectPublicInvocations) {
-				throw new IllegalArgumentException("Secure object invocation " + object
-						+ " was denied as public invocations are not allowed via this interceptor. "
-						+ "This indicates a configuration error because the "
-						+ "rejectPublicInvocations property is set to 'true'");
-			}
-
-			if (debug) {
-				this.logger.debug("Public object - authentication not attempted");
-			}
-
+		if (CollectionUtils.isEmpty(attributes)) {
+			Assert.isTrue(!this.rejectPublicInvocations,
+					() -> "Secure object invocation " + object
+							+ " was denied as public invocations are not allowed via this interceptor. "
+							+ "This indicates a configuration error because the "
+							+ "rejectPublicInvocations property is set to 'true'");
+			this.logger.debug("Public object - authentication not attempted");
 			publishEvent(new PublicInvocationEvent(object));
-
 			return null; // no further work post-invocation
 		}
-
-		if (debug) {
-			this.logger.debug("Secure object: " + object + "; Attributes: " + attributes);
-		}
-
+		this.logger.debug(LogMessage.format("Secure object: %s; Attributes: %s", object, attributes));
 		if (SecurityContextHolder.getContext().getAuthentication() == null) {
 			credentialsNotFound(this.messages.getMessage("AbstractSecurityInterceptor.authenticationNotFound",
 					"An Authentication object was not found in the SecurityContext"), object, attributes);
 		}
-
 		Authentication authenticated = authenticateIfRequired();
-
 		// Attempt authorization
-		try {
-			this.accessDecisionManager.decide(authenticated, object, attributes);
-		}
-		catch (AccessDeniedException accessDeniedException) {
-			publishEvent(new AuthorizationFailureEvent(object, attributes, authenticated, accessDeniedException));
-
-			throw accessDeniedException;
-		}
-
-		if (debug) {
-			this.logger.debug("Authorization successful");
-		}
-
+		attemptAuthorization(object, attributes, authenticated);
+		this.logger.debug("Authorization successful");
 		if (this.publishAuthorizationSuccess) {
 			publishEvent(new AuthorizedEvent(object, attributes, authenticated));
 		}
 
 		// Attempt to run as a different user
 		Authentication runAs = this.runAsManager.buildRunAs(authenticated, object, attributes);
-
-		if (runAs == null) {
-			if (debug) {
-				this.logger.debug("RunAsManager did not change Authentication object");
-			}
-
-			// no further work post-invocation
-			return new InterceptorStatusToken(SecurityContextHolder.getContext(), false, attributes, object);
-		}
-		else {
-			if (debug) {
-				this.logger.debug("Switching to RunAs Authentication: " + runAs);
-			}
-
+		if (runAs != null) {
+			this.logger.debug(LogMessage.format("Switching to RunAs Authentication: %s", runAs));
 			SecurityContext origCtx = SecurityContextHolder.getContext();
 			SecurityContextHolder.setContext(SecurityContextHolder.createEmptyContext());
 			SecurityContextHolder.getContext().setAuthentication(runAs);
-
 			// need to revert to token.Authenticated post-invocation
 			return new InterceptorStatusToken(origCtx, true, attributes, object);
 		}
+		this.logger.debug("RunAsManager did not change Authentication object");
+		// no further work post-invocation
+		return new InterceptorStatusToken(SecurityContextHolder.getContext(), false, attributes, object);
+
+	}
+
+	private void attemptAuthorization(Object object, Collection<ConfigAttribute> attributes,
+			Authentication authenticated) {
+		try {
+			this.accessDecisionManager.decide(authenticated, object, attributes);
+		}
+		catch (AccessDeniedException ex) {
+			publishEvent(new AuthorizationFailureEvent(object, attributes, authenticated, ex));
+			throw ex;
+		}
 	}
 
 	/**
@@ -266,11 +239,8 @@ public abstract class AbstractSecurityInterceptor
 	 */
 	protected void finallyInvocation(InterceptorStatusToken token) {
 		if (token != null && token.isContextHolderRefreshRequired()) {
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug(
-						"Reverting to original Authentication: " + token.getSecurityContext().getAuthentication());
-			}
-
+			this.logger.debug(LogMessage.of(
+					() -> "Reverting to original Authentication: " + token.getSecurityContext().getAuthentication()));
 			SecurityContextHolder.setContext(token.getSecurityContext());
 		}
 	}
@@ -289,24 +259,19 @@ public abstract class AbstractSecurityInterceptor
 			// public object
 			return returnedObject;
 		}
-
 		finallyInvocation(token); // continue to clean in this method for passivity
-
 		if (this.afterInvocationManager != null) {
 			// Attempt after invocation handling
 			try {
 				returnedObject = this.afterInvocationManager.decide(token.getSecurityContext().getAuthentication(),
 						token.getSecureObject(), token.getAttributes(), returnedObject);
 			}
-			catch (AccessDeniedException accessDeniedException) {
-				AuthorizationFailureEvent event = new AuthorizationFailureEvent(token.getSecureObject(),
-						token.getAttributes(), token.getSecurityContext().getAuthentication(), accessDeniedException);
-				publishEvent(event);
-
-				throw accessDeniedException;
+			catch (AccessDeniedException ex) {
+				publishEvent(new AuthorizationFailureEvent(token.getSecureObject(), token.getAttributes(),
+						token.getSecurityContext().getAuthentication(), ex));
+				throw ex;
 			}
 		}
-
 		return returnedObject;
 	}
 
@@ -318,25 +283,14 @@ public abstract class AbstractSecurityInterceptor
 	 */
 	private Authentication authenticateIfRequired() {
 		Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-
 		if (authentication.isAuthenticated() && !this.alwaysReauthenticate) {
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Previously Authenticated: " + authentication);
-			}
-
+			this.logger.debug(LogMessage.format("Previously Authenticated: %s", authentication));
 			return authentication;
 		}
-
 		authentication = this.authenticationManager.authenticate(authentication);
-
-		// We don't authenticated.setAuthentication(true), because each provider should do
-		// that
-		if (this.logger.isDebugEnabled()) {
-			this.logger.debug("Successfully Authenticated: " + authentication);
-		}
-
+		// Don't authenticated.setAuthentication(true) because each provider does that
+		this.logger.debug(LogMessage.format("Successfully Authenticated: %s", authentication));
 		SecurityContextHolder.getContext().setAuthentication(authentication);
-
 		return authentication;
 	}
 
@@ -351,11 +305,9 @@ public abstract class AbstractSecurityInterceptor
 	 */
 	private void credentialsNotFound(String reason, Object secureObject, Collection<ConfigAttribute> configAttribs) {
 		AuthenticationCredentialsNotFoundException exception = new AuthenticationCredentialsNotFoundException(reason);
-
 		AuthenticationCredentialsNotFoundEvent event = new AuthenticationCredentialsNotFoundEvent(secureObject,
 				configAttribs, exception);
 		publishEvent(event);
-
 		throw exception;
 	}
 

+ 7 - 16
core/src/main/java/org/springframework/security/access/intercept/AfterInvocationProviderManager.java

@@ -24,11 +24,13 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.AfterInvocationProvider;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.core.Authentication;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 
 /**
  * Provider-based implementation of {@link AfterInvocationManager}.
@@ -57,22 +59,13 @@ public class AfterInvocationProviderManager implements AfterInvocationManager, I
 		checkIfValidList(this.providers);
 	}
 
-	private void checkIfValidList(List<?> listToCheck) {
-		if ((listToCheck == null) || (listToCheck.size() == 0)) {
-			throw new IllegalArgumentException("A list of AfterInvocationProviders is required");
-		}
-	}
-
 	@Override
 	public Object decide(Authentication authentication, Object object, Collection<ConfigAttribute> config,
 			Object returnedObject) throws AccessDeniedException {
-
 		Object result = returnedObject;
-
 		for (AfterInvocationProvider provider : this.providers) {
 			result = provider.decide(authentication, object, config, result);
 		}
-
 		return result;
 	}
 
@@ -83,7 +76,6 @@ public class AfterInvocationProviderManager implements AfterInvocationManager, I
 	public void setProviders(List<?> newList) {
 		checkIfValidList(newList);
 		this.providers = new ArrayList<>(newList.size());
-
 		for (Object currentObject : newList) {
 			Assert.isInstanceOf(AfterInvocationProvider.class, currentObject, () -> "AfterInvocationProvider "
 					+ currentObject.getClass().getName() + " must implement AfterInvocationProvider");
@@ -91,18 +83,18 @@ public class AfterInvocationProviderManager implements AfterInvocationManager, I
 		}
 	}
 
+	private void checkIfValidList(List<?> listToCheck) {
+		Assert.isTrue(!CollectionUtils.isEmpty(listToCheck), "A list of AfterInvocationProviders is required");
+	}
+
 	@Override
 	public boolean supports(ConfigAttribute attribute) {
 		for (AfterInvocationProvider provider : this.providers) {
-			if (logger.isDebugEnabled()) {
-				logger.debug("Evaluating " + attribute + " against " + provider);
-			}
-
+			logger.debug(LogMessage.format("Evaluating %s against %s", attribute, provider));
 			if (provider.supports(attribute)) {
 				return true;
 			}
 		}
-
 		return false;
 	}
 
@@ -124,7 +116,6 @@ public class AfterInvocationProviderManager implements AfterInvocationManager, I
 				return false;
 			}
 		}
-
 		return true;
 	}
 

+ 10 - 20
core/src/main/java/org/springframework/security/access/intercept/MethodInvocationPrivilegeEvaluator.java

@@ -23,6 +23,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.core.Authentication;
@@ -54,36 +55,25 @@ public class MethodInvocationPrivilegeEvaluator implements InitializingBean {
 		Assert.notNull(this.securityInterceptor, "SecurityInterceptor required");
 	}
 
-	public boolean isAllowed(MethodInvocation mi, Authentication authentication) {
-		Assert.notNull(mi, "MethodInvocation required");
-		Assert.notNull(mi.getMethod(), "MethodInvocation must provide a non-null getMethod()");
-
-		Collection<ConfigAttribute> attrs = this.securityInterceptor.obtainSecurityMetadataSource().getAttributes(mi);
-
+	public boolean isAllowed(MethodInvocation invocation, Authentication authentication) {
+		Assert.notNull(invocation, "MethodInvocation required");
+		Assert.notNull(invocation.getMethod(), "MethodInvocation must provide a non-null getMethod()");
+		Collection<ConfigAttribute> attrs = this.securityInterceptor.obtainSecurityMetadataSource()
+				.getAttributes(invocation);
 		if (attrs == null) {
-			if (this.securityInterceptor.isRejectPublicInvocations()) {
-				return false;
-			}
-
-			return true;
+			return !this.securityInterceptor.isRejectPublicInvocations();
 		}
-
 		if (authentication == null || authentication.getAuthorities().isEmpty()) {
 			return false;
 		}
-
 		try {
-			this.securityInterceptor.getAccessDecisionManager().decide(authentication, mi, attrs);
+			this.securityInterceptor.getAccessDecisionManager().decide(authentication, invocation, attrs);
+			return true;
 		}
 		catch (AccessDeniedException unauthorized) {
-			if (logger.isDebugEnabled()) {
-				logger.debug(mi.toString() + " denied for " + authentication.toString(), unauthorized);
-			}
-
+			logger.debug(LogMessage.format("%s denied for %s", invocation, authentication), unauthorized);
 			return false;
 		}
-
-		return true;
 	}
 
 	public void setSecurityInterceptor(AbstractSecurityInterceptor securityInterceptor) {

+ 2 - 5
core/src/main/java/org/springframework/security/access/intercept/RunAsImplAuthenticationProvider.java

@@ -54,14 +54,11 @@ public class RunAsImplAuthenticationProvider implements InitializingBean, Authen
 	@Override
 	public Authentication authenticate(Authentication authentication) throws AuthenticationException {
 		RunAsUserToken token = (RunAsUserToken) authentication;
-
-		if (token.getKeyHash() == this.key.hashCode()) {
-			return authentication;
-		}
-		else {
+		if (token.getKeyHash() != this.key.hashCode()) {
 			throw new BadCredentialsException(this.messages.getMessage("RunAsImplAuthenticationProvider.incorrectKey",
 					"The presented RunAsUserToken does not contain the expected key"));
 		}
+		return authentication;
 	}
 
 	public String getKey() {

+ 0 - 4
core/src/main/java/org/springframework/security/access/intercept/RunAsManagerImpl.java

@@ -69,7 +69,6 @@ public class RunAsManagerImpl implements RunAsManager, InitializingBean {
 	public Authentication buildRunAs(Authentication authentication, Object object,
 			Collection<ConfigAttribute> attributes) {
 		List<GrantedAuthority> newAuthorities = new ArrayList<>();
-
 		for (ConfigAttribute attribute : attributes) {
 			if (this.supports(attribute)) {
 				GrantedAuthority extraAuthority = new SimpleGrantedAuthority(
@@ -77,14 +76,11 @@ public class RunAsManagerImpl implements RunAsManager, InitializingBean {
 				newAuthorities.add(extraAuthority);
 			}
 		}
-
 		if (newAuthorities.size() == 0) {
 			return null;
 		}
-
 		// Add existing authorities
 		newAuthorities.addAll(authentication.getAuthorities());
-
 		return new RunAsUserToken(this.key, authentication.getPrincipal(), authentication.getCredentials(),
 				newAuthorities, authentication.getClass());
 	}

+ 0 - 1
core/src/main/java/org/springframework/security/access/intercept/RunAsUserToken.java

@@ -75,7 +75,6 @@ public class RunAsUserToken extends AbstractAuthenticationToken {
 		StringBuilder sb = new StringBuilder(super.toString());
 		String className = (this.originalAuthentication != null) ? this.originalAuthentication.getName() : null;
 		sb.append("; Original Class: ").append(className);
-
 		return sb.toString();
 	}
 

+ 0 - 1
core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityInterceptor.java

@@ -56,7 +56,6 @@ public class MethodSecurityInterceptor extends AbstractSecurityInterceptor imple
 	@Override
 	public Object invoke(MethodInvocation mi) throws Throwable {
 		InterceptorStatusToken token = super.beforeInvocation(mi);
-
 		Object result;
 		try {
 			result = mi.proceed();

+ 4 - 7
core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityMetadataSourceAdvisor.java

@@ -20,7 +20,6 @@ import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.lang.reflect.Method;
-import java.util.Collection;
 
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
@@ -33,6 +32,7 @@ import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.security.access.method.MethodSecurityMetadataSource;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 
 /**
  * Advisor driven by a {@link MethodSecurityMetadataSource}, used to exclude a
@@ -85,7 +85,6 @@ public class MethodSecurityMetadataSourceAdvisor extends AbstractPointcutAdvisor
 		Assert.notNull(adviceBeanName, "The adviceBeanName cannot be null");
 		Assert.notNull(attributeSource, "The attributeSource cannot be null");
 		Assert.notNull(attributeSourceBeanName, "The attributeSourceBeanName cannot be null");
-
 		this.adviceBeanName = adviceBeanName;
 		this.attributeSource = attributeSource;
 		this.metadataSourceBeanName = attributeSourceBeanName;
@@ -123,11 +122,9 @@ public class MethodSecurityMetadataSourceAdvisor extends AbstractPointcutAdvisor
 	class MethodSecurityMetadataSourcePointcut extends StaticMethodMatcherPointcut implements Serializable {
 
 		@Override
-		@SuppressWarnings("unchecked")
-		public boolean matches(Method m, Class targetClass) {
-			Collection attributes = MethodSecurityMetadataSourceAdvisor.this.attributeSource.getAttributes(m,
-					targetClass);
-			return attributes != null && !attributes.isEmpty();
+		public boolean matches(Method m, Class<?> targetClass) {
+			MethodSecurityMetadataSource source = MethodSecurityMetadataSourceAdvisor.this.attributeSource;
+			return !CollectionUtils.isEmpty(source.getAttributes(m, targetClass));
 		}
 
 	}

+ 0 - 2
core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptor.java

@@ -55,7 +55,6 @@ public final class AspectJMethodSecurityInterceptor extends MethodSecurityInterc
 	 */
 	public Object invoke(JoinPoint jp, AspectJCallback advisorProceed) {
 		InterceptorStatusToken token = super.beforeInvocation(new MethodInvocationAdapter(jp));
-
 		Object result;
 		try {
 			result = advisorProceed.proceedWithObject();
@@ -63,7 +62,6 @@ public final class AspectJMethodSecurityInterceptor extends MethodSecurityInterc
 		finally {
 			super.finallyInvocation(token);
 		}
-
 		return super.afterInvocation(token, result);
 	}
 

+ 3 - 8
core/src/main/java/org/springframework/security/access/intercept/aspectj/MethodInvocationAdapter.java

@@ -24,6 +24,8 @@ import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.reflect.CodeSignature;
 
+import org.springframework.util.Assert;
+
 /**
  * Decorates a JoinPoint to allow it to be used with method-security infrastructure
  * classes which support {@code MethodInvocation} instances.
@@ -51,23 +53,17 @@ public final class MethodInvocationAdapter implements MethodInvocation {
 		String targetMethodName = jp.getStaticPart().getSignature().getName();
 		Class<?>[] types = ((CodeSignature) jp.getStaticPart().getSignature()).getParameterTypes();
 		Class<?> declaringType = jp.getStaticPart().getSignature().getDeclaringType();
-
 		this.method = findMethod(targetMethodName, declaringType, types);
-
-		if (this.method == null) {
-			throw new IllegalArgumentException("Could not obtain target method from JoinPoint: '" + jp + "'");
-		}
+		Assert.notNull(this.method, () -> "Could not obtain target method from JoinPoint: '" + jp + "'");
 	}
 
 	private Method findMethod(String name, Class<?> declaringType, Class<?>[] params) {
 		Method method = null;
-
 		try {
 			method = declaringType.getMethod(name, params);
 		}
 		catch (NoSuchMethodException ignored) {
 		}
-
 		if (method == null) {
 			try {
 				method = declaringType.getDeclaredMethod(name, params);
@@ -75,7 +71,6 @@ public final class MethodInvocationAdapter implements MethodInvocation {
 			catch (NoSuchMethodException ignored) {
 			}
 		}
-
 		return method;
 	}
 

+ 0 - 2
core/src/main/java/org/springframework/security/access/method/AbstractFallbackMethodSecurityMetadataSource.java

@@ -57,13 +57,11 @@ public abstract class AbstractFallbackMethodSecurityMetadataSource extends Abstr
 		if (attr != null) {
 			return attr;
 		}
-
 		// Second try is the config attribute on the target class.
 		attr = findAttributes(specificMethod.getDeclaringClass());
 		if (attr != null) {
 			return attr;
 		}
-
 		if (specificMethod != method || targetClass == null) {
 			// Fallback is to look at the original method.
 			attr = findAttributes(method, method.getDeclaringClass());

+ 0 - 2
core/src/main/java/org/springframework/security/access/method/AbstractMethodSecurityMetadataSource.java

@@ -42,7 +42,6 @@ public abstract class AbstractMethodSecurityMetadataSource implements MethodSecu
 			MethodInvocation mi = (MethodInvocation) object;
 			Object target = mi.getThis();
 			Class<?> targetClass = null;
-
 			if (target != null) {
 				targetClass = (target instanceof Class<?>) ? (Class<?>) target
 						: AopProxyUtils.ultimateTargetClass(target);
@@ -56,7 +55,6 @@ public abstract class AbstractMethodSecurityMetadataSource implements MethodSecu
 			}
 			return attrs;
 		}
-
 		throw new IllegalArgumentException("Object must be a non-null MethodInvocation");
 	}
 

+ 4 - 11
core/src/main/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSource.java

@@ -25,6 +25,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
@@ -56,11 +57,9 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod
 		synchronized (this.attributeCache) {
 			Collection<ConfigAttribute> cached = this.attributeCache.get(cacheKey);
 			// Check for canonical value indicating there is no config attribute,
-
 			if (cached != null) {
 				return cached;
 			}
-
 			// No cached value, so query the sources to find a result
 			Collection<ConfigAttribute> attributes = null;
 			for (MethodSecurityMetadataSource s : this.methodSecurityMetadataSources) {
@@ -69,19 +68,13 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod
 					break;
 				}
 			}
-
 			// Put it in the cache.
 			if (attributes == null || attributes.isEmpty()) {
 				this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE);
 				return NULL_CONFIG_ATTRIBUTE;
 			}
-
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Caching method [" + cacheKey + "] with attributes " + attributes);
-			}
-
+			this.logger.debug(LogMessage.format("Caching method [%s] with attributes %s", cacheKey, attributes));
 			this.attributeCache.put(cacheKey, attributes);
-
 			return attributes;
 		}
 	}
@@ -127,8 +120,8 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod
 
 		@Override
 		public String toString() {
-			return "CacheKey[" + ((this.targetClass != null) ? this.targetClass.getName() : "-") + "; " + this.method
-					+ "]";
+			String targetClassName = (this.targetClass != null) ? this.targetClass.getName() : "-";
+			return "CacheKey[" + targetClassName + "; " + this.method + "]";
 		}
 
 	}

+ 27 - 42
core/src/main/java/org/springframework/security/access/method/MapBasedMethodSecurityMetadataSource.java

@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Set;
 
 import org.springframework.beans.factory.BeanClassLoaderAware;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -47,10 +48,14 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 
 	private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
 
-	/** Map from RegisteredMethod to ConfigAttribute list */
+	/**
+	 * Map from RegisteredMethod to ConfigAttribute list
+	 */
 	protected final Map<RegisteredMethod, List<ConfigAttribute>> methodMap = new HashMap<>();
 
-	/** Map from RegisteredMethod to name pattern used for registration */
+	/**
+	 * Map from RegisteredMethod to name pattern used for registration
+	 */
 	private final Map<RegisteredMethod, String> nameMap = new HashMap<>();
 
 	public MapBasedMethodSecurityMetadataSource() {
@@ -83,7 +88,6 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 		if (targetClass == null) {
 			return null;
 		}
-
 		return findAttributesSpecifiedAgainst(method, targetClass);
 	}
 
@@ -107,17 +111,11 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 	 */
 	private void addSecureMethod(String name, List<ConfigAttribute> attr) {
 		int lastDotIndex = name.lastIndexOf(".");
-
-		if (lastDotIndex == -1) {
-			throw new IllegalArgumentException("'" + name + "' is not a valid method name: format is FQN.methodName");
-		}
-
+		Assert.isTrue(lastDotIndex != -1, () -> "'" + name + "' is not a valid method name: format is FQN.methodName");
 		String methodName = name.substring(lastDotIndex + 1);
 		Assert.hasText(methodName, () -> "Method not found for '" + name + "'");
-
 		String typeName = name.substring(0, lastDotIndex);
 		Class<?> type = ClassUtils.resolveClassName(typeName, this.beanClassLoader);
-
 		addSecureMethod(type, methodName, attr);
 	}
 
@@ -131,43 +129,38 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 	 */
 	public void addSecureMethod(Class<?> javaType, String mappedName, List<ConfigAttribute> attr) {
 		String name = javaType.getName() + '.' + mappedName;
-
-		if (this.logger.isDebugEnabled()) {
-			this.logger.debug("Request to add secure method [" + name + "] with attributes [" + attr + "]");
-		}
-
+		this.logger.debug(LogMessage.format("Request to add secure method [%s] with attributes [%s]", name, attr));
 		Method[] methods = javaType.getMethods();
 		List<Method> matchingMethods = new ArrayList<>();
-
-		for (Method m : methods) {
-			if (m.getName().equals(mappedName) || isMatch(m.getName(), mappedName)) {
-				matchingMethods.add(m);
+		for (Method method : methods) {
+			if (method.getName().equals(mappedName) || isMatch(method.getName(), mappedName)) {
+				matchingMethods.add(method);
 			}
 		}
+		Assert.notEmpty(matchingMethods, () -> "Couldn't find method '" + mappedName + "' on '" + javaType + "'");
+		registerAllMatchingMethods(javaType, attr, name, matchingMethods);
+	}
 
-		if (matchingMethods.isEmpty()) {
-			throw new IllegalArgumentException("Couldn't find method '" + mappedName + "' on '" + javaType + "'");
-		}
-
-		// register all matching methods
+	private void registerAllMatchingMethods(Class<?> javaType, List<ConfigAttribute> attr, String name,
+			List<Method> matchingMethods) {
 		for (Method method : matchingMethods) {
 			RegisteredMethod registeredMethod = new RegisteredMethod(method, javaType);
 			String regMethodName = this.nameMap.get(registeredMethod);
-
 			if ((regMethodName == null) || (!regMethodName.equals(name) && (regMethodName.length() <= name.length()))) {
 				// no already registered method name, or more specific
 				// method name specification (now) -> (re-)register method
 				if (regMethodName != null) {
-					this.logger.debug("Replacing attributes for secure method [" + method + "]: current name [" + name
-							+ "] is more specific than [" + regMethodName + "]");
+					this.logger.debug(LogMessage.format(
+							"Replacing attributes for secure method [%s]: current name [%s] is more specific than [%s]",
+							method, name, regMethodName));
 				}
-
 				this.nameMap.put(registeredMethod, name);
 				addSecureMethod(registeredMethod, attr);
 			}
 			else {
-				this.logger.debug("Keeping attributes for secure method [" + method + "]: current name [" + name
-						+ "] is not more specific than [" + regMethodName + "]");
+				this.logger.debug(LogMessage.format(
+						"Keeping attributes for secure method [%s]: current name [%s] is not more specific than [%s]",
+						method, name, regMethodName));
 			}
 		}
 	}
@@ -183,13 +176,11 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 	 */
 	public void addSecureMethod(Class<?> javaType, Method method, List<ConfigAttribute> attr) {
 		RegisteredMethod key = new RegisteredMethod(method, javaType);
-
 		if (this.methodMap.containsKey(key)) {
-			this.logger.debug(
-					"Method [" + method + "] is already registered with attributes [" + this.methodMap.get(key) + "]");
+			this.logger.debug(LogMessage.format("Method [%s] is already registered with attributes [%s]", method,
+					this.methodMap.get(key)));
 			return;
 		}
-
 		this.methodMap.put(key, attr);
 	}
 
@@ -201,9 +192,7 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 	private void addSecureMethod(RegisteredMethod method, List<ConfigAttribute> attr) {
 		Assert.notNull(method, "RegisteredMethod required");
 		Assert.notNull(attr, "Configuration attribute required");
-		if (this.logger.isInfoEnabled()) {
-			this.logger.info("Adding secure method [" + method + "] with attributes [" + attr + "]");
-		}
+		this.logger.info(LogMessage.format("Adding secure method [%s] with attributes [%s]", method, attr));
 		this.methodMap.put(method, attr);
 	}
 
@@ -214,11 +203,7 @@ public class MapBasedMethodSecurityMetadataSource extends AbstractFallbackMethod
 	@Override
 	public Collection<ConfigAttribute> getAllConfigAttributes() {
 		Set<ConfigAttribute> allAttributes = new HashSet<>();
-
-		for (List<ConfigAttribute> attributeList : this.methodMap.values()) {
-			allAttributes.addAll(attributeList);
-		}
-
+		this.methodMap.values().forEach(allAttributes::addAll);
 		return allAttributes;
 	}
 

+ 4 - 7
core/src/main/java/org/springframework/security/access/prepost/PostInvocationAdviceProvider.java

@@ -49,14 +49,12 @@ public class PostInvocationAdviceProvider implements AfterInvocationProvider {
 	@Override
 	public Object decide(Authentication authentication, Object object, Collection<ConfigAttribute> config,
 			Object returnedObject) throws AccessDeniedException {
-
-		PostInvocationAttribute pia = findPostInvocationAttribute(config);
-
-		if (pia == null) {
+		PostInvocationAttribute postInvocationAttribute = findPostInvocationAttribute(config);
+		if (postInvocationAttribute == null) {
 			return returnedObject;
 		}
-
-		return this.postAdvice.after(authentication, (MethodInvocation) object, pia, returnedObject);
+		return this.postAdvice.after(authentication, (MethodInvocation) object, postInvocationAttribute,
+				returnedObject);
 	}
 
 	private PostInvocationAttribute findPostInvocationAttribute(Collection<ConfigAttribute> config) {
@@ -65,7 +63,6 @@ public class PostInvocationAdviceProvider implements AfterInvocationProvider {
 				return (PostInvocationAttribute) attribute;
 			}
 		}
-
 		return null;
 	}
 

+ 2 - 10
core/src/main/java/org/springframework/security/access/prepost/PreInvocationAuthorizationAdviceVoter.java

@@ -61,21 +61,14 @@ public class PreInvocationAuthorizationAdviceVoter implements AccessDecisionVote
 
 	@Override
 	public int vote(Authentication authentication, MethodInvocation method, Collection<ConfigAttribute> attributes) {
-
 		// Find prefilter and preauth (or combined) attributes
-		// if both null, abstain
-		// else call advice with them
-
+		// if both null, abstain else call advice with them
 		PreInvocationAttribute preAttr = findPreInvocationAttribute(attributes);
-
 		if (preAttr == null) {
 			// No expression based metadata, so abstain
 			return ACCESS_ABSTAIN;
 		}
-
-		boolean allowed = this.preAdvice.before(authentication, method, preAttr);
-
-		return allowed ? ACCESS_GRANTED : ACCESS_DENIED;
+		return this.preAdvice.before(authentication, method, preAttr) ? ACCESS_GRANTED : ACCESS_DENIED;
 	}
 
 	private PreInvocationAttribute findPreInvocationAttribute(Collection<ConfigAttribute> config) {
@@ -84,7 +77,6 @@ public class PreInvocationAuthorizationAdviceVoter implements AccessDecisionVote
 				return (PreInvocationAttribute) attribute;
 			}
 		}
-
 		return null;
 	}
 

+ 4 - 12
core/src/main/java/org/springframework/security/access/prepost/PrePostAdviceReactiveMethodInterceptor.java

@@ -66,7 +66,6 @@ public class PrePostAdviceReactiveMethodInterceptor implements MethodInterceptor
 		Assert.notNull(attributeSource, "attributeSource cannot be null");
 		Assert.notNull(preInvocationAdvice, "preInvocationAdvice cannot be null");
 		Assert.notNull(postInvocationAdvice, "postInvocationAdvice cannot be null");
-
 		this.attributeSource = attributeSource;
 		this.preInvocationAdvice = preInvocationAdvice;
 		this.postAdvice = postInvocationAdvice;
@@ -76,31 +75,26 @@ public class PrePostAdviceReactiveMethodInterceptor implements MethodInterceptor
 	public Object invoke(final MethodInvocation invocation) {
 		Method method = invocation.getMethod();
 		Class<?> returnType = method.getReturnType();
-		if (!Publisher.class.isAssignableFrom(returnType)) {
-			throw new IllegalStateException("The returnType " + returnType + " on " + method
-					+ " must return an instance of org.reactivestreams.Publisher (i.e. Mono / Flux) in order to support Reactor Context");
-		}
+		Assert.state(Publisher.class.isAssignableFrom(returnType),
+				() -> "The returnType " + returnType + " on " + method
+						+ " must return an instance of org.reactivestreams.Publisher "
+						+ "(i.e. Mono / Flux) in order to support Reactor Context");
 		Class<?> targetClass = invocation.getThis().getClass();
 		Collection<ConfigAttribute> attributes = this.attributeSource.getAttributes(method, targetClass);
-
 		PreInvocationAttribute preAttr = findPreInvocationAttribute(attributes);
 		Mono<Authentication> toInvoke = ReactiveSecurityContextHolder.getContext()
 				.map(SecurityContext::getAuthentication).defaultIfEmpty(this.anonymous)
 				.filter((auth) -> this.preInvocationAdvice.before(auth, invocation, preAttr))
 				.switchIfEmpty(Mono.defer(() -> Mono.error(new AccessDeniedException("Denied"))));
-
 		PostInvocationAttribute attr = findPostInvocationAttribute(attributes);
-
 		if (Mono.class.isAssignableFrom(returnType)) {
 			return toInvoke.flatMap((auth) -> PrePostAdviceReactiveMethodInterceptor.<Mono<?>>proceed(invocation)
 					.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
 		}
-
 		if (Flux.class.isAssignableFrom(returnType)) {
 			return toInvoke.flatMapMany((auth) -> PrePostAdviceReactiveMethodInterceptor.<Flux<?>>proceed(invocation)
 					.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
 		}
-
 		return toInvoke.flatMapMany(
 				(auth) -> Flux.from(PrePostAdviceReactiveMethodInterceptor.<Publisher<?>>proceed(invocation))
 						.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
@@ -121,7 +115,6 @@ public class PrePostAdviceReactiveMethodInterceptor implements MethodInterceptor
 				return (PostInvocationAttribute) attribute;
 			}
 		}
-
 		return null;
 	}
 
@@ -131,7 +124,6 @@ public class PrePostAdviceReactiveMethodInterceptor implements MethodInterceptor
 				return (PreInvocationAttribute) attribute;
 			}
 		}
-
 		return null;
 	}
 

+ 7 - 21
core/src/main/java/org/springframework/security/access/prepost/PrePostAnnotationSecurityMetadataSource.java

@@ -23,6 +23,7 @@ import java.util.Collection;
 import java.util.Collections;
 
 import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.access.method.AbstractMethodSecurityMetadataSource;
 import org.springframework.util.ClassUtils;
@@ -61,45 +62,35 @@ public class PrePostAnnotationSecurityMetadataSource extends AbstractMethodSecur
 		if (method.getDeclaringClass() == Object.class) {
 			return Collections.emptyList();
 		}
-
-		this.logger.trace("Looking for Pre/Post annotations for method '" + method.getName() + "' on target class '"
-				+ targetClass + "'");
+		this.logger.trace(LogMessage.format("Looking for Pre/Post annotations for method '%s' on target class '%s'",
+				method.getName(), targetClass));
 		PreFilter preFilter = findAnnotation(method, targetClass, PreFilter.class);
 		PreAuthorize preAuthorize = findAnnotation(method, targetClass, PreAuthorize.class);
 		PostFilter postFilter = findAnnotation(method, targetClass, PostFilter.class);
 		// TODO: Can we check for void methods and throw an exception here?
 		PostAuthorize postAuthorize = findAnnotation(method, targetClass, PostAuthorize.class);
-
 		if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null) {
 			// There is no meta-data so return
 			this.logger.trace("No expression annotations found");
 			return Collections.emptyList();
 		}
-
 		String preFilterAttribute = (preFilter != null) ? preFilter.value() : null;
 		String filterObject = (preFilter != null) ? preFilter.filterTarget() : null;
 		String preAuthorizeAttribute = (preAuthorize != null) ? preAuthorize.value() : null;
 		String postFilterAttribute = (postFilter != null) ? postFilter.value() : null;
 		String postAuthorizeAttribute = (postAuthorize != null) ? postAuthorize.value() : null;
-
 		ArrayList<ConfigAttribute> attrs = new ArrayList<>(2);
-
 		PreInvocationAttribute pre = this.attributeFactory.createPreInvocationAttribute(preFilterAttribute,
 				filterObject, preAuthorizeAttribute);
-
 		if (pre != null) {
 			attrs.add(pre);
 		}
-
 		PostInvocationAttribute post = this.attributeFactory.createPostInvocationAttribute(postFilterAttribute,
 				postAuthorizeAttribute);
-
 		if (post != null) {
 			attrs.add(post);
 		}
-
 		attrs.trimToSize();
-
 		return attrs;
 	}
 
@@ -120,31 +111,26 @@ public class PrePostAnnotationSecurityMetadataSource extends AbstractMethodSecur
 		// If the target class is null, the method will be unchanged.
 		Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
 		A annotation = AnnotationUtils.findAnnotation(specificMethod, annotationClass);
-
 		if (annotation != null) {
-			this.logger.debug(annotation + " found on specific method: " + specificMethod);
+			this.logger.debug(LogMessage.format("%s found on specific method: %s", annotation, specificMethod));
 			return annotation;
 		}
-
 		// Check the original (e.g. interface) method
 		if (specificMethod != method) {
 			annotation = AnnotationUtils.findAnnotation(method, annotationClass);
-
 			if (annotation != null) {
-				this.logger.debug(annotation + " found on: " + method);
+				this.logger.debug(LogMessage.format("%s found on: %s", annotation, method));
 				return annotation;
 			}
 		}
-
 		// Check the class-level (note declaringClass, not targetClass, which may not
 		// actually implement the method)
 		annotation = AnnotationUtils.findAnnotation(specificMethod.getDeclaringClass(), annotationClass);
-
 		if (annotation != null) {
-			this.logger.debug(annotation + " found on: " + specificMethod.getDeclaringClass().getName());
+			this.logger.debug(
+					LogMessage.format("%s found on: %s", annotation, specificMethod.getDeclaringClass().getName()));
 			return annotation;
 		}
-
 		return null;
 	}
 

+ 2 - 4
core/src/main/java/org/springframework/security/access/vote/AbstractAccessDecisionManager.java

@@ -88,12 +88,11 @@ public abstract class AbstractAccessDecisionManager
 
 	@Override
 	public boolean supports(ConfigAttribute attribute) {
-		for (AccessDecisionVoter voter : this.decisionVoters) {
+		for (AccessDecisionVoter<?> voter : this.decisionVoters) {
 			if (voter.supports(attribute)) {
 				return true;
 			}
 		}
-
 		return false;
 	}
 
@@ -108,12 +107,11 @@ public abstract class AbstractAccessDecisionManager
 	 */
 	@Override
 	public boolean supports(Class<?> clazz) {
-		for (AccessDecisionVoter voter : this.decisionVoters) {
+		for (AccessDecisionVoter<?> voter : this.decisionVoters) {
 			if (!voter.supports(clazz)) {
 				return false;
 			}
 		}
-
 		return true;
 	}
 

+ 2 - 7
core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java

@@ -33,18 +33,13 @@ public abstract class AbstractAclVoter implements AccessDecisionVoter<MethodInvo
 	private Class<?> processDomainObjectClass;
 
 	protected Object getDomainObjectInstance(MethodInvocation invocation) {
-		Object[] args;
-		Class<?>[] params;
-
-		params = invocation.getMethod().getParameterTypes();
-		args = invocation.getArguments();
-
+		Object[] args = invocation.getArguments();
+		Class<?>[] params = invocation.getMethod().getParameterTypes();
 		for (int i = 0; i < params.length; i++) {
 			if (this.processDomainObjectClass.isAssignableFrom(params[i])) {
 				return args[i];
 			}
 		}
-
 		throw new AuthorizationServiceException("MethodInvocation: " + invocation
 				+ " did not provide any argument of type: " + this.processDomainObjectClass);
 	}

+ 3 - 11
core/src/main/java/org/springframework/security/access/vote/AffirmativeBased.java

@@ -19,6 +19,7 @@ package org.springframework.security.access.vote;
 import java.util.Collection;
 import java.util.List;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDecisionVoter;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
@@ -52,36 +53,27 @@ public class AffirmativeBased extends AbstractAccessDecisionManager {
 	 * @throws AccessDeniedException if access is denied
 	 */
 	@Override
+	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public void decide(Authentication authentication, Object object, Collection<ConfigAttribute> configAttributes)
 			throws AccessDeniedException {
 		int deny = 0;
-
 		for (AccessDecisionVoter voter : getDecisionVoters()) {
 			int result = voter.vote(authentication, object, configAttributes);
-
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Voter: " + voter + ", returned: " + result);
-			}
-
+			this.logger.debug(LogMessage.format("Voter: %s, returned: %s", voter, result));
 			switch (result) {
 			case AccessDecisionVoter.ACCESS_GRANTED:
 				return;
-
 			case AccessDecisionVoter.ACCESS_DENIED:
 				deny++;
-
 				break;
-
 			default:
 				break;
 			}
 		}
-
 		if (deny > 0) {
 			throw new AccessDeniedException(
 					this.messages.getMessage("AbstractAccessDecisionManager.accessDenied", "Access is denied"));
 		}
-
 		// To get this far, every AccessDecisionVoter abstained
 		checkAllowIfAllAbstainDecisions();
 	}

+ 0 - 5
core/src/main/java/org/springframework/security/access/vote/AuthenticatedVoter.java

@@ -87,24 +87,20 @@ public class AuthenticatedVoter implements AccessDecisionVoter<Object> {
 	@Override
 	public int vote(Authentication authentication, Object object, Collection<ConfigAttribute> attributes) {
 		int result = ACCESS_ABSTAIN;
-
 		for (ConfigAttribute attribute : attributes) {
 			if (this.supports(attribute)) {
 				result = ACCESS_DENIED;
-
 				if (IS_AUTHENTICATED_FULLY.equals(attribute.getAttribute())) {
 					if (isFullyAuthenticated(authentication)) {
 						return ACCESS_GRANTED;
 					}
 				}
-
 				if (IS_AUTHENTICATED_REMEMBERED.equals(attribute.getAttribute())) {
 					if (this.authenticationTrustResolver.isRememberMe(authentication)
 							|| isFullyAuthenticated(authentication)) {
 						return ACCESS_GRANTED;
 					}
 				}
-
 				if (IS_AUTHENTICATED_ANONYMOUSLY.equals(attribute.getAttribute())) {
 					if (this.authenticationTrustResolver.isAnonymous(authentication)
 							|| isFullyAuthenticated(authentication)
@@ -114,7 +110,6 @@ public class AuthenticatedVoter implements AccessDecisionVoter<Object> {
 				}
 			}
 		}
-
 		return result;
 	}
 

+ 5 - 18
core/src/main/java/org/springframework/security/access/vote/ConsensusBased.java

@@ -19,6 +19,7 @@ package org.springframework.security.access.vote;
 import java.util.Collection;
 import java.util.List;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDecisionVoter;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
@@ -59,53 +60,39 @@ public class ConsensusBased extends AbstractAccessDecisionManager {
 	 * @throws AccessDeniedException if access is denied
 	 */
 	@Override
+	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public void decide(Authentication authentication, Object object, Collection<ConfigAttribute> configAttributes)
 			throws AccessDeniedException {
 		int grant = 0;
 		int deny = 0;
-
 		for (AccessDecisionVoter voter : getDecisionVoters()) {
 			int result = voter.vote(authentication, object, configAttributes);
-
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Voter: " + voter + ", returned: " + result);
-			}
-
+			this.logger.debug(LogMessage.format("Voter: %s, returned: %s", voter, result));
 			switch (result) {
 			case AccessDecisionVoter.ACCESS_GRANTED:
 				grant++;
-
 				break;
-
 			case AccessDecisionVoter.ACCESS_DENIED:
 				deny++;
-
 				break;
-
 			default:
 				break;
 			}
 		}
-
 		if (grant > deny) {
 			return;
 		}
-
 		if (deny > grant) {
 			throw new AccessDeniedException(
 					this.messages.getMessage("AbstractAccessDecisionManager.accessDenied", "Access is denied"));
 		}
-
 		if ((grant == deny) && (grant != 0)) {
 			if (this.allowIfEqualGrantedDeniedDecisions) {
 				return;
 			}
-			else {
-				throw new AccessDeniedException(
-						this.messages.getMessage("AbstractAccessDecisionManager.accessDenied", "Access is denied"));
-			}
+			throw new AccessDeniedException(
+					this.messages.getMessage("AbstractAccessDecisionManager.accessDenied", "Access is denied"));
 		}
-
 		// To get this far, every AccessDecisionVoter abstained
 		checkAllowIfAllAbstainDecisions();
 	}

+ 0 - 3
core/src/main/java/org/springframework/security/access/vote/RoleVoter.java

@@ -89,11 +89,9 @@ public class RoleVoter implements AccessDecisionVoter<Object> {
 		}
 		int result = ACCESS_ABSTAIN;
 		Collection<? extends GrantedAuthority> authorities = extractAuthorities(authentication);
-
 		for (ConfigAttribute attribute : attributes) {
 			if (this.supports(attribute)) {
 				result = ACCESS_DENIED;
-
 				// Attempt to find a matching granted authority
 				for (GrantedAuthority authority : authorities) {
 					if (attribute.getAttribute().equals(authority.getAuthority())) {
@@ -102,7 +100,6 @@ public class RoleVoter implements AccessDecisionVoter<Object> {
 				}
 			}
 		}
-
 		return result;
 	}
 

+ 3 - 14
core/src/main/java/org/springframework/security/access/vote/UnanimousBased.java

@@ -20,6 +20,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDecisionVoter;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
@@ -56,45 +57,33 @@ public class UnanimousBased extends AbstractAccessDecisionManager {
 	 * @throws AccessDeniedException if access is denied
 	 */
 	@Override
+	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public void decide(Authentication authentication, Object object, Collection<ConfigAttribute> attributes)
 			throws AccessDeniedException {
-
 		int grant = 0;
-
 		List<ConfigAttribute> singleAttributeList = new ArrayList<>(1);
 		singleAttributeList.add(null);
-
 		for (ConfigAttribute attribute : attributes) {
 			singleAttributeList.set(0, attribute);
-
 			for (AccessDecisionVoter voter : getDecisionVoters()) {
 				int result = voter.vote(authentication, object, singleAttributeList);
-
-				if (this.logger.isDebugEnabled()) {
-					this.logger.debug("Voter: " + voter + ", returned: " + result);
-				}
-
+				this.logger.debug(LogMessage.format("Voter: %s, returned: %s", voter, result));
 				switch (result) {
 				case AccessDecisionVoter.ACCESS_GRANTED:
 					grant++;
-
 					break;
-
 				case AccessDecisionVoter.ACCESS_DENIED:
 					throw new AccessDeniedException(
 							this.messages.getMessage("AbstractAccessDecisionManager.accessDenied", "Access is denied"));
-
 				default:
 					break;
 				}
 			}
 		}
-
 		// To get this far, there were no deny votes
 		if (grant > 0) {
 			return;
 		}
-
 		// To get this far, every AccessDecisionVoter abstained
 		checkAllowIfAllAbstainDecisions();
 	}

+ 3 - 28
core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java

@@ -27,6 +27,7 @@ import org.springframework.security.core.CredentialsContainer;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.userdetails.UserDetails;
+import org.springframework.util.Assert;
 
 /**
  * Base class for <code>Authentication</code> objects.
@@ -54,15 +55,10 @@ public abstract class AbstractAuthenticationToken implements Authentication, Cre
 			this.authorities = AuthorityUtils.NO_AUTHORITIES;
 			return;
 		}
-
 		for (GrantedAuthority a : authorities) {
-			if (a == null) {
-				throw new IllegalArgumentException("Authorities collection cannot contain any null elements");
-			}
+			Assert.notNull(a, "Authorities collection cannot contain any null elements");
 		}
-		ArrayList<GrantedAuthority> temp = new ArrayList<>(authorities.size());
-		temp.addAll(authorities);
-		this.authorities = Collections.unmodifiableList(temp);
+		this.authorities = Collections.unmodifiableList(new ArrayList<>(authorities));
 	}
 
 	@Override
@@ -81,7 +77,6 @@ public abstract class AbstractAuthenticationToken implements Authentication, Cre
 		if (this.getPrincipal() instanceof Principal) {
 			return ((Principal) this.getPrincipal()).getName();
 		}
-
 		return (this.getPrincipal() == null) ? "" : this.getPrincipal().toString();
 	}
 
@@ -127,68 +122,52 @@ public abstract class AbstractAuthenticationToken implements Authentication, Cre
 		if (!(obj instanceof AbstractAuthenticationToken)) {
 			return false;
 		}
-
 		AbstractAuthenticationToken test = (AbstractAuthenticationToken) obj;
-
 		if (!this.authorities.equals(test.authorities)) {
 			return false;
 		}
-
 		if ((this.details == null) && (test.getDetails() != null)) {
 			return false;
 		}
-
 		if ((this.details != null) && (test.getDetails() == null)) {
 			return false;
 		}
-
 		if ((this.details != null) && (!this.details.equals(test.getDetails()))) {
 			return false;
 		}
-
 		if ((this.getCredentials() == null) && (test.getCredentials() != null)) {
 			return false;
 		}
-
 		if ((this.getCredentials() != null) && !this.getCredentials().equals(test.getCredentials())) {
 			return false;
 		}
-
 		if (this.getPrincipal() == null && test.getPrincipal() != null) {
 			return false;
 		}
-
 		if (this.getPrincipal() != null && !this.getPrincipal().equals(test.getPrincipal())) {
 			return false;
 		}
-
 		return this.isAuthenticated() == test.isAuthenticated();
 	}
 
 	@Override
 	public int hashCode() {
 		int code = 31;
-
 		for (GrantedAuthority authority : this.authorities) {
 			code ^= authority.hashCode();
 		}
-
 		if (this.getPrincipal() != null) {
 			code ^= this.getPrincipal().hashCode();
 		}
-
 		if (this.getCredentials() != null) {
 			code ^= this.getCredentials().hashCode();
 		}
-
 		if (this.getDetails() != null) {
 			code ^= this.getDetails().hashCode();
 		}
-
 		if (this.isAuthenticated()) {
 			code ^= -37;
 		}
-
 		return code;
 	}
 
@@ -200,23 +179,19 @@ public abstract class AbstractAuthenticationToken implements Authentication, Cre
 		sb.append("Credentials: [PROTECTED]; ");
 		sb.append("Authenticated: ").append(this.isAuthenticated()).append("; ");
 		sb.append("Details: ").append(this.getDetails()).append("; ");
-
 		if (!this.authorities.isEmpty()) {
 			sb.append("Granted Authorities: ");
-
 			int i = 0;
 			for (GrantedAuthority authority : this.authorities) {
 				if (i++ > 0) {
 					sb.append(", ");
 				}
-
 				sb.append(authority);
 			}
 		}
 		else {
 			sb.append("Not granted any authorities");
 		}
-
 		return sb.toString();
 	}
 

+ 28 - 23
core/src/main/java/org/springframework/security/authentication/AbstractUserDetailsReactiveAuthenticationManager.java

@@ -57,55 +57,60 @@ public abstract class AbstractUserDetailsReactiveAuthenticationManager implement
 
 	private Scheduler scheduler = Schedulers.boundedElastic();
 
-	private UserDetailsChecker preAuthenticationChecks = (user) -> {
+	private UserDetailsChecker preAuthenticationChecks = this::defaultPreAuthenticationChecks;
+
+	private UserDetailsChecker postAuthenticationChecks = this::defaultPostAuthenticationChecks;
+
+	private void defaultPreAuthenticationChecks(UserDetails user) {
 		if (!user.isAccountNonLocked()) {
 			this.logger.debug("User account is locked");
-
 			throw new LockedException(this.messages.getMessage("AbstractUserDetailsAuthenticationProvider.locked",
 					"User account is locked"));
 		}
-
 		if (!user.isEnabled()) {
 			this.logger.debug("User account is disabled");
-
 			throw new DisabledException(
 					this.messages.getMessage("AbstractUserDetailsAuthenticationProvider.disabled", "User is disabled"));
 		}
-
 		if (!user.isAccountNonExpired()) {
 			this.logger.debug("User account is expired");
-
 			throw new AccountExpiredException(this.messages
 					.getMessage("AbstractUserDetailsAuthenticationProvider.expired", "User account has expired"));
 		}
-	};
+	}
 
-	private UserDetailsChecker postAuthenticationChecks = (user) -> {
+	private void defaultPostAuthenticationChecks(UserDetails user) {
 		if (!user.isCredentialsNonExpired()) {
 			this.logger.debug("User account credentials have expired");
-
 			throw new CredentialsExpiredException(this.messages.getMessage(
 					"AbstractUserDetailsAuthenticationProvider.credentialsExpired", "User credentials have expired"));
 		}
-	};
+	}
 
 	@Override
 	public Mono<Authentication> authenticate(Authentication authentication) {
-		final String username = authentication.getName();
-		final String presentedPassword = (String) authentication.getCredentials();
+		String username = authentication.getName();
+		String presentedPassword = (String) authentication.getCredentials();
 		return retrieveUser(username).doOnNext(this.preAuthenticationChecks::check).publishOn(this.scheduler)
-				.filter((u) -> this.passwordEncoder.matches(presentedPassword, u.getPassword()))
+				.filter((userDetails) -> this.passwordEncoder.matches(presentedPassword, userDetails.getPassword()))
 				.switchIfEmpty(Mono.defer(() -> Mono.error(new BadCredentialsException("Invalid Credentials"))))
-				.flatMap((u) -> {
-					boolean upgradeEncoding = this.userDetailsPasswordService != null
-							&& this.passwordEncoder.upgradeEncoding(u.getPassword());
-					if (upgradeEncoding) {
-						String newPassword = this.passwordEncoder.encode(presentedPassword);
-						return this.userDetailsPasswordService.updatePassword(u, newPassword);
-					}
-					return Mono.just(u);
-				}).doOnNext(this.postAuthenticationChecks::check)
-				.map((u) -> new UsernamePasswordAuthenticationToken(u, u.getPassword(), u.getAuthorities()));
+				.flatMap((userDetails) -> upgradeEncodingIfNecessary(userDetails, presentedPassword))
+				.doOnNext(this.postAuthenticationChecks::check).map(this::createUsernamePasswordAuthenticationToken);
+	}
+
+	private Mono<UserDetails> upgradeEncodingIfNecessary(UserDetails userDetails, String presentedPassword) {
+		boolean upgradeEncoding = this.userDetailsPasswordService != null
+				&& this.passwordEncoder.upgradeEncoding(userDetails.getPassword());
+		if (upgradeEncoding) {
+			String newPassword = this.passwordEncoder.encode(presentedPassword);
+			return this.userDetailsPasswordService.updatePassword(userDetails, newPassword);
+		}
+		return Mono.just(userDetails);
+	}
+
+	private UsernamePasswordAuthenticationToken createUsernamePasswordAuthenticationToken(UserDetails userDetails) {
+		return new UsernamePasswordAuthenticationToken(userDetails, userDetails.getPassword(),
+				userDetails.getAuthorities());
 	}
 
 	/**

+ 0 - 3
core/src/main/java/org/springframework/security/authentication/AccountStatusUserDetailsChecker.java

@@ -37,17 +37,14 @@ public class AccountStatusUserDetailsChecker implements UserDetailsChecker, Mess
 			throw new LockedException(
 					this.messages.getMessage("AccountStatusUserDetailsChecker.locked", "User account is locked"));
 		}
-
 		if (!user.isEnabled()) {
 			throw new DisabledException(
 					this.messages.getMessage("AccountStatusUserDetailsChecker.disabled", "User is disabled"));
 		}
-
 		if (!user.isAccountNonExpired()) {
 			throw new AccountExpiredException(
 					this.messages.getMessage("AccountStatusUserDetailsChecker.expired", "User account has expired"));
 		}
-
 		if (!user.isCredentialsNonExpired()) {
 			throw new CredentialsExpiredException(this.messages
 					.getMessage("AccountStatusUserDetailsChecker.credentialsExpired", "User credentials have expired"));

+ 0 - 2
core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationProvider.java

@@ -49,12 +49,10 @@ public class AnonymousAuthenticationProvider implements AuthenticationProvider,
 		if (!supports(authentication.getClass())) {
 			return null;
 		}
-
 		if (this.key.hashCode() != ((AnonymousAuthenticationToken) authentication).getKeyHash()) {
 			throw new BadCredentialsException(this.messages.getMessage("AnonymousAuthenticationProvider.incorrectKey",
 					"The presented AnonymousAuthenticationToken does not contain the expected key"));
 		}
-
 		return authentication;
 	}
 

+ 2 - 13
core/src/main/java/org/springframework/security/authentication/AnonymousAuthenticationToken.java

@@ -57,12 +57,8 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im
 	private AnonymousAuthenticationToken(Integer keyHash, Object principal,
 			Collection<? extends GrantedAuthority> authorities) {
 		super(authorities);
-
-		if (principal == null || "".equals(principal)) {
-			throw new IllegalArgumentException("principal cannot be null or empty");
-		}
+		Assert.isTrue(principal != null && !"".equals(principal), "principal cannot be null or empty");
 		Assert.notEmpty(authorities, "authorities cannot be null or empty");
-
 		this.keyHash = keyHash;
 		this.principal = principal;
 		setAuthenticated(true);
@@ -78,17 +74,10 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken im
 		if (!super.equals(obj)) {
 			return false;
 		}
-
 		if (obj instanceof AnonymousAuthenticationToken) {
 			AnonymousAuthenticationToken test = (AnonymousAuthenticationToken) obj;
-
-			if (this.getKeyHash() != test.getKeyHash()) {
-				return false;
-			}
-
-			return true;
+			return (this.getKeyHash() == test.getKeyHash());
 		}
-
 		return false;
 	}
 

+ 0 - 2
core/src/main/java/org/springframework/security/authentication/AuthenticationTrustResolverImpl.java

@@ -48,7 +48,6 @@ public class AuthenticationTrustResolverImpl implements AuthenticationTrustResol
 		if ((this.anonymousClass == null) || (authentication == null)) {
 			return false;
 		}
-
 		return this.anonymousClass.isAssignableFrom(authentication.getClass());
 	}
 
@@ -57,7 +56,6 @@ public class AuthenticationTrustResolverImpl implements AuthenticationTrustResol
 		if ((this.rememberMeClass == null) || (authentication == null)) {
 			return false;
 		}
-
 		return this.rememberMeClass.isAssignableFrom(authentication.getClass());
 	}
 

+ 0 - 4
core/src/main/java/org/springframework/security/authentication/CachingUserDetailsService.java

@@ -47,16 +47,12 @@ public class CachingUserDetailsService implements UserDetailsService {
 	@Override
 	public UserDetails loadUserByUsername(String username) {
 		UserDetails user = this.userCache.getUserFromCache(username);
-
 		if (user == null) {
 			user = this.delegate.loadUserByUsername(username);
 		}
-
 		Assert.notNull(user, () -> "UserDetailsService " + this.delegate + " returned null for username " + username
 				+ ". " + "This is an interface contract violation");
-
 		this.userCache.putUserInCache(user);
-
 		return user;
 	}
 

+ 0 - 3
core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java

@@ -79,7 +79,6 @@ public class DefaultAuthenticationEventPublisher
 
 	public DefaultAuthenticationEventPublisher(ApplicationEventPublisher applicationEventPublisher) {
 		this.applicationEventPublisher = applicationEventPublisher;
-
 		addMapping(BadCredentialsException.class.getName(), AuthenticationFailureBadCredentialsEvent.class);
 		addMapping(UsernameNotFoundException.class.getName(), AuthenticationFailureBadCredentialsEvent.class);
 		addMapping(AccountExpiredException.class.getName(), AuthenticationFailureExpiredEvent.class);
@@ -105,7 +104,6 @@ public class DefaultAuthenticationEventPublisher
 	public void publishAuthenticationFailure(AuthenticationException exception, Authentication authentication) {
 		Constructor<? extends AbstractAuthenticationEvent> constructor = getEventConstructor(exception);
 		AbstractAuthenticationEvent event = null;
-
 		if (constructor != null) {
 			try {
 				event = constructor.newInstance(authentication, exception);
@@ -113,7 +111,6 @@ public class DefaultAuthenticationEventPublisher
 			catch (IllegalAccessException | InvocationTargetException | InstantiationException ignored) {
 			}
 		}
-
 		if (event != null) {
 			if (this.applicationEventPublisher != null) {
 				this.applicationEventPublisher.publishEvent(event);

+ 6 - 22
core/src/main/java/org/springframework/security/authentication/ProviderManager.java

@@ -27,6 +27,7 @@ import org.springframework.beans.factory.InitializingBean;
 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.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.CredentialsContainer;
@@ -134,13 +135,10 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 	}
 
 	private void checkState() {
-		if (this.parent == null && this.providers.isEmpty()) {
-			throw new IllegalArgumentException(
-					"A parent AuthenticationManager or a list " + "of AuthenticationProviders is required");
-		}
-		else if (CollectionUtils.contains(this.providers.iterator(), null)) {
-			throw new IllegalArgumentException("providers list cannot contain null values");
-		}
+		Assert.isTrue(this.parent != null || !this.providers.isEmpty(),
+				"A parent AuthenticationManager or a list of AuthenticationProviders is required");
+		Assert.isTrue(!CollectionUtils.contains(this.providers.iterator(), null),
+				"providers list cannot contain null values");
 	}
 
 	/**
@@ -170,20 +168,13 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 		AuthenticationException parentException = null;
 		Authentication result = null;
 		Authentication parentResult = null;
-		boolean debug = logger.isDebugEnabled();
-
 		for (AuthenticationProvider provider : getProviders()) {
 			if (!provider.supports(toTest)) {
 				continue;
 			}
-
-			if (debug) {
-				logger.debug("Authentication attempt using " + provider.getClass().getName());
-			}
-
+			logger.debug(LogMessage.format("Authentication attempt using %s", provider.getClass().getName()));
 			try {
 				result = provider.authenticate(authentication);
-
 				if (result != null) {
 					copyDetails(authentication, result);
 					break;
@@ -199,7 +190,6 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 				lastException = ex;
 			}
 		}
-
 		if (result == null && this.parent != null) {
 			// Allow the parent to try.
 			try {
@@ -217,14 +207,12 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 				lastException = ex;
 			}
 		}
-
 		if (result != null) {
 			if (this.eraseCredentialsAfterAuthentication && (result instanceof CredentialsContainer)) {
 				// Authentication is complete. Remove credentials and other secret data
 				// from authentication
 				((CredentialsContainer) result).eraseCredentials();
 			}
-
 			// If the parent AuthenticationManager was attempted and successful then it
 			// will publish an AuthenticationSuccessEvent
 			// This check prevents a duplicate AuthenticationSuccessEvent if the parent
@@ -236,12 +224,10 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 		}
 
 		// Parent was null, or didn't authenticate (or throw an exception).
-
 		if (lastException == null) {
 			lastException = new ProviderNotFoundException(this.messages.getMessage("ProviderManager.providerNotFound",
 					new Object[] { toTest.getName() }, "No AuthenticationProvider found for {0}"));
 		}
-
 		// If the parent AuthenticationManager was attempted and failed then it will
 		// publish an AbstractAuthenticationFailureEvent
 		// This check prevents a duplicate AbstractAuthenticationFailureEvent if the
@@ -249,7 +235,6 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 		if (parentException == null) {
 			prepareException(lastException, authentication);
 		}
-
 		throw lastException;
 	}
 
@@ -267,7 +252,6 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
 	private void copyDetails(Authentication source, Authentication dest) {
 		if ((dest instanceof AbstractAuthenticationToken) && (dest.getDetails() == null)) {
 			AbstractAuthenticationToken token = (AbstractAuthenticationToken) dest;
-
 			token.setDetails(source.getDetails());
 		}
 	}

+ 11 - 8
core/src/main/java/org/springframework/security/authentication/ReactiveAuthenticationManagerAdapter.java

@@ -47,14 +47,17 @@ public class ReactiveAuthenticationManagerAdapter implements ReactiveAuthenticat
 
 	@Override
 	public Mono<Authentication> authenticate(Authentication token) {
-		return Mono.just(token).publishOn(this.scheduler).flatMap((t) -> {
-			try {
-				return Mono.just(this.authenticationManager.authenticate(t));
-			}
-			catch (Throwable error) {
-				return Mono.error(error);
-			}
-		}).filter((a) -> a.isAuthenticated());
+		return Mono.just(token).publishOn(this.scheduler).flatMap(this::doAuthenticate)
+				.filter(Authentication::isAuthenticated);
+	}
+
+	private Mono<Authentication> doAuthenticate(Authentication authentication) {
+		try {
+			return Mono.just(this.authenticationManager.authenticate(authentication));
+		}
+		catch (Throwable ex) {
+			return Mono.error(ex);
+		}
 	}
 
 	/**

+ 0 - 2
core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationProvider.java

@@ -53,12 +53,10 @@ public class RememberMeAuthenticationProvider implements AuthenticationProvider,
 		if (!supports(authentication.getClass())) {
 			return null;
 		}
-
 		if (this.key.hashCode() != ((RememberMeAuthenticationToken) authentication).getKeyHash()) {
 			throw new BadCredentialsException(this.messages.getMessage("RememberMeAuthenticationProvider.incorrectKey",
 					"The presented RememberMeAuthenticationToken does not contain the expected key"));
 		}
-
 		return authentication;
 	}
 

+ 2 - 9
core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java

@@ -48,11 +48,9 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken {
 	public RememberMeAuthenticationToken(String key, Object principal,
 			Collection<? extends GrantedAuthority> authorities) {
 		super(authorities);
-
 		if ((key == null) || ("".equals(key)) || (principal == null) || "".equals(principal)) {
 			throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
 		}
-
 		this.keyHash = key.hashCode();
 		this.principal = principal;
 		setAuthenticated(true);
@@ -68,7 +66,6 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken {
 	private RememberMeAuthenticationToken(Integer keyHash, Object principal,
 			Collection<? extends GrantedAuthority> authorities) {
 		super(authorities);
-
 		this.keyHash = keyHash;
 		this.principal = principal;
 		setAuthenticated(true);
@@ -97,17 +94,13 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken {
 		if (!super.equals(obj)) {
 			return false;
 		}
-
 		if (obj instanceof RememberMeAuthenticationToken) {
-			RememberMeAuthenticationToken test = (RememberMeAuthenticationToken) obj;
-
-			if (this.getKeyHash() != test.getKeyHash()) {
+			RememberMeAuthenticationToken other = (RememberMeAuthenticationToken) obj;
+			if (this.getKeyHash() != other.getKeyHash()) {
 				return false;
 			}
-
 			return true;
 		}
-
 		return false;
 	}
 

+ 3 - 5
core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java

@@ -20,6 +20,7 @@ import java.util.Collection;
 
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.SpringSecurityCoreVersion;
+import org.springframework.util.Assert;
 
 /**
  * An {@link org.springframework.security.core.Authentication} implementation that is
@@ -82,11 +83,8 @@ public class UsernamePasswordAuthenticationToken extends AbstractAuthenticationT
 
 	@Override
 	public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException {
-		if (isAuthenticated) {
-			throw new IllegalArgumentException(
-					"Cannot set this token to trusted - use constructor which takes a GrantedAuthority list instead");
-		}
-
+		Assert.isTrue(!isAuthenticated,
+				"Cannot set this token to trusted - use constructor which takes a GrantedAuthority list instead");
 		super.setAuthenticated(false);
 	}
 

+ 19 - 39
core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java

@@ -124,67 +124,54 @@ public abstract class AbstractUserDetailsAuthenticationProvider
 		Assert.isInstanceOf(UsernamePasswordAuthenticationToken.class, authentication,
 				() -> this.messages.getMessage("AbstractUserDetailsAuthenticationProvider.onlySupports",
 						"Only UsernamePasswordAuthenticationToken is supported"));
-
-		// Determine username
-		String username = (authentication.getPrincipal() == null) ? "NONE_PROVIDED" : authentication.getName();
-
+		String username = determineUsername(authentication);
 		boolean cacheWasUsed = true;
 		UserDetails user = this.userCache.getUserFromCache(username);
-
 		if (user == null) {
 			cacheWasUsed = false;
-
 			try {
 				user = retrieveUser(username, (UsernamePasswordAuthenticationToken) authentication);
 			}
-			catch (UsernameNotFoundException notFound) {
+			catch (UsernameNotFoundException ex) {
 				this.logger.debug("User '" + username + "' not found");
-
-				if (this.hideUserNotFoundExceptions) {
-					throw new BadCredentialsException(this.messages
-							.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"));
-				}
-				else {
-					throw notFound;
+				if (!this.hideUserNotFoundExceptions) {
+					throw ex;
 				}
+				throw new BadCredentialsException(this.messages
+						.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"));
 			}
-
 			Assert.notNull(user, "retrieveUser returned null - a violation of the interface contract");
 		}
-
 		try {
 			this.preAuthenticationChecks.check(user);
 			additionalAuthenticationChecks(user, (UsernamePasswordAuthenticationToken) authentication);
 		}
-		catch (AuthenticationException exception) {
-			if (cacheWasUsed) {
-				// There was a problem, so try again after checking
-				// we're using latest data (i.e. not from the cache)
-				cacheWasUsed = false;
-				user = retrieveUser(username, (UsernamePasswordAuthenticationToken) authentication);
-				this.preAuthenticationChecks.check(user);
-				additionalAuthenticationChecks(user, (UsernamePasswordAuthenticationToken) authentication);
-			}
-			else {
-				throw exception;
+		catch (AuthenticationException ex) {
+			if (!cacheWasUsed) {
+				throw ex;
 			}
+			// There was a problem, so try again after checking
+			// we're using latest data (i.e. not from the cache)
+			cacheWasUsed = false;
+			user = retrieveUser(username, (UsernamePasswordAuthenticationToken) authentication);
+			this.preAuthenticationChecks.check(user);
+			additionalAuthenticationChecks(user, (UsernamePasswordAuthenticationToken) authentication);
 		}
-
 		this.postAuthenticationChecks.check(user);
-
 		if (!cacheWasUsed) {
 			this.userCache.putUserInCache(user);
 		}
-
 		Object principalToReturn = user;
-
 		if (this.forcePrincipalAsString) {
 			principalToReturn = user.getUsername();
 		}
-
 		return createSuccessAuthentication(principalToReturn, authentication, user);
 	}
 
+	private String determineUsername(Authentication authentication) {
+		return (authentication.getPrincipal() == null) ? "NONE_PROVIDED" : authentication.getName();
+	}
+
 	/**
 	 * Creates a successful {@link Authentication} object.
 	 * <p>
@@ -209,7 +196,6 @@ public abstract class AbstractUserDetailsAuthenticationProvider
 		UsernamePasswordAuthenticationToken result = new UsernamePasswordAuthenticationToken(principal,
 				authentication.getCredentials(), this.authoritiesMapper.mapAuthorities(user.getAuthorities()));
 		result.setDetails(authentication.getDetails());
-
 		return result;
 	}
 
@@ -333,21 +319,16 @@ public abstract class AbstractUserDetailsAuthenticationProvider
 		public void check(UserDetails user) {
 			if (!user.isAccountNonLocked()) {
 				AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account is locked");
-
 				throw new LockedException(AbstractUserDetailsAuthenticationProvider.this.messages
 						.getMessage("AbstractUserDetailsAuthenticationProvider.locked", "User account is locked"));
 			}
-
 			if (!user.isEnabled()) {
 				AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account is disabled");
-
 				throw new DisabledException(AbstractUserDetailsAuthenticationProvider.this.messages
 						.getMessage("AbstractUserDetailsAuthenticationProvider.disabled", "User is disabled"));
 			}
-
 			if (!user.isAccountNonExpired()) {
 				AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account is expired");
-
 				throw new AccountExpiredException(AbstractUserDetailsAuthenticationProvider.this.messages
 						.getMessage("AbstractUserDetailsAuthenticationProvider.expired", "User account has expired"));
 			}
@@ -361,7 +342,6 @@ public abstract class AbstractUserDetailsAuthenticationProvider
 		public void check(UserDetails user) {
 			if (!user.isCredentialsNonExpired()) {
 				AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account credentials have expired");
-
 				throw new CredentialsExpiredException(AbstractUserDetailsAuthenticationProvider.this.messages
 						.getMessage("AbstractUserDetailsAuthenticationProvider.credentialsExpired",
 								"User credentials have expired"));

+ 0 - 4
core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java

@@ -69,16 +69,12 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication
 			UsernamePasswordAuthenticationToken authentication) throws AuthenticationException {
 		if (authentication.getCredentials() == null) {
 			this.logger.debug("Authentication failed: no credentials provided");
-
 			throw new BadCredentialsException(this.messages
 					.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"));
 		}
-
 		String presentedPassword = authentication.getCredentials().toString();
-
 		if (!this.passwordEncoder.matches(presentedPassword, userDetails.getPassword())) {
 			this.logger.debug("Authentication failed: password does not match stored value");
-
 			throw new BadCredentialsException(this.messages
 					.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"));
 		}

+ 15 - 15
core/src/main/java/org/springframework/security/authentication/event/LoggerListener.java

@@ -20,6 +20,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.context.ApplicationListener;
+import org.springframework.core.log.LogMessage;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -44,23 +45,22 @@ public class LoggerListener implements ApplicationListener<AbstractAuthenticatio
 		if (!this.logInteractiveAuthenticationSuccessEvents && event instanceof InteractiveAuthenticationSuccessEvent) {
 			return;
 		}
+		logger.warn(LogMessage.of(() -> getLogMessage(event)));
+	}
 
-		if (logger.isWarnEnabled()) {
-			final StringBuilder builder = new StringBuilder();
-			builder.append("Authentication event ");
-			builder.append(ClassUtils.getShortName(event.getClass()));
-			builder.append(": ");
-			builder.append(event.getAuthentication().getName());
-			builder.append("; details: ");
-			builder.append(event.getAuthentication().getDetails());
-
-			if (event instanceof AbstractAuthenticationFailureEvent) {
-				builder.append("; exception: ");
-				builder.append(((AbstractAuthenticationFailureEvent) event).getException().getMessage());
-			}
-
-			logger.warn(builder.toString());
+	private String getLogMessage(AbstractAuthenticationEvent event) {
+		StringBuilder builder = new StringBuilder();
+		builder.append("Authentication event ");
+		builder.append(ClassUtils.getShortName(event.getClass()));
+		builder.append(": ");
+		builder.append(event.getAuthentication().getName());
+		builder.append("; details: ");
+		builder.append(event.getAuthentication().getDetails());
+		if (event instanceof AbstractAuthenticationFailureEvent) {
+			builder.append("; exception: ");
+			builder.append(((AbstractAuthenticationFailureEvent) event).getException().getMessage());
 		}
+		return builder.toString();
 	}
 
 	public boolean isLogInteractiveAuthenticationSuccessEvents() {

+ 38 - 45
core/src/main/java/org/springframework/security/authentication/jaas/AbstractJaasAuthenticationProvider.java

@@ -36,6 +36,7 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationEventPublisherAware;
 import org.springframework.context.ApplicationListener;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.authentication.jaas.event.JaasAuthenticationFailedEvent;
@@ -46,6 +47,7 @@ import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.session.SessionDestroyedEvent;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.ObjectUtils;
 
 /**
@@ -161,56 +163,51 @@ public abstract class AbstractJaasAuthenticationProvider implements Authenticati
 		if (!(auth instanceof UsernamePasswordAuthenticationToken)) {
 			return null;
 		}
-
 		UsernamePasswordAuthenticationToken request = (UsernamePasswordAuthenticationToken) auth;
 		Set<GrantedAuthority> authorities;
-
 		try {
 			// Create the LoginContext object, and pass our InternallCallbackHandler
 			LoginContext loginContext = createLoginContext(new InternalCallbackHandler(auth));
-
 			// Attempt to login the user, the LoginContext will call our
 			// InternalCallbackHandler at this point.
 			loginContext.login();
-
-			// Create a set to hold the authorities, and add any that have already been
-			// applied.
-			authorities = new HashSet<>();
-
 			// Get the subject principals and pass them to each of the AuthorityGranters
 			Set<Principal> principals = loginContext.getSubject().getPrincipals();
-
-			for (Principal principal : principals) {
-				for (AuthorityGranter granter : this.authorityGranters) {
-					Set<String> roles = granter.grant(principal);
-
-					// If the granter doesn't wish to grant any authorities, it should
-					// return null.
-					if ((roles != null) && !roles.isEmpty()) {
-						for (String role : roles) {
-							authorities.add(new JaasGrantedAuthority(role, principal));
-						}
-					}
-				}
-			}
-
+			// Create a set to hold the authorities, and add any that have already been
+			// applied.
+			authorities = getAuthorities(principals);
 			// Convert the authorities set back to an array and apply it to the token.
 			JaasAuthenticationToken result = new JaasAuthenticationToken(request.getPrincipal(),
 					request.getCredentials(), new ArrayList<>(authorities), loginContext);
-
 			// Publish the success event
 			publishSuccessEvent(result);
-
 			// we're done, return the token.
 			return result;
 
 		}
-		catch (LoginException loginException) {
-			AuthenticationException ase = this.loginExceptionResolver.resolveException(loginException);
+		catch (LoginException ex) {
+			AuthenticationException resolvedException = this.loginExceptionResolver.resolveException(ex);
+			publishFailureEvent(request, resolvedException);
+			throw resolvedException;
+		}
+	}
 
-			publishFailureEvent(request, ase);
-			throw ase;
+	private Set<GrantedAuthority> getAuthorities(Set<Principal> principals) {
+		Set<GrantedAuthority> authorities;
+		authorities = new HashSet<>();
+		for (Principal principal : principals) {
+			for (AuthorityGranter granter : this.authorityGranters) {
+				Set<String> roles = granter.grant(principal);
+				// If the granter doesn't wish to grant any authorities,
+				// it should return null.
+				if (!CollectionUtils.isEmpty(roles)) {
+					for (String role : roles) {
+						authorities.add(new JaasGrantedAuthority(role, principal));
+					}
+				}
+			}
 		}
+		return authorities;
 	}
 
 	/**
@@ -230,32 +227,17 @@ public abstract class AbstractJaasAuthenticationProvider implements Authenticati
 	 */
 	protected void handleLogout(SessionDestroyedEvent event) {
 		List<SecurityContext> contexts = event.getSecurityContexts();
-
 		if (contexts.isEmpty()) {
 			this.log.debug("The destroyed session has no SecurityContexts");
-
 			return;
 		}
-
 		for (SecurityContext context : contexts) {
 			Authentication auth = context.getAuthentication();
-
 			if ((auth != null) && (auth instanceof JaasAuthenticationToken)) {
 				JaasAuthenticationToken token = (JaasAuthenticationToken) auth;
-
 				try {
 					LoginContext loginContext = token.getLoginContext();
-					boolean debug = this.log.isDebugEnabled();
-					if (loginContext != null) {
-						if (debug) {
-							this.log.debug("Logging principal: [" + token.getPrincipal() + "] out of LoginContext");
-						}
-						loginContext.logout();
-					}
-					else if (debug) {
-						this.log.debug("Cannot logout principal: [" + token.getPrincipal() + "] from LoginContext. "
-								+ "The LoginContext is unavailable");
-					}
+					logout(token, loginContext);
 				}
 				catch (LoginException ex) {
 					this.log.warn("Error error logging out of LoginContext", ex);
@@ -264,6 +246,17 @@ public abstract class AbstractJaasAuthenticationProvider implements Authenticati
 		}
 	}
 
+	private void logout(JaasAuthenticationToken token, LoginContext loginContext) throws LoginException {
+		if (loginContext != null) {
+			this.log.debug(
+					LogMessage.of(() -> "Logging principal: [" + token.getPrincipal() + "] out of LoginContext"));
+			loginContext.logout();
+			return;
+		}
+		this.log.debug(LogMessage.of(() -> "Cannot logout principal: [" + token.getPrincipal()
+				+ "] from LoginContext. The LoginContext is unavailable"));
+	}
+
 	@Override
 	public void onApplicationEvent(SessionDestroyedEvent event) {
 		handleLogout(event);

+ 2 - 11
core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java

@@ -30,6 +30,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.core.io.Resource;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.authentication.jaas.event.JaasAuthenticationFailedEvent;
@@ -153,7 +154,6 @@ public class JaasAuthenticationProvider extends AbstractJaasAuthenticationProvid
 		Assert.hasLength(getLoginContextName(), () -> "loginContextName must be set on " + getClass());
 		Assert.notNull(this.loginConfig, () -> "loginConfig must be set on " + getClass());
 		configureJaas(this.loginConfig);
-
 		Assert.notNull(Configuration.getConfiguration(),
 				"As per https://java.sun.com/j2se/1.5.0/docs/api/javax/security/auth/login/Configuration.html "
 						+ "\"If a Configuration object was set via the Configuration.setConfiguration method, then that object is "
@@ -173,7 +173,6 @@ public class JaasAuthenticationProvider extends AbstractJaasAuthenticationProvid
 	 */
 	protected void configureJaas(Resource loginConfig) throws IOException {
 		configureJaasUsingLoop();
-
 		if (this.refreshConfigurationOnStartup) {
 			// Overcome issue in SEC-760
 			Configuration.getConfiguration().refresh();
@@ -189,38 +188,30 @@ public class JaasAuthenticationProvider extends AbstractJaasAuthenticationProvid
 	private void configureJaasUsingLoop() throws IOException {
 		String loginConfigUrl = convertLoginConfigToUrl();
 		boolean alreadySet = false;
-
 		int n = 1;
 		final String prefix = "login.config.url.";
 		String existing;
-
 		while ((existing = Security.getProperty(prefix + n)) != null) {
 			alreadySet = existing.equals(loginConfigUrl);
-
 			if (alreadySet) {
 				break;
 			}
-
 			n++;
 		}
-
 		if (!alreadySet) {
 			String key = prefix + n;
-			log.debug("Setting security property [" + key + "] to: " + loginConfigUrl);
+			log.debug(LogMessage.format("Setting security property [%s] to: %s", key, loginConfigUrl));
 			Security.setProperty(key, loginConfigUrl);
 		}
 	}
 
 	private String convertLoginConfigToUrl() throws IOException {
 		String loginConfigPath;
-
 		try {
 			loginConfigPath = this.loginConfig.getFile().getAbsolutePath().replace(File.separatorChar, '/');
-
 			if (!loginConfigPath.startsWith("/")) {
 				loginConfigPath = "/" + loginConfigPath;
 			}
-
 			return new URL("file", "", loginConfigPath).toString();
 		}
 		catch (IOException ex) {

+ 8 - 12
core/src/main/java/org/springframework/security/authentication/jaas/JaasNameCallbackHandler.java

@@ -46,20 +46,16 @@ public class JaasNameCallbackHandler implements JaasAuthenticationCallbackHandle
 	@Override
 	public void handle(Callback callback, Authentication authentication) {
 		if (callback instanceof NameCallback) {
-			NameCallback ncb = (NameCallback) callback;
-			String username;
-
-			Object principal = authentication.getPrincipal();
-
-			if (principal instanceof UserDetails) {
-				username = ((UserDetails) principal).getUsername();
-			}
-			else {
-				username = principal.toString();
-			}
+			((NameCallback) callback).setName(getUserName(authentication));
+		}
+	}
 
-			ncb.setName(username);
+	private String getUserName(Authentication authentication) {
+		Object principal = authentication.getPrincipal();
+		if (principal instanceof UserDetails) {
+			return ((UserDetails) principal).getUsername();
 		}
+		return principal.toString();
 	}
 
 }

+ 1 - 2
core/src/main/java/org/springframework/security/authentication/jaas/JaasPasswordCallbackHandler.java

@@ -47,8 +47,7 @@ public class JaasPasswordCallbackHandler implements JaasAuthenticationCallbackHa
 	@Override
 	public void handle(Callback callback, Authentication auth) {
 		if (callback instanceof PasswordCallback) {
-			PasswordCallback pc = (PasswordCallback) callback;
-			pc.setPassword(auth.getCredentials().toString().toCharArray());
+			((PasswordCallback) callback).setPassword(auth.getCredentials().toString().toCharArray());
 		}
 	}
 

+ 8 - 21
core/src/main/java/org/springframework/security/authentication/jaas/SecurityContextLoginModule.java

@@ -73,9 +73,7 @@ public class SecurityContextLoginModule implements LoginModule {
 		if (this.authen == null) {
 			return false;
 		}
-
 		this.authen = null;
-
 		return true;
 	}
 
@@ -91,9 +89,7 @@ public class SecurityContextLoginModule implements LoginModule {
 		if (this.authen == null) {
 			return false;
 		}
-
 		this.subject.getPrincipals().add(this.authen);
-
 		return true;
 	}
 
@@ -119,7 +115,6 @@ public class SecurityContextLoginModule implements LoginModule {
 	@SuppressWarnings("unchecked")
 	public void initialize(Subject subject, CallbackHandler callbackHandler, Map sharedState, Map options) {
 		this.subject = subject;
-
 		if (options != null) {
 			this.ignoreMissingAuthentication = "true".equals(options.get("ignoreMissingAuthentication"));
 		}
@@ -135,21 +130,15 @@ public class SecurityContextLoginModule implements LoginModule {
 	@Override
 	public boolean login() throws LoginException {
 		this.authen = SecurityContextHolder.getContext().getAuthentication();
-
-		if (this.authen == null) {
-			String msg = "Login cannot complete, authentication not found in security context";
-
-			if (this.ignoreMissingAuthentication) {
-				log.warn(msg);
-
-				return false;
-			}
-			else {
-				throw new LoginException(msg);
-			}
+		if (this.authen != null) {
+			return true;
 		}
-
-		return true;
+		String msg = "Login cannot complete, authentication not found in security context";
+		if (!this.ignoreMissingAuthentication) {
+			throw new LoginException(msg);
+		}
+		log.warn(msg);
+		return false;
 	}
 
 	/**
@@ -163,10 +152,8 @@ public class SecurityContextLoginModule implements LoginModule {
 		if (this.authen == null) {
 			return false;
 		}
-
 		this.subject.getPrincipals().remove(this.authen);
 		this.authen = null;
-
 		return true;
 	}
 

+ 2 - 3
core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationManagerImpl.java

@@ -46,12 +46,11 @@ public class RemoteAuthenticationManagerImpl implements RemoteAuthenticationMana
 	public Collection<? extends GrantedAuthority> attemptAuthentication(String username, String password)
 			throws RemoteAuthenticationException {
 		UsernamePasswordAuthenticationToken request = new UsernamePasswordAuthenticationToken(username, password);
-
 		try {
 			return this.authenticationManager.authenticate(request).getAuthorities();
 		}
-		catch (AuthenticationException authEx) {
-			throw new RemoteAuthenticationException(authEx.getMessage());
+		catch (AuthenticationException ex) {
+			throw new RemoteAuthenticationException(ex.getMessage());
 		}
 	}
 

+ 0 - 1
core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProvider.java

@@ -66,7 +66,6 @@ public class RemoteAuthenticationProvider implements AuthenticationProvider, Ini
 		String password = (credentials != null) ? credentials.toString() : null;
 		Collection<? extends GrantedAuthority> authorities = this.remoteAuthenticationManager
 				.attemptAuthentication(username, password);
-
 		return new UsernamePasswordAuthenticationToken(username, password, authorities);
 	}
 

+ 5 - 1
core/src/main/java/org/springframework/security/authorization/AuthenticatedReactiveAuthorizationManager.java

@@ -39,10 +39,14 @@ public class AuthenticatedReactiveAuthorizationManager<T> implements ReactiveAut
 
 	@Override
 	public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
-		return authentication.filter(this::isNotAnonymous).map((a) -> new AuthorizationDecision(a.isAuthenticated()))
+		return authentication.filter(this::isNotAnonymous).map(this::getAuthorizationDecision)
 				.defaultIfEmpty(new AuthorizationDecision(false));
 	}
 
+	private AuthorizationDecision getAuthorizationDecision(Authentication authentication) {
+		return new AuthorizationDecision(authentication.isAuthenticated());
+	}
+
 	/**
 	 * Verify (via {@link AuthenticationTrustResolver}) that the given authentication is
 	 * not anonymous.

+ 3 - 5
core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java

@@ -22,6 +22,7 @@ import java.util.List;
 import reactor.core.publisher.Mono;
 
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.util.Assert;
 
 /**
@@ -42,9 +43,8 @@ public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthori
 
 	@Override
 	public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
-		return authentication.filter((a) -> a.isAuthenticated()).flatMapIterable((a) -> a.getAuthorities())
-				.map((g) -> g.getAuthority()).any((a) -> this.authorities.contains(a))
-				.map((hasAuthority) -> new AuthorizationDecision(hasAuthority))
+		return authentication.filter((a) -> a.isAuthenticated()).flatMapIterable(Authentication::getAuthorities)
+				.map(GrantedAuthority::getAuthority).any(this.authorities::contains).map(AuthorizationDecision::new)
 				.defaultIfEmpty(new AuthorizationDecision(false));
 	}
 
@@ -74,7 +74,6 @@ public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthori
 		for (String authority : authorities) {
 			Assert.notNull(authority, "authority cannot be null");
 		}
-
 		return new AuthorityReactiveAuthorizationManager<>(authorities);
 	}
 
@@ -104,7 +103,6 @@ public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthori
 		for (String role : roles) {
 			Assert.notNull(role, "role cannot be null");
 		}
-
 		return hasAnyAuthority(toNamedRolesArray(roles));
 	}
 

+ 2 - 2
core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationManager.java

@@ -47,9 +47,9 @@ public interface ReactiveAuthorizationManager<T> {
 	 * denied
 	 */
 	default Mono<Void> verify(Mono<Authentication> authentication, T object) {
-		return check(authentication, object).filter((d) -> d.isGranted())
+		return check(authentication, object).filter(AuthorizationDecision::isGranted)
 				.switchIfEmpty(Mono.defer(() -> Mono.error(new AccessDeniedException("Access Denied"))))
-				.flatMap((d) -> Mono.empty());
+				.flatMap((decision) -> Mono.empty());
 	}
 
 }

+ 0 - 1
core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextCallable.java

@@ -79,7 +79,6 @@ public final class DelegatingSecurityContextCallable<V> implements Callable<V> {
 	@Override
 	public V call() throws Exception {
 		this.originalSecurityContext = SecurityContextHolder.getContext();
-
 		try {
 			SecurityContextHolder.setContext(this.delegateSecurityContext);
 			return this.delegate.call();

+ 1 - 2
core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextExecutor.java

@@ -59,8 +59,7 @@ public class DelegatingSecurityContextExecutor extends AbstractDelegatingSecurit
 
 	@Override
 	public final void execute(Runnable task) {
-		task = wrap(task);
-		this.delegate.execute(task);
+		this.delegate.execute(wrap(task));
 	}
 
 	protected final Executor getDelegateExecutor() {

+ 3 - 6
core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextExecutorService.java

@@ -90,20 +90,17 @@ public class DelegatingSecurityContextExecutorService extends DelegatingSecurity
 
 	@Override
 	public final <T> Future<T> submit(Callable<T> task) {
-		task = wrap(task);
-		return getDelegate().submit(task);
+		return getDelegate().submit(wrap(task));
 	}
 
 	@Override
 	public final <T> Future<T> submit(Runnable task, T result) {
-		task = wrap(task);
-		return getDelegate().submit(task, result);
+		return getDelegate().submit(wrap(task), result);
 	}
 
 	@Override
 	public final Future<?> submit(Runnable task) {
-		task = wrap(task);
-		return getDelegate().submit(task);
+		return getDelegate().submit(wrap(task));
 	}
 
 	@Override

+ 0 - 1
core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnable.java

@@ -77,7 +77,6 @@ public final class DelegatingSecurityContextRunnable implements Runnable {
 	@Override
 	public void run() {
 		this.originalSecurityContext = SecurityContextHolder.getContext();
-
 		try {
 			SecurityContextHolder.setContext(this.delegateSecurityContext);
 			this.delegate.run();

+ 4 - 8
core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextScheduledExecutorService.java

@@ -61,26 +61,22 @@ public final class DelegatingSecurityContextScheduledExecutorService extends Del
 
 	@Override
 	public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) {
-		command = wrap(command);
-		return getDelegate().schedule(command, delay, unit);
+		return getDelegate().schedule(wrap(command), delay, unit);
 	}
 
 	@Override
 	public <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUnit unit) {
-		callable = wrap(callable);
-		return getDelegate().schedule(callable, delay, unit);
+		return getDelegate().schedule(wrap(callable), delay, unit);
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit) {
-		command = wrap(command);
-		return getDelegate().scheduleAtFixedRate(command, initialDelay, period, unit);
+		return getDelegate().scheduleAtFixedRate(wrap(command), initialDelay, period, unit);
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit) {
-		command = wrap(command);
-		return getDelegate().scheduleWithFixedDelay(command, initialDelay, delay, unit);
+		return getDelegate().scheduleWithFixedDelay(wrap(command), initialDelay, delay, unit);
 	}
 
 	private ScheduledExecutorService getDelegate() {

+ 2 - 4
core/src/main/java/org/springframework/security/converter/RsaKeyConverters.java

@@ -72,7 +72,7 @@ public final class RsaKeyConverters {
 		return (source) -> {
 			List<String> lines = readAllLines(source);
 			Assert.isTrue(!lines.isEmpty() && lines.get(0).startsWith(PKCS8_PEM_HEADER),
-					"Key is not in PEM-encoded PKCS#8 format, " + "please check that the header begins with -----"
+					"Key is not in PEM-encoded PKCS#8 format, please check that the header begins with -----"
 							+ PKCS8_PEM_HEADER + "-----");
 			StringBuilder base64Encoded = new StringBuilder();
 			for (String line : lines) {
@@ -81,7 +81,6 @@ public final class RsaKeyConverters {
 				}
 			}
 			byte[] pkcs8 = Base64.getDecoder().decode(base64Encoded.toString());
-
 			try {
 				return (RSAPrivateKey) keyFactory.generatePrivate(new PKCS8EncodedKeySpec(pkcs8));
 			}
@@ -105,7 +104,7 @@ public final class RsaKeyConverters {
 		return (source) -> {
 			List<String> lines = readAllLines(source);
 			Assert.isTrue(!lines.isEmpty() && lines.get(0).startsWith(X509_PEM_HEADER),
-					"Key is not in PEM-encoded X.509 format, " + "please check that the header begins with -----"
+					"Key is not in PEM-encoded X.509 format, please check that the header begins with -----"
 							+ X509_PEM_HEADER + "-----");
 			StringBuilder base64Encoded = new StringBuilder();
 			for (String line : lines) {
@@ -114,7 +113,6 @@ public final class RsaKeyConverters {
 				}
 			}
 			byte[] x509 = Base64.getDecoder().decode(base64Encoded.toString());
-
 			try {
 				return (RSAPublicKey) keyFactory.generatePublic(new X509EncodedKeySpec(x509));
 			}

+ 5 - 10
core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java

@@ -53,14 +53,6 @@ public final class SpringSecurityCoreVersion {
 	private SpringSecurityCoreVersion() {
 	}
 
-	public static String getVersion() {
-		Package pkg = SpringSecurityCoreVersion.class.getPackage();
-		return (pkg != null) ? pkg.getImplementationVersion() : null;
-	}
-
-	/**
-	 * Performs version checks
-	 */
 	private static void performVersionChecks() {
 		performVersionChecks(MIN_SPRING_VERSION);
 	}
@@ -76,11 +68,9 @@ public final class SpringSecurityCoreVersion {
 		// Check Spring Compatibility
 		String springVersion = SpringVersion.getVersion();
 		String version = getVersion();
-
 		if (disableChecks(springVersion, version)) {
 			return;
 		}
-
 		logger.info("You are running with Spring Security Core " + version);
 		if (new ComparableVersion(springVersion).compareTo(new ComparableVersion(minSpringVersion)) < 0) {
 			logger.warn("**** You are advised to use Spring " + minSpringVersion
@@ -88,6 +78,11 @@ public final class SpringSecurityCoreVersion {
 		}
 	}
 
+	public static String getVersion() {
+		Package pkg = SpringSecurityCoreVersion.class.getPackage();
+		return (pkg != null) ? pkg.getImplementationVersion() : null;
+	}
+
 	/**
 	 * Disable if springVersion and springSecurityVersion are the same to allow working
 	 * with Uber Jars.

+ 4 - 5
core/src/main/java/org/springframework/security/core/authority/AuthorityUtils.java

@@ -34,10 +34,13 @@ import org.springframework.util.StringUtils;
  *
  * @author Luke Taylor
  */
-public abstract class AuthorityUtils {
+public final class AuthorityUtils {
 
 	public static final List<GrantedAuthority> NO_AUTHORITIES = Collections.emptyList();
 
+	private AuthorityUtils() {
+	}
+
 	/**
 	 * Creates a array of GrantedAuthority objects from a comma-separated string
 	 * representation (e.g. "ROLE_A, ROLE_B, ROLE_C").
@@ -56,11 +59,9 @@ public abstract class AuthorityUtils {
 	public static Set<String> authorityListToSet(Collection<? extends GrantedAuthority> userAuthorities) {
 		Assert.notNull(userAuthorities, "userAuthorities cannot be null");
 		Set<String> set = new HashSet<>(userAuthorities.size());
-
 		for (GrantedAuthority authority : userAuthorities) {
 			set.add(authority.getAuthority());
 		}
-
 		return set;
 	}
 
@@ -71,11 +72,9 @@ public abstract class AuthorityUtils {
 	 */
 	public static List<GrantedAuthority> createAuthorityList(String... authorities) {
 		List<GrantedAuthority> grantedAuthorities = new ArrayList<>(authorities.length);
-
 		for (String authority : authorities) {
 			grantedAuthorities.add(new SimpleGrantedAuthority(authority));
 		}
-
 		return grantedAuthorities;
 	}
 

+ 0 - 2
core/src/main/java/org/springframework/security/core/authority/SimpleGrantedAuthority.java

@@ -50,11 +50,9 @@ public final class SimpleGrantedAuthority implements GrantedAuthority {
 		if (this == obj) {
 			return true;
 		}
-
 		if (obj instanceof SimpleGrantedAuthority) {
 			return this.role.equals(((SimpleGrantedAuthority) obj).role);
 		}
-
 		return false;
 	}
 

+ 11 - 14
core/src/main/java/org/springframework/security/core/authority/mapping/MapBasedAttributes2GrantedAuthoritiesMapper.java

@@ -58,16 +58,15 @@ public class MapBasedAttributes2GrantedAuthoritiesMapper
 	 */
 	@Override
 	public List<GrantedAuthority> getGrantedAuthorities(Collection<String> attributes) {
-		ArrayList<GrantedAuthority> gaList = new ArrayList<>();
+		ArrayList<GrantedAuthority> result = new ArrayList<>();
 		for (String attribute : attributes) {
-			Collection<GrantedAuthority> c = this.attributes2grantedAuthoritiesMap.get(attribute);
-			if (c != null) {
-				gaList.addAll(c);
+			Collection<GrantedAuthority> granted = this.attributes2grantedAuthoritiesMap.get(attribute);
+			if (granted != null) {
+				result.addAll(granted);
 			}
 		}
-		gaList.trimToSize();
-
-		return gaList;
+		result.trimToSize();
+		return result;
 	}
 
 	/**
@@ -85,7 +84,6 @@ public class MapBasedAttributes2GrantedAuthoritiesMapper
 		Assert.notEmpty(attributes2grantedAuthoritiesMap,
 				"A non-empty attributes2grantedAuthoritiesMap must be supplied");
 		this.attributes2grantedAuthoritiesMap = preProcessMap(attributes2grantedAuthoritiesMap);
-
 		this.mappableAttributes = Collections.unmodifiableSet(this.attributes2grantedAuthoritiesMap.keySet());
 	}
 
@@ -96,7 +94,6 @@ public class MapBasedAttributes2GrantedAuthoritiesMapper
 	 */
 	private Map<String, Collection<GrantedAuthority>> preProcessMap(Map<?, ?> orgMap) {
 		Map<String, Collection<GrantedAuthority>> result = new HashMap<>(orgMap.size());
-
 		for (Map.Entry<?, ?> entry : orgMap.entrySet()) {
 			Assert.isInstanceOf(String.class, entry.getKey(),
 					"attributes2grantedAuthoritiesMap contains non-String objects as keys");
@@ -156,11 +153,11 @@ public class MapBasedAttributes2GrantedAuthoritiesMapper
 	}
 
 	private void addGrantedAuthorityCollection(Collection<GrantedAuthority> result, String value) {
-		StringTokenizer st = new StringTokenizer(value, this.stringSeparator, false);
-		while (st.hasMoreTokens()) {
-			String nextToken = st.nextToken();
-			if (StringUtils.hasText(nextToken)) {
-				result.add(new SimpleGrantedAuthority(nextToken));
+		StringTokenizer tokenizer = new StringTokenizer(value, this.stringSeparator, false);
+		while (tokenizer.hasMoreTokens()) {
+			String token = tokenizer.nextToken();
+			if (StringUtils.hasText(token)) {
+				result.add(new SimpleGrantedAuthority(token));
 			}
 		}
 	}

+ 0 - 4
core/src/main/java/org/springframework/security/core/authority/mapping/SimpleAuthorityMapper.java

@@ -63,11 +63,9 @@ public final class SimpleAuthorityMapper implements GrantedAuthoritiesMapper, In
 		for (GrantedAuthority authority : authorities) {
 			mapped.add(mapAuthority(authority.getAuthority()));
 		}
-
 		if (this.defaultAuthority != null) {
 			mapped.add(this.defaultAuthority);
 		}
-
 		return mapped;
 	}
 
@@ -78,11 +76,9 @@ public final class SimpleAuthorityMapper implements GrantedAuthoritiesMapper, In
 		else if (this.convertToLowerCase) {
 			name = name.toLowerCase();
 		}
-
 		if (this.prefix.length() > 0 && !name.startsWith(this.prefix)) {
 			name = this.prefix + name;
 		}
-
 		return new SimpleGrantedAuthority(name);
 	}
 

+ 0 - 1
core/src/main/java/org/springframework/security/core/context/GlobalSecurityContextHolderStrategy.java

@@ -41,7 +41,6 @@ final class GlobalSecurityContextHolderStrategy implements SecurityContextHolder
 		if (contextHolder == null) {
 			contextHolder = new SecurityContextImpl();
 		}
-
 		return contextHolder;
 	}
 

+ 0 - 2
core/src/main/java/org/springframework/security/core/context/InheritableThreadLocalSecurityContextHolderStrategy.java

@@ -37,12 +37,10 @@ final class InheritableThreadLocalSecurityContextHolderStrategy implements Secur
 	@Override
 	public SecurityContext getContext() {
 		SecurityContext ctx = contextHolder.get();
-
 		if (ctx == null) {
 			ctx = createEmptyContext();
 			contextHolder.set(ctx);
 		}
-
 		return ctx;
 	}
 

+ 10 - 2
core/src/main/java/org/springframework/security/core/context/ReactiveSecurityContextHolder.java

@@ -41,8 +41,16 @@ public final class ReactiveSecurityContextHolder {
 	 * @return the {@code Mono<SecurityContext>}
 	 */
 	public static Mono<SecurityContext> getContext() {
-		return Mono.subscriberContext().filter((c) -> c.hasKey(SECURITY_CONTEXT_KEY))
-				.flatMap((c) -> c.<Mono<SecurityContext>>get(SECURITY_CONTEXT_KEY));
+		return Mono.subscriberContext().filter(ReactiveSecurityContextHolder::hasSecurityContext)
+				.flatMap(ReactiveSecurityContextHolder::getSecurityContext);
+	}
+
+	private static boolean hasSecurityContext(Context context) {
+		return context.hasKey(SECURITY_CONTEXT_KEY);
+	}
+
+	private static Mono<SecurityContext> getSecurityContext(Context context) {
+		return context.<Mono<SecurityContext>>get(SECURITY_CONTEXT_KEY);
 	}
 
 	/**

+ 25 - 27
core/src/main/java/org/springframework/security/core/context/SecurityContextHolder.java

@@ -67,37 +67,11 @@ public class SecurityContextHolder {
 		initialize();
 	}
 
-	/**
-	 * Explicitly clears the context value from the current thread.
-	 */
-	public static void clearContext() {
-		strategy.clearContext();
-	}
-
-	/**
-	 * Obtain the current <code>SecurityContext</code>.
-	 * @return the security context (never <code>null</code>)
-	 */
-	public static SecurityContext getContext() {
-		return strategy.getContext();
-	}
-
-	/**
-	 * Primarily for troubleshooting purposes, this method shows how many times the class
-	 * has re-initialized its <code>SecurityContextHolderStrategy</code>.
-	 * @return the count (should be one unless you've called
-	 * {@link #setStrategyName(String)} to switch to an alternate strategy.
-	 */
-	public static int getInitializeCount() {
-		return initializeCount;
-	}
-
 	private static void initialize() {
 		if (!StringUtils.hasText(strategyName)) {
 			// Set default
 			strategyName = MODE_THREADLOCAL;
 		}
-
 		if (strategyName.equals(MODE_THREADLOCAL)) {
 			strategy = new ThreadLocalSecurityContextHolderStrategy();
 		}
@@ -118,10 +92,34 @@ public class SecurityContextHolder {
 				ReflectionUtils.handleReflectionException(ex);
 			}
 		}
-
 		initializeCount++;
 	}
 
+	/**
+	 * Explicitly clears the context value from the current thread.
+	 */
+	public static void clearContext() {
+		strategy.clearContext();
+	}
+
+	/**
+	 * Obtain the current <code>SecurityContext</code>.
+	 * @return the security context (never <code>null</code>)
+	 */
+	public static SecurityContext getContext() {
+		return strategy.getContext();
+	}
+
+	/**
+	 * Primarily for troubleshooting purposes, this method shows how many times the class
+	 * has re-initialized its <code>SecurityContextHolderStrategy</code>.
+	 * @return the count (should be one unless you've called
+	 * {@link #setStrategyName(String)} to switch to an alternate strategy.
+	 */
+	public static int getInitializeCount() {
+		return initializeCount;
+	}
+
 	/**
 	 * Associates a new <code>SecurityContext</code> with the current thread of execution.
 	 * @param context the new <code>SecurityContext</code> (may not be <code>null</code>)

+ 6 - 15
core/src/main/java/org/springframework/security/core/context/SecurityContextImpl.java

@@ -18,6 +18,7 @@ package org.springframework.security.core.context;
 
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.SpringSecurityCoreVersion;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Base implementation of {@link SecurityContext}.
@@ -42,18 +43,15 @@ public class SecurityContextImpl implements SecurityContext {
 	@Override
 	public boolean equals(Object obj) {
 		if (obj instanceof SecurityContextImpl) {
-			SecurityContextImpl test = (SecurityContextImpl) obj;
-
-			if ((this.getAuthentication() == null) && (test.getAuthentication() == null)) {
+			SecurityContextImpl other = (SecurityContextImpl) obj;
+			if ((this.getAuthentication() == null) && (other.getAuthentication() == null)) {
 				return true;
 			}
-
-			if ((this.getAuthentication() != null) && (test.getAuthentication() != null)
-					&& this.getAuthentication().equals(test.getAuthentication())) {
+			if ((this.getAuthentication() != null) && (other.getAuthentication() != null)
+					&& this.getAuthentication().equals(other.getAuthentication())) {
 				return true;
 			}
 		}
-
 		return false;
 	}
 
@@ -64,12 +62,7 @@ public class SecurityContextImpl implements SecurityContext {
 
 	@Override
 	public int hashCode() {
-		if (this.authentication == null) {
-			return -1;
-		}
-		else {
-			return this.authentication.hashCode();
-		}
+		return ObjectUtils.nullSafeHashCode(this.authentication);
 	}
 
 	@Override
@@ -81,14 +74,12 @@ public class SecurityContextImpl implements SecurityContext {
 	public String toString() {
 		StringBuilder sb = new StringBuilder();
 		sb.append(super.toString());
-
 		if (this.authentication == null) {
 			sb.append(": Null authentication");
 		}
 		else {
 			sb.append(": Authentication: ").append(this.authentication);
 		}
-
 		return sb.toString();
 	}
 

+ 0 - 2
core/src/main/java/org/springframework/security/core/context/ThreadLocalSecurityContextHolderStrategy.java

@@ -38,12 +38,10 @@ final class ThreadLocalSecurityContextHolderStrategy implements SecurityContextH
 	@Override
 	public SecurityContext getContext() {
 		SecurityContext ctx = contextHolder.get();
-
 		if (ctx == null) {
 			ctx = createEmptyContext();
 			contextHolder.set(ctx);
 		}
-
 		return ctx;
 	}
 

+ 6 - 6
core/src/main/java/org/springframework/security/core/parameters/AnnotationParameterNameDiscoverer.java

@@ -88,6 +88,11 @@ import org.springframework.util.ReflectionUtils;
  */
 public class AnnotationParameterNameDiscoverer implements ParameterNameDiscoverer {
 
+	private static final ParameterNameFactory<Constructor<?>> CONSTRUCTOR_METHODPARAM_FACTORY = (
+			constructor) -> constructor.getParameterAnnotations();
+
+	private static final ParameterNameFactory<Method> METHOD_METHODPARAM_FACTORY = Method::getParameterAnnotations;
+
 	private final Set<String> annotationClassesToUse;
 
 	public AnnotationParameterNameDiscoverer(String... annotationClassToUse) {
@@ -162,12 +167,6 @@ public class AnnotationParameterNameDiscoverer implements ParameterNameDiscovere
 		return null;
 	}
 
-	private static final ParameterNameFactory<Constructor<?>> CONSTRUCTOR_METHODPARAM_FACTORY = (
-			constructor) -> constructor.getParameterAnnotations();
-
-	private static final ParameterNameFactory<Method> METHOD_METHODPARAM_FACTORY = (method) -> method
-			.getParameterAnnotations();
-
 	/**
 	 * Strategy interface for looking up the parameter names.
 	 *
@@ -175,6 +174,7 @@ public class AnnotationParameterNameDiscoverer implements ParameterNameDiscovere
 	 * @author Rob Winch
 	 * @since 3.2
 	 */
+	@FunctionalInterface
 	private interface ParameterNameFactory<T extends AccessibleObject> {
 
 		/**

+ 0 - 7
core/src/main/java/org/springframework/security/core/parameters/DefaultSecurityParameterNameDiscoverer.java

@@ -21,9 +21,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
 import org.springframework.core.DefaultParameterNameDiscoverer;
 import org.springframework.core.LocalVariableTableParameterNameDiscoverer;
 import org.springframework.core.ParameterNameDiscoverer;
@@ -52,8 +49,6 @@ import org.springframework.util.ClassUtils;
  */
 public class DefaultSecurityParameterNameDiscoverer extends PrioritizedParameterNameDiscoverer {
 
-	private final Log logger = LogFactory.getLog(getClass());
-
 	private static final String DATA_PARAM_CLASSNAME = "org.springframework.data.repository.query.Param";
 
 	private static final boolean DATA_PARAM_PRESENT = ClassUtils.isPresent(DATA_PARAM_CLASSNAME,
@@ -79,14 +74,12 @@ public class DefaultSecurityParameterNameDiscoverer extends PrioritizedParameter
 		for (ParameterNameDiscoverer discover : parameterNameDiscovers) {
 			addDiscoverer(discover);
 		}
-
 		Set<String> annotationClassesToUse = new HashSet<>(2);
 		annotationClassesToUse.add("org.springframework.security.access.method.P");
 		annotationClassesToUse.add(P.class.getName());
 		if (DATA_PARAM_PRESENT) {
 			annotationClassesToUse.add(DATA_PARAM_CLASSNAME);
 		}
-
 		addDiscoverer(new AnnotationParameterNameDiscoverer(annotationClassesToUse));
 		addDiscoverer(new DefaultParameterNameDiscoverer());
 	}

+ 9 - 36
core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java

@@ -30,6 +30,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.context.ApplicationListener;
+import org.springframework.core.log.LogMessage;
 import org.springframework.util.Assert;
 
 /**
@@ -74,33 +75,26 @@ public class SessionRegistryImpl implements SessionRegistry, ApplicationListener
 
 	@Override
 	public List<SessionInformation> getAllSessions(Object principal, boolean includeExpiredSessions) {
-		final Set<String> sessionsUsedByPrincipal = this.principals.get(principal);
-
+		Set<String> sessionsUsedByPrincipal = this.principals.get(principal);
 		if (sessionsUsedByPrincipal == null) {
 			return Collections.emptyList();
 		}
-
 		List<SessionInformation> list = new ArrayList<>(sessionsUsedByPrincipal.size());
-
 		for (String sessionId : sessionsUsedByPrincipal) {
 			SessionInformation sessionInformation = getSessionInformation(sessionId);
-
 			if (sessionInformation == null) {
 				continue;
 			}
-
 			if (includeExpiredSessions || !sessionInformation.isExpired()) {
 				list.add(sessionInformation);
 			}
 		}
-
 		return list;
 	}
 
 	@Override
 	public SessionInformation getSessionInformation(String sessionId) {
 		Assert.hasText(sessionId, "SessionId required as per interface contract");
-
 		return this.sessionIds.get(sessionId);
 	}
 
@@ -123,9 +117,7 @@ public class SessionRegistryImpl implements SessionRegistry, ApplicationListener
 	@Override
 	public void refreshLastRequest(String sessionId) {
 		Assert.hasText(sessionId, "SessionId required as per interface contract");
-
 		SessionInformation info = getSessionInformation(sessionId);
-
 		if (info != null) {
 			info.refreshLastRequest();
 		}
@@ -135,26 +127,19 @@ public class SessionRegistryImpl implements SessionRegistry, ApplicationListener
 	public void registerNewSession(String sessionId, Object principal) {
 		Assert.hasText(sessionId, "SessionId required as per interface contract");
 		Assert.notNull(principal, "Principal required as per interface contract");
-
 		if (getSessionInformation(sessionId) != null) {
 			removeSessionInformation(sessionId);
 		}
-
 		if (this.logger.isDebugEnabled()) {
-			this.logger.debug("Registering session " + sessionId + ", for principal " + principal);
+			this.logger.debug(LogMessage.format("Registering session %s, for principal %s", sessionId, principal));
 		}
-
 		this.sessionIds.put(sessionId, new SessionInformation(principal, sessionId, new Date()));
-
 		this.principals.compute(principal, (key, sessionsUsedByPrincipal) -> {
 			if (sessionsUsedByPrincipal == null) {
 				sessionsUsedByPrincipal = new CopyOnWriteArraySet<>();
 			}
 			sessionsUsedByPrincipal.add(sessionId);
-
-			if (this.logger.isTraceEnabled()) {
-				this.logger.trace("Sessions used by '" + principal + "' : " + sessionsUsedByPrincipal);
-			}
+			this.logger.trace(LogMessage.format("Sessions used by '%s' : %s", principal, sessionsUsedByPrincipal));
 			return sessionsUsedByPrincipal;
 		});
 	}
@@ -162,37 +147,25 @@ public class SessionRegistryImpl implements SessionRegistry, ApplicationListener
 	@Override
 	public void removeSessionInformation(String sessionId) {
 		Assert.hasText(sessionId, "SessionId required as per interface contract");
-
 		SessionInformation info = getSessionInformation(sessionId);
-
 		if (info == null) {
 			return;
 		}
-
 		if (this.logger.isTraceEnabled()) {
 			this.logger.debug("Removing session " + sessionId + " from set of registered sessions");
 		}
-
 		this.sessionIds.remove(sessionId);
-
 		this.principals.computeIfPresent(info.getPrincipal(), (key, sessionsUsedByPrincipal) -> {
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug("Removing session " + sessionId + " from principal's set of registered sessions");
-			}
-
+			this.logger.debug(
+					LogMessage.format("Removing session %s from principal's set of registered sessions", sessionId));
 			sessionsUsedByPrincipal.remove(sessionId);
-
 			if (sessionsUsedByPrincipal.isEmpty()) {
 				// No need to keep object in principals Map anymore
-				if (this.logger.isDebugEnabled()) {
-					this.logger.debug("Removing principal " + info.getPrincipal() + " from registry");
-				}
+				this.logger.debug(LogMessage.format("Removing principal %s from registry", info.getPrincipal()));
 				sessionsUsedByPrincipal = null;
 			}
-
-			if (this.logger.isTraceEnabled()) {
-				this.logger.trace("Sessions used by '" + info.getPrincipal() + "' : " + sessionsUsedByPrincipal);
-			}
+			this.logger.trace(
+					LogMessage.format("Sessions used by '%s' : %s", info.getPrincipal(), sessionsUsedByPrincipal));
 			return sessionsUsedByPrincipal;
 		});
 	}

+ 6 - 11
core/src/main/java/org/springframework/security/core/token/KeyBasedPersistenceTokenService.java

@@ -89,13 +89,14 @@ public class KeyBasedPersistenceTokenService implements TokenService, Initializi
 		String serverSecret = computeServerSecretApplicableAt(creationTime);
 		String pseudoRandomNumber = generatePseudoRandomNumber();
 		String content = creationTime + ":" + pseudoRandomNumber + ":" + extendedInformation;
+		String key = computeKey(serverSecret, content);
+		return new DefaultToken(key, creationTime, extendedInformation);
+	}
 
-		// Compute key
+	private String computeKey(String serverSecret, String content) {
 		String sha512Hex = Sha512DigestUtils.shaHex(content + ":" + serverSecret);
 		String keyPayload = content + ":" + sha512Hex;
-		String key = Utf8.decode(Base64.getEncoder().encode(Utf8.encode(keyPayload)));
-
-		return new DefaultToken(key, creationTime, extendedInformation);
+		return Utf8.decode(Base64.getEncoder().encode(Utf8.encode(keyPayload)));
 	}
 
 	@Override
@@ -106,18 +107,15 @@ public class KeyBasedPersistenceTokenService implements TokenService, Initializi
 		String[] tokens = StringUtils
 				.delimitedListToStringArray(Utf8.decode(Base64.getDecoder().decode(Utf8.encode(key))), ":");
 		Assert.isTrue(tokens.length >= 4, () -> "Expected 4 or more tokens but found " + tokens.length);
-
 		long creationTime;
 		try {
 			creationTime = Long.decode(tokens[0]);
 		}
-		catch (NumberFormatException nfe) {
+		catch (NumberFormatException ex) {
 			throw new IllegalArgumentException("Expected number but found " + tokens[0]);
 		}
-
 		String serverSecret = computeServerSecretApplicableAt(creationTime);
 		String pseudoRandomNumber = tokens[1];
-
 		// Permit extendedInfo to itself contain ":" characters
 		StringBuilder extendedInfo = new StringBuilder();
 		for (int i = 2; i < tokens.length - 1; i++) {
@@ -126,14 +124,11 @@ public class KeyBasedPersistenceTokenService implements TokenService, Initializi
 			}
 			extendedInfo.append(tokens[i]);
 		}
-
 		String sha1Hex = tokens[tokens.length - 1];
-
 		// Verification
 		String content = creationTime + ":" + pseudoRandomNumber + ":" + extendedInfo.toString();
 		String expectedSha512Hex = Sha512DigestUtils.shaHex(content + ":" + serverSecret);
 		Assert.isTrue(expectedSha512Hex.equals(sha1Hex), "Key verification failure");
-
 		return new DefaultToken(key, creationTime, extendedInfo.toString());
 	}
 

+ 4 - 7
core/src/main/java/org/springframework/security/core/token/SecureRandomFactoryBean.java

@@ -38,19 +38,16 @@ public class SecureRandomFactoryBean implements FactoryBean<SecureRandom> {
 
 	@Override
 	public SecureRandom getObject() throws Exception {
-		SecureRandom rnd = SecureRandom.getInstance(this.algorithm);
-
+		SecureRandom random = SecureRandom.getInstance(this.algorithm);
 		// Request the next bytes, thus eagerly incurring the expense of default
 		// seeding and to prevent the see from replacing the entire state
-		rnd.nextBytes(new byte[1]);
-
+		random.nextBytes(new byte[1]);
 		if (this.seed != null) {
 			// Seed specified, so use it
 			byte[] seedBytes = FileCopyUtils.copyToByteArray(this.seed.getInputStream());
-			rnd.setSeed(seedBytes);
+			random.setSeed(seedBytes);
 		}
-
-		return rnd;
+		return random;
 	}
 
 	@Override

+ 9 - 4
core/src/main/java/org/springframework/security/core/userdetails/MapReactiveUserDetailsService.java

@@ -72,10 +72,15 @@ public class MapReactiveUserDetailsService implements ReactiveUserDetailsService
 
 	@Override
 	public Mono<UserDetails> updatePassword(UserDetails user, String newPassword) {
-		return Mono.just(user).map((u) -> User.withUserDetails(u).password(newPassword).build()).doOnNext((u) -> {
-			String key = getKey(user.getUsername());
-			this.users.put(key, u);
-		});
+		return Mono.just(user).map((userDetails) -> withNewPassword(userDetails, newPassword))
+				.doOnNext((userDetails) -> {
+					String key = getKey(user.getUsername());
+					this.users.put(key, userDetails);
+				});
+	}
+
+	private UserDetails withNewPassword(UserDetails userDetails, String newPassword) {
+		return User.withUserDetails(userDetails).password(newPassword).build();
 	}
 
 	private String getKey(String username) {

+ 9 - 21
core/src/main/java/org/springframework/security/core/userdetails/User.java

@@ -107,11 +107,8 @@ public class User implements UserDetails, CredentialsContainer {
 	public User(String username, String password, boolean enabled, boolean accountNonExpired,
 			boolean credentialsNonExpired, boolean accountNonLocked,
 			Collection<? extends GrantedAuthority> authorities) {
-
-		if (((username == null) || "".equals(username)) || (password == null)) {
-			throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
-		}
-
+		Assert.isTrue(username != null && !"".equals(username) && password != null,
+				"Cannot pass null or empty values to constructor");
 		this.username = username;
 		this.password = password;
 		this.enabled = enabled;
@@ -166,12 +163,10 @@ public class User implements UserDetails, CredentialsContainer {
 		// Ensure array iteration order is predictable (as per
 		// UserDetails.getAuthorities() contract and SEC-717)
 		SortedSet<GrantedAuthority> sortedAuthorities = new TreeSet<>(new AuthorityComparator());
-
 		for (GrantedAuthority grantedAuthority : authorities) {
 			Assert.notNull(grantedAuthority, "GrantedAuthority list cannot contain any null elements");
 			sortedAuthorities.add(grantedAuthority);
 		}
-
 		return sortedAuthorities;
 	}
 
@@ -183,9 +178,9 @@ public class User implements UserDetails, CredentialsContainer {
 	 * the same principal.
 	 */
 	@Override
-	public boolean equals(Object rhs) {
-		if (rhs instanceof User) {
-			return this.username.equals(((User) rhs).username);
+	public boolean equals(Object obj) {
+		if (obj instanceof User) {
+			return this.username.equals(((User) obj).username);
 		}
 		return false;
 	}
@@ -208,24 +203,20 @@ public class User implements UserDetails, CredentialsContainer {
 		sb.append("AccountNonExpired: ").append(this.accountNonExpired).append("; ");
 		sb.append("credentialsNonExpired: ").append(this.credentialsNonExpired).append("; ");
 		sb.append("AccountNonLocked: ").append(this.accountNonLocked).append("; ");
-
 		if (!this.authorities.isEmpty()) {
 			sb.append("Granted Authorities: ");
-
 			boolean first = true;
 			for (GrantedAuthority auth : this.authorities) {
 				if (!first) {
 					sb.append(",");
 				}
 				first = false;
-
 				sb.append(auth);
 			}
 		}
 		else {
 			sb.append("Not granted any authorities");
 		}
-
 		return sb.toString();
 	}
 
@@ -303,8 +294,8 @@ public class User implements UserDetails, CredentialsContainer {
 	 */
 	@Deprecated
 	public static UserBuilder withDefaultPasswordEncoder() {
-		logger.warn(
-				"User.withDefaultPasswordEncoder() is considered unsafe for production and is only intended for sample applications.");
+		logger.warn("User.withDefaultPasswordEncoder() is considered unsafe for production "
+				+ "and is only intended for sample applications.");
 		PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder();
 		return builder().passwordEncoder(encoder::encode);
 	}
@@ -323,17 +314,14 @@ public class User implements UserDetails, CredentialsContainer {
 		@Override
 		public int compare(GrantedAuthority g1, GrantedAuthority g2) {
 			// Neither should ever be null as each entry is checked before adding it to
-			// the set.
-			// If the authority is null, it is a custom authority and should precede
-			// others.
+			// the set. If the authority is null, it is a custom authority and should
+			// precede others.
 			if (g2.getAuthority() == null) {
 				return -1;
 			}
-
 			if (g1.getAuthority() == null) {
 				return 1;
 			}
-
 			return g1.getAuthority().compareTo(g2.getAuthority());
 		}
 

+ 5 - 20
core/src/main/java/org/springframework/security/core/userdetails/cache/EhCacheBasedUserCache.java

@@ -22,6 +22,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.core.userdetails.UserCache;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.util.Assert;
@@ -50,35 +51,19 @@ public class EhCacheBasedUserCache implements UserCache, InitializingBean {
 	@Override
 	public UserDetails getUserFromCache(String username) {
 		Element element = this.cache.get(username);
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("Cache hit: " + (element != null) + "; username: " + username);
-		}
-
-		if (element == null) {
-			return null;
-		}
-		else {
-			return (UserDetails) element.getValue();
-		}
+		logger.debug(LogMessage.of(() -> "Cache hit: " + (element != null) + "; username: " + username));
+		return (element != null) ? (UserDetails) element.getValue() : null;
 	}
 
 	@Override
 	public void putUserInCache(UserDetails user) {
 		Element element = new Element(user.getUsername(), user);
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("Cache put: " + element.getKey());
-		}
-
+		logger.debug(LogMessage.of(() -> "Cache put: " + element.getKey()));
 		this.cache.put(element);
 	}
 
 	public void removeUserFromCache(UserDetails user) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Cache remove: " + user.getUsername());
-		}
-
+		logger.debug(LogMessage.of(() -> "Cache remove: " + user.getUsername()));
 		this.removeUserFromCache(user.getUsername());
 	}
 

+ 5 - 18
core/src/main/java/org/springframework/security/core/userdetails/cache/SpringCacheBasedUserCache.java

@@ -20,6 +20,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.cache.Cache;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.core.userdetails.UserCache;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.util.Assert;
@@ -44,32 +45,18 @@ public class SpringCacheBasedUserCache implements UserCache {
 	@Override
 	public UserDetails getUserFromCache(String username) {
 		Cache.ValueWrapper element = (username != null) ? this.cache.get(username) : null;
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("Cache hit: " + (element != null) + "; username: " + username);
-		}
-
-		if (element == null) {
-			return null;
-		}
-		else {
-			return (UserDetails) element.get();
-		}
+		logger.debug(LogMessage.of(() -> "Cache hit: " + (element != null) + "; username: " + username));
+		return (element != null) ? (UserDetails) element.get() : null;
 	}
 
 	@Override
 	public void putUserInCache(UserDetails user) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Cache put: " + user.getUsername());
-		}
+		logger.debug(LogMessage.of(() -> "Cache put: " + user.getUsername()));
 		this.cache.put(user.getUsername(), user);
 	}
 
 	public void removeUserFromCache(UserDetails user) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Cache remove: " + user.getUsername());
-		}
-
+		logger.debug(LogMessage.of(() -> "Cache remove: " + user.getUsername()));
 		this.removeUserFromCache(user.getUsername());
 	}
 

+ 0 - 15
core/src/main/java/org/springframework/security/core/userdetails/jdbc/JdbcDaoImpl.java

@@ -171,37 +171,26 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService, M
 	@Override
 	public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
 		List<UserDetails> users = loadUsersByUsername(username);
-
 		if (users.size() == 0) {
 			this.logger.debug("Query returned no results for user '" + username + "'");
-
 			throw new UsernameNotFoundException(this.messages.getMessage("JdbcDaoImpl.notFound",
 					new Object[] { username }, "Username {0} not found"));
 		}
-
 		UserDetails user = users.get(0); // contains no GrantedAuthority[]
-
 		Set<GrantedAuthority> dbAuthsSet = new HashSet<>();
-
 		if (this.enableAuthorities) {
 			dbAuthsSet.addAll(loadUserAuthorities(user.getUsername()));
 		}
-
 		if (this.enableGroups) {
 			dbAuthsSet.addAll(loadGroupAuthorities(user.getUsername()));
 		}
-
 		List<GrantedAuthority> dbAuths = new ArrayList<>(dbAuthsSet);
-
 		addCustomAuthorities(user.getUsername(), dbAuths);
-
 		if (dbAuths.size() == 0) {
 			this.logger.debug("User '" + username + "' has no authorities and will be treated as 'not found'");
-
 			throw new UsernameNotFoundException(this.messages.getMessage("JdbcDaoImpl.noAuthority",
 					new Object[] { username }, "User {0} has no GrantedAuthority"));
 		}
-
 		return createUserDetails(username, user, dbAuths);
 	}
 
@@ -225,7 +214,6 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService, M
 	protected List<GrantedAuthority> loadUserAuthorities(String username) {
 		return getJdbcTemplate().query(this.authoritiesByUsernameQuery, new String[] { username }, (rs, rowNum) -> {
 			String roleName = JdbcDaoImpl.this.rolePrefix + rs.getString(2);
-
 			return new SimpleGrantedAuthority(roleName);
 		});
 	}
@@ -239,7 +227,6 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService, M
 		return getJdbcTemplate().query(this.groupAuthoritiesByUsernameQuery, new String[] { username },
 				(rs, rowNum) -> {
 					String roleName = getRolePrefix() + rs.getString(3);
-
 					return new SimpleGrantedAuthority(roleName);
 				});
 	}
@@ -256,11 +243,9 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService, M
 	protected UserDetails createUserDetails(String username, UserDetails userFromUserQuery,
 			List<GrantedAuthority> combinedAuthorities) {
 		String returnUsername = userFromUserQuery.getUsername();
-
 		if (!this.usernameBasedPrimaryKey) {
 			returnUsername = username;
 		}
-
 		return new User(returnUsername, userFromUserQuery.getPassword(), userFromUserQuery.isEnabled(),
 				userFromUserQuery.isAccountNonExpired(), userFromUserQuery.isCredentialsNonExpired(),
 				userFromUserQuery.isAccountNonLocked(), combinedAuthorities);

+ 21 - 31
core/src/main/java/org/springframework/security/core/userdetails/memory/UserAttributeEditor.java

@@ -32,42 +32,32 @@ public class UserAttributeEditor extends PropertyEditorSupport {
 
 	@Override
 	public void setAsText(String s) throws IllegalArgumentException {
-		if (StringUtils.hasText(s)) {
-			String[] tokens = StringUtils.commaDelimitedListToStringArray(s);
-			UserAttribute userAttrib = new UserAttribute();
-
-			List<String> authoritiesAsStrings = new ArrayList<>();
-
-			for (int i = 0; i < tokens.length; i++) {
-				String currentToken = tokens[i].trim();
-
-				if (i == 0) {
-					userAttrib.setPassword(currentToken);
+		if (!StringUtils.hasText(s)) {
+			setValue(null);
+			return;
+		}
+		String[] tokens = StringUtils.commaDelimitedListToStringArray(s);
+		UserAttribute userAttrib = new UserAttribute();
+		List<String> authoritiesAsStrings = new ArrayList<>();
+		for (int i = 0; i < tokens.length; i++) {
+			String currentToken = tokens[i].trim();
+			if (i == 0) {
+				userAttrib.setPassword(currentToken);
+			}
+			else {
+				if (currentToken.toLowerCase().equals("enabled")) {
+					userAttrib.setEnabled(true);
+				}
+				else if (currentToken.toLowerCase().equals("disabled")) {
+					userAttrib.setEnabled(false);
 				}
 				else {
-					if (currentToken.toLowerCase().equals("enabled")) {
-						userAttrib.setEnabled(true);
-					}
-					else if (currentToken.toLowerCase().equals("disabled")) {
-						userAttrib.setEnabled(false);
-					}
-					else {
-						authoritiesAsStrings.add(currentToken);
-					}
+					authoritiesAsStrings.add(currentToken);
 				}
 			}
-			userAttrib.setAuthoritiesAsString(authoritiesAsStrings);
-
-			if (userAttrib.isValid()) {
-				setValue(userAttrib);
-			}
-			else {
-				setValue(null);
-			}
-		}
-		else {
-			setValue(null);
 		}
+		userAttrib.setAuthoritiesAsString(authoritiesAsStrings);
+		setValue(userAttrib.isValid() ? userAttrib : null);
 	}
 
 }

+ 22 - 15
core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java

@@ -42,6 +42,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.core.log.LogMessage;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -97,22 +98,17 @@ public final class SecurityJackson2Modules {
 
 	@SuppressWarnings("unchecked")
 	private static Module loadAndGetInstance(String className, ClassLoader loader) {
-		Module instance = null;
 		try {
 			Class<? extends Module> securityModule = (Class<? extends Module>) ClassUtils.forName(className, loader);
 			if (securityModule != null) {
-				if (logger.isDebugEnabled()) {
-					logger.debug("Loaded module " + className + ", now registering");
-				}
-				instance = securityModule.newInstance();
+				logger.debug(LogMessage.format("Loaded module %s, now registering", className));
+				return securityModule.newInstance();
 			}
 		}
 		catch (Exception ex) {
-			if (logger.isDebugEnabled()) {
-				logger.debug("Cannot load module " + className, ex);
-			}
+			logger.debug(LogMessage.format("Cannot load module %s", className), ex);
 		}
-		return instance;
+		return null;
 	}
 
 	/**
@@ -193,12 +189,23 @@ public final class SecurityJackson2Modules {
 	 */
 	static class AllowlistTypeIdResolver implements TypeIdResolver {
 
-		private static final Set<String> ALLOWLIST_CLASS_NAMES = Collections
-				.unmodifiableSet(new HashSet(Arrays.asList("java.util.ArrayList", "java.util.Collections$EmptyList",
-						"java.util.Collections$EmptyMap", "java.util.Collections$UnmodifiableRandomAccessList",
-						"java.util.Collections$SingletonList", "java.util.Date", "java.time.Instant", "java.net.URL",
-						"java.util.TreeMap", "java.util.HashMap", "java.util.LinkedHashMap",
-						"org.springframework.security.core.context.SecurityContextImpl")));
+		private static final Set<String> ALLOWLIST_CLASS_NAMES;
+		static {
+			Set<String> names = new HashSet<>();
+			names.add("java.util.ArrayList");
+			names.add("java.util.Collections$EmptyList");
+			names.add("java.util.Collections$EmptyMap");
+			names.add("java.util.Collections$UnmodifiableRandomAccessList");
+			names.add("java.util.Collections$SingletonList");
+			names.add("java.util.Date");
+			names.add("java.time.Instant");
+			names.add("java.net.URL");
+			names.add("java.util.TreeMap");
+			names.add("java.util.HashMap");
+			names.add("java.util.LinkedHashMap");
+			names.add("org.springframework.security.core.context.SecurityContextImpl");
+			ALLOWLIST_CLASS_NAMES = Collections.unmodifiableSet(names);
+		}
 
 		private final TypeIdResolver delegate;
 

+ 14 - 9
core/src/main/java/org/springframework/security/jackson2/UserDeserializer.java

@@ -42,6 +42,9 @@ import org.springframework.security.core.userdetails.User;
  */
 class UserDeserializer extends JsonDeserializer<User> {
 
+	private static final TypeReference<Set<SimpleGrantedAuthority>> SIMPLE_GRANTED_AUTHORITY_SET = new TypeReference<Set<SimpleGrantedAuthority>>() {
+	};
+
 	/**
 	 * This method will create {@link User} object. It will ensure successful object
 	 * creation even if password key is null in serialized json, because credentials may
@@ -58,15 +61,17 @@ class UserDeserializer extends JsonDeserializer<User> {
 		ObjectMapper mapper = (ObjectMapper) jp.getCodec();
 		JsonNode jsonNode = mapper.readTree(jp);
 		Set<? extends GrantedAuthority> authorities = mapper.convertValue(jsonNode.get("authorities"),
-				new TypeReference<Set<SimpleGrantedAuthority>>() {
-				});
-		JsonNode password = readJsonNode(jsonNode, "password");
-		User result = new User(readJsonNode(jsonNode, "username").asText(), password.asText(""),
-				readJsonNode(jsonNode, "enabled").asBoolean(), readJsonNode(jsonNode, "accountNonExpired").asBoolean(),
-				readJsonNode(jsonNode, "credentialsNonExpired").asBoolean(),
-				readJsonNode(jsonNode, "accountNonLocked").asBoolean(), authorities);
-
-		if (password.asText(null) == null) {
+				SIMPLE_GRANTED_AUTHORITY_SET);
+		JsonNode passwordNode = readJsonNode(jsonNode, "password");
+		String username = readJsonNode(jsonNode, "username").asText();
+		String password = passwordNode.asText("");
+		boolean enabled = readJsonNode(jsonNode, "enabled").asBoolean();
+		boolean accountNonExpired = readJsonNode(jsonNode, "accountNonExpired").asBoolean();
+		boolean credentialsNonExpired = readJsonNode(jsonNode, "credentialsNonExpired").asBoolean();
+		boolean accountNonLocked = readJsonNode(jsonNode, "accountNonLocked").asBoolean();
+		User result = new User(username, password, enabled, accountNonExpired, credentialsNonExpired, accountNonLocked,
+				authorities);
+		if (passwordNode.asText(null) == null) {
 			result.eraseCredentials();
 		}
 		return result;

+ 30 - 25
core/src/main/java/org/springframework/security/jackson2/UsernamePasswordAuthenticationTokenDeserializer.java

@@ -19,11 +19,13 @@ package org.springframework.security.jackson2;
 import java.io.IOException;
 import java.util.List;
 
+import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.MissingNode;
@@ -48,6 +50,12 @@ import org.springframework.security.core.GrantedAuthority;
  */
 class UsernamePasswordAuthenticationTokenDeserializer extends JsonDeserializer<UsernamePasswordAuthenticationToken> {
 
+	private static final TypeReference<List<GrantedAuthority>> GRANTED_AUTHORITY_LIST = new TypeReference<List<GrantedAuthority>>() {
+	};
+
+	private static final TypeReference<Object> OBJECT = new TypeReference<Object>() {
+	};
+
 	/**
 	 * This method construct {@link UsernamePasswordAuthenticationToken} object from
 	 * serialized json.
@@ -60,47 +68,44 @@ class UsernamePasswordAuthenticationTokenDeserializer extends JsonDeserializer<U
 	@Override
 	public UsernamePasswordAuthenticationToken deserialize(JsonParser jp, DeserializationContext ctxt)
 			throws IOException, JsonProcessingException {
-		UsernamePasswordAuthenticationToken token = null;
 		ObjectMapper mapper = (ObjectMapper) jp.getCodec();
 		JsonNode jsonNode = mapper.readTree(jp);
 		Boolean authenticated = readJsonNode(jsonNode, "authenticated").asBoolean();
 		JsonNode principalNode = readJsonNode(jsonNode, "principal");
-		Object principal = null;
-		if (principalNode.isObject()) {
-			principal = mapper.readValue(principalNode.traverse(mapper), Object.class);
-		}
-		else {
-			principal = principalNode.asText();
-		}
+		Object principal = getPrincipal(mapper, principalNode);
 		JsonNode credentialsNode = readJsonNode(jsonNode, "credentials");
-		Object credentials;
-		if (credentialsNode.isNull() || credentialsNode.isMissingNode()) {
-			credentials = null;
-		}
-		else {
-			credentials = credentialsNode.asText();
-		}
+		Object credentials = getCredentials(credentialsNode);
 		List<GrantedAuthority> authorities = mapper.readValue(readJsonNode(jsonNode, "authorities").traverse(mapper),
-				new TypeReference<List<GrantedAuthority>>() {
-				});
-		if (authenticated) {
-			token = new UsernamePasswordAuthenticationToken(principal, credentials, authorities);
-		}
-		else {
-			token = new UsernamePasswordAuthenticationToken(principal, credentials);
-		}
+				GRANTED_AUTHORITY_LIST);
+		UsernamePasswordAuthenticationToken token = (!authenticated)
+				? new UsernamePasswordAuthenticationToken(principal, credentials)
+				: new UsernamePasswordAuthenticationToken(principal, credentials, authorities);
 		JsonNode detailsNode = readJsonNode(jsonNode, "details");
 		if (detailsNode.isNull() || detailsNode.isMissingNode()) {
 			token.setDetails(null);
 		}
 		else {
-			Object details = mapper.readValue(detailsNode.toString(), new TypeReference<Object>() {
-			});
+			Object details = mapper.readValue(detailsNode.toString(), OBJECT);
 			token.setDetails(details);
 		}
 		return token;
 	}
 
+	private Object getCredentials(JsonNode credentialsNode) {
+		if (credentialsNode.isNull() || credentialsNode.isMissingNode()) {
+			return null;
+		}
+		return credentialsNode.asText();
+	}
+
+	private Object getPrincipal(ObjectMapper mapper, JsonNode principalNode)
+			throws IOException, JsonParseException, JsonMappingException {
+		if (principalNode.isObject()) {
+			return mapper.readValue(principalNode.traverse(mapper), Object.class);
+		}
+		return principalNode.asText();
+	}
+
 	private JsonNode readJsonNode(JsonNode jsonNode, String field) {
 		return jsonNode.has(field) ? jsonNode.get(field) : MissingNode.getInstance();
 	}

部分文件因为文件数量过多而无法显示