Skip to content

Commit 11a2189

Browse files
jzheauxjzheaux
committed
Defer SecurityContextHolderStrategy Lookup
Due to how early method interceptors are loaded during startup it's reasonable to consider scenarios where applications are changing the global security context holder strategy during startup. Closes gh-12877
1 parent eff9814 commit 11a2189

8 files changed

+131
-62
lines changed

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -51,8 +51,7 @@
5151
public final class AuthorizationManagerAfterMethodInterceptor
5252
implements Ordered, MethodInterceptor, PointcutAdvisor, AopInfrastructureBean {
5353

54-
private Supplier<Authentication> authentication = getAuthentication(
55-
SecurityContextHolder.getContextHolderStrategy());
54+
private Supplier<SecurityContextHolderStrategy> securityContextHolderStrategy = SecurityContextHolder::getContextHolderStrategy;
5655

5756
private final Log logger = LogFactory.getLog(this.getClass());
5857

@@ -156,14 +155,14 @@ public boolean isPerInstance() {
156155
* @since 5.8
157156
*/
158157
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy strategy) {
159-
this.authentication = getAuthentication(strategy);
158+
this.securityContextHolderStrategy = () -> strategy;
160159
}
161160

162161
private void attemptAuthorization(MethodInvocation mi, Object result) {
163162
this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
164163
MethodInvocationResult object = new MethodInvocationResult(mi, result);
165-
AuthorizationDecision decision = this.authorizationManager.check(this.authentication, object);
166-
this.eventPublisher.publishAuthorizationEvent(this.authentication, object, decision);
164+
AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, object);
165+
this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, object, decision);
167166
if (decision != null && !decision.isGranted()) {
168167
this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
169168
+ this.authorizationManager + " and decision " + decision));
@@ -172,15 +171,13 @@ private void attemptAuthorization(MethodInvocation mi, Object result) {
172171
this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
173172
}
174173

