Browse Source

Support AuthorizationManager for intercept-methods Element

Closes gh-11328
Josh Cummings 3 years ago
parent
commit
74a007dc91

+ 190 - 2
config/src/main/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecorator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 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,22 @@
 
 package org.springframework.security.config.method;
 
+import java.lang.reflect.Method;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 
+import org.aopalliance.intercept.MethodInvocation;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
+import org.springframework.aop.ClassFilter;
+import org.springframework.aop.MethodMatcher;
+import org.springframework.aop.Pointcut;
 import org.springframework.aop.config.AbstractInterceptorDrivenBeanDefinitionDecorator;
+import org.springframework.aop.support.AopUtils;
+import org.springframework.aop.support.RootClassFilter;
+import org.springframework.beans.BeanMetadataElement;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.BeanDefinitionHolder;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
@@ -32,11 +41,24 @@ import org.springframework.beans.factory.support.ManagedMap;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.beans.factory.xml.BeanDefinitionDecorator;
 import org.springframework.beans.factory.xml.ParserContext;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.Expression;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.security.access.SecurityConfig;
+import org.springframework.security.access.expression.ExpressionUtils;
+import org.springframework.security.access.expression.SecurityExpressionHandler;
+import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
 import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
 import org.springframework.security.access.method.MapBasedMethodSecurityMetadataSource;
+import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationManager;
+import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor;
 import org.springframework.security.config.BeanIds;
 import org.springframework.security.config.Elements;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.access.expression.ExpressionAuthorizationDecision;
+import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 
@@ -68,9 +90,76 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
 
 		private static final String ATT_ACCESS_MGR = "access-decision-manager-ref";
 
+		private static final String ATT_USE_AUTHORIZATION_MGR = "use-authorization-manager";
+
+		private static final String ATT_AUTHORIZATION_MGR = "authorization-manager-ref";
+
+		private final ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
+
 		@Override
 		protected BeanDefinition createInterceptorDefinition(Node node) {
 			Element interceptMethodsElt = (Element) node;
+			if (Boolean.parseBoolean(interceptMethodsElt.getAttribute(ATT_USE_AUTHORIZATION_MGR))) {
+				return createAuthorizationManagerInterceptorDefinition(interceptMethodsElt);
+			}
+			if (StringUtils.hasText(interceptMethodsElt.getAttribute(ATT_AUTHORIZATION_MGR))) {
+				return createAuthorizationManagerInterceptorDefinition(interceptMethodsElt);
+			}
+			return createMethodSecurityInterceptorDefinition(interceptMethodsElt);
+		}
+
+		private BeanDefinition createAuthorizationManagerInterceptorDefinition(Element interceptMethodsElt) {
+			BeanDefinitionBuilder interceptor = BeanDefinitionBuilder
+					.rootBeanDefinition(AuthorizationManagerBeforeMethodInterceptor.class);
+			interceptor.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE);
+			Map<Pointcut, BeanMetadataElement> managers = new ManagedMap<>();
+			List<Element> methods = DomUtils.getChildElementsByTagName(interceptMethodsElt, Elements.PROTECT);
+			for (Element protectElt : methods) {
+				managers.put(pointcut(interceptMethodsElt, protectElt),
+						authorizationManager(interceptMethodsElt, protectElt));
+			}
+			return interceptor.addConstructorArgValue(Pointcut.TRUE)
+					.addConstructorArgValue(authorizationManager(managers)).getBeanDefinition();
+		}
+
+		private Pointcut pointcut(Element interceptorElt, Element protectElt) {
+			String method = protectElt.getAttribute(ATT_METHOD);
+			Class<?> javaType = javaType(interceptorElt, method);
+			return new PrefixBasedMethodMatcher(javaType, method);
+		}
+
+		private BeanMetadataElement authorizationManager(Element interceptMethodsElt, Element protectElt) {
+			String authorizationManager = interceptMethodsElt.getAttribute(ATT_AUTHORIZATION_MGR);
+			if (StringUtils.hasText(authorizationManager)) {
+				return new RuntimeBeanReference(authorizationManager);
+			}
+			String access = protectElt.getAttribute(ATT_ACCESS);
+			SpelExpressionParser parser = new SpelExpressionParser();
+			Expression expression = parser.parseExpression(access);
+			return BeanDefinitionBuilder.rootBeanDefinition(MethodExpressionAuthorizationManager.class)
+					.addConstructorArgValue(expression).getBeanDefinition();
+		}
+
+		private BeanMetadataElement authorizationManager(Map<Pointcut, BeanMetadataElement> managers) {
+			return BeanDefinitionBuilder.rootBeanDefinition(PointcutMatchingAuthorizationManager.class)
+					.addConstructorArgValue(managers).getBeanDefinition();
+		}
+
+		private Class<?> javaType(Element interceptMethodsElt, String method) {
+			int lastDotIndex = method.lastIndexOf(".");
+			String parentBeanClass = ((Element) interceptMethodsElt.getParentNode()).getAttribute("class");
+			Assert.isTrue(lastDotIndex != -1 || StringUtils.hasText(parentBeanClass),
+					() -> "'" + method + "' is not a valid method name: format is FQN.methodName");
+			if (lastDotIndex == -1) {
+				return ClassUtils.resolveClassName(parentBeanClass, this.beanClassLoader);
+			}
+			String methodName = method.substring(lastDotIndex + 1);
+			Assert.hasText(methodName, () -> "Method not found for '" + method + "'");
+			String typeName = method.substring(0, lastDotIndex);
+			return ClassUtils.resolveClassName(typeName, this.beanClassLoader);
+		}
+
+		private BeanDefinition createMethodSecurityInterceptorDefinition(Element interceptMethodsElt) {
 			BeanDefinitionBuilder interceptor = BeanDefinitionBuilder
 					.rootBeanDefinition(MethodSecurityInterceptor.class);
 			// Default to autowiring to pick up after invocation mgr
@@ -83,7 +172,7 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
 			interceptor.addPropertyValue("authenticationManager",
 					new RuntimeBeanReference(BeanIds.AUTHENTICATION_MANAGER));
 			// Lookup parent bean information
-			String parentBeanClass = ((Element) node.getParentNode()).getAttribute("class");
+			String parentBeanClass = ((Element) interceptMethodsElt.getParentNode()).getAttribute("class");
 			// Parse the included methods
 			List<Element> methods = DomUtils.getChildElementsByTagName(interceptMethodsElt, Elements.PROTECT);
 			Map<String, BeanDefinition> mappings = new ManagedMap<>();
@@ -108,4 +197,103 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
 
 	}
 
