Skip to content

Commit d0e1107

Browse files
jgrandjajgrandja
committed
HttpSessionSecurityContextRepository does not persist @transient Authentication
Related spring-projects/spring-security#9993 Closes gh-482
1 parent 42095a6 commit d0e1107

File tree

3 files changed

+262
-0
lines changed

3 files changed

+262
-0
lines changed

‎oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,24 @@
1919
import java.util.LinkedHashMap;
2020
import java.util.Map;
2121

22+
import javax.servlet.AsyncContext;
23+
import javax.servlet.ServletRequest;
24+
import javax.servlet.ServletResponse;
25+
import javax.servlet.http.HttpServletRequest;
26+
import javax.servlet.http.HttpServletRequestWrapper;
27+
import javax.servlet.http.HttpServletResponse;
28+
29+
import org.springframework.core.annotation.AnnotationUtils;
2230
import org.springframework.http.HttpMethod;
2331
import org.springframework.http.HttpStatus;
2432
import org.springframework.security.authentication.AuthenticationManager;
2533
import org.springframework.security.config.Customizer;
2634
import org.springframework.security.config.annotation.web.HttpSecurityBuilder;
2735
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
2836
import org.springframework.security.config.annotation.web.configurers.ExceptionHandlingConfigurer;
37+
import org.springframework.security.core.Authentication;
38+
import org.springframework.security.core.Transient;
39+
import org.springframework.security.core.context.SecurityContext;
2940
import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService;
3041
import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService;
3142
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2TokenIntrospectionAuthenticationProvider;
@@ -39,6 +50,10 @@
3950
import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
4051
import org.springframework.security.web.authentication.HttpStatusEntryPoint;
4152
import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter;
53+
import org.springframework.security.web.context.HttpRequestResponseHolder;
54+
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
55+
import org.springframework.security.web.context.SaveContextOnUpdateOrErrorResponseWrapper;
56+
import org.springframework.security.web.context.SecurityContextRepository;
4257
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
4358
import org.springframework.security.web.util.matcher.OrRequestMatcher;
4459
import org.springframework.security.web.util.matcher.RequestMatcher;
@@ -212,6 +227,105 @@ public void init(B builder) {
212227
this.tokenRevocationEndpointMatcher)
213228
);
214229
}
230+
231+
// gh-482
232+
initSecurityContextRepository(builder);
233+
}
234+
235+
private void initSecurityContextRepository(B builder) {
236+
// TODO This is a temporary fix and should be removed after upgrading to Spring Security 5.7.0 GA.
237+
//
238+
// See:
239+
// Prevent Save @Transient Authentication with existing HttpSession
240+
// https://github.com/spring-projects/spring-security/pull/9993
241+
242+
final SecurityContextRepository securityContextRepository = builder.getSharedObject(SecurityContextRepository.class);
243+
if (!(securityContextRepository instanceof HttpSessionSecurityContextRepository)) {
244+
return;
245+
}
246+
247+
SecurityContextRepository securityContextRepositoryTransientNotSaved = new SecurityContextRepository() {
248+
// OAuth2ClientAuthenticationToken is @Transient and is accepted by
249+
// OAuth2TokenEndpointFilter, OAuth2TokenIntrospectionEndpointFilter and OAuth2TokenRevocationEndpointFilter
250+
private final RequestMatcher clientAuthenticationRequestMatcher = new OrRequestMatcher(
251+
getRequestMatcher(OAuth2TokenEndpointConfigurer.class),
252+
OAuth2AuthorizationServerConfigurer.this.tokenIntrospectionEndpointMatcher,
253+
OAuth2AuthorizationServerConfigurer.this.tokenRevocationEndpointMatcher);
254+
255+
// JwtAuthenticationToken is @Transient and is accepted by
256+
// OidcUserInfoEndpointFilter and OidcClientRegistrationEndpointFilter
257+
private final RequestMatcher jwtAuthenticationRequestMatcher = getRequestMatcher(OidcConfigurer.class);
258+
259+
@Override
260+
public SecurityContext loadContext(HttpRequestResponseHolder requestResponseHolder) {
261+
final HttpServletRequest unwrappedRequest = requestResponseHolder.getRequest();
262+
final HttpServletResponse unwrappedResponse = requestResponseHolder.getResponse();
263+
264+
SecurityContext securityContext = securityContextRepository.loadContext(requestResponseHolder);
265+
266+
if (this.clientAuthenticationRequestMatcher.matches(unwrappedRequest) ||
267+
this.jwtAuthenticationRequestMatcher.matches(unwrappedRequest)) {
268+
269+
final SaveContextOnUpdateOrErrorResponseWrapper transientAuthenticationResponseWrapper =
270+
new SaveContextOnUpdateOrErrorResponseWrapper(unwrappedResponse, false) {
271+
272+
@Override
273+
protected void saveContext(SecurityContext context) {
274+
// @Transient Authentication should not be saved
275+
if (context.getAuthentication() != null) {
276+
Assert.state(isTransientAuthentication(context.getAuthentication()), "Expected @Transient Authentication");
277+
}
278+
}
279+
280+
};
281+
// Override the default HttpSessionSecurityContextRepository.SaveToSessionResponseWrapper
282+
requestResponseHolder.setResponse(transientAuthenticationResponseWrapper);
283+
284+
final HttpServletRequestWrapper transientAuthenticationRequestWrapper =
285+
new HttpServletRequestWrapper(unwrappedRequest) {
286+
287+
@Override
288+
public AsyncContext startAsync() {
289+
transientAuthenticationResponseWrapper.disableSaveOnResponseCommitted();
290+
return super.startAsync();
291+
}
292+
293+
@Override
294+
public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse)
295+
throws IllegalStateException {
296+
transientAuthenticationResponseWrapper.disableSaveOnResponseCommitted();
297+
return super.startAsync(servletRequest, servletResponse);
298+
}
299+
300+
};
301+
// Override the default HttpSessionSecurityContextRepository.SaveToSessionRequestWrapper
302+
requestResponseHolder.setRequest(transientAuthenticationRequestWrapper);
303+
}
304+
305+
return securityContext;
306+
}
307+
308+
@Override
309+
public void saveContext(SecurityContext context, HttpServletRequest request, HttpServletResponse response) {
310+
Authentication authentication = context.getAuthentication();
311+
if (authentication == null || isTransientAuthentication(authentication)) {
312+
return;
313+
}
314+
securityContextRepository.saveContext(context, request, response);
315+
}
316+
317+
@Override
318+
public boolean containsContext(HttpServletRequest request) {
319+
return securityContextRepository.containsContext(request);
320+
}
321+
322+
private boolean isTransientAuthentication(Authentication authentication) {
323+
return AnnotationUtils.getAnnotation(authentication.getClass(), Transient.class) != null;
324+
}
325+
326+
};
327+
328+
builder.setSharedObject(SecurityContextRepository.class, securityContextRepositoryTransientNotSaved);
215329
}
216330

