From 74a007dc917243a7daad246d70f9be909830b000 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 6 Jul 2022 12:52:32 -0600 Subject: [PATCH] Support AuthorizationManager for intercept-methods Element Closes gh-11328 --- ...terceptMethodsBeanDefinitionDecorator.java | 192 +++++++++++++++++- .../security/config/spring-security-5.8.rnc | 7 +- .../security/config/spring-security-5.8.xsd | 13 ++ ...ptMethodsBeanDefinitionDecoratorTests.java | 67 +++++- .../security/config/method-security.xml | 27 +++ .../appendix/namespace/method-security.adoc | 7 + 6 files changed, 308 insertions(+), 5 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecorator.java b/config/src/main/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecorator.java index ca4a87b477..45b60e20b0 100644 --- a/config/src/main/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecorator.java +++ b/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 managers = new ManagedMap<>(); + List 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 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 methods = DomUtils.getChildElementsByTagName(interceptMethodsElt, Elements.PROTECT); Map 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 { + + private final Map> managers; + + PointcutMatchingAuthorizationManager(Map> managers) { + this.managers = managers; + } + + @Override + public AuthorizationDecision check(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 new AuthorizationDecision(false); + } + + } + + private static class MethodExpressionAuthorizationManager implements AuthorizationManager { + + private final Expression expression; + + private SecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); + + MethodExpressionAuthorizationManager(Expression expression) { + this.expression = expression; + } + + @Override + public AuthorizationDecision check(Supplier 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 expressionHandler) { + this.expressionHandler = expressionHandler; + } + + } + } diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.8.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-5.8.rnc index 7987dc62ed..af8b10425a 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.8.rnc +++ b/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". diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.8.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-5.8.xsd index 510ae32fa8..29e56d18b0 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.8.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.8.xsd @@ -540,6 +540,19 @@ + + + Use the AuthorizationManager API instead of AccessDecisionManager (defaults to false) + + + + + + Use this AuthorizationManager instead of the default (supercedes + use-authorization-manager) + + + 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 e7dd933100..b6be974650 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 @@ -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 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; diff --git a/config/src/test/resources/org/springframework/security/config/method-security.xml b/config/src/test/resources/org/springframework/security/config/method-security.xml index 5a62445638..deb40d17b8 100644 --- a/config/src/test/resources/org/springframework/security/config/method-security.xml +++ b/config/src/test/resources/org/springframework/security/config/method-security.xml @@ -30,6 +30,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + @@ -39,4 +63,7 @@ + + + diff --git a/docs/modules/ROOT/pages/servlet/appendix/namespace/method-security.adoc b/docs/modules/ROOT/pages/servlet/appendix/namespace/method-security.adoc index aba3e89846..995007a40d 100644 --- a/docs/modules/ROOT/pages/servlet/appendix/namespace/method-security.adoc +++ b/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]] === 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**