Add AuthorizationReturnObject

Closes spring-projectsgh-14597

aspects/src/main/java/org/springframework/security/authorization/method/aspectj/AuthorizeReturnObjectAspect.aj

@@ -0,0 +1,42 @@
+/*
+ * 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.method.aspectj;
+
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
+
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.security.authorization.method.AuthorizeReturnObject;
+
+/**
+ * Concrete AspectJ aspect using Spring Security @AuthorizeReturnObject annotation.
+ *
+ * <p>
+ * When using this aspect, you <i>must</i> annotate the implementation class
+ * (and/or methods within that class), <i>not</i> the interface (if any) that
+ * the class implements. AspectJ follows Java's rule that annotations on
+ * interfaces are <i>not</i> inherited. This will vary from Spring AOP.
+ *
+ * @author Josh Cummings
+ * @since 6.3
+ */
+public aspect AuthorizeReturnObjectAspect extends AbstractMethodInterceptorAspect {
+
+	/**
+	 * Matches the execution of any method with a AuthorizeReturnObject annotation.
+	 */
+	protected pointcut executionOfAnnotatedMethod() : execution(* *(..)) && @annotation(AuthorizeReturnObject);
+}

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

@@ -19,6 +19,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.aopalliance.intercept.MethodInterceptor;
+
 import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -28,17 +30,29 @@
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory;
 import org.springframework.security.authorization.method.AuthorizationAdvisor;
+import org.springframework.security.authorization.method.AuthorizationProxyMethodInterceptor;
+import org.springframework.security.config.Customizer;
 
 @Configuration(proxyBeanMethods = false)
 final class AuthorizationProxyConfiguration implements AopInfrastructureBean {
 
 	@Bean
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
-	static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider<AuthorizationAdvisor> provider) {
+	static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider<AuthorizationAdvisor> provider,
+			ObjectProvider<Customizer<AuthorizationAdvisorProxyFactory>> customizers) {
 		List<AuthorizationAdvisor> advisors = new ArrayList<>();
 		provider.forEach(advisors::add);
 		AnnotationAwareOrderComparator.sort(advisors);
-		return new AuthorizationAdvisorProxyFactory(advisors);
+		AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors);
+		customizers.forEach((customizer) -> customizer.customize(factory));
+		return factory;
+	}
+
+	@Bean
+	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
+	static MethodInterceptor authorizationProxyMethodInterceptor(
+			AuthorizationAdvisorProxyFactory authorizationProxyFactory) {
+		return new AuthorizationProxyMethodInterceptor(authorizationProxyFactory);
 	}
 
 }

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

@@ -33,6 +33,7 @@ public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, B
 		registerAsAdvisor("postAuthorizeAuthorization", registry);
 		registerAsAdvisor("securedAuthorization", registry);
 		registerAsAdvisor("jsr250Authorization", registry);
+		registerAsAdvisor("authorizationProxy", registry);
 	}
 
 	private void registerAsAdvisor(String prefix, BeanDefinitionRegistry registry) {

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

@@ -60,6 +60,9 @@ public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, B
 				"postAuthorizeAspect$0", registry);
 		registerBeanDefinition("securedAuthorizationMethodInterceptor",
 				"org.springframework.security.authorization.method.aspectj.SecuredAspect", "securedAspect$0", registry);
