From c622edcd68fcdf983f72a029ba741c23f290f6b7 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 19 Mar 2024 18:45:51 -0600 Subject: [PATCH] Add Value-Type Ignore Support Issue gh-14597 --- .../AuthorizationProxyConfiguration.java | 7 +- ...activeAuthorizationProxyConfiguration.java | 12 +- ...ePostMethodSecurityConfigurationTests.java | 10 +- ...ctiveMethodSecurityConfigurationTests.java | 13 +- .../AuthorizationAdvisorProxyFactory.java | 474 ++++++++++++------ ...ctiveAuthorizationAdvisorProxyFactory.java | 139 ----- ...AuthorizationAdvisorProxyFactoryTests.java | 61 ++- ...AuthorizationAdvisorProxyFactoryTests.java | 12 +- .../authorization/method-security.adoc | 43 +- 9 files changed, 449 insertions(+), 322 deletions(-) delete mode 100644 core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java index 5772e6afafb..5b58329e85e 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java @@ -30,16 +30,19 @@ import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory; import org.springframework.security.authorization.method.AuthorizationAdvisor; import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; +import org.springframework.security.config.Customizer; @Configuration(proxyBeanMethods = false) final class AuthorizationProxyConfiguration implements AopInfrastructureBean { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider provider) { + static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider provider, + ObjectProvider> customizers) { List advisors = new ArrayList<>(); provider.forEach(advisors::add); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); + customizers.forEach((c) -> c.customize(factory)); factory.setAdvisors(advisors); return factory; } diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java index da16b6a968a..8d727d0cfa1 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java @@ -27,20 +27,22 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Role; -import org.springframework.security.authorization.ReactiveAuthorizationAdvisorProxyFactory; +import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory; import org.springframework.security.authorization.method.AuthorizationAdvisor; import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; +import org.springframework.security.config.Customizer; @Configuration(proxyBeanMethods = false) final class ReactiveAuthorizationProxyConfiguration implements AopInfrastructureBean { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - static ReactiveAuthorizationAdvisorProxyFactory authorizationProxyFactory( - ObjectProvider provider) { + static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider provider, + ObjectProvider> customizers) { List advisors = new ArrayList<>(); provider.forEach(advisors::add); - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); + customizers.forEach((c) -> c.customize(factory)); factory.setAdvisors(advisors); return factory; } @@ -48,7 +50,7 @@ static ReactiveAuthorizationAdvisorProxyFactory authorizationProxyFactory( @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) static MethodInterceptor authorizeReturnObjectMethodInterceptor(ObjectProvider provider, - ReactiveAuthorizationAdvisorProxyFactory authorizationProxyFactory) { + AuthorizationAdvisorProxyFactory authorizationProxyFactory) { AuthorizeReturnObjectMethodInterceptor interceptor = new AuthorizeReturnObjectMethodInterceptor( authorizationProxyFactory); List advisors = new ArrayList<>(); diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java index 203d00e9fba..0d94d44ffeb 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java @@ -58,6 +58,7 @@ import org.springframework.security.access.prepost.PostFilter; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.access.prepost.PreFilter; +import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationEventPublisher; import org.springframework.security.authorization.AuthorizationManager; @@ -66,6 +67,7 @@ import org.springframework.security.authorization.method.AuthorizeReturnObject; import org.springframework.security.authorization.method.MethodInvocationResult; import org.springframework.security.authorization.method.PrePostTemplateDefaults; +import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig; import org.springframework.security.config.core.GrantedAuthorityDefaults; import org.springframework.security.config.test.SpringTestContext; @@ -1143,6 +1145,12 @@ List resultsContainDave(List list) { @Configuration static class AuthorizeResultConfig { + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + static Customizer skipValueTypes() { + return (f) -> f.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES); + } + @Bean FlightRepository flights() { FlightRepository flights = new FlightRepository(); @@ -1186,6 +1194,7 @@ void remove(String id) { } + @AuthorizeReturnObject static class Flight { private final String id; @@ -1216,7 +1225,6 @@ Integer getSeats() { return this.seats; } - @AuthorizeReturnObject @PostAuthorize("hasAuthority('seating:read')") @PostFilter("filterObject.name != 'Kevin Mitnick'") List getPassengers() { diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java index b3a566086f5..0629549c0a7 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java @@ -30,8 +30,10 @@ import reactor.test.StepVerifier; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Role; import org.springframework.expression.EvaluationContext; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.expression.SecurityExpressionRoot; @@ -42,7 +44,9 @@ import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.access.prepost.PreFilter; import org.springframework.security.authentication.TestAuthentication; +import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory; import org.springframework.security.authorization.method.AuthorizeReturnObject; +import org.springframework.security.config.Customizer; import org.springframework.security.config.core.GrantedAuthorityDefaults; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; @@ -238,6 +242,13 @@ public void bar(String param) { @Configuration static class AuthorizeResultConfig { + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + static Customizer skipValueTypes() { + return (factory) -> factory + .setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES); + } + @Bean FlightRepository flights() { FlightRepository flights = new FlightRepository(); @@ -282,6 +293,7 @@ Mono remove(String id) { } + @AuthorizeReturnObject static class Flight { private final String id; @@ -312,7 +324,6 @@ Mono getSeats() { return Mono.just(this.seats); } - @AuthorizeReturnObject @PostAuthorize("hasAnyAuthority('seating:read', 'airplane:read')") @PostFilter("@isNotKevin.apply(filterObject)") Flux getPassengers() { diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java index 9e7db95dab2..415c25b3874 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java @@ -36,15 +36,25 @@ import java.util.TreeSet; import java.util.stream.Stream; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + import org.springframework.aop.Advisor; import org.springframework.aop.framework.ProxyFactory; import org.springframework.core.annotation.AnnotationAwareOrderComparator; +import org.springframework.lang.NonNull; import org.springframework.security.authorization.method.AuthorizationAdvisor; import org.springframework.security.authorization.method.AuthorizationManagerAfterMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizationManagerAfterReactiveMethodInterceptor; import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizationManagerBeforeReactiveMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizeReturnObject; import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; import org.springframework.security.authorization.method.PostFilterAuthorizationMethodInterceptor; +import org.springframework.security.authorization.method.PostFilterAuthorizationReactiveMethodInterceptor; import org.springframework.security.authorization.method.PreFilterAuthorizationMethodInterceptor; +import org.springframework.security.authorization.method.PreFilterAuthorizationReactiveMethodInterceptor; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -63,8 +73,7 @@ * like so: * *
- *     AuthorizationManagerBeforeMethodInterceptor preAuthorize = AuthorizationManagerBeforeMethodInterceptor.preAuthorize();
- *     AuthorizationProxyFactory proxyFactory = new AuthorizationProxyFactory(preAuthorize);
+ *     AuthorizationProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withDefaults();
  *     Foo foo = new Foo();
  *     foo.bar(); // passes
  *     Foo securedFoo = proxyFactory.proxy(foo);
@@ -74,18 +83,66 @@
  * @author Josh Cummings
  * @since 6.3
  */
-public final class AuthorizationAdvisorProxyFactory implements AuthorizationProxyFactory {
+public final class AuthorizationAdvisorProxyFactory
+		implements AuthorizationProxyFactory, Iterable {
+
+	private static final boolean isReactivePresent = ClassUtils.isPresent("reactor.core.publisher.Mono", null);
+
+	/**
+	 * The default {@link TargetVisitor}, which will proxy {@link Class} instances as well
+	 * as instances contained in reactive types (if reactor is present), collection types,
+	 * and other container types like {@link Optional}
+	 */
+	public static final TargetVisitor DEFAULT_VISITOR = isReactivePresent
+			? new DelegateVisitor(new ClassVisitor(), new ReactiveTypeVisitor(), new ContainerTypeVisitor())
+			: new DelegateVisitor(new ClassVisitor(), new ContainerTypeVisitor());
+
+	/**
+	 * The default {@link TargetVisitor} that also skips any value types (for example,
+	 * {@link String}, {@link Integer}). This is handy for annotations like
+	 * {@link AuthorizeReturnObject} when used at the class level
+	 */
+	public static final TargetVisitor DEFAULT_VISITOR_SKIP_VALUE_TYPES = new DelegateVisitor(new ClassVisitor(),
+			new IgnoreValueTypeVisitor(), DEFAULT_VISITOR);
 
-	private List advisors = new ArrayList<>();
+	private List advisors;
 
-	public AuthorizationAdvisorProxyFactory() {
+	private TargetVisitor visitor = DEFAULT_VISITOR;
+
+	private AuthorizationAdvisorProxyFactory(List advisors) {
+		this.advisors = new ArrayList<>(advisors);
+		this.advisors.add(new AuthorizeReturnObjectMethodInterceptor(this));
+		setAdvisors(this.advisors);
+	}
+
+	/**
+	 * Construct an {@link AuthorizationAdvisorProxyFactory} with the defaults needed for
+	 * wrapping objects in Spring Security's pre-post method security support.
+	 * @return an {@link AuthorizationAdvisorProxyFactory} for adding pre-post method
+	 * security support
+	 */
+	public static AuthorizationAdvisorProxyFactory withDefaults() {
 		List advisors = new ArrayList<>();
 		advisors.add(AuthorizationManagerBeforeMethodInterceptor.preAuthorize());
 		advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize());
 		advisors.add(new PreFilterAuthorizationMethodInterceptor());
 		advisors.add(new PostFilterAuthorizationMethodInterceptor());
-		advisors.add(new AuthorizeReturnObjectMethodInterceptor(this));
-		setAdvisors(advisors);
+		return new AuthorizationAdvisorProxyFactory(advisors);
+	}
+
+	/**
+	 * Construct an {@link AuthorizationAdvisorProxyFactory} with the defaults needed for
+	 * wrapping objects in Spring Security's pre-post reactive method security support.
+	 * @return an {@link AuthorizationAdvisorProxyFactory} for adding pre-post reactive
+	 * method security support
+	 */
+	public static AuthorizationAdvisorProxyFactory withReactiveDefaults() {
+		List advisors = new ArrayList<>();
+		advisors.add(AuthorizationManagerBeforeReactiveMethodInterceptor.preAuthorize());
+		advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize());
+		advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor());
+		advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor());
+		return new AuthorizationAdvisorProxyFactory(advisors);
 	}
 
 	/**
@@ -111,41 +168,9 @@ public Object proxy(Object target) {
 		if (target == null) {
 			return null;
 		}
-		if (target instanceof Class targetClass) {
-			return proxyClass(targetClass);
-		}
-		if (target instanceof Iterator iterator) {
-			return proxyIterator(iterator);
-		}
-		if (target instanceof Queue queue) {
-			return proxyQueue(queue);
-		}
-		if (target instanceof List list) {
-			return proxyList(list);
-		}
-		if (target instanceof SortedSet set) {
-			return proxySortedSet(set);
-		}
-		if (target instanceof Set set) {
-			return proxySet(set);
-		}
-		if (target.getClass().isArray()) {
-			return proxyArray((Object[]) target);
-		}
-		if (target instanceof SortedMap map) {
-			return proxySortedMap(map);
-		}
-		if (target instanceof Iterable iterable) {
-			return proxyIterable(iterable);
-		}
-		if (target instanceof Map map) {
-			return proxyMap(map);
-		}
-		if (target instanceof Stream stream) {
-			return proxyStream(stream);
-		}
-		if (target instanceof Optional optional) {
-			return proxyOptional(optional);
+		Object proxied = this.visitor.visit(this, target);
+		if (proxied != null) {
+			return proxied;
 		}
 		ProxyFactory factory = new ProxyFactory(target);
 		for (Advisor advisor : this.advisors) {
@@ -179,144 +204,309 @@ public void setAdvisors(Collection advisors) {
 		AnnotationAwareOrderComparator.sort(this.advisors);
 	}
 
-	@SuppressWarnings("unchecked")
-	private  T proxyCast(T target) {
-		return (T) proxy(target);
+	/**
+	 * Use this visitor to navigate the proxy target's hierarchy.
+	 *
+	 * 

+ * This can be helpful when you want a specialized behavior for a type or set of + * types. For example, if you want to have this factory skip primitives and wrappers, + * then you can do: + * + *

+	 * 	AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory();
+	 * 	proxyFactory.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_IGNORE_VALUE_TYPES);
+	 * 
+ * @param visitor the visitor to use to introduce specialized behavior for a type + */ + public void setTargetVisitor(TargetVisitor visitor) { + Assert.notNull(visitor, "delegate cannot be null"); + this.visitor = visitor; } - private Class proxyClass(Class targetClass) { - ProxyFactory factory = new ProxyFactory(); - factory.setTargetClass(targetClass); - factory.setInterfaces(ClassUtils.getAllInterfacesForClass(targetClass)); - factory.setProxyTargetClass(!Modifier.isFinal(targetClass.getModifiers())); - for (Advisor advisor : this.advisors) { - factory.addAdvisors(advisor); - } - return factory.getProxyClass(getClass().getClassLoader()); + @Override + @NonNull + public Iterator iterator() { + return this.advisors.iterator(); } - private Iterable proxyIterable(Iterable iterable) { - return () -> proxyIterator(iterable.iterator()); + /** + * An interface to handle how the {@link AuthorizationAdvisorProxyFactory} should step + * through the target's object hierarchy. + * + * @author Josh Cummings + * @since 6.3 + * @see AuthorizationAdvisorProxyFactory#setTargetVisitor + */ + public interface TargetVisitor { + + /** + * Visit and possibly proxy this object. + * + *

+ * Visiting may take the form of walking down this object's hierarchy and proxying + * sub-objects. + * + *

+ * An example is a visitor that proxies the elements of a {@link List} instead of + * the list itself + * + *

+ * Returning {@code null} implies that this visitor does not want to proxy this + * object + * @param proxyFactory the proxy factory to delegate proxying to for any + * sub-objects + * @param target the object to proxy + * @return the visited (and possibly proxied) object + */ + Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object target); + } - private Iterator proxyIterator(Iterator iterator) { - return new Iterator<>() { - @Override - public boolean hasNext() { - return iterator.hasNext(); - } + private static final class IgnoreValueTypeVisitor implements TargetVisitor { - @Override - public T next() { - return proxyCast(iterator.next()); + @Override + public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object object) { + if (ClassUtils.isSimpleValueType(object.getClass())) { + return object; } - }; + return null; + } + } - private SortedSet proxySortedSet(SortedSet set) { - SortedSet proxies = new TreeSet<>(set.comparator()); - for (T toProxy : set) { - proxies.add(proxyCast(toProxy)); - } - try { - set.clear(); - set.addAll(proxies); - return proxies; - } - catch (UnsupportedOperationException ex) { - return Collections.unmodifiableSortedSet(proxies); + private static final class ClassVisitor implements TargetVisitor { + + @Override + public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object object) { + if (object instanceof Class targetClass) { + ProxyFactory factory = new ProxyFactory(); + factory.setTargetClass(targetClass); + factory.setInterfaces(ClassUtils.getAllInterfacesForClass(targetClass)); + factory.setProxyTargetClass(!Modifier.isFinal(targetClass.getModifiers())); + for (Advisor advisor : proxyFactory) { + factory.addAdvisors(advisor); + } + return factory.getProxyClass(getClass().getClassLoader()); + } + return null; } + } - private Set proxySet(Set set) { - Set proxies = new LinkedHashSet<>(set.size()); - for (T toProxy : set) { - proxies.add(proxyCast(toProxy)); + private static final class ContainerTypeVisitor implements TargetVisitor { + + @Override + public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object target) { + if (target instanceof Iterator iterator) { + return proxyIterator(proxyFactory, iterator); + } + if (target instanceof Queue queue) { + return proxyQueue(proxyFactory, queue); + } + if (target instanceof List list) { + return proxyList(proxyFactory, list); + } + if (target instanceof SortedSet set) { + return proxySortedSet(proxyFactory, set); + } + if (target instanceof Set set) { + return proxySet(proxyFactory, set); + } + if (target.getClass().isArray()) { + return proxyArray(proxyFactory, (Object[]) target); + } + if (target instanceof SortedMap map) { + return proxySortedMap(proxyFactory, map); + } + if (target instanceof Iterable iterable) { + return proxyIterable(proxyFactory, iterable); + } + if (target instanceof Map map) { + return proxyMap(proxyFactory, map); + } + if (target instanceof Stream stream) { + return proxyStream(proxyFactory, stream); + } + if (target instanceof Optional optional) { + return proxyOptional(proxyFactory, optional); + } + return null; + } + + @SuppressWarnings("unchecked") + private T proxyCast(AuthorizationProxyFactory proxyFactory, T target) { + return (T) proxyFactory.proxy(target); } - try { - set.clear(); - set.addAll(proxies); - return proxies; + + private Iterable proxyIterable(AuthorizationProxyFactory proxyFactory, Iterable iterable) { + return () -> proxyIterator(proxyFactory, iterable.iterator()); } - catch (UnsupportedOperationException ex) { - return Collections.unmodifiableSet(proxies); + + private Iterator proxyIterator(AuthorizationProxyFactory proxyFactory, Iterator iterator) { + return new Iterator<>() { + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public T next() { + return proxyCast(proxyFactory, iterator.next()); + } + }; } - } - private Queue proxyQueue(Queue queue) { - Queue proxies = new LinkedList<>(); - for (T toProxy : queue) { - proxies.add(proxyCast(toProxy)); + private SortedSet proxySortedSet(AuthorizationProxyFactory proxyFactory, SortedSet set) { + SortedSet proxies = new TreeSet<>(set.comparator()); + for (T toProxy : set) { + proxies.add(proxyCast(proxyFactory, toProxy)); + } + try { + set.clear(); + set.addAll(proxies); + return proxies; + } + catch (UnsupportedOperationException ex) { + return Collections.unmodifiableSortedSet(proxies); + } } - queue.clear(); - queue.addAll(proxies); - return proxies; - } - private List proxyList(List list) { - List proxies = new ArrayList<>(list.size()); - for (T toProxy : list) { - proxies.add(proxyCast(toProxy)); + private Set proxySet(AuthorizationProxyFactory proxyFactory, Set set) { + Set proxies = new LinkedHashSet<>(set.size()); + for (T toProxy : set) { + proxies.add(proxyCast(proxyFactory, toProxy)); + } + try { + set.clear(); + set.addAll(proxies); + return proxies; + } + catch (UnsupportedOperationException ex) { + return Collections.unmodifiableSet(proxies); + } } - try { - list.clear(); - list.addAll(proxies); + + private Queue proxyQueue(AuthorizationProxyFactory proxyFactory, Queue queue) { + Queue proxies = new LinkedList<>(); + for (T toProxy : queue) { + proxies.add(proxyCast(proxyFactory, toProxy)); + } + queue.clear(); + queue.addAll(proxies); return proxies; } - catch (UnsupportedOperationException ex) { - return Collections.unmodifiableList(proxies); + + private List proxyList(AuthorizationProxyFactory proxyFactory, List list) { + List proxies = new ArrayList<>(list.size()); + for (T toProxy : list) { + proxies.add(proxyCast(proxyFactory, toProxy)); + } + try { + list.clear(); + list.addAll(proxies); + return proxies; + } + catch (UnsupportedOperationException ex) { + return Collections.unmodifiableList(proxies); + } } - } - private Object[] proxyArray(Object[] objects) { - List retain = new ArrayList<>(objects.length); - for (Object object : objects) { - retain.add(proxy(object)); + private Object[] proxyArray(AuthorizationProxyFactory proxyFactory, Object[] objects) { + List retain = new ArrayList<>(objects.length); + for (Object object : objects) { + retain.add(proxyFactory.proxy(object)); + } + Object[] proxies = (Object[]) Array.newInstance(objects.getClass().getComponentType(), retain.size()); + for (int i = 0; i < retain.size(); i++) { + proxies[i] = retain.get(i); + } + return proxies; } - Object[] proxies = (Object[]) Array.newInstance(objects.getClass().getComponentType(), retain.size()); - for (int i = 0; i < retain.size(); i++) { - proxies[i] = retain.get(i); + + private SortedMap proxySortedMap(AuthorizationProxyFactory proxyFactory, SortedMap entries) { + SortedMap proxies = new TreeMap<>(entries.comparator()); + for (Map.Entry entry : entries.entrySet()) { + proxies.put(entry.getKey(), proxyCast(proxyFactory, entry.getValue())); + } + try { + entries.clear(); + entries.putAll(proxies); + return entries; + } + catch (UnsupportedOperationException ex) { + return Collections.unmodifiableSortedMap(proxies); + } } - return proxies; - } - private SortedMap proxySortedMap(SortedMap entries) { - SortedMap proxies = new TreeMap<>(entries.comparator()); - for (Map.Entry entry : entries.entrySet()) { - proxies.put(entry.getKey(), proxyCast(entry.getValue())); + private Map proxyMap(AuthorizationProxyFactory proxyFactory, Map entries) { + Map proxies = new LinkedHashMap<>(entries.size()); + for (Map.Entry entry : entries.entrySet()) { + proxies.put(entry.getKey(), proxyCast(proxyFactory, entry.getValue())); + } + try { + entries.clear(); + entries.putAll(proxies); + return entries; + } + catch (UnsupportedOperationException ex) { + return Collections.unmodifiableMap(proxies); + } } - try { - entries.clear(); - entries.putAll(proxies); - return entries; + + private Stream proxyStream(AuthorizationProxyFactory proxyFactory, Stream stream) { + return stream.map(proxyFactory::proxy).onClose(stream::close); } - catch (UnsupportedOperationException ex) { - return Collections.unmodifiableSortedMap(proxies); + + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") + private Optional proxyOptional(AuthorizationProxyFactory proxyFactory, Optional optional) { + return optional.map(proxyFactory::proxy); } + } - private Map proxyMap(Map entries) { - Map proxies = new LinkedHashMap<>(entries.size()); - for (Map.Entry entry : entries.entrySet()) { - proxies.put(entry.getKey(), proxyCast(entry.getValue())); + private static class ReactiveTypeVisitor implements TargetVisitor { + + @Override + @SuppressWarnings("ReactiveStreamsUnusedPublisher") + public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object target) { + if (target instanceof Mono mono) { + return proxyMono(proxyFactory, mono); + } + if (target instanceof Flux flux) { + return proxyFlux(proxyFactory, flux); + } + return null; } - try { - entries.clear(); - entries.putAll(proxies); - return entries; + + private Mono proxyMono(AuthorizationProxyFactory proxyFactory, Mono mono) { + return mono.map(proxyFactory::proxy); } - catch (UnsupportedOperationException ex) { - return Collections.unmodifiableMap(proxies); + + private Flux proxyFlux(AuthorizationProxyFactory proxyFactory, Flux flux) { + return flux.map(proxyFactory::proxy); } - } - private Stream proxyStream(Stream stream) { - return stream.map(this::proxy).onClose(stream::close); } - @SuppressWarnings("OptionalUsedAsFieldOrParameterType") - private Optional proxyOptional(Optional optional) { - return optional.map(this::proxy); + private static class DelegateVisitor implements TargetVisitor { + + private final Collection visitors; + + DelegateVisitor(TargetVisitor... visitors) { + this.visitors = List.of(visitors); + } + + @Override + public Object visit(AuthorizationAdvisorProxyFactory proxyFactory, Object target) { + for (TargetVisitor visitor : this.visitors) { + Object result = visitor.visit(proxyFactory, target); + if (result != null) { + return result; + } + } + return null; + } + } } diff --git a/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java deleted file mode 100644 index 2e410f4d141..00000000000 --- a/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * 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.authorization; - -import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Stream; - -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; - -import org.springframework.aop.framework.ProxyFactory; -import org.springframework.security.authorization.method.AuthorizationAdvisor; -import org.springframework.security.authorization.method.AuthorizationManagerAfterReactiveMethodInterceptor; -import org.springframework.security.authorization.method.AuthorizationManagerBeforeReactiveMethodInterceptor; -import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; -import org.springframework.security.authorization.method.PostFilterAuthorizationReactiveMethodInterceptor; -import org.springframework.security.authorization.method.PreFilterAuthorizationReactiveMethodInterceptor; - -/** - * A proxy factory for applying authorization advice to an arbitrary object. - * - *

- * For example, consider a non-Spring-managed object {@code Foo}:

- *     class Foo {
- *         @PreAuthorize("hasAuthority('bar:read')")
- *         String bar() { ... }
- *     }
- * 
- * - * Use {@link ReactiveAuthorizationAdvisorProxyFactory} to wrap the instance in Spring - * Security's {@link org.springframework.security.access.prepost.PreAuthorize} method - * interceptor like so: - * - *
- *     AuthorizationManagerBeforeMethodInterceptor preAuthorize = AuthorizationManagerBeforeMethodInterceptor.preAuthorize();
- *     AuthorizationProxyFactory proxyFactory = new AuthorizationProxyFactory(preAuthorize);
- *     Foo foo = new Foo();
- *     foo.bar(); // passes
- *     Foo securedFoo = proxyFactory.proxy(foo);
- *     securedFoo.bar(); // access denied!
- * 
- * - * @author Josh Cummings - * @since 6.3 - */ -public final class ReactiveAuthorizationAdvisorProxyFactory implements AuthorizationProxyFactory { - - private final AuthorizationAdvisorProxyFactory defaults = new AuthorizationAdvisorProxyFactory(); - - public ReactiveAuthorizationAdvisorProxyFactory() { - List advisors = new ArrayList<>(); - advisors.add(AuthorizationManagerBeforeReactiveMethodInterceptor.preAuthorize()); - advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize()); - advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor()); - advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor()); - advisors.add(new AuthorizeReturnObjectMethodInterceptor(this)); - this.defaults.setAdvisors(advisors); - } - - /** - * Proxy an object to enforce authorization advice. - * - *

- * Proxies any instance of a non-final class or a class that implements more than one - * interface. - * - *

- * If {@code target} is an {@link Iterator}, {@link Collection}, {@link Array}, - * {@link Map}, {@link Stream}, or {@link Optional}, then the element or value type is - * proxied. - * - *

- * If {@code target} is a {@link Class}, then {@link ProxyFactory#getProxyClass} is - * invoked instead. - * @param target the instance to proxy - * @return the proxied instance - */ - @Override - public Object proxy(Object target) { - if (target instanceof Mono mono) { - return proxyMono(mono); - } - if (target instanceof Flux flux) { - return proxyFlux(flux); - } - return this.defaults.proxy(target); - } - - /** - * Add advisors that should be included to each proxy created. - * - *

- * All advisors are re-sorted by their advisor order. - * @param advisors the advisors to add - */ - public void setAdvisors(AuthorizationAdvisor... advisors) { - this.defaults.setAdvisors(advisors); - } - - /** - * Add advisors that should be included to each proxy created. - * - *

- * All advisors are re-sorted by their advisor order. - * @param advisors the advisors to add - */ - public void setAdvisors(Collection advisors) { - this.defaults.setAdvisors(advisors); - } - - private Mono proxyMono(Mono mono) { - return mono.map(this::proxy); - } - - private Flux proxyFlux(Flux flux) { - return flux.map(this::proxy); - } - -} diff --git a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java index a027013170a..12ebba9560d 100644 --- a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java +++ b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java @@ -40,12 +40,14 @@ import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.authentication.TestAuthentication; +import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory.TargetVisitor; import org.springframework.security.authorization.method.AuthorizationAdvisor; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; 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.atLeastOnce; import static org.mockito.Mockito.mock; @@ -64,7 +66,7 @@ public class AuthorizationAdvisorProxyFactoryTests { @Test public void proxyWhenPreAuthorizeThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Flight flight = new Flight(); assertThat(flight.getAltitude()).isEqualTo(35000d); Flight secured = proxy(factory, flight); @@ -75,7 +77,7 @@ public void proxyWhenPreAuthorizeThenHonors() { @Test public void proxyWhenPreAuthorizeOnInterfaceThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); assertThat(this.alan.getFirstName()).isEqualTo("alan"); User secured = proxy(factory, this.alan); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(secured::getFirstName); @@ -89,7 +91,7 @@ public void proxyWhenPreAuthorizeOnInterfaceThenHonors() { @Test public void proxyWhenPreAuthorizeOnRecordThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); HasSecret repo = new Repository("secret"); assertThat(repo.secret()).isEqualTo("secret"); HasSecret secured = proxy(factory, repo); @@ -102,7 +104,7 @@ public void proxyWhenPreAuthorizeOnRecordThenHonors() { @Test public void proxyWhenImmutableListThenReturnsSecuredImmutableList() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); List flights = List.of(this.flight); List secured = proxy(factory, flights); secured.forEach( @@ -114,7 +116,7 @@ public void proxyWhenImmutableListThenReturnsSecuredImmutableList() { @Test public void proxyWhenImmutableSetThenReturnsSecuredImmutableSet() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Set flights = Set.of(this.flight); Set secured = proxy(factory, flights); secured.forEach( @@ -126,7 +128,7 @@ public void proxyWhenImmutableSetThenReturnsSecuredImmutableSet() { @Test public void proxyWhenQueueThenReturnsSecuredQueue() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Queue flights = new LinkedList<>(List.of(this.flight)); Queue secured = proxy(factory, flights); assertThat(flights.size()).isEqualTo(secured.size()); @@ -138,7 +140,7 @@ public void proxyWhenQueueThenReturnsSecuredQueue() { @Test public void proxyWhenImmutableSortedSetThenReturnsSecuredImmutableSortedSet() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); SortedSet users = Collections.unmodifiableSortedSet(new TreeSet<>(Set.of(this.alan))); SortedSet secured = proxy(factory, users); secured @@ -150,7 +152,7 @@ public void proxyWhenImmutableSortedSetThenReturnsSecuredImmutableSortedSet() { @Test public void proxyWhenImmutableSortedMapThenReturnsSecuredImmutableSortedMap() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); SortedMap users = Collections .unmodifiableSortedMap(new TreeMap<>(Map.of(this.alan.getId(), this.alan))); SortedMap secured = proxy(factory, users); @@ -163,7 +165,7 @@ public void proxyWhenImmutableSortedMapThenReturnsSecuredImmutableSortedMap() { @Test public void proxyWhenImmutableMapThenReturnsSecuredImmutableMap() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Map users = Map.of(this.alan.getId(), this.alan); Map secured = proxy(factory, users); secured.forEach( @@ -175,7 +177,7 @@ public void proxyWhenImmutableMapThenReturnsSecuredImmutableMap() { @Test public void proxyWhenMutableListThenReturnsSecuredMutableList() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); List flights = new ArrayList<>(List.of(this.flight)); List secured = proxy(factory, flights); secured.forEach( @@ -187,7 +189,7 @@ public void proxyWhenMutableListThenReturnsSecuredMutableList() { @Test public void proxyWhenMutableSetThenReturnsSecuredMutableSet() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Set flights = new HashSet<>(Set.of(this.flight)); Set secured = proxy(factory, flights); secured.forEach( @@ -199,7 +201,7 @@ public void proxyWhenMutableSetThenReturnsSecuredMutableSet() { @Test public void proxyWhenMutableSortedSetThenReturnsSecuredMutableSortedSet() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); SortedSet users = new TreeSet<>(Set.of(this.alan)); SortedSet secured = proxy(factory, users); secured.forEach((u) -> assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(u::getFirstName)); @@ -210,7 +212,7 @@ public void proxyWhenMutableSortedSetThenReturnsSecuredMutableSortedSet() { @Test public void proxyWhenMutableSortedMapThenReturnsSecuredMutableSortedMap() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); SortedMap users = new TreeMap<>(Map.of(this.alan.getId(), this.alan)); SortedMap secured = proxy(factory, users); secured.forEach((id, u) -> assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(u::getFirstName)); @@ -221,7 +223,7 @@ public void proxyWhenMutableSortedMapThenReturnsSecuredMutableSortedMap() { @Test public void proxyWhenMutableMapThenReturnsSecuredMutableMap() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Map users = new HashMap<>(Map.of(this.alan.getId(), this.alan)); Map secured = proxy(factory, users); secured.forEach((id, u) -> assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(u::getFirstName)); @@ -232,7 +234,7 @@ public void proxyWhenMutableMapThenReturnsSecuredMutableMap() { @Test public void proxyWhenPreAuthorizeForOptionalThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Optional flights = Optional.of(this.flight); assertThat(flights.get().getAltitude()).isEqualTo(35000d); Optional secured = proxy(factory, flights); @@ -243,7 +245,7 @@ public void proxyWhenPreAuthorizeForOptionalThenHonors() { @Test public void proxyWhenPreAuthorizeForStreamThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Stream flights = Stream.of(this.flight); Stream secured = proxy(factory, flights); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(() -> secured.forEach(Flight::getAltitude)); @@ -253,7 +255,7 @@ public void proxyWhenPreAuthorizeForStreamThenHonors() { @Test public void proxyWhenPreAuthorizeForArrayThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Flight[] flights = { this.flight }; Flight[] secured = proxy(factory, flights); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(secured[0]::getAltitude); @@ -263,7 +265,7 @@ public void proxyWhenPreAuthorizeForArrayThenHonors() { @Test public void proxyWhenPreAuthorizeForIteratorThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Iterator flights = List.of(this.flight).iterator(); Iterator secured = proxy(factory, flights); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(() -> secured.next().getAltitude()); @@ -273,7 +275,7 @@ public void proxyWhenPreAuthorizeForIteratorThenHonors() { @Test public void proxyWhenPreAuthorizeForIterableThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Iterable users = new UserRepository(); Iterable secured = proxy(factory, users); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(() -> secured.forEach(User::getFirstName)); @@ -282,7 +284,7 @@ public void proxyWhenPreAuthorizeForIterableThenHonors() { @Test public void proxyWhenPreAuthorizeForClassThenHonors() { - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); Class clazz = proxy(factory, Flight.class); assertThat(clazz.getSimpleName()).contains("SpringCGLIB$$"); Flight secured = proxy(factory, this.flight); @@ -297,13 +299,30 @@ public void setAdvisorsWhenProxyThenVisits() { AuthorizationAdvisor advisor = mock(AuthorizationAdvisor.class); given(advisor.getAdvice()).willReturn(advisor); given(advisor.getPointcut()).willReturn(Pointcut.TRUE); - AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); factory.setAdvisors(advisor); Flight flight = proxy(factory, this.flight); flight.getAltitude(); verify(advisor, atLeastOnce()).getPointcut(); } + @Test + public void setTargetVisitorThenUses() { + TargetVisitor visitor = mock(TargetVisitor.class); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); + factory.setTargetVisitor(visitor); + factory.proxy(new Flight()); + verify(visitor).visit(any(), any()); + } + + @Test + public void setTargetVisitorIgnoreValueTypesThenIgnores() { + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); + assertThatExceptionOfType(ClassCastException.class).isThrownBy(() -> ((Integer) factory.proxy(35)).intValue()); + factory.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES); + assertThat(factory.proxy(35)).isEqualTo(35); + } + private Authentication authenticated(String user, String... authorities) { return TestAuthentication.authenticated(TestAuthentication.withUsername(user).authorities(authorities).build()); } diff --git a/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java b/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java index 5b90c954c67..67e5309bd5c 100644 --- a/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java +++ b/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java @@ -52,7 +52,7 @@ public class ReactiveAuthorizationAdvisorProxyFactoryTests { @Test public void proxyWhenPreAuthorizeThenHonors() { - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); Flight flight = new Flight(); StepVerifier .create(flight.getAltitude().contextWrite(ReactiveSecurityContextHolder.withAuthentication(this.user))) @@ -67,7 +67,7 @@ public void proxyWhenPreAuthorizeThenHonors() { @Test public void proxyWhenPreAuthorizeOnInterfaceThenHonors() { SecurityContextHolder.getContext().setAuthentication(this.user); - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); StepVerifier .create(this.alan.getFirstName().contextWrite(ReactiveSecurityContextHolder.withAuthentication(this.user))) .expectNext("alan") @@ -89,7 +89,7 @@ public void proxyWhenPreAuthorizeOnInterfaceThenHonors() { @Test public void proxyWhenPreAuthorizeOnRecordThenHonors() { - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); HasSecret repo = new Repository(Mono.just("secret")); StepVerifier.create(repo.secret().contextWrite(ReactiveSecurityContextHolder.withAuthentication(this.user))) .expectNext("secret") @@ -104,7 +104,7 @@ public void proxyWhenPreAuthorizeOnRecordThenHonors() { @Test public void proxyWhenPreAuthorizeOnFluxThenHonors() { - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); Flux flights = Flux.just(this.flight); Flux secured = proxy(factory, flights); StepVerifier @@ -115,7 +115,7 @@ public void proxyWhenPreAuthorizeOnFluxThenHonors() { @Test public void proxyWhenPreAuthorizeForClassThenHonors() { - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); Class clazz = proxy(factory, Flight.class); assertThat(clazz.getSimpleName()).contains("SpringCGLIB$$"); Flight secured = proxy(factory, this.flight); @@ -129,7 +129,7 @@ public void setAdvisorsWhenProxyThenVisits() { AuthorizationAdvisor advisor = mock(AuthorizationAdvisor.class); given(advisor.getAdvice()).willReturn(advisor); given(advisor.getPointcut()).willReturn(Pointcut.TRUE); - ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); + AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); factory.setAdvisors(advisor); Flight flight = proxy(factory, this.flight); flight.getAltitude(); diff --git a/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc b/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc index e6bbee7abc6..7c6665ac601 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc @@ -1812,12 +1812,40 @@ fun getEmailWhenProxiedThenAuthorizes() { ---- ====== -[NOTE] -==== -`@AuthorizeReturnObject` can be placed at the class level. Note, though, that this means Spring Security will proxy any return object, including ``String``, ``Integer`` and other types. +=== Using `@AuthorizeReturnObject` at the class level + +`@AuthorizeReturnObject` can be placed at the class level. Note, though, that this means Spring Security will attempt to proxy any return object, including ``String``, ``Integer`` and other types. This is often not what you want to do. -In most cases, you will want to annotate the individual methods. +If you want to use `@AuthorizeReturnObject` on a class or interface whose methods return value types, like `int`, `String`, `Double` or collections of those types, then you should also publish the appropriate `AuthorizationAdvisorProxyFactory.TargetVisitor` as follows: + + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +static Customizer skipValueTypes() { + return (factory) -> factory.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +open fun skipValueTypes() = Customizer { + it.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES) +} +---- +====== + +[TIP] +==== +You can set your own `AuthorizationAdvisorProxyFactory.TargetVisitor` to customize the proxying for any set of types ==== === Programmatically Proxying @@ -1878,10 +1906,12 @@ Java:: [source,java,role="primary"] ---- import static org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor.preAuthorize; - +import static org.springframework.security.authorization.method.AuthorizationProxyTargetVisitors.*; // ... AuthorizationProxyFactory proxyFactory = new AuthorizationProxyFactory(preAuthorize()); +// and if needing to ignore value types +proxyFactory.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES); ---- Kotlin:: @@ -1889,10 +1919,13 @@ Kotlin:: [source,kotlin,role="secondary"] ---- import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor.preAuthorize +import org.springframework.security.authorization.method.AuthorizationProxyTargetVisitors.* // ... val proxyFactory: AuthorizationProxyFactory = AuthorizationProxyFactory(preAuthorize()) +// and if needing to ignore value types +proxyFactory.setTargetVisitor(AuthorizationAdvisorProxyFactory.DEFAULT_VISITOR_SKIP_VALUE_TYPES) ---- ======