Improve Method Security logging

core/src/main/java/org/springframework/security/access/prepost/PrePostAnnotationSecurityMetadataSource.java

@@ -62,16 +62,13 @@ public Collection<ConfigAttribute> getAttributes(Method method, Class<?> targetC
 		if (method.getDeclaringClass() == Object.class) {
 			return Collections.emptyList();
 		}
-		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;

core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationDecision.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2002-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.authorization;
+
+import java.util.Collection;
+
+import org.springframework.security.core.GrantedAuthority;
+
+/**
+ * Represents an {@link AuthorizationDecision} based on a collection of authorities
+ *
+ * @author Marcus Da Coregio
+ * @since 5.6
+ */
+public class AuthorityAuthorizationDecision extends AuthorizationDecision {
+
+	private final Collection<GrantedAuthority> authorities;
+
+	public AuthorityAuthorizationDecision(boolean granted, Collection<GrantedAuthority> authorities) {
+		super(granted);
+		this.authorities = authorities;
+	}
+
+	public Collection<GrantedAuthority> getAuthorities() {
+		return this.authorities;
+	}
+
+	@Override
+	public String toString() {
+		return getClass().getSimpleName() + " [" + "granted=" + isGranted() + ", authorities=" + this.authorities + ']';
+	}
+
+}

core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java

@@ -16,13 +16,13 @@
 
 package org.springframework.security.authorization;
 
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.function.Supplier;
 
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.util.Assert;
 
 /**
@@ -37,10 +37,10 @@ public final class AuthorityAuthorizationManager<T> implements AuthorizationMana
 
 	private static final String ROLE_PREFIX = "ROLE_";
 
-	private final Set<String> authorities;
+	private final Set<GrantedAuthority> authorities;
 
 	private AuthorityAuthorizationManager(String... authorities) {
-		this.authorities = new HashSet<>(Arrays.asList(authorities));
+		this.authorities = new HashSet<>(AuthorityUtils.createAuthorityList(authorities));
 	}
 
 	/**
@@ -124,7 +124,7 @@ private static String[] toNamedRolesArray(String rolePrefix, String[] roles) {
 	@Override
 	public AuthorizationDecision check(Supplier<Authentication> authentication, T object) {
 		boolean granted = isGranted(authentication.get());
-		return new AuthorizationDecision(granted);
+		return new AuthorityAuthorizationDecision(granted, this.authorities);
 	}
 
 	private boolean isGranted(Authentication authentication) {
@@ -133,8 +133,7 @@ private boolean isGranted(Authentication authentication) {
 
 	private boolean isAuthorized(Authentication authentication) {
 		for (GrantedAuthority grantedAuthority : authentication.getAuthorities()) {
-			String authority = grantedAuthority.getAuthority();
-			if (this.authorities.contains(authority)) {
+			if (this.authorities.contains(grantedAuthority)) {
 				return true;
 			}
 		}

core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,13 +16,13 @@
 
 package org.springframework.security.authorization;
 
-import java.util.Arrays;
 import java.util.List;
 
 import reactor.core.publisher.Mono;
 
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.util.Assert;
 
 /**
@@ -35,21 +35,20 @@
  */
 public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthorizationManager<T> {
 
-	private final List<String> authorities;
+	private final List<GrantedAuthority> authorities;
 
 	AuthorityReactiveAuthorizationManager(String... authorities) {
-		this.authorities = Arrays.asList(authorities);
+		this.authorities = AuthorityUtils.createAuthorityList(authorities);
 	}
 
 	@Override
 	public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
 		// @formatter:off
 		return authentication.filter((a) -> a.isAuthenticated())
 				.flatMapIterable(Authentication::getAuthorities)
-				.map(GrantedAuthority::getAuthority)
 				.any(this.authorities::contains)
-				.map(AuthorizationDecision::new)
-				.defaultIfEmpty(new AuthorizationDecision(false));
+				.map((granted) -> ((AuthorizationDecision) new AuthorityAuthorizationDecision(granted, this.authorities)))
+				.defaultIfEmpty(new AuthorityAuthorizationDecision(false, this.authorities));
 		// @formatter:on
 	}
 

core/src/main/java/org/springframework/security/authorization/AuthorizationDecision.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,4 +32,9 @@ public boolean isGranted() {
 		return this.granted;
 	}
 
+	@Override
+	public String toString() {
+		return getClass().getSimpleName() + " [granted=" + this.granted + "]";
+	}
+
 }