217331
@Override

‎oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@
3737
import org.assertj.core.matcher.AssertionMatcher;
3838
import org.junit.After;
3939
import org.junit.AfterClass;
40+
import org.junit.Before;
4041
import org.junit.BeforeClass;
4142
import org.junit.Rule;
4243
import org.junit.Test;
44+
import org.mockito.ArgumentCaptor;
4345

4446
import org.springframework.beans.factory.annotation.Autowired;
4547
import org.springframework.context.annotation.Bean;
@@ -56,6 +58,7 @@
5658
import org.springframework.mock.web.MockHttpServletResponse;
5759
import org.springframework.security.authentication.AuthenticationProvider;
5860
import org.springframework.security.authentication.TestingAuthenticationToken;
61+
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
5962
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
6063
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
6164
import org.springframework.security.config.annotation.web.configuration.OAuth2AuthorizationServerConfiguration;
@@ -102,6 +105,8 @@
102105
import org.springframework.security.web.authentication.AuthenticationConverter;
103106
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
104107
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
108+
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
109+
import org.springframework.security.web.context.SecurityContextRepository;
105110
import org.springframework.security.web.util.matcher.RequestMatcher;
106111
import org.springframework.test.web.servlet.MockMvc;
107112
import org.springframework.test.web.servlet.MvcResult;
@@ -116,6 +121,10 @@
116121
import static org.mockito.ArgumentMatchers.any;
117122
import static org.mockito.ArgumentMatchers.eq;
118123
import static org.mockito.Mockito.mock;
124+
import static org.mockito.Mockito.never;
125+
import static org.mockito.Mockito.reset;
126+
import static org.mockito.Mockito.spy;
127+
import static org.mockito.Mockito.times;
119128
import static org.mockito.Mockito.verify;
120129
import static org.mockito.Mockito.when;
121130
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
@@ -154,6 +163,7 @@ public class OAuth2AuthorizationCodeGrantTests {
154163
private static AuthenticationProvider authorizationRequestAuthenticationProvider;
155164
private static AuthenticationSuccessHandler authorizationResponseHandler;
156165
private static AuthenticationFailureHandler authorizationErrorResponseHandler;
166+
private static SecurityContextRepository securityContextRepository;
157167
private static String consentPage = "/oauth2/consent";
158168

159169
@Rule
@@ -187,6 +197,7 @@ public static void init() {
187197
authorizationRequestAuthenticationProvider = mock(AuthenticationProvider.class);
188198
authorizationResponseHandler = mock(AuthenticationSuccessHandler.class);
189199
authorizationErrorResponseHandler = mock(AuthenticationFailureHandler.class);
200+
securityContextRepository = spy(new HttpSessionSecurityContextRepository());
190201
db = new EmbeddedDatabaseBuilder()
191202
.generateUniqueName(true)
192203
.setType(EmbeddedDatabaseType.HSQL)
@@ -197,6 +208,11 @@ public static void init() {
197208
.build();
198209
}
199210

211+
@Before
212+
public void setup() {
213+
reset(securityContextRepository);
214+
}
215+
200216
@After
201217
public void tearDown() {
202218
jdbcOperations.update("truncate table oauth2_authorization");
@@ -615,6 +631,48 @@ public void requestWhenAuthorizationEndpointCustomizedThenUsed() throws Exceptio
615631
verify(authorizationResponseHandler).onAuthenticationSuccess(any(), any(), eq(authorizationCodeRequestAuthenticationResult));
616632
}
617633

634+
// gh-482
635+
@Test
636+
public void requestWhenClientObtainsAccessTokenThenClientAuthenticationNotPersisted() throws Exception {
637+
this.spring.register(AuthorizationServerConfigurationWithSecurityContextRepository.class).autowire();
638+
639+
RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build();
640+
this.registeredClientRepository.save(registeredClient);
641+
642+
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
643+
.params(getAuthorizationRequestParameters(registeredClient))
644+
.param(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
645+
.param(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
646+
.with(user("user")))
647+
.andExpect(status().is3xxRedirection())
648+
.andReturn();
649+
650+
ArgumentCaptor<org.springframework.security.core.context.SecurityContext> securityContextCaptor =
651+
ArgumentCaptor.forClass(org.springframework.security.core.context.SecurityContext.class);
652+
verify(securityContextRepository, times(2)).saveContext(securityContextCaptor.capture(), any(), any());
653+
securityContextCaptor.getAllValues().forEach(securityContext ->
654+
assertThat(securityContext.getAuthentication()).isInstanceOf(UsernamePasswordAuthenticationToken.class));
655+
reset(securityContextRepository);
656+
657+
String authorizationCode = extractParameterFromRedirectUri(mvcResult.getResponse().getRedirectedUrl(), "code");
658+
OAuth2Authorization authorizationCodeAuthorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE);
659+
660+
this.mvc.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
661+
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
662+
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
663+
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER))
664+
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
665+
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
666+
.andExpect(status().isOk())
667+
.andExpect(jsonPath("$.access_token").isNotEmpty())
668+
.andExpect(jsonPath("$.token_type").isNotEmpty())
669+
.andExpect(jsonPath("$.expires_in").isNotEmpty())
670+
.andExpect(jsonPath("$.refresh_token").doesNotExist())
671+
.andExpect(jsonPath("$.scope").isNotEmpty());
672+
673+
verify(securityContextRepository, never()).saveContext(any(), any(), any());
674+
}
675+
618676
private static MultiValueMap<String, String> getAuthorizationRequestParameters(RegisteredClient registeredClient) {
619677
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
620678
parameters.set(OAuth2ParameterNames.RESPONSE_TYPE, OAuth2AuthorizationResponseType.CODE.getValue());
@@ -739,6 +797,29 @@ static class ParametersMapper extends JdbcOAuth2AuthorizationService.OAuth2Autho
739797

740798
}
741799

