Skip to content

Commit a516734

Browse files
jzheauxAyush Kohli
jzheaux
authored and
Ayush Kohli
committed
Use Interceptors instead of Advice
- Interceptor is a more descriptive term for what method security is doing - This also allows the code to follow a delegate pattern that unifies both before-method and after- method authorization Issue spring-projectsgh-9289
1 parent 8e5d8b0 commit a516734

File tree

37 files changed

+1004
-1238
lines changed

37 files changed

+1004
-1238
lines changed

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

Lines changed: 40 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,11 @@
1616

1717
package org.springframework.security.config.annotation.method.configuration;
1818

19-
import java.lang.annotation.Annotation;
2019
import java.util.ArrayList;
2120
import java.util.List;
2221
import java.util.Map;
2322

24-
import javax.annotation.security.DenyAll;
25-
import javax.annotation.security.PermitAll;
26-
import javax.annotation.security.RolesAllowed;
27-
28-
import org.springframework.aop.Pointcut;
29-
import org.springframework.aop.support.ComposablePointcut;
3023
import org.springframework.aop.support.DefaultPointcutAdvisor;
31-
import org.springframework.aop.support.Pointcuts;
32-
import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;
3324
import org.springframework.beans.factory.InitializingBean;
3425
import org.springframework.beans.factory.annotation.Autowired;
3526
import org.springframework.beans.factory.config.BeanDefinition;
@@ -39,26 +30,16 @@
3930
import org.springframework.context.annotation.Role;
4031
import org.springframework.core.annotation.AnnotationAttributes;
4132
import org.springframework.core.type.AnnotationMetadata;
42-
import org.springframework.security.access.annotation.Secured;
4333
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
4434
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
45-
import org.springframework.security.access.prepost.PostAuthorize;
46-
import org.springframework.security.access.prepost.PostFilter;
47-
import org.springframework.security.access.prepost.PreAuthorize;
48-
import org.springframework.security.access.prepost.PreFilter;
49-
import org.springframework.security.authorization.method.AuthorizationManagerMethodAfterAdvice;
50-
import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice;
51-
import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice;
52-
import org.springframework.security.authorization.method.AuthorizationMethodBeforeAdvice;
5335
import org.springframework.security.authorization.method.AuthorizationMethodInterceptor;
54-
import org.springframework.security.authorization.method.DelegatingAuthorizationMethodAfterAdvice;
55-
import org.springframework.security.authorization.method.DelegatingAuthorizationMethodBeforeAdvice;
36+
import org.springframework.security.authorization.method.AuthorizationMethodInterceptors;
37+
import org.springframework.security.authorization.method.DelegatingAuthorizationMethodInterceptor;
5638
import org.springframework.security.authorization.method.Jsr250AuthorizationManager;
57-
import org.springframework.security.authorization.method.MethodAuthorizationContext;
5839
import org.springframework.security.authorization.method.PostAuthorizeAuthorizationManager;
59-
import org.springframework.security.authorization.method.PostFilterAuthorizationMethodAfterAdvice;
40+
import org.springframework.security.authorization.method.PostFilterAuthorizationMethodInterceptor;
6041
import org.springframework.security.authorization.method.PreAuthorizeAuthorizationManager;
61-
import org.springframework.security.authorization.method.PreFilterAuthorizationMethodBeforeAdvice;
42+
import org.springframework.security.authorization.method.PreFilterAuthorizationMethodInterceptor;
6243
import org.springframework.security.authorization.method.SecuredAuthorizationManager;
6344
import org.springframework.security.config.core.GrantedAuthorityDefaults;
6445
import org.springframework.util.Assert;
@@ -79,30 +60,19 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean
7960

8061
private GrantedAuthorityDefaults grantedAuthorityDefaults;
8162

82-
private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> authorizationMethodBeforeAdvice;
83-
84-
private AuthorizationMethodAfterAdvice<MethodAuthorizationContext> authorizationMethodAfterAdvice;
63+
private AuthorizationMethodInterceptor interceptor;
8564

8665
private AnnotationAttributes enableMethodSecurity;
8766