core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java

@@ -21,14 +21,18 @@
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import org.springframework.aop.Pointcut;
 import org.springframework.aop.PointcutAdvisor;
 import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.core.Ordered;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
+import org.springframework.security.authorization.AuthorizationDecision;
 import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -54,6 +58,8 @@ public final class AuthorizationManagerAfterMethodInterceptor
 		return authentication;
 	};
 
+	private final Log logger = LogFactory.getLog(this.getClass());
+
 	private final Pointcut pointcut;
 
 	private final AuthorizationManager<MethodInvocationResult> authorizationManager;
@@ -103,7 +109,7 @@ public static AuthorizationManagerAfterMethodInterceptor postAuthorize(
 	@Override
 	public Object invoke(MethodInvocation mi) throws Throwable {
 		Object result = mi.proceed();
-		this.authorizationManager.verify(AUTHENTICATION_SUPPLIER, new MethodInvocationResult(mi, result));
+		attemptAuthorization(mi, result);
 		return result;
 	}
 
@@ -134,4 +140,16 @@ public boolean isPerInstance() {
 		return true;
 	}
 
+	private void attemptAuthorization(MethodInvocation mi, Object result) {
+		this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
+		AuthorizationDecision decision = this.authorizationManager.check(AUTHENTICATION_SUPPLIER,
+				new MethodInvocationResult(mi, result));
+		if (decision != null && !decision.isGranted()) {
+			this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
+					+ this.authorizationManager + " and decision " + decision));
+			throw new AccessDeniedException("Access Denied");
+		}
+		this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
+	}
+
 }

core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java

@@ -25,15 +25,19 @@
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import org.springframework.aop.Pointcut;
 import org.springframework.aop.PointcutAdvisor;
 import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.core.Ordered;
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.annotation.Secured;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
+import org.springframework.security.authorization.AuthorizationDecision;
 import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -59,6 +63,8 @@ public final class AuthorizationManagerBeforeMethodInterceptor
 		return authentication;
 	};
 
+	private final Log logger = LogFactory.getLog(this.getClass());
+
 	private final Pointcut pointcut;
 
 	private final AuthorizationManager<MethodInvocation> authorizationManager;
@@ -149,7 +155,7 @@ public static AuthorizationManagerBeforeMethodInterceptor jsr250(Jsr250Authoriza
 	 */
 	@Override
 	public Object invoke(MethodInvocation mi) throws Throwable {
-		this.authorizationManager.verify(AUTHENTICATION_SUPPLIER, mi);
+		attemptAuthorization(mi);
 		return mi.proceed();
 	}
 
@@ -180,4 +186,15 @@ public boolean isPerInstance() {
 		return true;
 	}
 
+	private void attemptAuthorization(MethodInvocation mi) {
+		this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
+		AuthorizationDecision decision = this.authorizationManager.check(AUTHENTICATION_SUPPLIER, mi);
+		if (decision != null && !decision.isGranted()) {
+			this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
+					+ this.authorizationManager + " and decision " + decision));
+			throw new AccessDeniedException("Access Denied");
+		}
+		this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
+	}
+
 }

core/src/main/java/org/springframework/security/authorization/method/ExpressionAttribute.java

@@ -49,4 +49,10 @@ Expression getExpression() {
 		return this.expression;
 	}
 
+	@Override
+	public String toString() {
+		return getClass().getSimpleName() + " [Expression="
+				+ ((this.expression != null) ? this.expression.getExpressionString() : null) + "]";
+	}
+
 }