+	private static class PrefixBasedMethodMatcher implements MethodMatcher, Pointcut {
+
+		private final ClassFilter classFilter;
+
+		private final Class<?> javaType;
+
+		private final String methodPrefix;
+
+		PrefixBasedMethodMatcher(Class<?> javaType, String methodPrefix) {
+			this.classFilter = new RootClassFilter(javaType);
+			this.javaType = javaType;
+			this.methodPrefix = methodPrefix;
+		}
+
+		@Override
+		public ClassFilter getClassFilter() {
+			return this.classFilter;
+		}
+
+		@Override
+		public MethodMatcher getMethodMatcher() {
+			return this;
+		}
+
+		@Override
+		public boolean matches(Method method, Class<?> targetClass) {
+			return matches(this.methodPrefix, method.getName());
+		}
+
+		@Override
+		public boolean isRuntime() {
+			return false;
+		}
+
+		@Override
+		public boolean matches(Method method, Class<?> targetClass, Object... args) {
+			return matches(this.methodPrefix, method.getName());
+		}
+
+		private boolean matches(String mappedName, String methodName) {
+			boolean equals = methodName.equals(mappedName);
+			return equals || prefixMatches(mappedName, methodName) || suffixMatches(mappedName, methodName);
+		}
+
+		private boolean prefixMatches(String mappedName, String methodName) {
+			return mappedName.endsWith("*") && methodName.startsWith(mappedName.substring(0, mappedName.length() - 1));
+		}
+
+		private boolean suffixMatches(String mappedName, String methodName) {
+			return mappedName.startsWith("*") && methodName.endsWith(mappedName.substring(1));
+		}
+
+	}
+
+	private static class PointcutMatchingAuthorizationManager implements AuthorizationManager<MethodInvocation> {
+
+		private final Map<Pointcut, AuthorizationManager<MethodInvocation>> managers;
+
+		PointcutMatchingAuthorizationManager(Map<Pointcut, AuthorizationManager<MethodInvocation>> managers) {
+			this.managers = managers;
+		}
+
+		@Override
+		public AuthorizationDecision check(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 new AuthorizationDecision(false);
+		}
+
+	}
+
+	private static class MethodExpressionAuthorizationManager implements AuthorizationManager<MethodInvocation> {
+
+		private final Expression expression;
+
+		private SecurityExpressionHandler<MethodInvocation> expressionHandler = new DefaultMethodSecurityExpressionHandler();
+
+		MethodExpressionAuthorizationManager(Expression expression) {
+			this.expression = expression;
+		}
+
+		@Override
+		public AuthorizationDecision check(Supplier<Authentication> authentication, MethodInvocation invocation) {
+			EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication, invocation);
+			boolean granted = ExpressionUtils.evaluateAsBoolean(this.expression, ctx);
+			return new ExpressionAuthorizationDecision(granted, this.expression);
+		}
+
+		void setExpressionHandler(SecurityExpressionHandler<MethodInvocation> expressionHandler) {
+			this.expressionHandler = expressionHandler;
+		}
+
+	}
+
 }

