Skip to content

Commit f180aa3

Browse files
jzheauxjzheaux
committed
Add AuthorizationReturnObject
Closes spring-projectsgh-14597
1 parent 0d7aa07 commit f180aa3

File tree

8 files changed

+407
-2
lines changed

8 files changed

+407
-2
lines changed

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

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.security.authorization.method.aspectj;
17+
18+
import org.aopalliance.intercept.MethodInterceptor;
19+
import org.aopalliance.intercept.MethodInvocation;
20+
21+
import org.springframework.beans.factory.InitializingBean;
22+
import org.springframework.security.authorization.method.AuthorizeReturnObject;
23+
24+
/**
25+
* Concrete AspectJ aspect using Spring Security @AuthorizeReturnObject annotation.
26+
*
27+
* <p>
28+
* When using this aspect, you <i>must</i> annotate the implementation class
29+
* (and/or methods within that class), <i>not</i> the interface (if any) that
30+
* the class implements. AspectJ follows Java's rule that annotations on
31+
* interfaces are <i>not</i> inherited. This will vary from Spring AOP.
32+
*
33+
* @author Josh Cummings
34+
* @since 6.3
35+
*/
36+
public aspect AuthorizeReturnObjectAspect extends AbstractMethodInterceptorAspect {
37+
38+
/**
39+
* Matches the execution of any method with a AuthorizeReturnObject annotation.
40+
*/
41+
protected pointcut executionOfAnnotatedMethod() : execution(* *(..)) && @annotation(AuthorizeReturnObject);
42+
}

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

+15
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.util.ArrayList;
2020
import java.util.List;
2121

22+
import org.aopalliance.intercept.MethodInterceptor;
23+
2224
import org.springframework.aop.framework.AopInfrastructureBean;
2325
import org.springframework.beans.factory.ObjectProvider;
2426
import org.springframework.beans.factory.config.BeanDefinition;
@@ -28,6 +30,8 @@
2830
import org.springframework.core.annotation.AnnotationAwareOrderComparator;
2931
import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory;
3032
import org.springframework.security.authorization.method.AuthorizationAdvisor;
33+
import org.springframework.security.authorization.method.AuthorizationProxyMethodInterceptor;
34+
import org.springframework.security.config.Customizer;
3135

3236
@Configuration(proxyBeanMethods = false)
3337
final class AuthorizationProxyConfiguration implements AopInfrastructureBean {
@@ -41,4 +45,15 @@ static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider
4145
return new AuthorizationAdvisorProxyFactory(advisors);
4246
}
4347

48+
@Bean
49+
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
50+
static MethodInterceptor authorizationProxyMethodInterceptor(
51+
AuthorizationAdvisorProxyFactory authorizationProxyFactory,
52+
ObjectProvider<Customizer<AuthorizationProxyMethodInterceptor>> customizers) {
53+
AuthorizationProxyMethodInterceptor interceptor = new AuthorizationProxyMethodInterceptor(
54+
authorizationProxyFactory);
55+
customizers.forEach((customizer) -> customizer.customize(interceptor));
56+
return interceptor;
57+
}
58+
4459
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, B
3333
registerAsAdvisor("postAuthorizeAuthorization", registry);
3434
registerAsAdvisor("securedAuthorization", registry);
3535
registerAsAdvisor("jsr250Authorization", registry);
36+
registerAsAdvisor("authorizationProxy", registry);
3637
}
3738

3839
private void registerAsAdvisor(String prefix, BeanDefinitionRegistry registry) {

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

+3
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, B
6060
"postAuthorizeAspect$0", registry);
6161
registerBeanDefinition("securedAuthorizationMethodInterceptor",
6262
"org.springframework.security.authorization.method.aspectj.SecuredAspect", "securedAspect$0", registry);
63+
registerBeanDefinition("authorizeReturnObjectMethodInterceptor",
64+
"org.springframework.security.authorization.method.aspectj.AuthorizeReturnObjectAspect",
65+
"authorizeReturnObjectAspect$0", registry);
6366
}
6467