core/src/main/java/org/springframework/security/authorization/method/ExpressionAttributeAuthorizationDecision.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2002-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.authorization.method;
+
+import org.springframework.security.authorization.AuthorizationDecision;
+
+/**
+ * Represents an {@link AuthorizationDecision} based on a {@link ExpressionAttribute}
+ *
+ * @author Marcus Da Coregio
+ * @since 5.6
+ */
+public class ExpressionAttributeAuthorizationDecision extends AuthorizationDecision {
+
+	private final ExpressionAttribute expressionAttribute;
+
+	public ExpressionAttributeAuthorizationDecision(boolean granted, ExpressionAttribute expressionAttribute) {
+		super(granted);
+		this.expressionAttribute = expressionAttribute;
+	}
+
+	public ExpressionAttribute getExpressionAttribute() {
+		return this.expressionAttribute;
+	}
+
+	@Override
+	public String toString() {
+		return getClass().getSimpleName() + " [" + "granted=" + isGranted() + ", expressionAttribute="
+				+ this.expressionAttribute + ']';
+	}
+
+}

core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManager.java

@@ -76,7 +76,7 @@ public AuthorizationDecision check(Supplier<Authentication> authentication, Meth
 				mi.getMethodInvocation());
 		this.expressionHandler.setReturnObject(mi.getResult(), ctx);
 		boolean granted = ExpressionUtils.evaluateAsBoolean(attribute.getExpression(), ctx);
-		return new AuthorizationDecision(granted);
+		return new ExpressionAttributeAuthorizationDecision(granted, attribute);
 	}
 
 	private final class PostAuthorizeExpressionAttributeRegistry

core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManager.java

@@ -74,7 +74,7 @@ public AuthorizationDecision check(Supplier<Authentication> authentication, Meth
 		}
 		EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), mi);
 		boolean granted = ExpressionUtils.evaluateAsBoolean(attribute.getExpression(), ctx);
-		return new AuthorizationDecision(granted);
+		return new ExpressionAttributeAuthorizationDecision(granted, attribute);
 	}
 
 	private final class PreAuthorizeExpressionAttributeRegistry

core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java

@@ -53,7 +53,7 @@ public void instantiateWhenAuthorizationManagerNullThenException() {
 	}
 
 	@Test
-	public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() throws Throwable {
+	public void beforeWhenMockAuthorizationManagerThenCheckAndReturnedObject() throws Throwable {
 		MethodInvocation mockMethodInvocation = mock(MethodInvocation.class);
 		MethodInvocationResult result = new MethodInvocationResult(mockMethodInvocation, new Object());
 		given(mockMethodInvocation.proceed()).willReturn(result.getResult());
@@ -62,7 +62,7 @@ public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() thro
 				Pointcut.TRUE, mockAuthorizationManager);
 		Object returnedObject = advice.invoke(mockMethodInvocation);
 		assertThat(returnedObject).isEqualTo(result.getResult());
-		verify(mockAuthorizationManager).verify(eq(AuthorizationManagerAfterMethodInterceptor.AUTHENTICATION_SUPPLIER),
+		verify(mockAuthorizationManager).check(eq(AuthorizationManagerAfterMethodInterceptor.AUTHENTICATION_SUPPLIER),
 				any(MethodInvocationResult.class));
 	}
 

core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java

@@ -49,13 +49,13 @@ public void instantiateWhenAuthorizationManagerNullThenException() {
 	}
 
 	@Test
-	public void beforeWhenMockAuthorizationManagerThenVerify() throws Throwable {
+	public void beforeWhenMockAuthorizationManagerThenCheck() throws Throwable {
 		MethodInvocation mockMethodInvocation = mock(MethodInvocation.class);
 		AuthorizationManager<MethodInvocation> mockAuthorizationManager = mock(AuthorizationManager.class);
 		AuthorizationManagerBeforeMethodInterceptor advice = new AuthorizationManagerBeforeMethodInterceptor(
 				Pointcut.TRUE, mockAuthorizationManager);
 		advice.invoke(mockMethodInvocation);
-		verify(mockAuthorizationManager).verify(AuthorizationManagerBeforeMethodInterceptor.AUTHENTICATION_SUPPLIER,
+		verify(mockAuthorizationManager).check(AuthorizationManagerBeforeMethodInterceptor.AUTHENTICATION_SUPPLIER,
 				mockMethodInvocation);
 	}