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,6 +30,8 @@
 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 {
@@ -41,4 +45,15 @@ static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider
 		return new AuthorizationAdvisorProxyFactory(advisors);
 	}
 
+	@Bean
+	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
+	static MethodInterceptor authorizationProxyMethodInterceptor(
+			AuthorizationAdvisorProxyFactory authorizationProxyFactory,
+			ObjectProvider<Customizer<AuthorizationProxyMethodInterceptor>> customizers) {
+		AuthorizationProxyMethodInterceptor interceptor = new AuthorizationProxyMethodInterceptor(
+				authorizationProxyFactory);
+		customizers.forEach((customizer) -> customizer.customize(interceptor));
+		return interceptor;
+	}
+
 }

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

@@ -19,9 +19,13 @@
 import java.io.Serializable;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.lang.reflect.Method;
 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;
 
@@ -31,8 +35,11 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import org.springframework.aop.Advisor;
+import org.springframework.aop.Pointcut;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 import org.springframework.aop.support.JdkRegexpMethodPointcut;
+import org.springframework.aop.support.Pointcuts;
+import org.springframework.aop.support.StaticMethodMatcherPointcut;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.context.annotation.AdviceMode;
@@ -60,8 +67,11 @@
 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.AuthorizationProxyMethodInterceptor;
+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;
@@ -75,11 +85,13 @@
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.TestExecutionListeners;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
+import org.springframework.util.ClassUtils;
 import org.springframework.web.context.ConfigurableWebApplicationContext;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 
 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 +674,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 +1146,129 @@ List<String> resultsContainDave(List<String> list) {
 
 	}
 
+	@EnableMethodSecurity
+	@Configuration
+	static class AuthorizeResultConfig {
+
+		@Bean
+		static Customizer<AuthorizationProxyMethodInterceptor> returnObject() {
+			return (interceptor) -> {
+				Pointcut pointcut = interceptor.getPointcut();
+				interceptor.setPointcut(Pointcuts.intersection(new NotValueReturnTypePointcut(), pointcut));
+			};
+		}
+
+		@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();
+		}
+
+		private static class NotValueReturnTypePointcut extends StaticMethodMatcherPointcut {
+
+			@Override
+			public boolean matches(Method method, Class<?> targetClass) {
+				return !ClassUtils.isSimpleValueType(method.getReturnType());
+			}
+
+		}
+	}
+
+	@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;
+		}
+
+	}
+
 }

core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java

@@ -43,12 +43,14 @@ public enum AuthorizationInterceptorsOrder {
 
 	JSR250,
 
-	POST_AUTHORIZE,
+	SECURE_RESULT(450),
+
+	POST_AUTHORIZE(500),
 
 	/**
 	 * {@link PostFilterAuthorizationMethodInterceptor}
 	 */
-	POST_FILTER,
+	POST_FILTER(600),
 
 	LAST(Integer.MAX_VALUE);