6568
private void registerBeanDefinition(String beanName, String aspectClassName, String aspectBeanName,

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

+211
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,13 @@
1919
import java.io.Serializable;
2020
import java.lang.annotation.Retention;
2121
import java.lang.annotation.RetentionPolicy;
22+
import java.lang.reflect.Method;
2223
import java.util.ArrayList;
2324
import java.util.Arrays;
25+
import java.util.Iterator;
2426
import java.util.List;
27+
import java.util.Map;
28+
import java.util.concurrent.ConcurrentHashMap;
2529
import java.util.function.Consumer;
2630
import java.util.function.Supplier;
2731

@@ -31,8 +35,11 @@
3135
import org.junit.jupiter.api.extension.ExtendWith;
3236

3337
import org.springframework.aop.Advisor;
38+
import org.springframework.aop.Pointcut;
3439
import org.springframework.aop.support.DefaultPointcutAdvisor;
3540
import org.springframework.aop.support.JdkRegexpMethodPointcut;
41+
import org.springframework.aop.support.Pointcuts;
42+
import org.springframework.aop.support.StaticMethodMatcherPointcut;
3643
import org.springframework.beans.factory.annotation.Autowired;
3744
import org.springframework.beans.factory.config.BeanDefinition;
3845
import org.springframework.context.annotation.AdviceMode;
@@ -60,8 +67,11 @@
6067
import org.springframework.security.authorization.AuthorizationManager;
6168
import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder;
6269
import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor;
70+
import org.springframework.security.authorization.method.AuthorizationProxyMethodInterceptor;
71+
import org.springframework.security.authorization.method.AuthorizeReturnObject;
6372
import org.springframework.security.authorization.method.MethodInvocationResult;
6473
import org.springframework.security.authorization.method.PrePostTemplateDefaults;
74+
import org.springframework.security.config.Customizer;
6575
import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig;
6676
import org.springframework.security.config.core.GrantedAuthorityDefaults;
6777
import org.springframework.security.config.test.SpringTestContext;
@@ -75,11 +85,13 @@
7585
import org.springframework.test.context.ContextConfiguration;
7686
import org.springframework.test.context.TestExecutionListeners;
7787
import org.springframework.test.context.junit.jupiter.SpringExtension;
88+
import org.springframework.util.ClassUtils;
7889
import org.springframework.web.context.ConfigurableWebApplicationContext;
7990
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
8091

8192
import static org.assertj.core.api.Assertions.assertThat;
8293
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
94+
import static org.assertj.core.api.Assertions.assertThatNoException;
8395
import static org.mockito.ArgumentMatchers.any;
8496
import static org.mockito.Mockito.atLeastOnce;
8597
import static org.mockito.Mockito.mock;
@@ -662,6 +674,79 @@ public void methodWhenPostFilterMetaAnnotationThenFilters() {
662674
.containsExactly("dave");
663675
}
664676

677+
@Test
678+
@WithMockUser(authorities = "airplane:read")
679+
public void findByIdWhenAuthorizedResultThenAuthorizes() {
680+
this.spring.register(AuthorizeResultConfig.class).autowire();
681+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
682+
Flight flight = flights.findById("1");
683+
assertThatNoException().isThrownBy(flight::getAltitude);
684+
assertThatNoException().isThrownBy(flight::getSeats);
685+
}
686+
687+
@Test
688+
@WithMockUser(authorities = "seating:read")
689+
public void findByIdWhenUnauthorizedResultThenDenies() {
690+
this.spring.register(AuthorizeResultConfig.class).autowire();
691+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
692+
Flight flight = flights.findById("1");
693+
assertThatNoException().isThrownBy(flight::getSeats);
694+
assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(flight::getAltitude);
695+
}
696+
697+
@Test
698+
@WithMockUser(authorities = "seating:read")
699+
public void findAllWhenUnauthorizedResultThenDenies() {
700+
this.spring.register(AuthorizeResultConfig.class).autowire();
701+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
702+
flights.findAll().forEachRemaining((flight) -> {
703+
assertThatNoException().isThrownBy(flight::getSeats);
704+
assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(flight::getAltitude);
705+
});
706+
}
707+
708+
@Test
709+
public void removeWhenAuthorizedResultThenRemoves() {
710+
this.spring.register(AuthorizeResultConfig.class).autowire();
711+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
712+
flights.remove("1");
713+
}
714+
715+
@Test
716+
@WithMockUser(authorities = "airplane:read")
717+
public void findAllWhenPostFilterThenFilters() {
718+
this.spring.register(AuthorizeResultConfig.class).autowire();
719+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
720+
flights.findAll()
721+
.forEachRemaining((flight) -> assertThat(flight.getPassengers()).extracting(Passenger::getName)
722+
.doesNotContain("Kevin Mitnick"));
723+
}
724+
725+
@Test
726+
@WithMockUser(authorities = "airplane:read")
727+
public void findAllWhenPreFilterThenFilters() {
728+
this.spring.register(AuthorizeResultConfig.class).autowire();
729+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
730+
flights.findAll().forEachRemaining((flight) -> {
731+
flight.board(new ArrayList<>(List.of("John")));
732+
assertThat(flight.getPassengers()).extracting(Passenger::getName).doesNotContain("John");
733+
flight.board(new ArrayList<>(List.of("John Doe")));
734+
assertThat(flight.getPassengers()).extracting(Passenger::getName).contains("John Doe");
735+
});
736+
}
737+
738+
@Test
739+
@WithMockUser(authorities = "seating:read")
740+
public void findAllWhenNestedPreAuthorizeThenAuthorizes() {
741+
this.spring.register(AuthorizeResultConfig.class).autowire();
742+
FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class);
743+
flights.findAll().forEachRemaining((flight) -> {
744+
List<Passenger> passengers = flight.getPassengers();
745+
passengers.forEach((passenger) -> assertThatExceptionOfType(AccessDeniedException.class)
746+
.isThrownBy(passenger::getName));
747+
});
748+
}
749+
665750
private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
666751
return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
667752
}
@@ -1061,4 +1146,130 @@ List<String> resultsContainDave(List<String> list) {
10611146

10621147
}
10631148

