Handle SpEL AuthorizationDeniedExceptions

Closes spring-projectsgh-14600
jzheaux · Apr 4, 2024 · e48aadb · e48aadb
1 parent 7e69289
commit e48aadb
Show file tree

Hide file tree

Showing 13 changed files with 211 additions and 29 deletions.
diff --git a/...ngframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java b/...ngframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java
@@ -30,6 +30,7 @@
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.AuthorizationEventPublisher;
 import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
@@ -173,16 +174,28 @@ private Object attemptAuthorization(MethodInvocation mi, Object result) {
 		this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
 		MethodInvocationResult object = new MethodInvocationResult(mi, result);
 		AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, object);
-		this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, object, decision);
-		if (decision != null && !decision.isGranted()) {
-			this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
-					+ this.authorizationManager + " and decision " + decision));
-			return postProcess(object, decision);
+		try {
+			this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, object, decision);
+			if (decision != null && !decision.isGranted()) {
+				this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
+						+ this.authorizationManager + " and decision " + decision));
+				return postProcess(object, decision);
+			}
+		}
+		catch (AuthorizationDeniedException denied) {
+			return postProcess(object, denied);
 		}
 		this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
 		return result;
 	}
 
+	private Object postProcess(MethodInvocationResult mi, AuthorizationDeniedException denied) {
+		if (this.authorizationManager instanceof MethodAuthorizationDeniedPostProcessor postProcessableDecision) {
+			return postProcessableDecision.postProcessResult(mi, denied);
+		}
+		return this.defaultPostProcessor.postProcessResult(mi, denied);
+	}
+
 	private Object postProcess(MethodInvocationResult mi, AuthorizationDecision decision) {
 		if (this.authorizationManager instanceof MethodAuthorizationDeniedPostProcessor postProcessableDecision) {
 			return postProcessableDecision.postProcessResult(mi, decision);

diff --git a/...ork/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java b/...ork/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java
@@ -34,6 +34,7 @@
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.ReactiveAuthorizationManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.util.Assert;
@@ -151,7 +152,23 @@ private Mono<Object> postAuthorize(Mono<Authentication> authentication, MethodIn
 		MethodInvocationResult invocationResult = new MethodInvocationResult(mi, result);
 		return this.authorizationManager.check(authentication, invocationResult)
 			.switchIfEmpty(Mono.just(new AuthorizationDecision(false)))
-			.flatMap((decision) -> postProcess(decision, invocationResult));
+			.flatMap((decision) -> postProcess(decision, invocationResult))
+			.onErrorResume(AuthorizationDeniedException.class, (denied) -> postProcess(denied, invocationResult));
+	}
+
+	private Mono<Object> postProcess(AuthorizationDeniedException denied,
+			MethodInvocationResult methodInvocationResult) {
+		return Mono.fromSupplier(() -> {
+			if (this.authorizationManager instanceof MethodAuthorizationDeniedPostProcessor postProcessableDecision) {
+				return postProcessableDecision.postProcessResult(methodInvocationResult, denied);
+			}
+			return this.defaultPostProcessor.postProcessResult(methodInvocationResult, denied);
+		}).flatMap((processedResult) -> {
+			if (Mono.class.isAssignableFrom(processedResult.getClass())) {
+				return (Mono<?>) processedResult;
+			}
+			return Mono.justOrEmpty(processedResult);
+		});
 	}
 
 	private Mono<Object> postProcess(AuthorizationDecision decision, MethodInvocationResult methodInvocationResult) {

diff --git a/...gframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java b/...gframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java
@@ -34,8 +34,10 @@
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.AuthorizationEventPublisher;
 import org.springframework.security.authorization.AuthorizationManager;
+import org.springframework.security.authorization.AuthorizationResult;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.core.context.SecurityContextHolderStrategy;
@@ -245,18 +247,30 @@ public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy secur
 
 	private Object attemptAuthorization(MethodInvocation mi) throws Throwable {
 		this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
-		AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, mi);
-		this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, mi, decision);
-		if (decision != null && !decision.isGranted()) {
-			this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
-					+ this.authorizationManager + " and decision " + decision));
-			return handle(mi, decision);
+		try {
+			AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, mi);
+			this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, mi, decision);
+			if (decision != null && !decision.isGranted()) {
+				this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
+						+ this.authorizationManager + " and decision " + decision));
+				return handle(mi, decision);
+			}
+		}
+		catch (AuthorizationDeniedException denied) {
+			return handle(mi, denied);
 		}
 		this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
 		return mi.proceed();
 	}
 