800+
@EnableWebSecurity
801+
static class AuthorizationServerConfigurationWithSecurityContextRepository extends AuthorizationServerConfiguration {
802+
// @formatter:off
803+
@Bean
804+
public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception {
805+
OAuth2AuthorizationServerConfigurer<HttpSecurity> authorizationServerConfigurer =
806+
new OAuth2AuthorizationServerConfigurer<>();
807+
RequestMatcher endpointsMatcher = authorizationServerConfigurer.getEndpointsMatcher();
808+
809+
http
810+
.requestMatcher(endpointsMatcher)
811+
.authorizeRequests(authorizeRequests ->
812+
authorizeRequests.anyRequest().authenticated()
813+
)
814+
.csrf(csrf -> csrf.ignoringRequestMatchers(endpointsMatcher))
815+
.securityContext(securityContext ->
816+
securityContext.securityContextRepository(securityContextRepository))
817+
.apply(authorizationServerConfigurer);
818+
return http.build();
819+
}
820+
// @formatter:on
821+
}
822+
742823
@EnableWebSecurity
743824
@Import(OAuth2AuthorizationServerConfiguration.class)
744825
static class AuthorizationServerConfigurationWithJwtEncoder extends AuthorizationServerConfiguration {

0 commit comments

Comments
 (0)