diff --git a/aspects/src/main/java/org/springframework/security/authorization/method/aspectj/AuthorizeReturnObjectAspect.aj b/aspects/src/main/java/org/springframework/security/authorization/method/aspectj/AuthorizeReturnObjectAspect.aj new file mode 100644 index 00000000000..1c6c9f29d4d --- /dev/null +++ b/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. + * + *

+ * When using this aspect, you must annotate the implementation class + * (and/or methods within that class), not the interface (if any) that + * the class implements. AspectJ follows Java's rule that annotations on + * interfaces are not 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); +} 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 6aa247c6670..cc53ba2d7c9 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 @@ -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> customizers) { + AuthorizationProxyMethodInterceptor interceptor = new AuthorizationProxyMethodInterceptor( + authorizationProxyFactory); + customizers.forEach((customizer) -> customizer.customize(interceptor)); + return interceptor; + } + } diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java index 409f6fa1ea2..2495188b866 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java +++ b/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) { diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAspectJAutoProxyRegistrar.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAspectJAutoProxyRegistrar.java index 16a31924d9a..8961bf99697 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAspectJAutoProxyRegistrar.java +++ b/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, 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 f653d3c5875..7d2ac3df84e 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 @@ -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 passengers = flight.getPassengers(); + passengers.forEach((passenger) -> assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(passenger::getName)); + }); + } + private static Consumer disallowBeanOverriding() { return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false); } @@ -1061,4 +1146,130 @@ List resultsContainDave(List list) { } + @EnableMethodSecurity + @Configuration + static class AuthorizeResultConfig { + + @Bean + static Customizer 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 flights = new ConcurrentHashMap<>(); + + Iterator 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 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 getPassengers() { + return this.passengers; + } + + @PreAuthorize("hasAuthority('seating:read')") + @PreFilter("filterObject.contains(' ')") + void board(List 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; + } + + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java index da6a26bf6e0..c92d39db185 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java +++ b/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); diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationProxyMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationProxyMethodInterceptor.java new file mode 100644 index 00000000000..8185203a35b --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationProxyMethodInterceptor.java @@ -0,0 +1,103 @@ +/* + * 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; + +import java.lang.reflect.Method; +import java.util.function.Predicate; + +import org.aopalliance.aop.Advice; +import org.aopalliance.intercept.MethodInvocation; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.Pointcuts; +import org.springframework.aop.support.StaticMethodMatcherPointcut; +import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory; +import org.springframework.security.authorization.AuthorizationProxyFactory; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; + +public final class AuthorizationProxyMethodInterceptor implements AuthorizationAdvisor { + + private final AuthorizationProxyFactory authorizationProxyFactory; + + private Pointcut pointcut = Pointcuts.intersection( + new MethodReturnTypePointcut(Predicate.not(ClassUtils::isVoidType)), + AuthorizationMethodPointcuts.forAnnotations(AuthorizeReturnObject.class)); + + private int order = AuthorizationInterceptorsOrder.SECURE_RESULT.getOrder(); + + public AuthorizationProxyMethodInterceptor(AuthorizationAdvisorProxyFactory authorizationProxyFactory) { + Assert.notNull(authorizationProxyFactory, "authorizationManager cannot be null"); + this.authorizationProxyFactory = authorizationProxyFactory.withAdvisors(this); + } + + @Override + public Object invoke(MethodInvocation mi) throws Throwable { + Object result = mi.proceed(); + if (result == null) { + return null; + } + return this.authorizationProxyFactory.proxy(result); + } + + @Override + public int getOrder() { + return this.order; + } + + public void setOrder(int order) { + this.order = order; + } + + /** + * {@inheritDoc} + */ + @Override + public Pointcut getPointcut() { + return this.pointcut; + } + + public void setPointcut(Pointcut pointcut) { + this.pointcut = pointcut; + } + + @Override + public Advice getAdvice() { + return this; + } + + @Override + public boolean isPerInstance() { + return true; + } + + static final class MethodReturnTypePointcut extends StaticMethodMatcherPointcut { + + private final Predicate> returnTypeMatches; + + MethodReturnTypePointcut(Predicate> returnTypeMatches) { + this.returnTypeMatches = returnTypeMatches; + } + + @Override + public boolean matches(Method method, Class targetClass) { + return this.returnTypeMatches.test(method.getReturnType()); + } + + } + +} diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObject.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObject.java new file mode 100644 index 00000000000..38c256c29be --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObject.java @@ -0,0 +1,28 @@ +/* + * 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; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.METHOD }) +public @interface AuthorizeReturnObject { + +}