Polish InterceptMethodsBeanDefinitionDecorator

Issue gh-11328

config/src/main/java/org/springframework/security/config/method/InterceptMethodsBeanDefinitionDecorator.java

@@ -69,27 +69,26 @@ import org.springframework.util.xml.DomUtils;
  */
 public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDecorator {
 
+	private final InternalAuthorizationManagerInterceptMethodsBeanDefinitionDecorator authorizationManagerDelegate = new InternalAuthorizationManagerInterceptMethodsBeanDefinitionDecorator();
+
 	private final BeanDefinitionDecorator delegate = new InternalInterceptMethodsBeanDefinitionDecorator();
 
 	@Override
 	public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder definition, ParserContext parserContext) {
+		if (this.authorizationManagerDelegate.supports(node)) {
+			return this.authorizationManagerDelegate.decorate(node, definition, parserContext);
+		}
 		MethodConfigUtils.registerDefaultMethodAccessManagerIfNecessary(parserContext);
 		return this.delegate.decorate(node, definition, parserContext);
 	}
 
-	/**
-	 * This is the real class which does the work. We need access to the ParserContext in
-	 * order to do bean registration.
-	 */
-	static class InternalInterceptMethodsBeanDefinitionDecorator
+	static class InternalAuthorizationManagerInterceptMethodsBeanDefinitionDecorator
 			extends AbstractInterceptorDrivenBeanDefinitionDecorator {
 
 		static final String ATT_METHOD = "method";
 
 		static final String ATT_ACCESS = "access";
 
-		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";
@@ -99,16 +98,6 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
 		@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);
@@ -122,6 +111,14 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
 					.addConstructorArgValue(authorizationManager(managers)).getBeanDefinition();
 		}
 
+		boolean supports(Node node) {
+			Element interceptMethodsElt = (Element) node;
+			if (Boolean.parseBoolean(interceptMethodsElt.getAttribute(ATT_USE_AUTHORIZATION_MGR))) {
+				return true;
+			}
+			return StringUtils.hasText(interceptMethodsElt.getAttribute(ATT_AUTHORIZATION_MGR));
+		}
+
 		private Pointcut pointcut(Element interceptorElt, Element protectElt) {
 			String method = protectElt.getAttribute(ATT_METHOD);
 			Class<?> javaType = javaType(interceptorElt, method);
@@ -159,139 +156,155 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
 			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
-			interceptor.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE);
-			String accessManagerId = interceptMethodsElt.getAttribute(ATT_ACCESS_MGR);
-			if (!StringUtils.hasText(accessManagerId)) {
-				accessManagerId = BeanIds.METHOD_ACCESS_MANAGER;
+		private static class PrefixBasedMethodMatcher implements MethodMatcher, Pointcut {
+
+			private final ClassFilter classFilter;
+
+			private final String methodPrefix;
+
+			PrefixBasedMethodMatcher(Class<?> javaType, String methodPrefix) {
+				this.classFilter = new RootClassFilter(javaType);
+				this.methodPrefix = methodPrefix;
 			}
-			interceptor.addPropertyValue("accessDecisionManager", new RuntimeBeanReference(accessManagerId));
-			interceptor.addPropertyValue("authenticationManager",
-					new RuntimeBeanReference(BeanIds.AUTHENTICATION_MANAGER));
-			// Lookup parent bean information
-			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<>();
-			for (Element protectmethodElt : methods) {
-				BeanDefinitionBuilder attributeBuilder = BeanDefinitionBuilder.rootBeanDefinition(SecurityConfig.class);
-				attributeBuilder.setFactoryMethod("createListFromCommaDelimitedString");
-				attributeBuilder.addConstructorArgValue(protectmethodElt.getAttribute(ATT_ACCESS));
-				// Support inference of class names
-				String methodName = protectmethodElt.getAttribute(ATT_METHOD);
-				if (methodName.lastIndexOf(".") == -1) {
-					if (parentBeanClass != null && !"".equals(parentBeanClass)) {
-						methodName = parentBeanClass + "." + methodName;
-					}
-				}
-				mappings.put(methodName, attributeBuilder.getBeanDefinition());
+
+			@Override
+			public ClassFilter getClassFilter() {
+				return this.classFilter;
 			}
-			BeanDefinition metadataSource = new RootBeanDefinition(MapBasedMethodSecurityMetadataSource.class);
-			metadataSource.getConstructorArgumentValues().addGenericArgumentValue(mappings);
-			interceptor.addPropertyValue("securityMetadataSource", metadataSource);
-			return interceptor.getBeanDefinition();
-		}
 
-	}
+			@Override
+			public MethodMatcher getMethodMatcher() {
+				return this;
+			}
 
-	private static class PrefixBasedMethodMatcher implements MethodMatcher, Pointcut {
+			@Override
+			public boolean matches(Method method, Class<?> targetClass) {
+				return matches(this.methodPrefix, method.getName());
+			}
 
-		private final ClassFilter classFilter;
+			@Override
+			public boolean isRuntime() {
+				return false;
+			}
 
-		private final Class<?> javaType;
+			@Override
+			public boolean matches(Method method, Class<?> targetClass, Object... args) {
+				return matches(this.methodPrefix, method.getName());
+			}
 
-		private final String methodPrefix;
+			private boolean matches(String mappedName, String methodName) {
+				boolean equals = methodName.equals(mappedName);
+				return equals || prefixMatches(mappedName, methodName) || suffixMatches(mappedName, methodName);
+			}
 
-		PrefixBasedMethodMatcher(Class<?> javaType, String methodPrefix) {
-			this.classFilter = new RootClassFilter(javaType);
-			this.javaType = javaType;
-			this.methodPrefix = methodPrefix;
-		}
+			private boolean prefixMatches(String mappedName, String methodName) {
+				return mappedName.endsWith("*")
+						&& methodName.startsWith(mappedName.substring(0, mappedName.length() - 1));
+			}
 
-		@Override
-		public ClassFilter getClassFilter() {
-			return this.classFilter;
-		}
+			private boolean suffixMatches(String mappedName, String methodName) {
+				return mappedName.startsWith("*") && methodName.endsWith(mappedName.substring(1));
+			}
 
-		@Override
-		public MethodMatcher getMethodMatcher() {
-			return this;
 		}
 
-		@Override
-		public boolean matches(Method method, Class<?> targetClass) {
-			return matches(this.methodPrefix, method.getName());
-		}
+		private static class PointcutMatchingAuthorizationManager implements AuthorizationManager<MethodInvocation> {
 
-		@Override
-		public boolean isRuntime() {
-			return false;
-		}
+			private final Map<Pointcut, AuthorizationManager<MethodInvocation>> managers;
 
-		@Override
-		public boolean matches(Method method, Class<?> targetClass, Object... args) {
-			return matches(this.methodPrefix, method.getName());
-		}
+			PointcutMatchingAuthorizationManager(Map<Pointcut, AuthorizationManager<MethodInvocation>> managers) {
+				this.managers = managers;
+			}
 
-		private boolean matches(String mappedName, String methodName) {
-			boolean equals = methodName.equals(mappedName);
-			return equals || prefixMatches(mappedName, methodName) || suffixMatches(mappedName, methodName);
-		}
+			@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 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 MethodExpressionAuthorizationManager implements AuthorizationManager<MethodInvocation> {
 
-	}
+			private final Expression expression;
 
-	private static class PointcutMatchingAuthorizationManager implements AuthorizationManager<MethodInvocation> {
+			private SecurityExpressionHandler<MethodInvocation> expressionHandler = new DefaultMethodSecurityExpressionHandler();
 
-		private final Map<Pointcut, AuthorizationManager<MethodInvocation>> managers;
+			MethodExpressionAuthorizationManager(Expression expression) {
+				this.expression = expression;
+			}
 
-		PointcutMatchingAuthorizationManager(Map<Pointcut, AuthorizationManager<MethodInvocation>> managers) {
-			this.managers = managers;
-		}
+			@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);
+			}
 
-		@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);
-				}
+			void setExpressionHandler(SecurityExpressionHandler<MethodInvocation> expressionHandler) {
+				this.expressionHandler = expressionHandler;
 			}