-	private Object handle(MethodInvocation mi, AuthorizationDecision decision) {
+	private Object handle(MethodInvocation mi, AuthorizationDeniedException denied) {
+		if (this.authorizationManager instanceof MethodAuthorizationDeniedHandler handler) {
+			return handler.handle(mi, denied);
+		}
+		return this.defaultHandler.handle(mi, denied);
+	}
+
+	private Object handle(MethodInvocation mi, AuthorizationResult decision) {
 		if (this.authorizationManager instanceof MethodAuthorizationDeniedHandler handler) {
 			return handler.handle(mi, decision);
 		}

diff --git a/...rk/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java b/...rk/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java
@@ -33,6 +33,7 @@
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.ReactiveAuthorizationManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.util.Assert;
@@ -145,7 +146,8 @@ private Flux<Object> preAuthorized(MethodInvocation mi, Flux<Object> mapping) {
 					return mapping;
 				}
 				return postProcess(decision, mi);
-			});
+			})
+			.onErrorResume(AuthorizationDeniedException.class, (denied) -> postProcess(denied, mi));
 	}
 
 	private Mono<Object> preAuthorized(MethodInvocation mi, Mono<Object> mapping) {
@@ -157,7 +159,22 @@ private Mono<Object> preAuthorized(MethodInvocation mi, Mono<Object> mapping) {
 					return mapping;
 				}
 				return postProcess(decision, mi);
-			});
+			})
+			.onErrorResume(AuthorizationDeniedException.class, (denied) -> postProcess(denied, mi));
+	}
+
+	private Mono<Object> postProcess(AuthorizationDeniedException denied, MethodInvocation mi) {
+		return Mono.fromSupplier(() -> {
+			if (this.authorizationManager instanceof MethodAuthorizationDeniedHandler handler) {
+				return handler.handle(mi, denied);
+			}
+			return this.defaultHandler.handle(mi, denied);
+		}).flatMap((processedResult) -> {
+			if (Mono.class.isAssignableFrom(processedResult.getClass())) {
+				return (Mono<?>) processedResult;
+			}
+			return Mono.justOrEmpty(processedResult);
+		});
 	}
 
 	private Mono<Object> postProcess(AuthorizationDecision decision, MethodInvocation mi) {

diff --git a/...a/org/springframework/security/authorization/method/MethodAuthorizationDeniedHandler.java b/...a/org/springframework/security/authorization/method/MethodAuthorizationDeniedHandler.java
@@ -19,6 +19,7 @@
 import org.aopalliance.intercept.MethodInvocation;
 
 import org.springframework.lang.Nullable;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.AuthorizationResult;
 
 /**
@@ -43,4 +44,8 @@ public interface MethodAuthorizationDeniedHandler {
 	@Nullable
 	Object handle(MethodInvocation methodInvocation, AuthorizationResult authorizationResult);
 
+	default Object handle(MethodInvocation methodInvocation, AuthorizationDeniedException authorizationDenied) {
+		return handle(methodInvocation, authorizationDenied.getAuthorizationResult());
+	}
+
 }
diff --git a/...springframework/security/authorization/method/MethodAuthorizationDeniedPostProcessor.java b/...springframework/security/authorization/method/MethodAuthorizationDeniedPostProcessor.java
@@ -17,6 +17,7 @@
 package org.springframework.security.authorization.method;
 
 import org.springframework.lang.Nullable;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.AuthorizationResult;
 
 /**
@@ -43,4 +44,9 @@ public interface MethodAuthorizationDeniedPostProcessor {
 	@Nullable
 	Object postProcessResult(MethodInvocationResult methodInvocationResult, AuthorizationResult authorizationResult);
 
+	default Object postProcessResult(MethodInvocationResult methodInvocationResult,
+			AuthorizationDeniedException authorizationDenied) {
+		return postProcessResult(methodInvocationResult, authorizationDenied.getAuthorizationResult());
+	}
+
 }
diff --git a/...ringframework/security/authorization/method/ThrowingMethodAuthorizationDeniedHandler.java b/...ringframework/security/authorization/method/ThrowingMethodAuthorizationDeniedHandler.java
@@ -35,4 +35,9 @@ public Object handle(MethodInvocation methodInvocation, AuthorizationResult resu
 		throw new AuthorizationDeniedException("Access Denied", result);
 	}
 
+	@Override
+	public Object handle(MethodInvocation methodInvocation, AuthorizationDeniedException authorizationDenied) {
+		throw authorizationDenied;
+	}
+
 }
diff --git a/...amework/security/authorization/method/ThrowingMethodAuthorizationDeniedPostProcessor.java b/...amework/security/authorization/method/ThrowingMethodAuthorizationDeniedPostProcessor.java
@@ -33,4 +33,10 @@ public Object postProcessResult(MethodInvocationResult methodInvocationResult, A
 		throw new AuthorizationDeniedException("Access Denied", result);
 	}
 
+	@Override
+	public Object postProcessResult(MethodInvocationResult methodInvocationResult,
+			AuthorizationDeniedException authorizationDenied) {
+		throw authorizationDenied;
+	}
+
 }
diff --git a/...mework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java b/...mework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java
@@ -26,8 +26,10 @@
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authorization.AuthenticatedAuthorizationManager;
 import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationDeniedException;
 import org.springframework.security.authorization.AuthorizationEventPublisher;
 import org.springframework.security.authorization.AuthorizationManager;
+import org.springframework.security.authorization.AuthorizationResult;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.context.SecurityContext;
@@ -36,6 +38,7 @@
 import org.springframework.security.core.context.SecurityContextImpl;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
@@ -139,4 +142,24 @@ public void invokeWhenAuthorizationEventPublisherThenUses() throws Throwable {
 				any(AuthorizationDecision.class));
 	}
 
+	@Test
+	public void invokeWhenCustomAuthorizationDeniedExceptionThenThrows() throws Throwable {
+		MethodInvocation mi = mock(MethodInvocation.class);
+		given(mi.proceed()).willReturn("ok");
+		AuthorizationManager<MethodInvocationResult> manager = mock(AuthorizationManager.class);
+		given(manager.check(any(), any()))
+			.willThrow(new MyAuthzDeniedException("denied", new AuthorizationDecision(false)));
+		AuthorizationManagerAfterMethodInterceptor advice = new AuthorizationManagerAfterMethodInterceptor(
+				Pointcut.TRUE, manager);
+		assertThatExceptionOfType(MyAuthzDeniedException.class).isThrownBy(() -> advice.invoke(mi));
+	}
+
+	static class MyAuthzDeniedException extends AuthorizationDeniedException {
+
+		MyAuthzDeniedException(String msg, AuthorizationResult authorizationResult) {
+			super(msg, authorizationResult);
+		}
+
+	}
+
 }
diff --git a/...ecurity/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptorTests.java b/...ecurity/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptorTests.java
@@ -28,6 +28,7 @@
 import org.springframework.security.access.intercept.method.MockMethodInvocation;
 import org.springframework.security.authorization.AuthorizationDecision;
 import org.springframework.security.authorization.AuthorizationDeniedException;
+import org.springframework.security.authorization.AuthorizationResult;
 import org.springframework.security.authorization.ReactiveAuthorizationManager;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -126,7 +127,8 @@ public void invokeFluxWhenAllValuesDeniedAndPostProcessorThenPostProcessorApplie
 		given(mockMethodInvocation.proceed()).willReturn(Flux.just("john", "bob"));
 		HandlingReactiveAuthorizationManager mockReactiveAuthorizationManager = mock(
 				HandlingReactiveAuthorizationManager.class);
-		given(mockReactiveAuthorizationManager.postProcessResult(any(), any())).willAnswer(this::masking);
+		given(mockReactiveAuthorizationManager.postProcessResult(any(), any(AuthorizationResult.class)))
+			.willAnswer(this::masking);
 		given(mockReactiveAuthorizationManager.check(any(), any())).willReturn(Mono.empty());
 		AuthorizationManagerAfterReactiveMethodInterceptor interceptor = new AuthorizationManagerAfterReactiveMethodInterceptor(
 				Pointcut.TRUE, mockReactiveAuthorizationManager);
@@ -145,13 +147,14 @@ public void invokeFluxWhenOneValueDeniedAndPostProcessorThenPostProcessorApplied
 		given(mockMethodInvocation.proceed()).willReturn(Flux.just("john", "bob"));
 		HandlingReactiveAuthorizationManager mockReactiveAuthorizationManager = mock(
 				HandlingReactiveAuthorizationManager.class);
-		given(mockReactiveAuthorizationManager.postProcessResult(any(), any())).willAnswer((invocation) -> {
-			MethodInvocationResult argument = invocation.getArgument(0);
-			if (!"john".equals(argument.getResult())) {
-				return monoMasking(invocation);
-			}
-			return Mono.just(argument.getResult());
-		});
+		given(mockReactiveAuthorizationManager.postProcessResult(any(), any(AuthorizationResult.class)))
+			.willAnswer((invocation) -> {
+				MethodInvocationResult argument = invocation.getArgument(0);
+				if (!"john".equals(argument.getResult())) {
+					return monoMasking(invocation);
+				}
+				return Mono.just(argument.getResult());
+			});
 		given(mockReactiveAuthorizationManager.check(any(), any())).willReturn(Mono.empty());
 		AuthorizationManagerAfterReactiveMethodInterceptor interceptor = new AuthorizationManagerAfterReactiveMethodInterceptor(
 				Pointcut.TRUE, mockReactiveAuthorizationManager);
@@ -170,7 +173,8 @@ public void invokeMonoWhenPostProcessableDecisionThenPostProcess() throws Throwa
 		given(mockMethodInvocation.proceed()).willReturn(Mono.just("john"));
 		HandlingReactiveAuthorizationManager mockReactiveAuthorizationManager = mock(
 				HandlingReactiveAuthorizationManager.class);
-		given(mockReactiveAuthorizationManager.postProcessResult(any(), any())).willAnswer(this::masking);
+		given(mockReactiveAuthorizationManager.postProcessResult(any(), any(AuthorizationResult.class)))
+			.willAnswer(this::masking);
 		given(mockReactiveAuthorizationManager.check(any(), any())).willReturn(Mono.empty());
 		AuthorizationManagerAfterReactiveMethodInterceptor interceptor = new AuthorizationManagerAfterReactiveMethodInterceptor(
 				Pointcut.TRUE, mockReactiveAuthorizationManager);
@@ -188,7 +192,8 @@ public void invokeMonoWhenPostProcessableDecisionAndPostProcessResultIsMonoThenP
 		given(mockMethodInvocation.proceed()).willReturn(Mono.just("john"));
 		HandlingReactiveAuthorizationManager mockReactiveAuthorizationManager = mock(
 				HandlingReactiveAuthorizationManager.class);
-		given(mockReactiveAuthorizationManager.postProcessResult(any(), any())).willAnswer(this::monoMasking);
+		given(mockReactiveAuthorizationManager.postProcessResult(any(), any(AuthorizationResult.class)))
+			.willAnswer(this::monoMasking);
 		given(mockReactiveAuthorizationManager.check(any(), any())).willReturn(Mono.empty());
 		AuthorizationManagerAfterReactiveMethodInterceptor interceptor = new AuthorizationManagerAfterReactiveMethodInterceptor(
 				Pointcut.TRUE, mockReactiveAuthorizationManager);
@@ -206,7 +211,8 @@ public void invokeMonoWhenPostProcessableDecisionAndPostProcessResultIsNullThenP
 		given(mockMethodInvocation.proceed()).willReturn(Mono.just("john"));
 		HandlingReactiveAuthorizationManager mockReactiveAuthorizationManager = mock(
 				HandlingReactiveAuthorizationManager.class);
-		given(mockReactiveAuthorizationManager.postProcessResult(any(), any())).willReturn(null);
+		given(mockReactiveAuthorizationManager.postProcessResult(any(), any(AuthorizationResult.class)))
+			.willReturn(null);
 		given(mockReactiveAuthorizationManager.check(any(), any())).willReturn(Mono.empty());
 		AuthorizationManagerAfterReactiveMethodInterceptor interceptor = new AuthorizationManagerAfterReactiveMethodInterceptor(
 				Pointcut.TRUE, mockReactiveAuthorizationManager);
@@ -235,6 +241,20 @@ public void invokeMonoWhenEmptyDecisionThenUseDefaultPostProcessor() throws Thro
 		verify(mockReactiveAuthorizationManager).check(any(), any());
 	}
 
+	@Test
+	public void invokeWhenCustomAuthorizationDeniedExceptionThenThrows() throws Throwable {
+		MethodInvocation mockMethodInvocation = spy(
+				new MockMethodInvocation(new Sample(), Sample.class.getDeclaredMethod("mono")));
+		given(mockMethodInvocation.proceed()).willReturn(Mono.just("ok"));
+		ReactiveAuthorizationManager<MethodInvocationResult> manager = mock(ReactiveAuthorizationManager.class);
+		given(manager.check(any(), any()))
+			.willReturn(Mono.error(new MyAuthzDeniedException("denied", new AuthorizationDecision(false))));
+		AuthorizationManagerAfterReactiveMethodInterceptor advice = new AuthorizationManagerAfterReactiveMethodInterceptor(
+				Pointcut.TRUE, manager);
+		assertThatExceptionOfType(MyAuthzDeniedException.class)
+			.isThrownBy(() -> ((Mono<?>) advice.invoke(mockMethodInvocation)).block());
+	}
+
 	private Object masking(InvocationOnMock invocation) {
 		MethodInvocationResult result = invocation.getArgument(0);
 		return result.getResult() + "-masked";
@@ -262,4 +282,12 @@ Flux<String> flux() {
 
 	}
 
+	static class MyAuthzDeniedException extends AuthorizationDeniedException {
+
+		MyAuthzDeniedException(String msg, AuthorizationResult authorizationResult) {
+			super(msg, authorizationResult);
+		}
+
+	}
+
 }