+ 6 - 1
config/src/main/resources/org/springframework/security/config/spring-security-5.8.rnc

@@ -177,7 +177,12 @@ intercept-methods =
 intercept-methods.attlist &=
 	## Optional AccessDecisionManager bean ID to be used by the created method security interceptor.
 	attribute access-decision-manager-ref {xsd:token}?
-
+intercept-methods.attlist &=
+	## Use the AuthorizationManager API instead of AccessDecisionManager (defaults to false)
+	attribute use-authorization-manager {xsd:boolean}?
+intercept-methods.attlist &=
+	## Use this AuthorizationManager instead of the default (supercedes use-authorization-manager)
+	attribute authorization-manager-ref {xsd:token}?
 
 protect =
 	## Defines a protected method and the access control configuration attributes that apply to it. We strongly advise you NOT to mix "protect" declarations with any services provided "global-method-security".

+ 13 - 0
config/src/main/resources/org/springframework/security/config/spring-security-5.8.xsd

@@ -540,6 +540,19 @@
                 </xs:documentation>
          </xs:annotation>
       </xs:attribute>
+      <xs:attribute name="use-authorization-manager" type="xs:boolean">
+         <xs:annotation>
+            <xs:documentation>Use the AuthorizationManager API instead of AccessDecisionManager (defaults to false)
+                </xs:documentation>
+         </xs:annotation>
+      </xs:attribute>
+      <xs:attribute name="authorization-manager-ref" type="xs:token">
+         <xs:annotation>
+            <xs:documentation>Use this AuthorizationManager instead of the default (supercedes
+                use-authorization-manager)
+                </xs:documentation>
+         </xs:annotation>
+      </xs:attribute>
   </xs:attributeGroup>
   
   <xs:attributeGroup name="protect.attlist">

+ 65 - 2
config/src/test/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecoratorTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.security.config.method;
 
