From 933ef676376fd903d7012dc12261d06077f0c5fb Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 11 Apr 2024 13:49:14 -0600 Subject: [PATCH] Polish AuthorizationDeniedException Handling Issue gh-14600 --- .../AuthorizationDeniedException.java | 7 ++++++- ...orizationManagerAfterMethodInterceptor.java | 10 ++-------- ...nManagerAfterReactiveMethodInterceptor.java | 18 ++---------------- ...rizationManagerBeforeMethodInterceptor.java | 7 ------- ...ManagerBeforeReactiveMethodInterceptor.java | 17 ++--------------- .../MethodAuthorizationDeniedHandler.java | 15 --------------- ...MethodAuthorizationDeniedPostProcessor.java | 18 ------------------ ...rowingMethodAuthorizationDeniedHandler.java | 8 +++----- ...MethodAuthorizationDeniedPostProcessor.java | 9 +++------ 9 files changed, 18 insertions(+), 91 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorizationDeniedException.java b/core/src/main/java/org/springframework/security/authorization/AuthorizationDeniedException.java index f38ec0d29f..4bd8fd2bf6 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorizationDeniedException.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorizationDeniedException.java @@ -25,7 +25,7 @@ import org.springframework.util.Assert; * @author Marcus da Coregio * @since 6.3 */ -public class AuthorizationDeniedException extends AccessDeniedException { +public class AuthorizationDeniedException extends AccessDeniedException implements AuthorizationResult { private final AuthorizationResult result; @@ -40,4 +40,9 @@ public class AuthorizationDeniedException extends AccessDeniedException { return this.result; } + @Override + public boolean isGranted() { + return false; + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java index 0eaf7c51e9..ef37e58d5f 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java @@ -33,6 +33,7 @@ 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; @@ -190,14 +191,7 @@ public final class AuthorizationManagerAfterMethodInterceptor implements Authori 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) { + private Object postProcess(MethodInvocationResult mi, AuthorizationResult decision) { if (this.authorizationManager instanceof MethodAuthorizationDeniedPostProcessor postProcessableDecision) { return postProcessableDecision.postProcessResult(mi, decision); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java index 2e56a3e865..d996242115 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java @@ -35,6 +35,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.AuthorizationResult; import org.springframework.security.authorization.ReactiveAuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.util.Assert; @@ -165,22 +166,7 @@ public final class AuthorizationManagerAfterReactiveMethodInterceptor implements }); } - private Mono 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 postProcess(AuthorizationDecision decision, MethodInvocationResult methodInvocationResult) { + private Mono postProcess(AuthorizationResult decision, MethodInvocationResult methodInvocationResult) { if (decision.isGranted()) { return Mono.just(methodInvocationResult.getResult()); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java index 4266efc68c..5d85967faa 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java @@ -264,13 +264,6 @@ public final class AuthorizationManagerBeforeMethodInterceptor implements Author return mi.proceed(); } - 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/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java index e8d8179930..71a1f00881 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java @@ -34,6 +34,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.AuthorizationResult; import org.springframework.security.authorization.ReactiveAuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.util.Assert; @@ -177,21 +178,7 @@ public final class AuthorizationManagerBeforeReactiveMethodInterceptor implement }); } - private Mono 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 postProcess(AuthorizationDecision decision, MethodInvocation mi) { + private Mono postProcess(AuthorizationResult decision, MethodInvocation mi) { return Mono.fromSupplier(() -> { if (this.authorizationManager instanceof MethodAuthorizationDeniedHandler handler) { return handler.handle(mi, decision); diff --git a/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedHandler.java b/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedHandler.java index 051134f046..8ccc797d98 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedHandler.java +++ b/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedHandler.java @@ -19,7 +19,6 @@ package org.springframework.security.authorization.method; import org.aopalliance.intercept.MethodInvocation; import org.springframework.lang.Nullable; -import org.springframework.security.authorization.AuthorizationDeniedException; import org.springframework.security.authorization.AuthorizationResult; /** @@ -44,18 +43,4 @@ public interface MethodAuthorizationDeniedHandler { @Nullable Object handle(MethodInvocation methodInvocation, AuthorizationResult authorizationResult); - /** - * Handle denied method invocations, implementations might either throw an - * {@link org.springframework.security.access.AccessDeniedException} or a replacement - * result instead of invoking the method, e.g. a masked value. - * @param methodInvocation the {@link MethodInvocation} related to the authorization - * denied - * @param authorizationDenied the authorization denied exception - * @return a replacement result for the denied method invocation, or null, or a - * {@link reactor.core.publisher.Mono} for reactive applications - */ - default Object handle(MethodInvocation methodInvocation, AuthorizationDeniedException authorizationDenied) { - return handle(methodInvocation, authorizationDenied.getAuthorizationResult()); - } - } diff --git a/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedPostProcessor.java b/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedPostProcessor.java index 2d4c201309..a72b2b09e3 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedPostProcessor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationDeniedPostProcessor.java @@ -17,7 +17,6 @@ package org.springframework.security.authorization.method; import org.springframework.lang.Nullable; -import org.springframework.security.authorization.AuthorizationDeniedException; import org.springframework.security.authorization.AuthorizationResult; /** @@ -44,21 +43,4 @@ public interface MethodAuthorizationDeniedPostProcessor { @Nullable Object postProcessResult(MethodInvocationResult methodInvocationResult, AuthorizationResult authorizationResult); - /** - * Post-process the denied result produced by a method invocation, implementations - * might either throw an - * {@link org.springframework.security.access.AccessDeniedException} or return a - * replacement result instead of the denied result, e.g. a masked value. - * @param methodInvocationResult the object containing the method invocation and the - * result produced - * @param authorizationDenied the {@link AuthorizationDeniedException} containing the - * authorization denied details - * @return a replacement result for the denied result, or null, or a - * {@link reactor.core.publisher.Mono} for reactive applications - */ - default Object postProcessResult(MethodInvocationResult methodInvocationResult, - AuthorizationDeniedException authorizationDenied) { - return postProcessResult(methodInvocationResult, authorizationDenied.getAuthorizationResult()); - } - } diff --git a/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedHandler.java b/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedHandler.java index 763cac16a2..e92e1345fd 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedHandler.java +++ b/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedHandler.java @@ -32,12 +32,10 @@ public final class ThrowingMethodAuthorizationDeniedHandler implements MethodAut @Override public Object handle(MethodInvocation methodInvocation, AuthorizationResult result) { + if (result instanceof AuthorizationDeniedException denied) { + throw denied; + } throw new AuthorizationDeniedException("Access Denied", result); } - @Override - public Object handle(MethodInvocation methodInvocation, AuthorizationDeniedException authorizationDenied) { - throw authorizationDenied; - } - } diff --git a/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedPostProcessor.java b/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedPostProcessor.java index bb468a6e20..5a7e2c16b1 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedPostProcessor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/ThrowingMethodAuthorizationDeniedPostProcessor.java @@ -30,13 +30,10 @@ public final class ThrowingMethodAuthorizationDeniedPostProcessor implements Met @Override public Object postProcessResult(MethodInvocationResult methodInvocationResult, AuthorizationResult result) { + if (result instanceof AuthorizationDeniedException denied) { + throw denied; + } throw new AuthorizationDeniedException("Access Denied", result); } - @Override - public Object postProcessResult(MethodInvocationResult methodInvocationResult, - AuthorizationDeniedException authorizationDenied) { - throw authorizationDenied; - } - }