Polish AuthorizationManager#authorize

Issue gh-14843
This commit is contained in:
Josh Cummings 2024-10-14 11:20:33 -06:00
parent e7644925f8
commit 9ce5a76e8c
20 changed files with 160 additions and 73 deletions

View File

@ -221,9 +221,8 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
AuthorizationManager<HttpServletRequest> 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 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
return () -> {
AuthorizationManager<HttpServletRequest> 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;

View File

@ -25,6 +25,7 @@ import org.springframework.aop.Pointcut;
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<MethodInvocation> {
@ -37,11 +38,24 @@ class PointcutDelegatingAuthorizationManager implements AuthorizationManager<Met
@Override
public AuthorizationDecision check(Supplier<Authentication> 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> authentication, MethodInvocation object) {
for (Map.Entry<Pointcut, AuthorizationManager<MethodInvocation>> 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);

View File

@ -168,6 +168,7 @@ public class InterceptMethodsBeanDefinitionDecoratorTests implements Application
@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());

View File

@ -139,7 +139,9 @@ public final class AuthorityAuthorizationManager<T> implements AuthorizationMana
* @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> authentication, T object) {
return this.delegate.check(authentication, this.authorities);

View File

@ -18,6 +18,9 @@ package org.springframework.security.authorization;
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 final class AuthorizationManagers {
@SafeVarargs
public static <T> AuthorizationManager<T> anyOf(AuthorizationDecision allAbstainDefaultDecision,
AuthorizationManager<T>... managers) {
return (authentication, object) -> {
List<AuthorizationDecision> decisions = new ArrayList<>();
return (AuthorizationManagerCheckAdapter<T>) (authentication, object) -> {
List<AuthorizationResult> results = new ArrayList<>();
for (AuthorizationManager<T> 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 final class AuthorizationManagers {
@SafeVarargs
public static <T> AuthorizationManager<T> allOf(AuthorizationDecision allAbstainDefaultDecision,
AuthorizationManager<T>... managers) {
return (authentication, object) -> {
List<AuthorizationDecision> decisions = new ArrayList<>();
return (AuthorizationManagerCheckAdapter<T>) (authentication, object) -> {
List<AuthorizationResult> results = new ArrayList<>();
for (AuthorizationManager<T> 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 final class AuthorizationManagers {
*/
public static <T> AuthorizationManager<T> not(AuthorizationManager<T> 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 @@ public final class AuthorizationManagers {
private static final class CompositeAuthorizationDecision extends AuthorizationDecision {
private final List<AuthorizationDecision> decisions;
private final List<AuthorizationResult> results;
private CompositeAuthorizationDecision(boolean granted, List<AuthorizationDecision> decisions) {
private CompositeAuthorizationDecision(boolean granted, List<AuthorizationResult> 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<T> extends AuthorizationManager<T> {
@Override
default AuthorizationDecision check(Supplier<Authentication> 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> authentication, T object);
}
}

View File

@ -33,8 +33,6 @@ public class AuthorizationObservationContext<T> extends Observation.Context {
private final T object;
private AuthorizationDecision decision;
private AuthorizationResult authorizationResult;
public AuthorizationObservationContext(T object) {
@ -77,9 +75,14 @@ public class AuthorizationObservationContext<T> extends Observation.Context {
*/
@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 class AuthorizationObservationContext<T> extends Observation.Context {
*/
@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;
}
/**

View File

@ -61,6 +61,10 @@ public final class ObservationAuthorizationManager<T>
}
}
/**
* @deprecated please use {@link #authorize(Supplier, Object)} instead
*/
@Deprecated
@Override
public AuthorizationDecision check(Supplier<Authentication> authentication, T object) {
AuthorizationObservationContext<T> context = new AuthorizationObservationContext<>(object);

View File

@ -56,6 +56,10 @@ public final class ObservationReactiveAuthorizationManager<T>
}
}
/**
* @deprecated please use {@link #authorize(Mono, Object)} instead
*/
@Deprecated
@Override
public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
AuthorizationObservationContext<T> context = new AuthorizationObservationContext<>(object);

View File

@ -182,22 +182,22 @@ public final class AuthorizationManagerAfterMethodInterceptor implements Authori
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() {

View File

@ -246,18 +246,18 @@ public final class AuthorizationManagerBeforeMethodInterceptor implements Author
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 @@ public final class AuthorizationManagerBeforeMethodInterceptor implements Author
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() {

View File

@ -33,6 +33,7 @@ import org.springframework.lang.NonNull;
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 final class Jsr250AuthorizationManager implements AuthorizationManager<Me
* @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> authentication, MethodInvocation methodInvocation) {
AuthorizationManager<MethodInvocation> delegate = this.registry.getManager(methodInvocation);
@ -109,8 +112,9 @@ public final class Jsr250AuthorizationManager implements AuthorizationManager<Me
return (a, o) -> new AuthorizationDecision(true);
}
if (annotation instanceof RolesAllowed rolesAllowed) {
return (a, o) -> Jsr250AuthorizationManager.this.authoritiesAuthorizationManager.check(a,
getAllowedRolesWithPrefix(rolesAllowed));
return (AuthorizationManagerCheckAdapter<MethodInvocation>) (a,
o) -> Jsr250AuthorizationManager.this.authoritiesAuthorizationManager.authorize(a,
getAllowedRolesWithPrefix(rolesAllowed));
}
return NULL_MANAGER;
}
@ -130,4 +134,23 @@ public final class Jsr250AuthorizationManager implements AuthorizationManager<Me
}
private interface AuthorizationManagerCheckAdapter<T> extends AuthorizationManager<T> {
@Override
default AuthorizationDecision check(Supplier<Authentication> 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> authentication, T object);
}
}

View File

@ -73,7 +73,9 @@ public final class SecuredAuthorizationManager implements AuthorizationManager<M
* @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> authentication, MethodInvocation mi) {
Set<String> authorities = getAuthorities(mi);

View File

@ -38,6 +38,8 @@ import org.springframework.security.core.Authentication;
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 class Jsr250AuthorizationManagerTests {
@Test
public void setAuthoritiesAuthorizationManagerWhenNotNullThenVerifyUsage() throws Exception {
AuthorizationManager<Collection<String>> authoritiesAuthorizationManager = mock(AuthorizationManager.class);
given(authoritiesAuthorizationManager.authorize(any(), any())).willCallRealMethod();
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
manager.setAuthoritiesAuthorizationManager(authoritiesAuthorizationManager);
MockMethodInvocation methodInvocation = new MockMethodInvocation(new ClassLevelAnnotations(),

View File

@ -67,11 +67,11 @@ public final class AuthorizationChannelInterceptor implements ChannelInterceptor
@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 final class AuthorizationChannelInterceptor implements ChannelInterceptor
@Override
public <T> void publishAuthorizationEvent(Supplier<Authentication> authentication, T object,
AuthorizationResult result) {
}
}

View File

@ -60,7 +60,9 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho
* @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> authentication, Message<?> message) {
if (this.logger.isTraceEnabled()) {

View File

@ -50,6 +50,10 @@ public final class PayloadExchangeMatcherReactiveAuthorizationManager
this.mappings = mappings;
}
/**
* @deprecated please use {@link #authorize(Mono, Object)} instead
*/
@Deprecated
@Override
public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, PayloadExchange exchange) {
return Flux.fromIterable(this.mappings)

View File

@ -57,8 +57,8 @@ public final class AuthorizationManagerWebInvocationPrivilegeEvaluator
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

View File

@ -93,10 +93,10 @@ public class AuthorizationFilter extends GenericFilterBean {
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);
}

View File

@ -68,7 +68,9 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho
* @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> authentication, HttpServletRequest request) {
if (this.logger.isTraceEnabled()) {

View File

@ -48,6 +48,10 @@ public final class DelegatingReactiveAuthorizationManager implements ReactiveAut
this.mappings = mappings;
}
/**
* @deprecated please use {@link #authorize(Mono, Object)} instead
*/
@Deprecated
@Override
public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, ServerWebExchange exchange) {
return Flux.fromIterable(this.mappings)