+import org.aopalliance.intercept.MethodInvocation;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
@@ -32,6 +33,8 @@ import org.springframework.security.authentication.AuthenticationCredentialsNotF
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
+import org.springframework.security.authorization.AuthorizationDecision;
+import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.config.TestBusinessBean;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.authority.AuthorityUtils;
@@ -41,6 +44,9 @@ import org.springframework.test.context.junit.jupiter.SpringExtension;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.verify;
 
 /**
  * @author Luke Taylor
@@ -57,6 +63,21 @@ public class InterceptMethodsBeanDefinitionDecoratorTests implements Application
 	@Qualifier("transactionalTarget")
 	private TestBusinessBean transactionalTarget;
 
+	@Autowired
+	@Qualifier("targetAuthorizationManager")
+	private TestBusinessBean targetAuthorizationManager;
+
+	@Autowired
+	@Qualifier("transactionalTargetAuthorizationManager")
+	private TestBusinessBean transactionalTargetAuthorizationManager;
+
+	@Autowired
+	@Qualifier("targetCustomAuthorizationManager")
+	private TestBusinessBean targetCustomAuthorizationManager;
+
+	@Autowired
+	private AuthorizationManager<MethodInvocation> mockAuthorizationManager;
+
 	private ApplicationContext appContext;
 
 	@BeforeAll
@@ -72,10 +93,12 @@ public class InterceptMethodsBeanDefinitionDecoratorTests implements Application
 
 	@Test
 	public void targetDoesntLoseApplicationListenerInterface() {
-		assertThat(this.appContext.getBeansOfType(ApplicationListener.class)).hasSize(1);
-		assertThat(this.appContext.getBeanNamesForType(ApplicationListener.class)).hasSize(1);
+		assertThat(this.appContext.getBeansOfType(ApplicationListener.class)).isNotEmpty();
+		assertThat(this.appContext.getBeanNamesForType(ApplicationListener.class)).isNotEmpty();
 		this.appContext.publishEvent(new AuthenticationSuccessEvent(new TestingAuthenticationToken("user", "")));
 		assertThat(this.target).isInstanceOf(ApplicationListener.class);
+		assertThat(this.targetAuthorizationManager).isInstanceOf(ApplicationListener.class);
+		assertThat(this.targetCustomAuthorizationManager).isInstanceOf(ApplicationListener.class);
 	}
 
 	@Test
@@ -110,6 +133,46 @@ public class InterceptMethodsBeanDefinitionDecoratorTests implements Application
 		assertThatExceptionOfType(AuthenticationException.class).isThrownBy(this.transactionalTarget::doSomething);
 	}
 
+	@Test
+	public void targetAuthorizationManagerShouldAllowUnprotectedMethodInvocationWithNoContext() {
+		this.targetAuthorizationManager.unprotected();
+	}
+
+	@Test
+	public void targetAuthorizationManagerShouldPreventProtectedMethodInvocationWithNoContext() {
+		assertThatExceptionOfType(AuthenticationCredentialsNotFoundException.class)
+				.isThrownBy(this.targetAuthorizationManager::doSomething);
+	}
+
+	@Test
+	public void targetAuthorizationManagerShouldAllowProtectedMethodInvocationWithCorrectRole() {
+		UsernamePasswordAuthenticationToken token = UsernamePasswordAuthenticationToken.authenticated("Test",
+				"Password", AuthorityUtils.createAuthorityList("ROLE_USER"));
+		SecurityContextHolder.getContext().setAuthentication(token);
+		this.targetAuthorizationManager.doSomething();
+	}
+
+	@Test
+	public void targetAuthorizationManagerShouldPreventProtectedMethodInvocationWithIncorrectRole() {
+		UsernamePasswordAuthenticationToken token = UsernamePasswordAuthenticationToken.authenticated("Test",
+				"Password", AuthorityUtils.createAuthorityList("ROLE_SOMEOTHERROLE"));
+		SecurityContextHolder.getContext().setAuthentication(token);
+		assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.targetAuthorizationManager::doSomething);
+	}
+
+	@Test
+	public void transactionalAuthorizationManagerMethodsShouldBeSecured() {
+		assertThatExceptionOfType(AuthenticationException.class)
+				.isThrownBy(this.transactionalTargetAuthorizationManager::doSomething);
+	}
+
+	@Test
+	public void targetCustomAuthorizationManagerUsed() {
+		given(this.mockAuthorizationManager.check(any(), any())).willReturn(new AuthorizationDecision(true));
+		this.targetCustomAuthorizationManager.doSomething();
+		verify(this.mockAuthorizationManager).check(any(), any());
+	}
+
 	@Override
 	public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
 		this.appContext = applicationContext;

+ 27 - 0
config/src/test/resources/org/springframework/security/config/method-security.xml

@@ -30,6 +30,30 @@
 		</intercept-methods>
 	</b:bean>
 
+	<b:bean id="transactionalTargetAuthorizationManager" class="org.springframework.security.config.TransactionalTestBusinessBean">
+		<intercept-methods use-authorization-manager="true">
+			<protect method="*" access="hasRole('ROLE_USER')" />
+		</intercept-methods>
+	</b:bean>
+
+
+	<b:bean id="targetAuthorizationManager" class="org.springframework.security.config.TestBusinessBeanImpl">
+		<!-- This will add a security interceptor to the bean -->
+		<intercept-methods use-authorization-manager="true">
+			<protect method="org.springframework.security.config.TestBusinessBean.set*" access="hasRole('${admin.role}')" />
+			<protect method="get*" access="hasAnyRole('${admin.role}','ROLE_USER')" />
+			<protect method="doSomething" access="hasRole('ROLE_USER')" />
+			<protect method="*" access="permitAll"/>
+		</intercept-methods>
+	</b:bean>
+
+	<b:bean id="targetCustomAuthorizationManager" class="org.springframework.security.config.TestBusinessBeanImpl">
+		<!-- This will add a security interceptor to the bean -->
+		<intercept-methods authorization-manager-ref="authorization-manager">
+			<protect method="*" access="denyAll"/>
+		</intercept-methods>
+	</b:bean>
+
 	<authentication-manager>
 		<authentication-provider>
 			<user-service>
@@ -39,4 +63,7 @@
 		</authentication-provider>
 	</authentication-manager>
 
+	<b:bean id="authorization-manager" class="org.mockito.Mockito" factory-method="mock">
+		<b:constructor-arg value="org.springframework.security.authorization.AuthorizationManager"/>
+	</b:bean>
 </b:beans>

+ 7 - 0
docs/modules/ROOT/pages/servlet/appendix/namespace/method-security.adoc

@@ -271,6 +271,13 @@ Can be used inside a bean definition to add a security interceptor to the bean a
 [[nsa-intercept-methods-attributes]]
 === <intercept-methods> Attributes
 
+[[nsa-intercept-methods-use-authorization-manager]]
+* **use-authorization-manager**
+Use AuthorizationManager API instead of AccessDecisionManager
+
+[[nsa-intercept-methods-authorization-manager-ref]]
+* **authorization-manager-ref**
+Optional AuthorizationManager bean ID to be used instead of the default (supercedes use-authorization-manager)
 
 [[nsa-intercept-methods-access-decision-manager-ref]]
 * **access-decision-manager-ref**