-			return new AuthorizationDecision(false);
+
 		}
 
 	}
 
-	private static class MethodExpressionAuthorizationManager implements AuthorizationManager<MethodInvocation> {
+	/**
+	 * This is the real class which does the work. We need access to the ParserContext in
+	 * order to do bean registration.
+	 */
+	static class InternalInterceptMethodsBeanDefinitionDecorator
+			extends AbstractInterceptorDrivenBeanDefinitionDecorator {
 
-		private final Expression expression;
+		static final String ATT_METHOD = "method";
 
-		private SecurityExpressionHandler<MethodInvocation> expressionHandler = new DefaultMethodSecurityExpressionHandler();
+		static final String ATT_ACCESS = "access";
 
-		MethodExpressionAuthorizationManager(Expression expression) {
-			this.expression = expression;
-		}
+		private static final String ATT_ACCESS_MGR = "access-decision-manager-ref";
 
 		@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;
+		protected BeanDefinition createInterceptorDefinition(Node node) {
+			Element interceptMethodsElt = (Element) node;
+			BeanDefinitionBuilder interceptor = BeanDefinitionBuilder
+					.rootBeanDefinition(MethodSecurityInterceptor.class);
+			// Default to autowiring to pick up after invocation mgr
+			interceptor.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE);
+			String accessManagerId = interceptMethodsElt.getAttribute(ATT_ACCESS_MGR);
+			if (!StringUtils.hasText(accessManagerId)) {
+				accessManagerId = BeanIds.METHOD_ACCESS_MANAGER;
+			}
+			interceptor.addPropertyValue("accessDecisionManager", new RuntimeBeanReference(accessManagerId));
+			interceptor.addPropertyValue("authenticationManager",
+					new RuntimeBeanReference(BeanIds.AUTHENTICATION_MANAGER));
+			// Lookup parent bean information
+			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<>();
+			for (Element protectmethodElt : methods) {
+				BeanDefinitionBuilder attributeBuilder = BeanDefinitionBuilder.rootBeanDefinition(SecurityConfig.class);
+				attributeBuilder.setFactoryMethod("createListFromCommaDelimitedString");
+				attributeBuilder.addConstructorArgValue(protectmethodElt.getAttribute(ATT_ACCESS));
+				// Support inference of class names
+				String methodName = protectmethodElt.getAttribute(ATT_METHOD);
+				if (methodName.lastIndexOf(".") == -1) {
+					if (parentBeanClass != null && !"".equals(parentBeanClass)) {
+						methodName = parentBeanClass + "." + methodName;
+					}
+				}
+				mappings.put(methodName, attributeBuilder.getBeanDefinition());
+			}
+			BeanDefinition metadataSource = new RootBeanDefinition(MapBasedMethodSecurityMetadataSource.class);
+			metadataSource.getConstructorArgumentValues().addGenericArgumentValue(mappings);
+			interceptor.addPropertyValue("securityMetadataSource", metadataSource);
+			return interceptor.getBeanDefinition();
 		}
 
 	}