+		registerBeanDefinition("authorizeReturnObjectMethodInterceptor",
+				"org.springframework.security.authorization.method.aspectj.AuthorizeReturnObjectAspect",
+				"authorizeReturnObjectAspect$0", registry);
 	}
 
 	private void registerBeanDefinition(String beanName, String aspectClassName, String aspectBeanName,

config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java

@@ -21,7 +21,10 @@
 import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
@@ -55,13 +58,16 @@
 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;
 import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder;
 import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor;
+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;
@@ -80,6 +86,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
@@ -662,6 +669,79 @@ public void methodWhenPostFilterMetaAnnotationThenFilters() {
 			.containsExactly("dave");
 	}
 
+	@Test
+	@WithMockUser(authorities = "airplane:read")
+	public void findByIdWhenAuthorizedResultThenAuthorizes() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		Flight flight = flights.findById("1");
+		assertThatNoException().isThrownBy(flight::getAltitude);
+		assertThatNoException().isThrownBy(flight::getSeats);
+	}
+
+	@Test
+	@WithMockUser(authorities = "seating:read")
+	public void findByIdWhenUnauthorizedResultThenDenies() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		Flight flight = flights.findById("1");
+		assertThatNoException().isThrownBy(flight::getSeats);
+		assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(flight::getAltitude);
+	}
+
+	@Test
+	@WithMockUser(authorities = "seating:read")
+	public void findAllWhenUnauthorizedResultThenDenies() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		flights.findAll().forEachRemaining((flight) -> {
+			assertThatNoException().isThrownBy(flight::getSeats);
+			assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(flight::getAltitude);
+		});
+	}
+
+	@Test
+	public void removeWhenAuthorizedResultThenRemoves() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		flights.remove("1");
+	}
+
+	@Test
+	@WithMockUser(authorities = "airplane:read")
+	public void findAllWhenPostFilterThenFilters() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		flights.findAll()
+			.forEachRemaining((flight) -> assertThat(flight.getPassengers()).extracting(Passenger::getName)
+				.doesNotContain("Kevin Mitnick"));
+	}
+
+	@Test
+	@WithMockUser(authorities = "airplane:read")
+	public void findAllWhenPreFilterThenFilters() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		flights.findAll().forEachRemaining((flight) -> {
+			flight.board(new ArrayList<>(List.of("John")));
+			assertThat(flight.getPassengers()).extracting(Passenger::getName).doesNotContain("John");
+			flight.board(new ArrayList<>(List.of("John Doe")));
+			assertThat(flight.getPassengers()).extracting(Passenger::getName).contains("John Doe");
+		});
+	}
+
+	@Test
+	@WithMockUser(authorities = "seating:read")
+	public void findAllWhenNestedPreAuthorizeThenAuthorizes() {
+		this.spring.register(AuthorizeResultConfig.class).autowire();
+		FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
+		flights.findAll().forEachRemaining((flight) -> {
+			List<Passenger> passengers = flight.getPassengers();
+			passengers.forEach((passenger) -> assertThatExceptionOfType(AccessDeniedException.class)
+				.isThrownBy(passenger::getName));
+		});
+	}
+
 	private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
 		return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
 	}
@@ -1061,4 +1141,118 @@ List<String> resultsContainDave(List<String> list) {
 
 	}
 
+	@EnableMethodSecurity
+	@Configuration
+	static class AuthorizeResultConfig {
+
+		@Bean
+		static Customizer<AuthorizationAdvisorProxyFactory> returnObject() {
+			return (proxyFactory) -> proxyFactory.setSkipProxy(AuthorizationAdvisorProxyFactory.SKIP_VALUE_TYPES);
+		}
+
+		@Bean
+		FlightRepository flights() {
+			FlightRepository flights = new FlightRepository();
+			Flight one = new Flight("1", 35000d, 35);
+			one.board(new ArrayList<>(List.of("Marie Curie", "Kevin Mitnick", "Ada Lovelace")));
+			flights.save(one);
+			Flight two = new Flight("2", 32000d, 72);
+			two.board(new ArrayList<>(List.of("Albert Einstein")));
+			flights.save(two);
+			return flights;
+		}
+
+		@Bean
+		RoleHierarchy roleHierarchy() {
+			return RoleHierarchyImpl.withRolePrefix("").role("airplane:read").implies("seating:read").build();
+		}
+
+	}
+
+	@AuthorizeReturnObject
+	static class FlightRepository {
+
+		private final Map<String, Flight> flights = new ConcurrentHashMap<>();
+
+		Iterator<Flight> findAll() {
+			return this.flights.values().iterator();
+		}
+
+		Flight findById(String id) {
+			return this.flights.get(id);
+		}
+
+		Flight save(Flight flight) {
+			this.flights.put(flight.getId(), flight);
+			return flight;
+		}
+
+		void remove(String id) {
+			this.flights.remove(id);
+		}
+
+	}
+
+	@AuthorizeReturnObject
+	static class Flight {
+
+		private final String id;
+
+		private final Double altitude;
+
+		private final Integer seats;
+
+		private final List<Passenger> passengers = new ArrayList<>();
+
+		Flight(String id, Double altitude, Integer seats) {
+			this.id = id;
+			this.altitude = altitude;
+			this.seats = seats;
+		}
+
+		String getId() {
+			return this.id;
+		}
+
+		@PreAuthorize("hasAuthority('airplane:read')")
+		Double getAltitude() {
+			return this.altitude;
+		}
+
+		@PreAuthorize("hasAuthority('seating:read')")
+		Integer getSeats() {
+			return this.seats;
+		}
+
+		@PostAuthorize("hasAuthority('seating:read')")
+		@PostFilter("filterObject.name != 'Kevin Mitnick'")
+		List<Passenger> getPassengers() {
+			return this.passengers;
+		}
+
+		@PreAuthorize("hasAuthority('seating:read')")
+		@PreFilter("filterObject.contains(' ')")
+		void board(List<String> passengers) {
+			for (String passenger : passengers) {
+				this.passengers.add(new Passenger(passenger));
+			}
+		}
+
+	}
+
+	public static class Passenger {
+
+		String name;
+
+		public Passenger(String name) {
+			this.name = name;
+		}
+
+		@PreAuthorize("hasAuthority('airplane:read')")
+		public String getName() {
+			return this.name;
+		}
+
+	}
+
 }