8867
@Bean
8968
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
90-
DefaultPointcutAdvisor methodSecurityAdvisor(AuthorizationMethodInterceptor interceptor) {
91-
AuthorizationMethodBeforeAdvice<?> beforeAdvice = getAuthorizationMethodBeforeAdvice();
92-
AuthorizationMethodAfterAdvice<?> afterAdvice = getAuthorizationMethodAfterAdvice();
93-
Pointcut pointcut = Pointcuts.union(beforeAdvice.getPointcut(), afterAdvice.getPointcut());
94-
DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(pointcut, interceptor);
69+
DefaultPointcutAdvisor methodSecurityAdvisor() {
70+
AuthorizationMethodInterceptor interceptor = getInterceptor();
71+
DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(interceptor.getPointcut(), interceptor);
9572
advisor.setOrder(order());
9673
return advisor;
9774
}
9875

99-
@Bean
100-
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
101-
AuthorizationMethodInterceptor authorizationMethodInterceptor() {
102-
return new AuthorizationMethodInterceptor(getAuthorizationMethodBeforeAdvice(),
103-
getAuthorizationMethodAfterAdvice());
104-
}
105-
10676
private MethodSecurityExpressionHandler getMethodSecurityExpressionHandler() {
10777
if (this.methodSecurityExpressionHandler == null) {
10878
DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler();
@@ -124,15 +94,18 @@ void setGrantedAuthorityDefaults(GrantedAuthorityDefaults grantedAuthorityDefaul
12494
this.grantedAuthorityDefaults = grantedAuthorityDefaults;
12595
}
12696

127-
private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> getAuthorizationMethodBeforeAdvice() {
128-
if (this.authorizationMethodBeforeAdvice == null) {
129-
this.authorizationMethodBeforeAdvice = createDefaultAuthorizationMethodBeforeAdvice();
97+
private AuthorizationMethodInterceptor getInterceptor() {
98+
if (this.interceptor != null) {
99+
return this.interceptor;
130100
}
131-
return this.authorizationMethodBeforeAdvice;
101+
List<AuthorizationMethodInterceptor> interceptors = new ArrayList<>();
102+
interceptors.addAll(createDefaultAuthorizationMethodBeforeAdvice());
103+
interceptors.addAll(createDefaultAuthorizationMethodAfterAdvice());
104+
return new DelegatingAuthorizationMethodInterceptor(interceptors);
132105
}
133106

134-
private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> createDefaultAuthorizationMethodBeforeAdvice() {
135-
List<AuthorizationMethodBeforeAdvice<MethodAuthorizationContext>> beforeAdvices = new ArrayList<>();
107+
private List<AuthorizationMethodInterceptor> createDefaultAuthorizationMethodBeforeAdvice() {
108+
List<AuthorizationMethodInterceptor> beforeAdvices = new ArrayList<>();
136109
beforeAdvices.add(getPreFilterAuthorizationMethodBeforeAdvice());
137110
beforeAdvices.add(getPreAuthorizeAuthorizationMethodBeforeAdvice());
138111
if (securedEnabled()) {
@@ -141,79 +114,55 @@ private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> createDefaul
141114
if (jsr250Enabled()) {
142115
beforeAdvices.add(getJsr250AuthorizationMethodBeforeAdvice());
143116
}
144-
return new DelegatingAuthorizationMethodBeforeAdvice<>(beforeAdvices);
117+
return beforeAdvices;
145118
}
146119

147-
private PreFilterAuthorizationMethodBeforeAdvice getPreFilterAuthorizationMethodBeforeAdvice() {
148-
Pointcut pointcut = forAnnotation(PreFilter.class);
149-
PreFilterAuthorizationMethodBeforeAdvice preFilterBeforeAdvice = new PreFilterAuthorizationMethodBeforeAdvice(
150-
pointcut);
151-
preFilterBeforeAdvice.setExpressionHandler(getMethodSecurityExpressionHandler());
152-
return preFilterBeforeAdvice;
120+
private PreFilterAuthorizationMethodInterceptor getPreFilterAuthorizationMethodBeforeAdvice() {
121+
PreFilterAuthorizationMethodInterceptor interceptor = new PreFilterAuthorizationMethodInterceptor();
122+
interceptor.setExpressionHandler(getMethodSecurityExpressionHandler());
123+
return interceptor;
153124
}
154125

155-
private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> getPreAuthorizeAuthorizationMethodBeforeAdvice() {
156-
Pointcut pointcut = forAnnotation(PreAuthorize.class);
126+
private AuthorizationMethodInterceptor getPreAuthorizeAuthorizationMethodBeforeAdvice() {
157127
PreAuthorizeAuthorizationManager authorizationManager = new PreAuthorizeAuthorizationManager();
158128
authorizationManager.setExpressionHandler(getMethodSecurityExpressionHandler());
159-
return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager);
129+
return AuthorizationMethodInterceptors.preAuthorize(authorizationManager);
160130
}
161131

162-
private AuthorizationManagerMethodBeforeAdvice<MethodAuthorizationContext> getSecuredAuthorizationMethodBeforeAdvice() {
163-
Pointcut pointcut = forAnnotation(Secured.class);
164-
SecuredAuthorizationManager authorizationManager = new SecuredAuthorizationManager();
165-
return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager);
132+
private AuthorizationMethodInterceptor getSecuredAuthorizationMethodBeforeAdvice() {
133+
return AuthorizationMethodInterceptors.secured(new SecuredAuthorizationManager());
166134
}
167135

168-
private AuthorizationManagerMethodBeforeAdvice<MethodAuthorizationContext> getJsr250AuthorizationMethodBeforeAdvice() {
169-
Pointcut pointcut = new ComposablePointcut(forAnnotation(DenyAll.class)).union(forAnnotation(PermitAll.class))
170-
.union(forAnnotation(RolesAllowed.class));
136+
private AuthorizationMethodInterceptor getJsr250AuthorizationMethodBeforeAdvice() {
171137
Jsr250AuthorizationManager authorizationManager = new Jsr250AuthorizationManager();
172138
if (this.grantedAuthorityDefaults != null) {
173139
authorizationManager.setRolePrefix(this.grantedAuthorityDefaults.getRolePrefix());
174140
}
175-
return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager);
141+
return AuthorizationMethodInterceptors.jsr250(authorizationManager);
176142
}
177143

178144
@Autowired(required = false)
179-
void setAuthorizationMethodBeforeAdvice(
180-
AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> authorizationMethodBeforeAdvice) {
181-
this.authorizationMethodBeforeAdvice = authorizationMethodBeforeAdvice;
182-
}
183-
184-
private AuthorizationMethodAfterAdvice<MethodAuthorizationContext> getAuthorizationMethodAfterAdvice() {
185-
if (this.authorizationMethodAfterAdvice == null) {
186-
this.authorizationMethodAfterAdvice = createDefaultAuthorizationMethodAfterAdvice();
187-
}
188-
return this.authorizationMethodAfterAdvice;
145+
void setAuthorizationMethodInterceptor(AuthorizationMethodInterceptor interceptor) {
146+
this.interceptor = interceptor;
189147
}
190148

191-
private AuthorizationMethodAfterAdvice<MethodAuthorizationContext> createDefaultAuthorizationMethodAfterAdvice() {
192-
List<AuthorizationMethodAfterAdvice<MethodAuthorizationContext>> afterAdvices = new ArrayList<>();
149+
private List<AuthorizationMethodInterceptor> createDefaultAuthorizationMethodAfterAdvice() {
150+
List<AuthorizationMethodInterceptor> afterAdvices = new ArrayList<>();
193151
afterAdvices.add(getPostFilterAuthorizationMethodAfterAdvice());
194152
afterAdvices.add(getPostAuthorizeAuthorizationMethodAfterAdvice());
195-
return new DelegatingAuthorizationMethodAfterAdvice<>(afterAdvices);
153+
return afterAdvices;
196154
}
197155

198-
private PostFilterAuthorizationMethodAfterAdvice getPostFilterAuthorizationMethodAfterAdvice() {
199-
Pointcut pointcut = forAnnotation(PostFilter.class);
200-
PostFilterAuthorizationMethodAfterAdvice postFilterAfterAdvice = new PostFilterAuthorizationMethodAfterAdvice(
201-
pointcut);
202-
postFilterAfterAdvice.setExpressionHandler(getMethodSecurityExpressionHandler());
203-
return postFilterAfterAdvice;
156+
private AuthorizationMethodInterceptor getPostFilterAuthorizationMethodAfterAdvice() {
157+
PostFilterAuthorizationMethodInterceptor interceptor = new PostFilterAuthorizationMethodInterceptor();
158+
interceptor.setExpressionHandler(getMethodSecurityExpressionHandler());
159+
return interceptor;
204160
}
205161

206-
private AuthorizationManagerMethodAfterAdvice<MethodAuthorizationContext> getPostAuthorizeAuthorizationMethodAfterAdvice() {
207-
Pointcut pointcut = forAnnotation(PostAuthorize.class);
162+
private AuthorizationMethodInterceptor getPostAuthorizeAuthorizationMethodAfterAdvice() {
208163
PostAuthorizeAuthorizationManager authorizationManager = new PostAuthorizeAuthorizationManager();
209164
authorizationManager.setExpressionHandler(getMethodSecurityExpressionHandler());
210-
return new AuthorizationManagerMethodAfterAdvice<>(pointcut, authorizationManager);
211-
}
212-
213-
@Autowired(required = false)
214-
void setAuthorizationMethodAfterAdvice(
215-
AuthorizationMethodAfterAdvice<MethodAuthorizationContext> authorizationMethodAfterAdvice) {
216-
this.authorizationMethodAfterAdvice = authorizationMethodAfterAdvice;
165+
return AuthorizationMethodInterceptors.postAuthorize(authorizationManager);
217166
}
218167

219168
@Override
@@ -227,7 +176,7 @@ public void afterPropertiesSet() throws Exception {
227176
if (!securedEnabled() && !jsr250Enabled()) {
228177
return;
229178
}
230-
Assert.isNull(this.authorizationMethodBeforeAdvice,
179+
Assert.isNull(this.interceptor,
231180
"You have specified your own advice, meaning that the annotation attributes securedEnabled and jsr250Enabled will be ignored. Please choose one or the other.");
232181
}
233182

@@ -243,9 +192,4 @@ private int order() {
243192
return this.enableMethodSecurity.getNumber("order");
244193
}
245194

246-
private Pointcut forAnnotation(Class<? extends Annotation> annotationClass) {
247-
return Pointcuts.union(new AnnotationMatchingPointcut(annotationClass, true),
248-
new AnnotationMatchingPointcut(null, annotationClass, true));
249-
}
250-
251195
}

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

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818

1919
import java.io.Serializable;
2020
import java.util.ArrayList;
21+
import java.util.Arrays;
2122
import java.util.List;
2223
import java.util.function.Supplier;
2324

25+
import org.aopalliance.intercept.MethodInvocation;
2426
import org.junit.Rule;
2527
import org.junit.Test;
2628
import org.junit.runner.RunWith;
@@ -38,10 +40,8 @@
3840
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
3941
import org.springframework.security.authorization.AuthorizationDecision;
4042
import org.springframework.security.authorization.AuthorizationManager;
41-
import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice;
42-
import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice;
43-
import org.springframework.security.authorization.method.AuthorizationMethodBeforeAdvice;
44-
import org.springframework.security.authorization.method.MethodAuthorizationContext;
43+
import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor;
44+
import org.springframework.security.authorization.method.AuthorizationMethodInterceptor;
4545
import org.springframework.security.config.test.SpringTestRule;
4646
import org.springframework.security.core.Authentication;
4747
import org.springframework.security.test.context.annotation.SecurityTestExecutionListeners;
@@ -273,6 +273,43 @@ public void rolesAllowedUserWhenRoleUserThenPasses() {
273273
this.businessService.rolesAllowedUser();
274274
}
275275

276+
@WithMockUser(roles = { "ADMIN", "USER" })
277+
@Test
278+
public void manyAnnotationsWhenMeetsConditionsThenReturnsFilteredList() throws Exception {
279+
List<String> names = Arrays.asList("harold", "jonathan", "pete", "bo");
280+
this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire();
281+
List<String> filtered = this.methodSecurityService.manyAnnotations(new ArrayList<>(names));
282+
assertThat(filtered).hasSize(2);
283+
assertThat(filtered).containsExactly("harold", "jonathan");
284+
}
285+
286+
@WithMockUser
287+
@Test
288+
public void manyAnnotationsWhenUserThenFails() {
289+
List<String> names = Arrays.asList("harold", "jonathan", "pete", "bo");
290+
this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire();
291+
assertThatExceptionOfType(AccessDeniedException.class)
292+
.isThrownBy(() -> this.methodSecurityService.manyAnnotations(new ArrayList<>(names)));
293+
}
294+
295+
@WithMockUser
296+
@Test
297+
public void manyAnnotationsWhenShortListThenFails() {
298+
List<String> names = Arrays.asList("harold", "jonathan", "pete");
299+
this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire();
300+
assertThatExceptionOfType(AccessDeniedException.class)
301+
.isThrownBy(() -> this.methodSecurityService.manyAnnotations(new ArrayList<>(names)));
302+
}
303+
304+
@WithMockUser(roles = "ADMIN")
305+
@Test
306+
public void manyAnnotationsWhenAdminThenFails() {
307+
List<String> names = Arrays.asList("harold", "jonathan", "pete", "bo");
308+
this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire();
309+
assertThatExceptionOfType(AccessDeniedException.class)
310+
.isThrownBy(() -> this.methodSecurityService.manyAnnotations(new ArrayList<>(names)));
311+
}
312+
276313
@Test
277314
public void configureWhenCustomAdviceAndSecureEnabledThenException() {
278315
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(() -> this.spring
@@ -338,12 +375,12 @@ public boolean hasPermission(Authentication authentication, Serializable targetI
338375
static class CustomAuthorizationManagerBeforeAdviceConfig {
339376

340377
@Bean
341-
AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> customBeforeAdvice() {
342-
JdkRegexpMethodPointcut methodMatcher = new JdkRegexpMethodPointcut();
343-
methodMatcher.setPattern(".*MethodSecurityServiceImpl.*securedUser");
344-
AuthorizationManager<MethodAuthorizationContext> authorizationManager = (a,
378+
AuthorizationMethodInterceptor customBeforeAdvice() {
379+
JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut();
380+
pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser");
381+
AuthorizationManager<MethodInvocation> authorizationManager = (a,
345382
o) -> new AuthorizationDecision("bob".equals(a.get().getName()));
346-
return new AuthorizationManagerMethodBeforeAdvice<>(methodMatcher, authorizationManager);
383+
return new AuthorizationManagerBeforeMethodInterceptor(pointcut, authorizationManager);
347384
}
348385

349386
}
@@ -352,25 +389,26 @@ AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> customBeforeAdvice()
352389
static class CustomAuthorizationManagerAfterAdviceConfig {
353390

354391
@Bean
355-
AuthorizationMethodAfterAdvice<MethodAuthorizationContext> customAfterAdvice() {
392+
393+
AuthorizationMethodInterceptor customAfterAdvice() {
356394
JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut();
357395
pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser");
358-
return new AuthorizationMethodAfterAdvice<MethodAuthorizationContext>() {
396+
AuthorizationMethodInterceptor interceptor = new AuthorizationMethodInterceptor() {
359397
@Override
360398
public Pointcut getPointcut() {
361399
return pointcut;
362400
}
363401

364402
@Override
365-
public Object after(Supplier<Authentication> authentication,
366-
MethodAuthorizationContext methodAuthorizationContext, Object returnedObject) {
403+
public Object invoke(Supplier<Authentication> authentication, MethodInvocation mi) {
367404
Authentication auth = authentication.get();
368405
if ("bob".equals(auth.getName())) {
369406
return "granted";
370407
}
371408
throw new AccessDeniedException("Access Denied for User '" + auth.getName() + "'");
372409
}
373410
};
411+
return interceptor;
374412
}
375413

376414
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,16 @@
1616

1717
package org.springframework.security.config.annotation.method.configuration;
1818

19+
import java.util.List;
20+
1921
import javax.annotation.security.DenyAll;
2022
import javax.annotation.security.PermitAll;
2123

2224
import org.springframework.security.access.annotation.Secured;
2325
import org.springframework.security.access.prepost.PostAuthorize;
26+
import org.springframework.security.access.prepost.PostFilter;
2427
import org.springframework.security.access.prepost.PreAuthorize;
28+
import org.springframework.security.access.prepost.PreFilter;
2529
import org.springframework.security.core.Authentication;
2630
import org.springframework.security.core.parameters.P;
2731

@@ -69,4 +73,11 @@ public interface MethodSecurityService {
6973
@PostAuthorize("#o?.contains('grant')")
7074
String postAnnotation(@P("o") String object);
7175

76+
@PreFilter("filterObject.length > 3")
77+
@PreAuthorize("hasRole('ADMIN')")
78+
@Secured("ROLE_USER")
79+
@PostFilter("filterObject.length > 5")
80+
@PostAuthorize("returnObject.size > 1")
81+
List<String> manyAnnotations(List<String> array);
82+
7283
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.security.config.annotation.method.configuration;
1818

19+
import java.util.List;
20+
1921
import org.springframework.security.core.Authentication;
2022
import org.springframework.security.core.context.SecurityContextHolder;
2123

@@ -86,4 +88,9 @@ public String postAnnotation(String object) {
8688
return null;
8789
}
8890

91+
@Override
92+
public List<String> manyAnnotations(List<String> object) {
93+
return object;
94+
}
95+
8996
}

0 commit comments

Comments
 (0)