Feedback

Author: marcusdacoregio

config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java

@@ -35,7 +35,7 @@
 import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
 import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
 import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
-import org.springframework.security.aot.hint.PrePostAuthorizeExpressionBeanHintsRegistrar;
+import org.springframework.security.aot.hint.PrePostAuthorizeHintsRegistrar;
 import org.springframework.security.aot.hint.SecurityHintsRegistrar;
 import org.springframework.security.authorization.AuthorizationEventPublisher;
 import org.springframework.security.authorization.ObservationAuthorizationManager;
@@ -195,8 +195,8 @@ static MethodInterceptor postFilterAuthorizationMethodInterceptor(
 
 	@Bean
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
-	static SecurityHintsRegistrar prePostAuthorizeExpressionBeanHintsRegistrar() {
-		return new PrePostAuthorizeExpressionBeanHintsRegistrar();
+	static SecurityHintsRegistrar prePostAuthorizeExpressionHintsRegistrar() {
+		return new PrePostAuthorizeHintsRegistrar();
 	}
 
 	@Override

core/src/main/java/org/springframework/security/aot/hint/PrePostAuthorizeExpressionBeanHintsRegistrar.java

@@ -18,15 +18,15 @@
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import org.springframework.aot.hint.MemberCategory;
 import org.springframework.aot.hint.RuntimeHints;
 import org.springframework.aot.hint.TypeReference;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
-import org.springframework.beans.factory.support.RegisteredBean;
 import org.springframework.expression.spel.SpelNode;
 import org.springframework.expression.spel.ast.BeanReference;
 import org.springframework.expression.spel.standard.SpelExpression;
@@ -36,11 +36,30 @@
 import org.springframework.security.authorization.method.AuthorizeReturnObject;
 import org.springframework.security.core.annotation.SecurityAnnotationScanner;
 import org.springframework.security.core.annotation.SecurityAnnotationScanners;
+import org.springframework.util.Assert;
 
 /**
- * A {@link SecurityHintsRegistrar} that scans all beans for methods that use
+ * A {@link SecurityHintsRegistrar} that scans all provided classes for methods that use
  * {@link PreAuthorize} or {@link PostAuthorize} and registers hints for the beans used
- * within the expressions.
+ * within the security expressions.
+ *
+ * <p>
+ * It will also scan return types of methods annotated with {@link AuthorizeReturnObject}.
+ *
+ * <p>
+ * This may be used by an application to register specific Security-adjacent classes that
+ * were otherwise missed by Spring Security's reachability scans.
+ *
+ * <p>
+ * Remember to register this as an infrastructural bean like so:
+ *
+ * <pre>
+ *	&#064;Bean
+ *	&#064;Role(BeanDefinition.ROLE_INFRASTRUCTURE)
+ *	static SecurityHintsRegistrar registerThese() {
+ *		return new PrePostAuthorizeExpressionBeanHintsRegistrar(MyClass.class);
+ *	}
+ * </pre>
  *
  * @author Marcus da Coregio
  * @since 6.4
@@ -59,14 +78,24 @@ public final class PrePostAuthorizeExpressionBeanHintsRegistrar implements Secur
 
 	private final SpelExpressionParser expressionParser = new SpelExpressionParser();
 
+	private final Set<Class<?>> visitedClasses = new HashSet<>();
+
+	private final List<Class<?>> toVisit;
+
+	public PrePostAuthorizeExpressionBeanHintsRegistrar(Class<?>... toVisit) {
+		this(Arrays.asList(toVisit));
+	}
+
+	public PrePostAuthorizeExpressionBeanHintsRegistrar(List<Class<?>> toVisit) {
+		Assert.notEmpty(toVisit, "toVisit cannot be empty");
+		Assert.noNullElements(toVisit, "toVisit cannot contain null elements");
+		this.toVisit = toVisit;
+	}
+
 	@Override
 	public void registerHints(RuntimeHints hints, ConfigurableListableBeanFactory beanFactory) {
-		Set<? extends Class<?>> beans = Arrays.stream(beanFactory.getBeanDefinitionNames())
-			.map((beanName) -> RegisteredBean.of(beanFactory, beanName).getBeanClass())
-			.collect(Collectors.toSet());
-
 		Set<String> expressions = new HashSet<>();
-		for (Class<?> bean : beans) {
+		for (Class<?> bean : this.toVisit) {
 			expressions.addAll(extractSecurityExpressions(bean));
 		}
 		Set<String> beanNamesToRegister = new HashSet<>();
@@ -83,6 +112,10 @@ public void registerHints(RuntimeHints hints, ConfigurableListableBeanFactory be
 	}
 
 	private Set<String> extractSecurityExpressions(Class<?> clazz) {
+		if (this.visitedClasses.contains(clazz)) {
+			return Collections.emptySet();
+		}
+		this.visitedClasses.add(clazz);
 		Set<String> expressions = new HashSet<>();
 		for (Method method : clazz.getDeclaredMethods()) {
 			PreAuthorize preAuthorize = this.preAuthorizeScanner.scan(method, clazz);

core/src/main/java/org/springframework/security/aot/hint/PrePostAuthorizeHintsRegistrar.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2002-2024 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.aot.hint;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.springframework.aot.hint.RuntimeHints;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.beans.factory.support.RegisteredBean;
+import org.springframework.security.access.prepost.PostAuthorize;
+import org.springframework.security.access.prepost.PreAuthorize;
+
+/**
+ * A {@link SecurityHintsRegistrar} that scans all beans for methods that use
+ * {@link PreAuthorize} or {@link PostAuthorize} and registers appropriate hints for the
+ * annotations.
+ *
+ * @author Marcus da Coregio
+ * @since 6.4
+ * @see SecurityHintsAotProcessor
+ * @see PrePostAuthorizeExpressionBeanHintsRegistrar
+ */
+public final class PrePostAuthorizeHintsRegistrar implements SecurityHintsRegistrar {
+
+	@Override
+	public void registerHints(RuntimeHints hints, ConfigurableListableBeanFactory beanFactory) {
+		List<Class<?>> beans = Arrays.stream(beanFactory.getBeanDefinitionNames())
+			.map((beanName) -> RegisteredBean.of(beanFactory, beanName).getBeanClass())
+			.collect(Collectors.toList());
+		new PrePostAuthorizeExpressionBeanHintsRegistrar(beans).registerHints(hints, beanFactory);
+	}
+
+}

core/src/test/java/org/springframework/security/aot/hint/PrePostAuthorizeExpressionBeanHintsRegistrarTests.java renamed to core/src/test/java/org/springframework/security/aot/hint/PrePostAuthorizeHintsRegistrarTests.java

@@ -22,17 +22,21 @@
 import org.springframework.aot.hint.MemberCategory;
 import org.springframework.aot.hint.predicate.RuntimeHintsPredicates;
 import org.springframework.aot.test.generate.TestGenerationContext;
+import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Role;
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.authorization.method.AuthorizeReturnObject;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 
-class PrePostAuthorizeExpressionBeanHintsRegistrarTests {
+class PrePostAuthorizeHintsRegistrarTests {
 
-	private final PrePostAuthorizeExpressionBeanHintsRegistrar registrar = new PrePostAuthorizeExpressionBeanHintsRegistrar();
+	private final PrePostAuthorizeHintsRegistrar registrar = new PrePostAuthorizeHintsRegistrar();
 
 	private final GenerationContext generationContext = new TestGenerationContext();
 
@@ -158,6 +162,11 @@ void registerHintsWhenSecurityAnnotationsInsideAuthorizeReturnObjectOnClassThenR
 			.accepts(this.generationContext.getRuntimeHints());
 	}
 
+	@Test
+	void registerHintsWhenCyclicDependencyThenNoStackOverflowException() {
+		assertThatNoException().isThrownBy(() -> process(AService.class));
+	}
+
 	private void process(Class<?>... beanClasses) {
 		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		for (Class<?> beanClass : beanClasses) {
@@ -312,4 +321,41 @@ String getFullName() {
 
 	}
 
+	static class AService {
+
+		@AuthorizeReturnObject
+		A getA() {
+			return new A();
+		}
+
+	}
+
+	static class A {
+
+		@AuthorizeReturnObject
+		B getB() {
+			return null;
+		}
+
+	}
+
+	static class B {
+
+		@AuthorizeReturnObject
+		A getA() {
+			return null;
+		}
+
+	}
+
+	static class UserSecurityHintsRegistrar {
+
+		@Bean
+		@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
+		static SecurityHintsRegistrar registerTheseToo() {
+			return new PrePostAuthorizeExpressionBeanHintsRegistrar(User.class);
+		}
+
+	}
+
 }

docs/modules/ROOT/pages/servlet/authorization/method-security.adoc

@@ -1536,7 +1536,6 @@ If it finds a method that uses `@AuthorizeReturnObject`, it will recursively sea
 
 For example, consider the following Spring Boot application:
 
-.Custom MethodSecurityExpressionHandler
 [tabs]
 ======
 Java::
@@ -1633,6 +1632,72 @@ class User(private val fullName: String) {
 <5> Finding another `@AuthorizeReturnObject` it will look again into the method's return type
 <6> Now, a `@PostAuthorize` is found with yet another bean name used: `myOtherAuthz`; the runtime hints are registered for the bean class as well
 
+There will be many times when Spring Security cannot determine the actual return type of the method ahead of time since it may be hidden in an erased generic type.
+
+Consider the following service:
+
+[tabs]
+======
+Java::
++
+[source,java,role="primary"]
+----
+@Service
+public class AccountService {
+
+    @AuthorizeReturnObject
+    public List<Account> getAllAccounts() {
+        // ...
+    }
+
+}
+----
+
+Kotlin::
++
+[source,kotlin,role="secondary"]
+----
+@Service
+class AccountService {
+
+    @AuthorizeReturnObject
+    fun getAllAccounts(): List<Account> {
+        // ...
+    }
+
+}
+----
+======
+
+In this case, the generic type is erased and so it isn’t apparent to Spring Security ahead-of-time that `Account` needs to be visited in order to check for `@PreAuthorize` and `@PostAuthorize`.
+
+To address this, you can publish a javadoc:org.springframework.security.aot.hint.PrePostAuthorizeExpressionBeanHintsRegistrar[`PrePostAuthorizeExpressionBeanHintsRegistrar`] like so:
+
+[tabs]
+======
+Java::
++
+[source,java,role="primary"]
+----
+@Bean
+@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
+static SecurityHintsRegistrar registerTheseToo() {
+    return new PrePostAuthorizeExpressionBeanHintsRegistrar(Account.class);
+}
+----
+
+Kotlin::
++
+[source,kotlin,role="secondary"]
+----
+@Bean
+@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
+fun registerTheseToo(): SecurityHintsRegistrar {
+    return PrePostAuthorizeExpressionBeanHintsRegistrar(Account::class.java)
+}
+----
+======
+
 [[use-aspectj]]
 == Authorizing with AspectJ