175-
private Supplier<Authentication> getAuthentication(SecurityContextHolderStrategy strategy) {
176-
return () -> {
177-
Authentication authentication = strategy.getContext().getAuthentication();
178-
if (authentication == null) {
179-
throw new AuthenticationCredentialsNotFoundException(
180-
"An Authentication object was not found in the SecurityContext");
181-
}
182-
return authentication;
183-
};
174+
private Authentication getAuthentication() {
175+
Authentication authentication = this.securityContextHolderStrategy.get().getContext().getAuthentication();
176+
if (authentication == null) {
177+
throw new AuthenticationCredentialsNotFoundException(
178+
"An Authentication object was not found in the SecurityContext");
179+
}
180+
return authentication;
184181
}
185182

186183
private static <T> void noPublish(Supplier<Authentication> authentication, T object,

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -56,8 +56,7 @@
5656
public final class AuthorizationManagerBeforeMethodInterceptor
5757
implements Ordered, MethodInterceptor, PointcutAdvisor, AopInfrastructureBean {
5858

59-
private Supplier<Authentication> authentication = getAuthentication(
60-
SecurityContextHolder.getContextHolderStrategy());
59+
private Supplier<SecurityContextHolderStrategy> securityContextHolderStrategy = SecurityContextHolder::getContextHolderStrategy;
6160

6261
private final Log logger = LogFactory.getLog(this.getClass());
6362

@@ -202,13 +201,13 @@ public boolean isPerInstance() {
202201
* @since 5.8
203202
*/
204203
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) {
205-
this.authentication = getAuthentication(securityContextHolderStrategy);
204+
this.securityContextHolderStrategy = () -> securityContextHolderStrategy;
206205
}
207206

208207
private void attemptAuthorization(MethodInvocation mi) {
209208
this.logger.debug(LogMessage.of(() -> "Authorizing method invocation " + mi));
210-
AuthorizationDecision decision = this.authorizationManager.check(this.authentication, mi);
211-
this.eventPublisher.publishAuthorizationEvent(this.authentication, mi, decision);
209+
AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, mi);
210+
this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, mi, decision);
212211
if (decision != null && !decision.isGranted()) {
213212
this.logger.debug(LogMessage.of(() -> "Failed to authorize " + mi + " with authorization manager "
214213
+ this.authorizationManager + " and decision " + decision));
@@ -217,15 +216,13 @@ private void attemptAuthorization(MethodInvocation mi) {
217216
this.logger.debug(LogMessage.of(() -> "Authorized method invocation " + mi));
218217
}
219218

220-
private Supplier<Authentication> getAuthentication(SecurityContextHolderStrategy strategy) {
221-
return () -> {
222-
Authentication authentication = strategy.getContext().getAuthentication();
223-
if (authentication == null) {
224-
throw new AuthenticationCredentialsNotFoundException(
225-
"An Authentication object was not found in the SecurityContext");
226-
}
227-
return authentication;
228-
};
219+
private Authentication getAuthentication() {
220+
Authentication authentication = this.securityContextHolderStrategy.get().getContext().getAuthentication();
221+
if (authentication == null) {
222+
throw new AuthenticationCredentialsNotFoundException(
223+
"An Authentication object was not found in the SecurityContext");
224+
}
225+
return authentication;
229226
}
230227

231228
private static <T> void noPublish(Supplier<Authentication> authentication, T object,

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -46,8 +46,7 @@
4646
public final class PostFilterAuthorizationMethodInterceptor
4747
implements Ordered, MethodInterceptor, PointcutAdvisor, AopInfrastructureBean {
4848

49-
private Supplier<Authentication> authentication = getAuthentication(
50-
SecurityContextHolder.getContextHolderStrategy());
49+
private Supplier<SecurityContextHolderStrategy> securityContextHolderStrategy = SecurityContextHolder::getContextHolderStrategy;
5150

5251
private PostFilterExpressionAttributeRegistry registry = new PostFilterExpressionAttributeRegistry();
5352

@@ -108,7 +107,7 @@ public boolean isPerInstance() {
108107
* @since 5.8
109108
*/
110109
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy strategy) {
111-
this.authentication = getAuthentication(strategy);
110+
this.securityContextHolderStrategy = () -> strategy;
112111
}
113112

114113
/**
@@ -125,19 +124,17 @@ public Object invoke(MethodInvocation mi) throws Throwable {
125124
return returnedObject;
126125
}
127126
MethodSecurityExpressionHandler expressionHandler = this.registry.getExpressionHandler();
128-
EvaluationContext ctx = expressionHandler.createEvaluationContext(this.authentication, mi);
127+
EvaluationContext ctx = expressionHandler.createEvaluationContext(this::getAuthentication, mi);
129128
return expressionHandler.filter(returnedObject, attribute.getExpression(), ctx);
130129
}
131130

132-
private Supplier<Authentication> getAuthentication(SecurityContextHolderStrategy strategy) {
133-
return () -> {
134-
Authentication authentication = strategy.getContext().getAuthentication();
135-
if (authentication == null) {
136-
throw new AuthenticationCredentialsNotFoundException(
137-
"An Authentication object was not found in the SecurityContext");
138-
}
139-
return authentication;
140-
};
131+
private Authentication getAuthentication() {
132+
Authentication authentication = this.securityContextHolderStrategy.get().getContext().getAuthentication();
133+
if (authentication == null) {
134+
throw new AuthenticationCredentialsNotFoundException(
135+
"An Authentication object was not found in the SecurityContext");
136+
}
137+
return authentication;
141138
}
142139

143140
}

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -47,8 +47,7 @@
4747
public final class PreFilterAuthorizationMethodInterceptor
4848
implements Ordered, MethodInterceptor, PointcutAdvisor, AopInfrastructureBean {
4949

50-
private Supplier<Authentication> authentication = getAuthentication(
51-
SecurityContextHolder.getContextHolderStrategy());
50+
private Supplier<SecurityContextHolderStrategy> securityContextHolderStrategy = SecurityContextHolder::getContextHolderStrategy;
5251

5352
private PreFilterExpressionAttributeRegistry registry = new PreFilterExpressionAttributeRegistry();
5453

@@ -109,7 +108,7 @@ public boolean isPerInstance() {
109108
* @since 5.8
110109
*/
111110
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy strategy) {
112-
this.authentication = getAuthentication(strategy);
111+
this.securityContextHolderStrategy = () -> strategy;
113112
}
114113

115114
/**
@@ -124,7 +123,7 @@ public Object invoke(MethodInvocation mi) throws Throwable {
124123
return mi.proceed();
125124
}
126125
MethodSecurityExpressionHandler expressionHandler = this.registry.getExpressionHandler();
127-
EvaluationContext ctx = expressionHandler.createEvaluationContext(this.authentication, mi);
126+
EvaluationContext ctx = expressionHandler.createEvaluationContext(this::getAuthentication, mi);
128127
Object filterTarget = findFilterTarget(attribute.getFilterTarget(), ctx, mi);
129128
expressionHandler.filter(filterTarget, attribute.getExpression(), ctx);
130129
return mi.proceed();
@@ -150,15 +149,13 @@ private Object findFilterTarget(String filterTargetName, EvaluationContext ctx,
150149
return filterTarget;
151150
}
152151

153-
private Supplier<Authentication> getAuthentication(SecurityContextHolderStrategy strategy) {
154-
return () -> {
155-
Authentication authentication = strategy.getContext().getAuthentication();
156-
if (authentication == null) {
157-
throw new AuthenticationCredentialsNotFoundException(
158-
"An Authentication object was not found in the SecurityContext");
159-
}
160-
return authentication;
161-
};
152+
private Authentication getAuthentication() {
153+
Authentication authentication = this.securityContextHolderStrategy.get().getContext().getAuthentication();
154+
if (authentication == null) {
155+
throw new AuthenticationCredentialsNotFoundException(
156+
"An Authentication object was not found in the SecurityContext");
157+
}
158+
return authentication;
162159
}
163160

164161
}

‎core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
2929
import org.springframework.security.authorization.AuthorizationEventPublisher;
3030
import org.springframework.security.authorization.AuthorizationManager;
3131
import org.springframework.security.core.Authentication;
32+
import org.springframework.security.core.authority.AuthorityUtils;
3233
import org.springframework.security.core.context.SecurityContext;
3334
import org.springframework.security.core.context.SecurityContextHolder;
3435
import org.springframework.security.core.context.SecurityContextHolderStrategy;
@@ -91,6 +92,25 @@ public void afterWhenMockSecurityContextHolderStrategyThenUses() throws Throwabl
9192
verify(strategy).getContext();
9293
}
9394

95+
// gh-12877
96+
@Test
97+
public void afterWhenStaticSecurityContextHolderStrategyAfterConstructorThenUses() throws Throwable {
98+
SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class);
99+
Authentication authentication = new TestingAuthenticationToken("john", "password",
100+
AuthorityUtils.createAuthorityList("authority"));
101+
given(strategy.getContext()).willReturn(new SecurityContextImpl(authentication));
102+
MethodInvocation invocation = mock(MethodInvocation.class);
103+
AuthorizationManager<MethodInvocationResult> authorizationManager = AuthenticatedAuthorizationManager
104+
.authenticated();
105+
AuthorizationManagerAfterMethodInterceptor advice = new AuthorizationManagerAfterMethodInterceptor(
106+
Pointcut.TRUE, authorizationManager);
107+
SecurityContextHolderStrategy saved = SecurityContextHolder.getContextHolderStrategy();
108+
SecurityContextHolder.setContextHolderStrategy(strategy);
109+
advice.invoke(invocation);
110+
verify(strategy).getContext();
111+
SecurityContextHolder.setContextHolderStrategy(saved);
112+
}
113+
94114
@Test
95115
public void configureWhenAuthorizationEventPublisherIsNullThenIllegalArgument() {
96116
AuthorizationManagerAfterMethodInterceptor advice = new AuthorizationManagerAfterMethodInterceptor(

‎core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -87,6 +87,24 @@ public void beforeWhenMockSecurityContextHolderStrategyThenUses() throws Throwab
8787
verify(strategy).getContext();
8888
}
8989

90+
// gh-12877
91+
@Test
92+
public void beforeWhenStaticSecurityContextHolderStrategyAfterConstructorThenUses() throws Throwable {
93+
SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class);
94+
Authentication authentication = new TestingAuthenticationToken("john", "password",
95+
AuthorityUtils.createAuthorityList("authority"));
96+
given(strategy.getContext()).willReturn(new SecurityContextImpl(authentication));
97+
MethodInvocation invocation = mock(MethodInvocation.class);
98+
AuthorizationManager<MethodInvocation> authorizationManager = AuthenticatedAuthorizationManager.authenticated();
99+
AuthorizationManagerBeforeMethodInterceptor advice = new AuthorizationManagerBeforeMethodInterceptor(
100+
Pointcut.TRUE, authorizationManager);
101+
SecurityContextHolderStrategy saved = SecurityContextHolder.getContextHolderStrategy();
102+
SecurityContextHolder.setContextHolderStrategy(strategy);
103+
advice.invoke(invocation);
104+
verify(strategy).getContext();
105+
SecurityContextHolder.setContextHolderStrategy(saved);
106+
}
107+
90108
@Test
91109
public void configureWhenAuthorizationEventPublisherIsNullThenIllegalArgument() {
92110
AuthorizationManagerBeforeMethodInterceptor advice = new AuthorizationManagerBeforeMethodInterceptor(

‎core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptorTests.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -147,6 +147,29 @@ public Object proceed() {
147147
verify(strategy).getContext();
148148
}
149149

150+
// gh-12877
151+
@Test
152+
public void postFilterWhenStaticSecurityContextHolderStrategyAfterConstructorThenUses() throws Throwable {
153+
SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class);
154+
Authentication authentication = new TestingAuthenticationToken("john", "password",
155+
AuthorityUtils.createAuthorityList("authority"));
156+
given(strategy.getContext()).willReturn(new SecurityContextImpl(authentication));
157+
String[] array = { "john", "bob" };
158+
MockMethodInvocation invocation = new MockMethodInvocation(new TestClass(), TestClass.class,
159+
"doSomethingArrayAuthentication", new Class[] { String[].class }, new Object[] { array }) {
160+
@Override
161+
public Object proceed() {
162+
return array;
163+
}
164+
};
165+
PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor();
166+
SecurityContextHolderStrategy saved = SecurityContextHolder.getContextHolderStrategy();
167+
SecurityContextHolder.setContextHolderStrategy(strategy);
168+
advice.invoke(invocation);
169+
verify(strategy).getContext();
170+
SecurityContextHolder.setContextHolderStrategy(saved);
171+
}
172+
150173
@PostFilter("filterObject == 'john'")
151174
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
152175

‎core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptorTests.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -204,6 +204,26 @@ public void preFilterWhenMockSecurityContextHolderStrategyThenUses() throws Thro
204204
verify(strategy).getContext();
205205
}
206206

207+
// gh-12877
208+
@Test
209+
public void preFilterWhenStaticSecurityContextHolderStrategyAfterConstructorThenUses() throws Throwable {
210+
SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class);
211+
Authentication authentication = new TestingAuthenticationToken("john", "password",
212+
AuthorityUtils.createAuthorityList("authority"));
213+
given(strategy.getContext()).willReturn(new SecurityContextImpl(authentication));
214+
List<String> list = new ArrayList<>();
215+
list.add("john");
216+
list.add("bob");
217+
MockMethodInvocation invocation = new MockMethodInvocation(new TestClass(), TestClass.class,
218+
"doSomethingArrayFilterAuthentication", new Class[] { List.class }, new Object[] { list });
219+
PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor();
220+
SecurityContextHolderStrategy saved = SecurityContextHolder.getContextHolderStrategy();
221+
SecurityContextHolder.setContextHolderStrategy(strategy);
222+
advice.invoke(invocation);
223+
verify(strategy).getContext();
224+
SecurityContextHolder.setContextHolderStrategy(saved);
225+
}
226+
207227
@PreFilter("filterObject == 'john'")
208228
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
209229

0 commit comments

Comments
 (0)