Add AfterMethodAuthorizationManager

- Removes the need to keep MethodAuthorizationContext#returnObject
in sync with other method parameters
- Restores MethodAuthorizationContext's immutability

Closes gh-9591

config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java

@@ -141,7 +141,7 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean
 		if (jsr250Enabled()) {
 			beforeAdvices.add(getJsr250AuthorizationMethodBeforeAdvice());
 		}
-		return new DelegatingAuthorizationMethodBeforeAdvice(beforeAdvices);
+		return new DelegatingAuthorizationMethodBeforeAdvice<>(beforeAdvices);
 	}
 
 	private PreFilterAuthorizationMethodBeforeAdvice getPreFilterAuthorizationMethodBeforeAdvice() {
@@ -192,7 +192,7 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean
 		List<AuthorizationMethodAfterAdvice<MethodAuthorizationContext>> afterAdvices = new ArrayList<>();
 		afterAdvices.add(getPostFilterAuthorizationMethodAfterAdvice());
 		afterAdvices.add(getPostAuthorizeAuthorizationMethodAfterAdvice());
-		return new DelegatingAuthorizationMethodAfterAdvice(afterAdvices);
+		return new DelegatingAuthorizationMethodAfterAdvice<>(afterAdvices);
 	}
 
 	private PostFilterAuthorizationMethodAfterAdvice getPostFilterAuthorizationMethodAfterAdvice() {

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

@@ -0,0 +1,65 @@
+/*
+ * 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 java.util.function.Supplier;
+
+import org.aopalliance.intercept.MethodInvocation;
+
+import org.springframework.lang.Nullable;
+import org.springframework.security.access.AccessDeniedException;
+import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.core.Authentication;
+
+/**
+ * An Authorization manager which can determine if an {@link Authentication} has access to
+ * a specific object and associated return object. Intended for use specifically to
+ * evaluate the returning state of a method invocation.
+ *
+ * @param <T> the type of object that the authorization check is being done one.
+ * @author Josh Cummings
+ * @author Evgeniy Cheban
+ * @since 5.5
+ */
+public interface AfterMethodAuthorizationManager<T> {
+
+	/**
+	 * Determines if access should be granted for a specific authentication and
+	 * returnedObject.
+	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
+	 * @param object the {@code T} object to check, typically a {@link MethodInvocation}
+	 * @param returnedObject the returnedObject from the method invocation to check
+	 * @throws AccessDeniedException if access is not granted
+	 */
+	default void verify(Supplier<Authentication> authentication, T object, Object returnedObject) {
+		AuthorizationDecision decision = check(authentication, object, returnedObject);
+		if (decision != null && !decision.isGranted()) {
+			throw new AccessDeniedException("Access Denied");
+		}
+	}
+
+	/**
+	 * Determines if access is granted for a specific authentication and returnedObject.
+	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
+	 * @param object the {@code T} object to check, typically a {@link MethodInvocation}
+	 * @param returnedObject the returned object from the method invocation to check
+	 * @return an {@link AuthorizationDecision} or null if no decision could be made
+	 */
+	@Nullable
+	AuthorizationDecision check(Supplier<Authentication> authentication, T object, Object returnedObject);
+
+}

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

@@ -0,0 +1,62 @@
+/*
+ * 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 java.util.function.Supplier;
+
+import org.aopalliance.intercept.MethodInvocation;
+
+import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationManager;
+import org.springframework.security.core.Authentication;
+
+/**
+ * Adapts an {@link AuthorizationManager} into an {@link AfterMethodAuthorizationManager}
+ *
+ * @param <T> the {@code T} object to authorize, typically a {@link MethodInvocation}
+ * @author Josh Cummings
+ * @since 5.5
+ */
+public final class AfterMethodAuthorizationManagerAdapter<T> implements AfterMethodAuthorizationManager<T> {
+
+	private final AuthorizationManager<T> authorizationManager;
+
+	/**
+	 * Construct a {@link AfterMethodAuthorizationManagerAdapter} with the provided
+	 * parameters
+	 * @param authorizationManager the {@link AuthorizationManager} to adapt
+	 */
+	public AfterMethodAuthorizationManagerAdapter(AuthorizationManager<T> authorizationManager) {
+		this.authorizationManager = authorizationManager;
+	}
+
+	/**
+	 * Determine if access is granted for a specific authentication and {@code T} object.
+	 *
+	 * Note that the {@code returnedObject} parameter is ignored
+	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
+	 * @param object the {@code T} object to check, typically a {@link MethodInvocation}
+	 * @param returnedObject the returned object from the method invocation, ignored in
+	 * this implementation
+	 * @return an {@link AuthorizationDecision} or null if no decision could be made
+	 */
+	@Override
+	public AuthorizationDecision check(Supplier<Authentication> authentication, T object, Object returnedObject) {
+		return this.authorizationManager.check(authentication, object);
+	}
+
+}

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

@@ -39,14 +39,15 @@ public final class AuthorizationManagerMethodAfterAdvice<T> implements Authoriza
 
 	private final Pointcut pointcut;
 
-	private final AuthorizationManager<T> authorizationManager;
+	private final AfterMethodAuthorizationManager<T> authorizationManager;
 
 	/**
 	 * Creates an instance.
 	 * @param pointcut the {@link Pointcut} to use
 	 * @param authorizationManager the {@link AuthorizationManager} to use
 	 */
-	public AuthorizationManagerMethodAfterAdvice(Pointcut pointcut, AuthorizationManager<T> authorizationManager) {
+	public AuthorizationManagerMethodAfterAdvice(Pointcut pointcut,
+			AfterMethodAuthorizationManager<T> authorizationManager) {
 		Assert.notNull(pointcut, "pointcut cannot be null");
 		Assert.notNull(authorizationManager, "authorizationManager cannot be null");
 		this.pointcut = pointcut;
@@ -57,13 +58,14 @@ public final class AuthorizationManagerMethodAfterAdvice<T> implements Authoriza
 	 * Determines if an {@link Authentication} has access to the {@link T} object using
 	 * the {@link AuthorizationManager}.
 	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
-	 * @param object the {@link T} object to check
+	 * @param object the {@link T} object to check - note that {@code T} should contain
+	 * the returned object
 	 * @throws AccessDeniedException if access is not granted
 	 */
 	@Override
-	public Object after(Supplier<Authentication> authentication, T object, Object returnedObject) {
-		this.authorizationManager.verify(authentication, object);
-		return returnedObject;
+	public Object after(Supplier<Authentication> authentication, T context, Object object) {
+		this.authorizationManager.verify(authentication, context, object);
+		return object;
 	}
 
 	/**

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

@@ -27,9 +27,9 @@ import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.security.core.Authentication;
 
 /**
- * An {@link Advice} which can determine if an {@link Authentication} has
- * access to the returned object from the {@link MethodInvocation}. {@link #getPointcut()}
- * describes when the advice applies for the method.
+ * An {@link Advice} which can determine if an {@link Authentication} has access to the
+ * returned object from the {@link MethodInvocation}. {@link #getPointcut()} describes
+ * when the advice applies for the method.
  *
  * @param <T> the type of object that the authorization check is being done one.
  * @author Evgeniy Cheban

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

@@ -38,21 +38,19 @@ import org.springframework.util.Assert;
  * @author Josh Cummings
  * @since 5.5
  */
-public final class DelegatingAuthorizationMethodAfterAdvice
-		implements AuthorizationMethodAfterAdvice<MethodAuthorizationContext> {
+public final class DelegatingAuthorizationMethodAfterAdvice<T> implements AuthorizationMethodAfterAdvice<T> {
 
 	private final Log logger = LogFactory.getLog(getClass());
 
 	private final Pointcut pointcut;
 
-	private final List<AuthorizationMethodAfterAdvice<MethodAuthorizationContext>> delegates;
+	private final List<AuthorizationMethodAfterAdvice<T>> delegates;
 
 	/**
 	 * Creates an instance.
 	 * @param delegates the {@link AuthorizationMethodAfterAdvice}s to use
 	 */
-	public DelegatingAuthorizationMethodAfterAdvice(
-			List<AuthorizationMethodAfterAdvice<MethodAuthorizationContext>> delegates) {
+	public DelegatingAuthorizationMethodAfterAdvice(List<AuthorizationMethodAfterAdvice<T>> delegates) {
 		Assert.notEmpty(delegates, "delegates cannot be empty");
 		this.delegates = delegates;
 		ComposablePointcut pointcut = null;
@@ -79,26 +77,24 @@ public final class DelegatingAuthorizationMethodAfterAdvice
 	 * Delegates to specific {@link AuthorizationMethodAfterAdvice}s and returns the
 	 * <code>returnedObject</code> (possibly modified) from the method argument.
 	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
-	 * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check
+	 * @param object the {@link MethodAuthorizationContext} to check
 	 * @param returnedObject the returned object from the {@link MethodInvocation} to
 	 * check
 	 * @return the <code>returnedObject</code> (possibly modified) from the method
 	 * argument
 	 */
 	@Override
-	public Object after(Supplier<Authentication> authentication, MethodAuthorizationContext methodAuthorizationContext,
-			Object returnedObject) {
+	public Object after(Supplier<Authentication> authentication, T object, Object returnedObject) {
 		if (this.logger.isTraceEnabled()) {
-			this.logger.trace(
-					LogMessage.format("Post Authorizing %s from %s", returnedObject, methodAuthorizationContext));
+			this.logger.trace(LogMessage.format("Post Authorizing %s from %s", returnedObject, object));
 		}
 		Object result = returnedObject;
-		for (AuthorizationMethodAfterAdvice<MethodAuthorizationContext> delegate : this.delegates) {
+		for (AuthorizationMethodAfterAdvice<T> delegate : this.delegates) {
 			if (this.logger.isTraceEnabled()) {
-				this.logger.trace(LogMessage.format("Checking authorization on %s from %s using %s", result,
-						methodAuthorizationContext, delegate));
+				this.logger.trace(
+						LogMessage.format("Checking authorization on %s from %s using %s", result, object, delegate));
 			}
-			result = delegate.after(authentication, methodAuthorizationContext, result);
+			result = delegate.after(authentication, object, result);
 		}
 		return result;
 	}

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

@@ -38,21 +38,19 @@ import org.springframework.util.Assert;
  * @author Josh Cummings
  * @since 5.5
  */
-public final class DelegatingAuthorizationMethodBeforeAdvice
-		implements AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> {
+public final class DelegatingAuthorizationMethodBeforeAdvice<T> implements AuthorizationMethodBeforeAdvice<T> {
 
 	private final Log logger = LogFactory.getLog(getClass());
 
 	private final Pointcut pointcut;
 
-	private final List<AuthorizationMethodBeforeAdvice<MethodAuthorizationContext>> delegates;
+	private final List<AuthorizationMethodBeforeAdvice<T>> delegates;
 
 	/**
 	 * Creates an instance.
 	 * @param delegates the {@link AuthorizationMethodBeforeAdvice}s to use
 	 */
-	public DelegatingAuthorizationMethodBeforeAdvice(
-			List<AuthorizationMethodBeforeAdvice<MethodAuthorizationContext>> delegates) {
+	public DelegatingAuthorizationMethodBeforeAdvice(List<AuthorizationMethodBeforeAdvice<T>> delegates) {
 		Assert.notEmpty(delegates, "delegates cannot be empty");
 		this.delegates = delegates;
 		ComposablePointcut pointcut = null;
@@ -80,19 +78,18 @@ public final class DelegatingAuthorizationMethodBeforeAdvice
 	 * if all {@link AuthorizationMethodBeforeAdvice}s granted or abstained. Denies only
 	 * if one of the {@link AuthorizationMethodBeforeAdvice}s denied.
 	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
-	 * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check
+	 * @param object the {@link MethodAuthorizationContext} to check
 	 */
 	@Override
-	public void before(Supplier<Authentication> authentication, MethodAuthorizationContext methodAuthorizationContext) {
+	public void before(Supplier<Authentication> authentication, T object) {
 		if (this.logger.isTraceEnabled()) {
-			this.logger.trace(LogMessage.format("Pre Authorizing %s", methodAuthorizationContext));
+			this.logger.trace(LogMessage.format("Pre Authorizing %s", object));
 		}
-		for (AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> delegate : this.delegates) {
+		for (AuthorizationMethodBeforeAdvice<T> delegate : this.delegates) {
 			if (this.logger.isTraceEnabled()) {
-				this.logger.trace(LogMessage.format("Checking authorization on %s using %s", methodAuthorizationContext,
-						delegate));
+				this.logger.trace(LogMessage.format("Checking authorization on %s using %s", object, delegate));
 			}
-			delegate.before(authentication, methodAuthorizationContext);
+			delegate.before(authentication, object);
 		}
 	}
 

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

@@ -31,8 +31,6 @@ public final class MethodAuthorizationContext {
 
 	private final Class<?> targetClass;
 
-	private Object returnObject;
-
 	/**
 	 * Creates an instance.
 	 * @param methodInvocation the {@link MethodInvocation} to use
@@ -59,26 +57,10 @@ public final class MethodAuthorizationContext {
 		return this.targetClass;
 	}
 
-	/**
-	 * Returns the returned object from the {@link MethodInvocation}.
-	 * @return the returned object from the {@link MethodInvocation} to use
-	 */
-	public Object getReturnObject() {
-		return this.returnObject;
-	}
-
-	/**
-	 * Sets the returned object from the {@link MethodInvocation}.
-	 * @param returnObject the returned object from the {@link MethodInvocation} to use
-	 */
-	public void setReturnObject(Object returnObject) {
-		this.returnObject = returnObject;
-	}
-
 	@Override
 	public String toString() {
 		return "MethodAuthorizationContext[methodInvocation=" + this.methodInvocation + ", targetClass="
-				+ this.targetClass + ", returnObject=" + this.returnObject + ']';
+				+ this.targetClass + ']';
 	}
 
 }

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

@@ -43,7 +43,8 @@ import org.springframework.util.Assert;
  * @author Evgeniy Cheban
  * @since 5.5
  */
-public final class PostAuthorizeAuthorizationManager implements AuthorizationManager<MethodAuthorizationContext> {
+public final class PostAuthorizeAuthorizationManager
+		implements AfterMethodAuthorizationManager<MethodAuthorizationContext> {
 
 	private final PostAuthorizeExpressionAttributeRegistry registry = new PostAuthorizeExpressionAttributeRegistry();
 
@@ -68,14 +69,14 @@ public final class PostAuthorizeAuthorizationManager implements AuthorizationMan
 	 */
 	@Override
 	public AuthorizationDecision check(Supplier<Authentication> authentication,
-			MethodAuthorizationContext methodAuthorizationContext) {
+			MethodAuthorizationContext methodAuthorizationContext, Object returnedObject) {
 		ExpressionAttribute attribute = this.registry.getAttribute(methodAuthorizationContext);
 		if (attribute == ExpressionAttribute.NULL_ATTRIBUTE) {
 			return null;
 		}
 		EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(),
 				methodAuthorizationContext.getMethodInvocation());
-		this.expressionHandler.setReturnObject(methodAuthorizationContext.getReturnObject(), ctx);
+		this.expressionHandler.setReturnObject(returnedObject, ctx);
 		boolean granted = ExpressionUtils.evaluateAsBoolean(attribute.getExpression(), ctx);
 		return new AuthorizationDecision(granted);
 	}

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

@@ -82,7 +82,6 @@ public final class PostFilterAuthorizationMethodAfterAdvice
 	 * evaluating an expression from the {@link PostFilter} annotation.
 	 * @param authentication the {@link Supplier} of the {@link Authentication} to check
 	 * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check
-	 * @param returnedObject the returned object from the {@link MethodInvocation} to
 	 * check
 	 * @return filtered <code>returnedObject</code> from the {@link MethodInvocation}
 	 */
@@ -98,9 +97,7 @@ public final class PostFilterAuthorizationMethodAfterAdvice
 		}
 		EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(),
 				methodAuthorizationContext.getMethodInvocation());
-		Object result = this.expressionHandler.filter(returnedObject, attribute.getExpression(), ctx);
-		methodAuthorizationContext.setReturnObject(result);
-		return result;
+		return this.expressionHandler.filter(returnedObject, attribute.getExpression(), ctx);
 	}
 
 	private final class PostFilterExpressionAttributeRegistry

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

@@ -23,7 +23,6 @@ import org.junit.Test;
 
 import org.springframework.aop.Pointcut;
 import org.springframework.security.authentication.TestAuthentication;
-import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -40,8 +39,10 @@ public class AuthorizationManagerMethodAfterAdviceTests {
 
 	@Test
 	public void instantiateWhenMethodMatcherNullThenException() {
+		AfterMethodAuthorizationManager<MethodInvocation> mockAuthorizationManager = mock(
+				AfterMethodAuthorizationManager.class);
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(null, mock(AuthorizationManager.class)))
+				.isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(null, mockAuthorizationManager))
 				.withMessage("pointcut cannot be null");
 	}
 
@@ -57,12 +58,13 @@ public class AuthorizationManagerMethodAfterAdviceTests {
 		Supplier<Authentication> authentication = TestAuthentication::authenticatedUser;
 		MethodInvocation mockMethodInvocation = mock(MethodInvocation.class);
 		Object returnedObject = new Object();
-		AuthorizationManager<MethodInvocation> mockAuthorizationManager = mock(AuthorizationManager.class);
+		AfterMethodAuthorizationManager<MethodInvocation> mockAuthorizationManager = mock(
+				AfterMethodAuthorizationManager.class);
 		AuthorizationManagerMethodAfterAdvice<MethodInvocation> advice = new AuthorizationManagerMethodAfterAdvice<>(
 				mock(Pointcut.class), mockAuthorizationManager);
 		Object result = advice.after(authentication, mockMethodInvocation, returnedObject);
 		assertThat(result).isEqualTo(returnedObject);
-		verify(mockAuthorizationManager).verify(authentication, mockMethodInvocation);
+		verify(mockAuthorizationManager).verify(authentication, mockMethodInvocation, returnedObject);
 	}
 
 }

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

@@ -62,7 +62,7 @@ public class PostAuthorizeAuthorizationManagerTests {
 				TestClass.class);
 		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
 		AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser,
-				methodAuthorizationContext);
+				methodAuthorizationContext, null);
 		assertThat(decision).isNull();
 	}
 
@@ -74,7 +74,7 @@ public class PostAuthorizeAuthorizationManagerTests {
 				TestClass.class);
 		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
 		AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser,
-				methodAuthorizationContext);
+				methodAuthorizationContext, null);
 		assertThat(decision).isNotNull();
 		assertThat(decision.isGranted()).isTrue();
 	}
@@ -87,7 +87,7 @@ public class PostAuthorizeAuthorizationManagerTests {
 				TestClass.class);
 		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
 		AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser,
-				methodAuthorizationContext);
+				methodAuthorizationContext, null);
 		assertThat(decision).isNotNull();
 		assertThat(decision.isGranted()).isFalse();
 	}
@@ -99,10 +99,9 @@ public class PostAuthorizeAuthorizationManagerTests {
 				"doSomethingList", new Class[] { List.class }, new Object[] { list });
 		MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation,
 				TestClass.class);
-		methodAuthorizationContext.setReturnObject(list);
 		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
 		AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser,
-				methodAuthorizationContext);
+				methodAuthorizationContext, list);
 		assertThat(decision).isNotNull();
 		assertThat(decision.isGranted()).isTrue();
 	}
@@ -114,10 +113,9 @@ public class PostAuthorizeAuthorizationManagerTests {
 				"doSomethingList", new Class[] { List.class }, new Object[] { list });
 		MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation,
 				TestClass.class);
-		methodAuthorizationContext.setReturnObject(list);
 		PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
 		AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser,
-				methodAuthorizationContext);
+				methodAuthorizationContext, list);
 		assertThat(decision).isNotNull();
 		assertThat(decision.isGranted()).isFalse();
 	}