1149+
@EnableMethodSecurity
1150+
@Configuration
1151+
static class AuthorizeResultConfig {
1152+
1153+
@Bean
1154+
static Customizer<AuthorizationProxyMethodInterceptor> returnObject() {
1155+
return (interceptor) -> {
1156+
Pointcut pointcut = interceptor.getPointcut();
1157+
interceptor.setPointcut(Pointcuts.intersection(new NotValueReturnTypePointcut(), pointcut));
1158+
};
1159+
}
1160+
1161+
@Bean
1162+
FlightRepository flights() {
1163+
FlightRepository flights = new FlightRepository();
1164+
Flight one = new Flight("1", 35000d, 35);
1165+
one.board(new ArrayList<>(List.of("Marie Curie", "Kevin Mitnick", "Ada Lovelace")));
1166+
flights.save(one);
1167+
Flight two = new Flight("2", 32000d, 72);
1168+
two.board(new ArrayList<>(List.of("Albert Einstein")));
1169+
flights.save(two);
1170+
return flights;
1171+
}
1172+
1173+
@Bean
1174+
RoleHierarchy roleHierarchy() {
1175+
return RoleHierarchyImpl.withRolePrefix("").role("airplane:read").implies("seating:read").build();
1176+
}
1177+
1178+
private static class NotValueReturnTypePointcut extends StaticMethodMatcherPointcut {
1179+
1180+
@Override
1181+
public boolean matches(Method method, Class<?> targetClass) {
1182+
return !ClassUtils.isSimpleValueType(method.getReturnType());
1183+
}
1184+
1185+
}
1186+
1187+
}
1188+
1189+
@AuthorizeReturnObject
1190+
static class FlightRepository {
1191+
1192+
private final Map<String, Flight> flights = new ConcurrentHashMap<>();
1193+
1194+
Iterator<Flight> findAll() {
1195+
return this.flights.values().iterator();
1196+
}
1197+
1198+
Flight findById(String id) {
1199+
return this.flights.get(id);
1200+
}
1201+
1202+
Flight save(Flight flight) {
1203+
this.flights.put(flight.getId(), flight);
1204+
return flight;
1205+
}
1206+
1207+
void remove(String id) {
1208+
this.flights.remove(id);
1209+
}
1210+
1211+
}
1212+
1213+
@AuthorizeReturnObject
1214+
static class Flight {
1215+
1216+
private final String id;
1217+
1218+
private final Double altitude;
1219+
1220+
private final Integer seats;
1221+
1222+
private final List<Passenger> passengers = new ArrayList<>();
1223+
1224+
Flight(String id, Double altitude, Integer seats) {
1225+
this.id = id;
1226+
this.altitude = altitude;
1227+
this.seats = seats;
1228+
}
1229+
1230+
String getId() {
1231+
return this.id;
1232+
}
1233+
1234+
@PreAuthorize("hasAuthority('airplane:read')")
1235+
Double getAltitude() {
1236+
return this.altitude;
1237+
}
1238+
1239+
@PreAuthorize("hasAuthority('seating:read')")
1240+
Integer getSeats() {
1241+
return this.seats;
1242+
}
1243+
1244+
@PostAuthorize("hasAuthority('seating:read')")
1245+
@PostFilter("filterObject.name != 'Kevin Mitnick'")
1246+
List<Passenger> getPassengers() {
1247+
return this.passengers;
1248+
}
1249+
1250+
@PreAuthorize("hasAuthority('seating:read')")
1251+
@PreFilter("filterObject.contains(' ')")
1252+
void board(List<String> passengers) {
1253+
for (String passenger : passengers) {
1254+
this.passengers.add(new Passenger(passenger));
1255+
}
1256+
}
1257+
1258+
}
1259+
1260+
public static class Passenger {
1261+
1262+
String name;
1263+
1264+
public Passenger(String name) {
1265+
this.name = name;
1266+
}
1267+
1268+
@PreAuthorize("hasAuthority('airplane:read')")
1269+
public String getName() {
1270+
return this.name;
1271+
}
1272+
1273+
}
1274+
10641275
}

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,14 @@ public enum AuthorizationInterceptorsOrder {
4343

4444
JSR250,
4545

46-
POST_AUTHORIZE,
46+
SECURE_RESULT(450),
47+
48+
POST_AUTHORIZE(500),
4749

4850
/**
4951
* {@link PostFilterAuthorizationMethodInterceptor}
5052
*/
51-
POST_FILTER,
53+
POST_FILTER(600),
5254

5355
LAST(Integer.MAX_VALUE);
5456

0 commit comments

Comments
 (0)