From 9ce5a76e8c2322a07f04923e8ce92d62cc7a8b0d Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Mon, 14 Oct 2024 11:20:33 -0600 Subject: [PATCH] Polish AuthorizationManager#authorize Issue gh-14843 --- .../http/DefaultFilterChainValidator.java | 10 +-- ...ointcutDelegatingAuthorizationManager.java | 16 +++- ...ptMethodsBeanDefinitionDecoratorTests.java | 1 + .../AuthorityAuthorizationManager.java | 2 + .../authorization/AuthorizationManagers.java | 82 ++++++++++++------- .../AuthorizationObservationContext.java | 15 ++-- .../ObservationAuthorizationManager.java | 4 + ...servationReactiveAuthorizationManager.java | 4 + ...rizationManagerAfterMethodInterceptor.java | 16 ++-- ...izationManagerBeforeMethodInterceptor.java | 18 ++-- .../method/Jsr250AuthorizationManager.java | 27 +++++- .../method/SecuredAuthorizationManager.java | 2 + .../Jsr250AuthorizationManagerTests.java | 3 + .../AuthorizationChannelInterceptor.java | 9 +- ...MatcherDelegatingAuthorizationManager.java | 2 + ...geMatcherReactiveAuthorizationManager.java | 4 + ...anagerWebInvocationPrivilegeEvaluator.java | 4 +- .../access/intercept/AuthorizationFilter.java | 8 +- ...MatcherDelegatingAuthorizationManager.java | 2 + ...elegatingReactiveAuthorizationManager.java | 4 + 20 files changed, 160 insertions(+), 73 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java index 1c140c3edd5..ce7c50be584 100644 --- a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java +++ b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java @@ -221,9 +221,8 @@ private boolean checkLoginPageIsPublic(List filters, FilterInvocation lo AuthorizationManager authorizationManager = authorizationFilter .getAuthorizationManager(); try { - AuthorizationResult decision = authorizationManager.authorize(() -> TEST, - loginRequest.getHttpRequest()); - return decision != null && decision.isGranted(); + AuthorizationResult result = authorizationManager.authorize(() -> TEST, loginRequest.getHttpRequest()); + return result != null && result.isGranted(); } catch (Exception ex) { return false; @@ -253,9 +252,8 @@ private Supplier deriveAnonymousCheck(List filters, FilterInvoc return () -> { AuthorizationManager authorizationManager = authorizationFilter .getAuthorizationManager(); - AuthorizationResult decision = authorizationManager.authorize(() -> token, - loginRequest.getHttpRequest()); - return decision != null && decision.isGranted(); + AuthorizationResult result = authorizationManager.authorize(() -> token, loginRequest.getHttpRequest()); + return result != null && result.isGranted(); }; } return () -> true; diff --git a/config/src/main/java/org/springframework/security/config/method/PointcutDelegatingAuthorizationManager.java b/config/src/main/java/org/springframework/security/config/method/PointcutDelegatingAuthorizationManager.java index d6aa4767f8f..1a47fc455ab 100644 --- a/config/src/main/java/org/springframework/security/config/method/PointcutDelegatingAuthorizationManager.java +++ b/config/src/main/java/org/springframework/security/config/method/PointcutDelegatingAuthorizationManager.java @@ -25,6 +25,7 @@ import org.springframework.aop.support.AopUtils; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; import org.springframework.security.core.Authentication; class PointcutDelegatingAuthorizationManager implements AuthorizationManager { @@ -37,11 +38,24 @@ class PointcutDelegatingAuthorizationManager implements AuthorizationManager authentication, MethodInvocation object) { + AuthorizationResult result = authorize(authentication, object); + if (result == null) { + return null; + } + if (result instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( + "Please either call authorize or ensure that the returned result is of type AuthorizationDecision"); + } + + @Override + public AuthorizationResult authorize(Supplier authentication, MethodInvocation object) { for (Map.Entry> entry : this.managers.entrySet()) { Class targetClass = (object.getThis() != null) ? AopUtils.getTargetClass(object.getThis()) : null; if (entry.getKey().getClassFilter().matches(targetClass) && entry.getKey().getMethodMatcher().matches(object.getMethod(), targetClass)) { - return entry.getValue().check(authentication, object); + return entry.getValue().authorize(authentication, object); } } return new AuthorizationDecision(false); diff --git a/config/src/test/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecoratorTests.java b/config/src/test/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecoratorTests.java index a4257f1732f..c71acbe5a44 100644 --- a/config/src/test/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecoratorTests.java +++ b/config/src/test/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecoratorTests.java @@ -168,6 +168,7 @@ public void transactionalAuthorizationManagerMethodsShouldBeSecured() { @Test public void targetCustomAuthorizationManagerUsed() { + given(this.mockAuthorizationManager.authorize(any(), any())).willCallRealMethod(); given(this.mockAuthorizationManager.check(any(), any())).willReturn(new AuthorizationDecision(true)); this.targetCustomAuthorizationManager.doSomething(); verify(this.mockAuthorizationManager).check(any(), any()); diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java index 1d93dffa08e..e71f99b37cc 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java @@ -139,7 +139,9 @@ private static String[] toNamedRolesArray(String rolePrefix, String[] roles) { * @param authentication the {@link Supplier} of the {@link Authentication} to check * @param object the {@link T} object to check * @return an {@link AuthorizationDecision} + * @deprecated please use {@link #authorize(Supplier, Object)} instead */ + @Deprecated @Override public AuthorizationDecision check(Supplier authentication, T object) { return this.delegate.check(authentication, this.authorities); diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorizationManagers.java b/core/src/main/java/org/springframework/security/authorization/AuthorizationManagers.java index b9031092050..f3893c97433 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorizationManagers.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorizationManagers.java @@ -18,6 +18,9 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; + +import org.springframework.security.core.Authentication; /** * A factory class to create an {@link AuthorizationManager} instances. @@ -55,22 +58,22 @@ public static AuthorizationManager anyOf(AuthorizationManager... manag @SafeVarargs public static AuthorizationManager anyOf(AuthorizationDecision allAbstainDefaultDecision, AuthorizationManager... managers) { - return (authentication, object) -> { - List decisions = new ArrayList<>(); + return (AuthorizationManagerCheckAdapter) (authentication, object) -> { + List results = new ArrayList<>(); for (AuthorizationManager manager : managers) { - AuthorizationDecision decision = manager.check(authentication, object); - if (decision == null) { + AuthorizationResult result = manager.authorize(authentication, object); + if (result == null) { continue; } - if (decision.isGranted()) { - return decision; + if (result.isGranted()) { + return result; } - decisions.add(decision); + results.add(result); } - if (decisions.isEmpty()) { + if (results.isEmpty()) { return allAbstainDefaultDecision; } - return new CompositeAuthorizationDecision(false, decisions); + return new CompositeAuthorizationDecision(false, results); }; } @@ -101,22 +104,22 @@ public static AuthorizationManager allOf(AuthorizationManager... manag @SafeVarargs public static AuthorizationManager allOf(AuthorizationDecision allAbstainDefaultDecision, AuthorizationManager... managers) { - return (authentication, object) -> { - List decisions = new ArrayList<>(); + return (AuthorizationManagerCheckAdapter) (authentication, object) -> { + List results = new ArrayList<>(); for (AuthorizationManager manager : managers) { - AuthorizationDecision decision = manager.check(authentication, object); - if (decision == null) { + AuthorizationResult result = manager.authorize(authentication, object); + if (result == null) { continue; } - if (!decision.isGranted()) { - return decision; + if (!result.isGranted()) { + return result; } - decisions.add(decision); + results.add(result); } - if (decisions.isEmpty()) { + if (results.isEmpty()) { return allAbstainDefaultDecision; } - return new CompositeAuthorizationDecision(true, decisions); + return new CompositeAuthorizationDecision(true, results); }; } @@ -131,11 +134,11 @@ public static AuthorizationManager allOf(AuthorizationDecision allAbstain */ public static AuthorizationManager not(AuthorizationManager manager) { return (authentication, object) -> { - AuthorizationDecision decision = manager.check(authentication, object); - if (decision == null) { + AuthorizationResult result = manager.authorize(authentication, object); + if (result == null) { return null; } - return new NotAuthorizationDecision(decision); + return new NotAuthorizationDecision(result); }; } @@ -144,34 +147,53 @@ private AuthorizationManagers() { private static final class CompositeAuthorizationDecision extends AuthorizationDecision { - private final List decisions; + private final List results; - private CompositeAuthorizationDecision(boolean granted, List decisions) { + private CompositeAuthorizationDecision(boolean granted, List results) { super(granted); - this.decisions = decisions; + this.results = results; } @Override public String toString() { - return "CompositeAuthorizationDecision [decisions=" + this.decisions + ']'; + return "CompositeAuthorizationDecision [results=" + this.results + ']'; } } private static final class NotAuthorizationDecision extends AuthorizationDecision { - private final AuthorizationDecision decision; + private final AuthorizationResult result; - private NotAuthorizationDecision(AuthorizationDecision decision) { - super(!decision.isGranted()); - this.decision = decision; + private NotAuthorizationDecision(AuthorizationResult result) { + super(!result.isGranted()); + this.result = result; } @Override public String toString() { - return "NotAuthorizationDecision [decision=" + this.decision + ']'; + return "NotAuthorizationDecision [result=" + this.result + ']'; } } + private interface AuthorizationManagerCheckAdapter extends AuthorizationManager { + + @Override + default AuthorizationDecision check(Supplier authentication, T object) { + AuthorizationResult result = authorize(authentication, object); + if (result == null) { + return null; + } + if (result instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( + "please call #authorize or ensure that the result is of type AuthorizationDecision"); + } + + AuthorizationResult authorize(Supplier authentication, T object); + + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorizationObservationContext.java b/core/src/main/java/org/springframework/security/authorization/AuthorizationObservationContext.java index 8374bd6e343..06c40e8fe02 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorizationObservationContext.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorizationObservationContext.java @@ -33,8 +33,6 @@ public class AuthorizationObservationContext extends Observation.Context { private final T object; - private AuthorizationDecision decision; - private AuthorizationResult authorizationResult; public AuthorizationObservationContext(T object) { @@ -77,9 +75,14 @@ public T getObject() { */ @Deprecated public AuthorizationDecision getDecision() { - Assert.isInstanceOf(AuthorizationDecision.class, this.authorizationResult, + if (this.authorizationResult == null) { + return null; + } + if (this.authorizationResult instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( "Please call getAuthorizationResult instead. If you must call getDecision, please ensure that the result you provide is of type AuthorizationDecision"); - return (AuthorizationDecision) this.authorizationResult; } /** @@ -89,9 +92,7 @@ public AuthorizationDecision getDecision() { */ @Deprecated public void setDecision(AuthorizationDecision decision) { - Assert.isInstanceOf(AuthorizationDecision.class, decision, - "Please call setAuthorizationResult instead. If you must call getDecision, please ensure that the result you provide is of type AuthorizationDecision"); - this.decision = decision; + this.authorizationResult = decision; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java index 3ee6a1f0e8a..0494dbfecb0 100644 --- a/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/ObservationAuthorizationManager.java @@ -61,6 +61,10 @@ public ObservationAuthorizationManager(ObservationRegistry registry, Authorizati } } + /** + * @deprecated please use {@link #authorize(Supplier, Object)} instead + */ + @Deprecated @Override public AuthorizationDecision check(Supplier authentication, T object) { AuthorizationObservationContext context = new AuthorizationObservationContext<>(object); diff --git a/core/src/main/java/org/springframework/security/authorization/ObservationReactiveAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/ObservationReactiveAuthorizationManager.java index 75e395f227e..a9d77c641b5 100644 --- a/core/src/main/java/org/springframework/security/authorization/ObservationReactiveAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/ObservationReactiveAuthorizationManager.java @@ -56,6 +56,10 @@ public ObservationReactiveAuthorizationManager(ObservationRegistry registry, } } + /** + * @deprecated please use {@link #authorize(Mono, Object)} instead + */ + @Deprecated @Override public Mono check(Mono authentication, T object) { AuthorizationObservationContext context = new AuthorizationObservationContext<>(object); 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 fdd8a039c76..a5604c5c15f 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 @@ -182,22 +182,22 @@ public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy strat private Object attemptAuthorization(MethodInvocation mi, Object result) { this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi)); MethodInvocationResult object = new MethodInvocationResult(mi, result); - AuthorizationResult decision = this.authorizationManager.authorize(this::getAuthentication, object); - this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, object, decision); - if (decision != null && !decision.isGranted()) { + AuthorizationResult authorizationResult = this.authorizationManager.authorize(this::getAuthentication, object); + this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, object, authorizationResult); + if (authorizationResult != null && !authorizationResult.isGranted()) { this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager " - + this.authorizationManager + " and decision " + decision)); - return handlePostInvocationDenied(object, decision); + + this.authorizationManager + " and authorizationResult " + authorizationResult)); + return handlePostInvocationDenied(object, authorizationResult); } this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi)); return result; } - private Object handlePostInvocationDenied(MethodInvocationResult mi, AuthorizationResult decision) { + private Object handlePostInvocationDenied(MethodInvocationResult mi, AuthorizationResult result) { if (this.authorizationManager instanceof MethodAuthorizationDeniedHandler deniedHandler) { - return deniedHandler.handleDeniedInvocationResult(mi, decision); + return deniedHandler.handleDeniedInvocationResult(mi, result); } - return this.defaultHandler.handleDeniedInvocationResult(mi, decision); + return this.defaultHandler.handleDeniedInvocationResult(mi, result); } private Authentication getAuthentication() { 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 689938ffaad..4e67ca9f802 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 @@ -246,18 +246,18 @@ public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy secur private Object attemptAuthorization(MethodInvocation mi) throws Throwable { this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi)); - AuthorizationResult decision; + AuthorizationResult result; try { - decision = this.authorizationManager.authorize(this::getAuthentication, mi); + result = this.authorizationManager.authorize(this::getAuthentication, mi); } catch (AuthorizationDeniedException denied) { return handle(mi, denied); } - this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, mi, decision); - if (decision != null && !decision.isGranted()) { + this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, mi, result); + if (result != null && !result.isGranted()) { this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager " - + this.authorizationManager + " and decision " + decision)); - return handle(mi, decision); + + this.authorizationManager + " and result " + result)); + return handle(mi, result); } this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi)); return proceed(mi); @@ -282,11 +282,11 @@ private Object handle(MethodInvocation mi, AuthorizationDeniedException denied) return this.defaultHandler.handleDeniedInvocation(mi, denied); } - private Object handle(MethodInvocation mi, AuthorizationResult decision) { + private Object handle(MethodInvocation mi, AuthorizationResult result) { if (this.authorizationManager instanceof MethodAuthorizationDeniedHandler handler) { - return handler.handleDeniedInvocation(mi, decision); + return handler.handleDeniedInvocation(mi, result); } - return this.defaultHandler.handleDeniedInvocation(mi, decision); + return this.defaultHandler.handleDeniedInvocation(mi, result); } private Authentication getAuthentication() { diff --git a/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java index 74d03e78622..30c3977db5e 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java @@ -33,6 +33,7 @@ import org.springframework.security.authorization.AuthoritiesAuthorizationManager; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; import org.springframework.security.core.Authentication; import org.springframework.security.core.annotation.SecurityAnnotationScanner; import org.springframework.security.core.annotation.SecurityAnnotationScanners; @@ -86,7 +87,9 @@ public void setRolePrefix(String rolePrefix) { * @param methodInvocation the {@link MethodInvocation} to check * @return an {@link AuthorizationDecision} or null if the JSR-250 security * annotations is not present + * @deprecated please use {@link #authorize(Supplier, Object)} instead */ + @Deprecated @Override public AuthorizationDecision check(Supplier authentication, MethodInvocation methodInvocation) { AuthorizationManager delegate = this.registry.getManager(methodInvocation); @@ -109,8 +112,9 @@ AuthorizationManager resolveManager(Method method, Class ta return (a, o) -> new AuthorizationDecision(true); } if (annotation instanceof RolesAllowed rolesAllowed) { - return (a, o) -> Jsr250AuthorizationManager.this.authoritiesAuthorizationManager.check(a, - getAllowedRolesWithPrefix(rolesAllowed)); + return (AuthorizationManagerCheckAdapter) (a, + o) -> Jsr250AuthorizationManager.this.authoritiesAuthorizationManager.authorize(a, + getAllowedRolesWithPrefix(rolesAllowed)); } return NULL_MANAGER; } @@ -130,4 +134,23 @@ private Set getAllowedRolesWithPrefix(RolesAllowed rolesAllowed) { } + private interface AuthorizationManagerCheckAdapter extends AuthorizationManager { + + @Override + default AuthorizationDecision check(Supplier authentication, T object) { + AuthorizationResult result = authorize(authentication, object); + if (result == null) { + return null; + } + if (result instanceof AuthorizationDecision decision) { + return decision; + } + throw new IllegalArgumentException( + "please call #authorize or ensure that the result is of type AuthorizationDecision"); + } + + AuthorizationResult authorize(Supplier authentication, T object); + + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java index ba71b56626d..efd5a34faa7 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java @@ -73,7 +73,9 @@ public void setAuthoritiesAuthorizationManager( * @param mi the {@link MethodInvocation} to check * @return an {@link AuthorizationDecision} or null if the {@link Secured} annotation * is not present + * @deprecated please use {@link #authorize(Supplier, Object)} instead */ + @Deprecated @Override public AuthorizationDecision check(Supplier authentication, MethodInvocation mi) { Set authorities = getAuthorities(mi); diff --git a/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java index ba7324d6a51..1b7d6e7a228 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java @@ -38,6 +38,8 @@ 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; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -78,6 +80,7 @@ public void setAuthoritiesAuthorizationManagerWhenNullThenException() { @Test public void setAuthoritiesAuthorizationManagerWhenNotNullThenVerifyUsage() throws Exception { AuthorizationManager> authoritiesAuthorizationManager = mock(AuthorizationManager.class); + given(authoritiesAuthorizationManager.authorize(any(), any())).willCallRealMethod(); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); manager.setAuthoritiesAuthorizationManager(authoritiesAuthorizationManager); MockMethodInvocation methodInvocation = new MockMethodInvocation(new ClassLevelAnnotations(), diff --git a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/AuthorizationChannelInterceptor.java b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/AuthorizationChannelInterceptor.java index 91f68118f5b..c17e2b8de94 100644 --- a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/AuthorizationChannelInterceptor.java +++ b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/AuthorizationChannelInterceptor.java @@ -67,11 +67,11 @@ public AuthorizationChannelInterceptor(AuthorizationManager> preSendA @Override public Message preSend(Message message, MessageChannel channel) { this.logger.debug(LogMessage.of(() -> "Authorizing message send")); - AuthorizationResult decision = this.preSendAuthorizationManager.authorize(this.authentication, message); - this.eventPublisher.publishAuthorizationEvent(this.authentication, message, decision); - if (decision == null || !decision.isGranted()) { // default deny + AuthorizationResult result = this.preSendAuthorizationManager.authorize(this.authentication, message); + this.eventPublisher.publishAuthorizationEvent(this.authentication, message, result); + if (result == null || !result.isGranted()) { // default deny this.logger.debug(LogMessage.of(() -> "Failed to authorize message with authorization manager " - + this.preSendAuthorizationManager + " and decision " + decision)); + + this.preSendAuthorizationManager + " and result " + result)); throw new AccessDeniedException("Access Denied"); } this.logger.debug(LogMessage.of(() -> "Authorized message send")); @@ -118,6 +118,7 @@ public void publishAuthorizationEvent(Supplier authenticatio @Override public void publishAuthorizationEvent(Supplier authentication, T object, AuthorizationResult result) { + } } diff --git a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java index d442e7532cf..d735fc5a422 100644 --- a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java +++ b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java @@ -60,7 +60,9 @@ private MessageMatcherDelegatingAuthorizationManager( * @return an {@link AuthorizationDecision}. If there is no {@link MessageMatcher} * matching the message, or the {@link AuthorizationManager} could not decide, then * null is returned + * @deprecated please use {@link #authorize(Supplier, Object)} instead */ + @Deprecated @Override public AuthorizationDecision check(Supplier authentication, Message message) { if (this.logger.isTraceEnabled()) { diff --git a/rsocket/src/main/java/org/springframework/security/rsocket/authorization/PayloadExchangeMatcherReactiveAuthorizationManager.java b/rsocket/src/main/java/org/springframework/security/rsocket/authorization/PayloadExchangeMatcherReactiveAuthorizationManager.java index 4f13b9fbfaa..fc7862f51e2 100644 --- a/rsocket/src/main/java/org/springframework/security/rsocket/authorization/PayloadExchangeMatcherReactiveAuthorizationManager.java +++ b/rsocket/src/main/java/org/springframework/security/rsocket/authorization/PayloadExchangeMatcherReactiveAuthorizationManager.java @@ -50,6 +50,10 @@ private PayloadExchangeMatcherReactiveAuthorizationManager( this.mappings = mappings; } + /** + * @deprecated please use {@link #authorize(Mono, Object)} instead + */ + @Deprecated @Override public Mono check(Mono authentication, PayloadExchange exchange) { return Flux.fromIterable(this.mappings) diff --git a/web/src/main/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluator.java b/web/src/main/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluator.java index d60f0e19652..a181db2cc0d 100644 --- a/web/src/main/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluator.java +++ b/web/src/main/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluator.java @@ -57,8 +57,8 @@ public boolean isAllowed(String uri, Authentication authentication) { public boolean isAllowed(String contextPath, String uri, String method, Authentication authentication) { FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext); HttpServletRequest httpRequest = this.requestTransformer.transform(filterInvocation.getHttpRequest()); - AuthorizationResult decision = this.authorizationManager.authorize(() -> authentication, httpRequest); - return decision == null || decision.isGranted(); + AuthorizationResult result = this.authorizationManager.authorize(() -> authentication, httpRequest); + return result == null || result.isGranted(); } @Override diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java b/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java index 3e8f6281984..80659363f21 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java @@ -93,10 +93,10 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo String alreadyFilteredAttributeName = getAlreadyFilteredAttributeName(); request.setAttribute(alreadyFilteredAttributeName, Boolean.TRUE); try { - AuthorizationResult decision = this.authorizationManager.authorize(this::getAuthentication, request); - this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, request, decision); - if (decision != null && !decision.isGranted()) { - throw new AuthorizationDeniedException("Access Denied", decision); + AuthorizationResult result = this.authorizationManager.authorize(this::getAuthentication, request); + this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, request, result); + if (result != null && !result.isGranted()) { + throw new AuthorizationDeniedException("Access Denied", result); } chain.doFilter(request, response); } diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java b/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java index da5b5ec1ec6..1c26f9b0a4d 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java @@ -68,7 +68,9 @@ private RequestMatcherDelegatingAuthorizationManager( * @return an {@link AuthorizationDecision}. If there is no {@link RequestMatcher} * matching the request, or the {@link AuthorizationManager} could not decide, then * null is returned + * @deprecated please use {@link #authorize(Supplier, Object)} instead */ + @Deprecated @Override public AuthorizationDecision check(Supplier authentication, HttpServletRequest request) { if (this.logger.isTraceEnabled()) { diff --git a/web/src/main/java/org/springframework/security/web/server/authorization/DelegatingReactiveAuthorizationManager.java b/web/src/main/java/org/springframework/security/web/server/authorization/DelegatingReactiveAuthorizationManager.java index 7e61d9911e8..ebdb2b8ccba 100644 --- a/web/src/main/java/org/springframework/security/web/server/authorization/DelegatingReactiveAuthorizationManager.java +++ b/web/src/main/java/org/springframework/security/web/server/authorization/DelegatingReactiveAuthorizationManager.java @@ -48,6 +48,10 @@ private DelegatingReactiveAuthorizationManager( this.mappings = mappings; } + /** + * @deprecated please use {@link #authorize(Mono, Object)} instead + */ + @Deprecated @Override public Mono check(Mono authentication, ServerWebExchange exchange) { return Flux.fromIterable(this.mappings)