diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationDecision.java b/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationDecision.java index 68f4303355..f9dd43a784 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationDecision.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationDecision.java @@ -18,22 +18,24 @@ package org.springframework.security.authorization; import java.util.Collection; +import org.springframework.security.core.GrantedAuthority; + /** * Represents an {@link AuthorizationDecision} based on a collection of authorities * * @author Marcus Da Coregio * @since 5.6 */ -class AuthorityAuthorizationDecision extends AuthorizationDecision { +public class AuthorityAuthorizationDecision extends AuthorizationDecision { - private final Collection authorities; + private final Collection authorities; - AuthorityAuthorizationDecision(boolean granted, Collection authorities) { + public AuthorityAuthorizationDecision(boolean granted, Collection authorities) { super(granted); this.authorities = authorities; } - Collection getAuthorities() { + public Collection getAuthorities() { return this.authorities; } 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 03d3d65e3f..8cfc0dcf0a 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java @@ -16,13 +16,13 @@ package org.springframework.security.authorization; -import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.util.function.Supplier; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.util.Assert; /** @@ -37,10 +37,10 @@ public final class AuthorityAuthorizationManager implements AuthorizationMana private static final String ROLE_PREFIX = "ROLE_"; - private final Set authorities; + private final Set authorities; private AuthorityAuthorizationManager(String... authorities) { - this.authorities = new HashSet<>(Arrays.asList(authorities)); + this.authorities = new HashSet<>(AuthorityUtils.createAuthorityList(authorities)); } /** @@ -133,8 +133,7 @@ public final class AuthorityAuthorizationManager implements AuthorizationMana private boolean isAuthorized(Authentication authentication) { for (GrantedAuthority grantedAuthority : authentication.getAuthorities()) { - String authority = grantedAuthority.getAuthority(); - if (this.authorities.contains(authority)) { + if (this.authorities.contains(grantedAuthority)) { return true; } } diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java index 27171923c3..a3de6cba76 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,13 +16,13 @@ package org.springframework.security.authorization; -import java.util.Arrays; import java.util.List; import reactor.core.publisher.Mono; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.util.Assert; /** @@ -35,10 +35,10 @@ import org.springframework.util.Assert; */ public class AuthorityReactiveAuthorizationManager implements ReactiveAuthorizationManager { - private final List authorities; + private final List authorities; AuthorityReactiveAuthorizationManager(String... authorities) { - this.authorities = Arrays.asList(authorities); + this.authorities = AuthorityUtils.createAuthorityList(authorities); } @Override @@ -46,7 +46,6 @@ public class AuthorityReactiveAuthorizationManager implements ReactiveAuthori // @formatter:off return authentication.filter((a) -> a.isAuthenticated()) .flatMapIterable(Authentication::getAuthorities) - .map(GrantedAuthority::getAuthority) .any(this.authorities::contains) .map((granted) -> ((AuthorizationDecision) new AuthorityAuthorizationDecision(granted, this.authorities))) .defaultIfEmpty(new AuthorityAuthorizationDecision(false, this.authorities)); 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 cbd43c6270..e93777aaa0 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 @@ -21,14 +21,18 @@ import java.util.function.Supplier; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.aop.Pointcut; import org.springframework.aop.PointcutAdvisor; import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.core.Ordered; +import org.springframework.core.log.LogMessage; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PostAuthorize; import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; +import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; @@ -54,6 +58,8 @@ public final class AuthorizationManagerAfterMethodInterceptor return authentication; }; + private final Log logger = LogFactory.getLog(this.getClass()); + private final Pointcut pointcut; private final AuthorizationManager authorizationManager; @@ -103,7 +109,7 @@ public final class AuthorizationManagerAfterMethodInterceptor @Override public Object invoke(MethodInvocation mi) throws Throwable { Object result = mi.proceed(); - this.authorizationManager.verify(AUTHENTICATION_SUPPLIER, new MethodInvocationResult(mi, result)); + attemptAuthorization(mi, result); return result; } @@ -134,4 +140,16 @@ public final class AuthorizationManagerAfterMethodInterceptor return true; } + private void attemptAuthorization(MethodInvocation mi, Object result) { + this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi)); + AuthorizationDecision decision = this.authorizationManager.check(AUTHENTICATION_SUPPLIER, + new MethodInvocationResult(mi, result)); + if (decision != null && !decision.isGranted()) { + this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager " + + this.authorizationManager + " and decision " + decision)); + throw new AccessDeniedException("Access Denied"); + } + this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi)); + } + } 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 0bd1ea05ee..38b6f03b1b 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 @@ -25,15 +25,19 @@ import javax.annotation.security.RolesAllowed; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.aop.Pointcut; import org.springframework.aop.PointcutAdvisor; import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.core.Ordered; +import org.springframework.core.log.LogMessage; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.annotation.Secured; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; +import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; @@ -59,6 +63,8 @@ public final class AuthorizationManagerBeforeMethodInterceptor return authentication; }; + private final Log logger = LogFactory.getLog(this.getClass()); + private final Pointcut pointcut; private final AuthorizationManager authorizationManager; @@ -149,7 +155,7 @@ public final class AuthorizationManagerBeforeMethodInterceptor */ @Override public Object invoke(MethodInvocation mi) throws Throwable { - this.authorizationManager.verify(AUTHENTICATION_SUPPLIER, mi); + attemptAuthorization(mi); return mi.proceed(); } @@ -180,4 +186,15 @@ public final class AuthorizationManagerBeforeMethodInterceptor return true; } + private void attemptAuthorization(MethodInvocation mi) { + this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi)); + AuthorizationDecision decision = this.authorizationManager.check(AUTHENTICATION_SUPPLIER, mi); + if (decision != null && !decision.isGranted()) { + this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager " + + this.authorizationManager + " and decision " + decision)); + throw new AccessDeniedException("Access Denied"); + } + this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi)); + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/ExpressionAttributeAuthorizationDecision.java b/core/src/main/java/org/springframework/security/authorization/method/ExpressionAttributeAuthorizationDecision.java index c958777fdb..f43d20fc67 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/ExpressionAttributeAuthorizationDecision.java +++ b/core/src/main/java/org/springframework/security/authorization/method/ExpressionAttributeAuthorizationDecision.java @@ -24,16 +24,16 @@ import org.springframework.security.authorization.AuthorizationDecision; * @author Marcus Da Coregio * @since 5.6 */ -class ExpressionAttributeAuthorizationDecision extends AuthorizationDecision { +public class ExpressionAttributeAuthorizationDecision extends AuthorizationDecision { private final ExpressionAttribute expressionAttribute; - ExpressionAttributeAuthorizationDecision(boolean granted, ExpressionAttribute expressionAttribute) { + public ExpressionAttributeAuthorizationDecision(boolean granted, ExpressionAttribute expressionAttribute) { super(granted); this.expressionAttribute = expressionAttribute; } - ExpressionAttribute getExpressionAttribute() { + public ExpressionAttribute getExpressionAttribute() { return this.expressionAttribute; } diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java index ddd622bfcf..2a129cb56a 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java @@ -53,7 +53,7 @@ public class AuthorizationManagerAfterMethodInterceptorTests { } @Test - public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() throws Throwable { + public void beforeWhenMockAuthorizationManagerThenCheckAndReturnedObject() throws Throwable { MethodInvocation mockMethodInvocation = mock(MethodInvocation.class); MethodInvocationResult result = new MethodInvocationResult(mockMethodInvocation, new Object()); given(mockMethodInvocation.proceed()).willReturn(result.getResult()); @@ -62,7 +62,7 @@ public class AuthorizationManagerAfterMethodInterceptorTests { Pointcut.TRUE, mockAuthorizationManager); Object returnedObject = advice.invoke(mockMethodInvocation); assertThat(returnedObject).isEqualTo(result.getResult()); - verify(mockAuthorizationManager).verify(eq(AuthorizationManagerAfterMethodInterceptor.AUTHENTICATION_SUPPLIER), + verify(mockAuthorizationManager).check(eq(AuthorizationManagerAfterMethodInterceptor.AUTHENTICATION_SUPPLIER), any(MethodInvocationResult.class)); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java index 38a5316771..9065e11040 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java @@ -49,13 +49,13 @@ public class AuthorizationManagerBeforeMethodInterceptorTests { } @Test - public void beforeWhenMockAuthorizationManagerThenVerify() throws Throwable { + public void beforeWhenMockAuthorizationManagerThenCheck() throws Throwable { MethodInvocation mockMethodInvocation = mock(MethodInvocation.class); AuthorizationManager mockAuthorizationManager = mock(AuthorizationManager.class); AuthorizationManagerBeforeMethodInterceptor advice = new AuthorizationManagerBeforeMethodInterceptor( Pointcut.TRUE, mockAuthorizationManager); advice.invoke(mockMethodInvocation); - verify(mockAuthorizationManager).verify(AuthorizationManagerBeforeMethodInterceptor.AUTHENTICATION_SUPPLIER, + verify(mockAuthorizationManager).check(AuthorizationManagerBeforeMethodInterceptor.AUTHENTICATION_SUPPLIER, mockMethodInvocation); }