Skip to content

Commit 211fa52

Browse files
sjohnrsjohnr
committed
Favor provided instances over shared objects
Prior to this commit, providing oauth2Login() and oauth2Client() with clientRegistrationRepository() and authorizedClientRepository() caused objects to be shared across both configurers. These configurers will now prefer explicitly provided instances of those objects when they are available. Closes gh-16105
1 parent 7f410ce commit 211fa52

File tree

4 files changed

+201
-14
lines changed

4 files changed

+201
-14
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurer.java

+22-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -98,6 +98,10 @@ public final class OAuth2ClientConfigurer<B extends HttpSecurityBuilder<B>>
9898

9999
private AuthorizationCodeGrantConfigurer authorizationCodeGrantConfigurer = new AuthorizationCodeGrantConfigurer();
100100

101+
private ClientRegistrationRepository clientRegistrationRepository;
102+
103+
private OAuth2AuthorizedClientRepository authorizedClientRepository;
104+
101105
/**
102106
* Sets the repository of client registrations.
103107
* @param clientRegistrationRepository the repository of client registrations
@@ -107,6 +111,7 @@ public OAuth2ClientConfigurer<B> clientRegistrationRepository(
107111
ClientRegistrationRepository clientRegistrationRepository) {
108112
Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null");
109113
this.getBuilder().setSharedObject(ClientRegistrationRepository.class, clientRegistrationRepository);
114+
this.clientRegistrationRepository = clientRegistrationRepository;
110115
return this;
111116
}
112117

@@ -119,6 +124,7 @@ public OAuth2ClientConfigurer<B> authorizedClientRepository(
119124
OAuth2AuthorizedClientRepository authorizedClientRepository) {
120125
Assert.notNull(authorizedClientRepository, "authorizedClientRepository cannot be null");
121126
this.getBuilder().setSharedObject(OAuth2AuthorizedClientRepository.class, authorizedClientRepository);
127+
this.authorizedClientRepository = authorizedClientRepository;
122128
return this;
123129
}
124130

@@ -283,17 +289,16 @@ private OAuth2AuthorizationRequestResolver getAuthorizationRequestResolver() {
283289
if (this.authorizationRequestResolver != null) {
284290
return this.authorizationRequestResolver;
285291
}
286-
ClientRegistrationRepository clientRegistrationRepository = OAuth2ClientConfigurerUtils
287-
.getClientRegistrationRepository(getBuilder());
292+
ClientRegistrationRepository clientRegistrationRepository = getClientRegistrationRepository(getBuilder());
288293
return new DefaultOAuth2AuthorizationRequestResolver(clientRegistrationRepository,
289294
OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI);
290295
}
291296

292297
private OAuth2AuthorizationCodeGrantFilter createAuthorizationCodeGrantFilter(B builder) {
293298
AuthenticationManager authenticationManager = builder.getSharedObject(AuthenticationManager.class);
294299
OAuth2AuthorizationCodeGrantFilter authorizationCodeGrantFilter = new OAuth2AuthorizationCodeGrantFilter(
295-
OAuth2ClientConfigurerUtils.getClientRegistrationRepository(builder),
296-
OAuth2ClientConfigurerUtils.getAuthorizedClientRepository(builder), authenticationManager);
300+
getClientRegistrationRepository(builder), getAuthorizedClientRepository(builder),
301+
authenticationManager);
297302
if (this.authorizationRequestRepository != null) {
298303
authorizationCodeGrantFilter.setAuthorizationRequestRepository(this.authorizationRequestRepository);
299304
}
@@ -315,6 +320,18 @@ private OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> get
315320
return (bean != null) ? bean : new DefaultAuthorizationCodeTokenResponseClient();
316321
}
317322

323+
private ClientRegistrationRepository getClientRegistrationRepository(B builder) {
324+
return (OAuth2ClientConfigurer.this.clientRegistrationRepository != null)
325+
? OAuth2ClientConfigurer.this.clientRegistrationRepository
326+
: OAuth2ClientConfigurerUtils.getClientRegistrationRepository(builder);
327+
}
328+
329+
private OAuth2AuthorizedClientRepository getAuthorizedClientRepository(B builder) {
330+
return (OAuth2ClientConfigurer.this.authorizedClientRepository != null)
331+
? OAuth2ClientConfigurer.this.authorizedClientRepository
332+
: OAuth2ClientConfigurerUtils.getAuthorizedClientRepository(builder);
333+
}
334+
318335
@SuppressWarnings("unchecked")
319336
private <T> T getBeanOrNull(ResolvableType type) {
320337
ApplicationContext context = getBuilder().getSharedObject(ApplicationContext.class);

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java

+20-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -172,6 +172,10 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>>
172172

173173
private String loginProcessingUrl = OAuth2LoginAuthenticationFilter.DEFAULT_FILTER_PROCESSES_URI;
174174

175+
private ClientRegistrationRepository clientRegistrationRepository;
176+
177+
private OAuth2AuthorizedClientRepository authorizedClientRepository;
178+
175179
/**
176180
* Sets the repository of client registrations.
177181
* @param clientRegistrationRepository the repository of client registrations
@@ -181,6 +185,7 @@ public OAuth2LoginConfigurer<B> clientRegistrationRepository(
181185
ClientRegistrationRepository clientRegistrationRepository) {
182186
Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null");
183187
this.getBuilder().setSharedObject(ClientRegistrationRepository.class, clientRegistrationRepository);
188+
this.clientRegistrationRepository = clientRegistrationRepository;
184189
return this;
185190
}
186191

@@ -194,6 +199,7 @@ public OAuth2LoginConfigurer<B> authorizedClientRepository(
194199
OAuth2AuthorizedClientRepository authorizedClientRepository) {
195200
Assert.notNull(authorizedClientRepository, "authorizedClientRepository cannot be null");
196201
this.getBuilder().setSharedObject(OAuth2AuthorizedClientRepository.class, authorizedClientRepository);
202+
this.authorizedClientRepository = authorizedClientRepository;
197203
return this;
198204
}
199205

@@ -339,8 +345,7 @@ public OAuth2LoginConfigurer<B> userInfoEndpoint(Customizer<UserInfoEndpointConf
339345
@Override
340346
public void init(B http) throws Exception {
341347
OAuth2LoginAuthenticationFilter authenticationFilter = new OAuth2LoginAuthenticationFilter(
342-
OAuth2ClientConfigurerUtils.getClientRegistrationRepository(this.getBuilder()),
343-
OAuth2ClientConfigurerUtils.getAuthorizedClientRepository(this.getBuilder()), this.loginProcessingUrl);
348+
this.getClientRegistrationRepository(), this.getAuthorizedClientRepository(), this.loginProcessingUrl);
344349
authenticationFilter.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy());
345350
this.setAuthenticationFilter(authenticationFilter);
346351
super.loginProcessingUrl(this.loginProcessingUrl);
@@ -406,8 +411,7 @@ public void configure(B http) throws Exception {
406411
authorizationRequestBaseUri = OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
407412
}
408413
authorizationRequestFilter = new OAuth2AuthorizationRequestRedirectFilter(
409-
OAuth2ClientConfigurerUtils.getClientRegistrationRepository(this.getBuilder()),
410-
authorizationRequestBaseUri);
414+
this.getClientRegistrationRepository(), authorizationRequestBaseUri);
411415
}
412416
if (this.authorizationEndpointConfig.authorizationRequestRepository != null) {
413417
authorizationRequestFilter
@@ -439,6 +443,16 @@ protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingU
439443
return new AntPathRequestMatcher(loginProcessingUrl);
440444
}
441445

446+
private ClientRegistrationRepository getClientRegistrationRepository() {
447+
return (this.clientRegistrationRepository != null) ? this.clientRegistrationRepository
448+
: OAuth2ClientConfigurerUtils.getClientRegistrationRepository(this.getBuilder());
449+
}
450+
451+
private OAuth2AuthorizedClientRepository getAuthorizedClientRepository() {
452+
return (this.authorizedClientRepository != null) ? this.authorizedClientRepository
453+
: OAuth2ClientConfigurerUtils.getAuthorizedClientRepository(this.getBuilder());
454+
}
455+
442456
@SuppressWarnings("unchecked")
443457
private JwtDecoderFactory<ClientRegistration> getJwtDecoderFactoryBean() {
444458
ResolvableType type = ResolvableType.forClassWithGenerics(JwtDecoderFactory.class, ClientRegistration.class);
@@ -529,8 +543,7 @@ private void initDefaultLoginFilter(B http) {
529543
@SuppressWarnings("unchecked")
530544
private Map<String, String> getLoginLinks() {
531545
Iterable<ClientRegistration> clientRegistrations = null;
532-
ClientRegistrationRepository clientRegistrationRepository = OAuth2ClientConfigurerUtils
533-
.getClientRegistrationRepository(this.getBuilder());
546+
ClientRegistrationRepository clientRegistrationRepository = this.getClientRegistrationRepository();
534547
ResolvableType type = ResolvableType.forInstance(clientRegistrationRepository).as(Iterable.class);
535548
if (type != ResolvableType.NONE && ClientRegistration.class.isAssignableFrom(type.resolveGenerics()[0])) {
536549
clientRegistrations = (Iterable<ClientRegistration>) clientRegistrationRepository;

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java

+92-1
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-2025 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.
@@ -75,6 +75,7 @@
7575
import static org.mockito.BDDMockito.given;
7676
import static org.mockito.Mockito.mock;
7777
import static org.mockito.Mockito.verify;
78+
import static org.mockito.Mockito.verifyNoInteractions;
7879
import static org.springframework.security.config.Customizer.withDefaults;
7980
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication;
8081
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
@@ -285,6 +286,49 @@ public void configureWhenCustomAuthorizationRedirectStrategySetThenAuthorization
285286
verify(authorizationRedirectStrategy).sendRedirect(any(), any(), anyString());
286287
}
287288

289+
@Test
290+
public void configureWhenOAuth2LoginBeansConfiguredThenNotShared() throws Exception {
291+
this.spring.register(OAuth2ClientConfigWithOAuth2Login.class).autowire();
292+
// Setup the Authorization Request in the session
293+
Map<String, Object> attributes = new HashMap<>();
294+
attributes.put(OAuth2ParameterNames.REGISTRATION_ID, this.registration1.getRegistrationId());
295+
// @formatter:off
296+
OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode()
297+
.authorizationUri(this.registration1.getProviderDetails().getAuthorizationUri())
298+
.clientId(this.registration1.getClientId())
299+
.redirectUri("http://localhost/client-1")
300+
.state("state")
301+
.attributes(attributes)
302+
.build();
303+
// @formatter:on
304+
AuthorizationRequestRepository<OAuth2AuthorizationRequest> authorizationRequestRepository = new HttpSessionOAuth2AuthorizationRequestRepository();
305+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
306+
MockHttpServletResponse response = new MockHttpServletResponse();
307+
authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest, request, response);
308+
MockHttpSession session = (MockHttpSession) request.getSession();
309+
String principalName = "user1";
310+
TestingAuthenticationToken authentication = new TestingAuthenticationToken(principalName, "password");
311+
// @formatter:off
312+
MockHttpServletRequestBuilder clientRequest = get("/client-1")
313+
.param(OAuth2ParameterNames.CODE, "code")
314+
.param(OAuth2ParameterNames.STATE, "state")
315+
.with(authentication(authentication))
316+
.session(session);
317+
this.mockMvc.perform(clientRequest)
318+
.andExpect(status().is3xxRedirection())
319+
.andExpect(redirectedUrl("http://localhost/client-1"));
320+
// @formatter:on
321+
OAuth2AuthorizedClient authorizedClient = authorizedClientRepository
322+
.loadAuthorizedClient(this.registration1.getRegistrationId(), authentication, request);
323+
assertThat(authorizedClient).isNotNull();
324+
// Ensure shared objects set for OAuth2 Client are not used
325+
ClientRegistrationRepository clientRegistrationRepository = this.spring.getContext()
326+
.getBean(ClientRegistrationRepository.class);
327+
OAuth2AuthorizedClientRepository authorizedClientRepository = this.spring.getContext()
328+
.getBean(OAuth2AuthorizedClientRepository.class);
329+
verifyNoInteractions(clientRegistrationRepository, authorizedClientRepository);
330+
}
331+
288332
@EnableWebSecurity
289333
@Configuration
290334
@EnableWebMvc
@@ -362,4 +406,51 @@ OAuth2AuthorizedClientRepository authorizedClientRepository() {
362406

363407
}
364408

409+
@Configuration
410+
@EnableWebSecurity
411+
@EnableWebMvc
412+
static class OAuth2ClientConfigWithOAuth2Login {
413+
414+
private final ClientRegistrationRepository clientRegistrationRepository = mock(
415+
ClientRegistrationRepository.class);
416+
417+
private final OAuth2AuthorizedClientRepository authorizedClientRepository = mock(
418+
OAuth2AuthorizedClientRepository.class);
419+
420+
@Bean
421+
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
422+
// @formatter:off
423+
http
424+
.authorizeHttpRequests((authorize) -> authorize
425+
.anyRequest().authenticated()
426+
)
427+
.oauth2Client((oauth2Client) -> oauth2Client
428+
.clientRegistrationRepository(OAuth2ClientConfigurerTests.clientRegistrationRepository)
429+
.authorizedClientService(OAuth2ClientConfigurerTests.authorizedClientService)
430+
.authorizationCodeGrant((authorizationCode) -> authorizationCode
431+
.authorizationRequestResolver(authorizationRequestResolver)
432+
.authorizationRedirectStrategy(authorizationRedirectStrategy)
433+
.accessTokenResponseClient(accessTokenResponseClient)
434+
)
435+
)
436+
.oauth2Login((oauth2Login) -> oauth2Login
437+
.clientRegistrationRepository(this.clientRegistrationRepository)
438+
.authorizedClientRepository(this.authorizedClientRepository)
439+
);
440+
// @formatter:on
441+
return http.build();
442+
}
443+
444+
@Bean
445+
ClientRegistrationRepository clientRegistrationRepository() {
446+
return this.clientRegistrationRepository;
447+
}
448+
449+
@Bean
450+
OAuth2AuthorizedClientRepository authorizedClientRepository() {
451+
return this.authorizedClientRepository;
452+
}
453+
454+
}
455+
365456
}

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java

+67-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -73,7 +73,9 @@
7373
import org.springframework.security.oauth2.client.userinfo.OAuth2UserService;
7474
import org.springframework.security.oauth2.client.web.AuthorizationRequestRepository;
7575
import org.springframework.security.oauth2.client.web.HttpSessionOAuth2AuthorizationRequestRepository;
76+
import org.springframework.security.oauth2.client.web.HttpSessionOAuth2AuthorizedClientRepository;
7677
import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestResolver;
78+
import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository;
7779
import org.springframework.security.oauth2.core.OAuth2AccessToken;
7880
import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
7981
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
@@ -115,6 +117,7 @@
115117
import static org.mockito.Mockito.atLeastOnce;
116118
import static org.mockito.Mockito.mock;
117119
import static org.mockito.Mockito.verify;
120+
import static org.mockito.Mockito.verifyNoInteractions;
118121
import static org.springframework.security.config.annotation.SecurityContextChangedListenerArgumentMatchers.setAuthentication;
119122
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication;
120123
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
@@ -669,6 +672,30 @@ public void oauth2LoginWhenDefaultsThenNoOidcSessionRegistry() {
669672
.collect(Collectors.toList())).isEmpty();
670673
}
671674

675+
@Test
676+
public void oidcLoginWhenOAuth2ClientBeansConfiguredThenNotShared() throws Exception {
677+
this.spring.register(OAuth2LoginConfigWithOAuth2Client.class, JwtDecoderFactoryConfig.class).autowire();
678+
OAuth2AuthorizationRequest authorizationRequest = createOAuth2AuthorizationRequest("openid");
679+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest, this.request, this.response);
680+
this.request.setParameter("code", "code123");
681+
this.request.setParameter("state", authorizationRequest.getState());
682+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.filterChain);
683+
Authentication authentication = this.securityContextRepository
684+
.loadContext(new HttpRequestResponseHolder(this.request, this.response))
685+
.getAuthentication();
686+
assertThat(authentication.getAuthorities()).hasSize(1);
687+
assertThat(authentication.getAuthorities()).first()
688+
.isInstanceOf(OidcUserAuthority.class)
689+
.hasToString("OIDC_USER");
690+
691+
// Ensure shared objects set for OAuth2 Client are not used
692+
ClientRegistrationRepository clientRegistrationRepository = this.spring.getContext()
693+
.getBean(ClientRegistrationRepository.class);
694+
OAuth2AuthorizedClientRepository authorizedClientRepository = this.spring.getContext()
695+
.getBean(OAuth2AuthorizedClientRepository.class);
696+
verifyNoInteractions(clientRegistrationRepository, authorizedClientRepository);
697+
}
698+
672699
private void loadConfig(Class<?>... configs) {
673700
AnnotationConfigWebApplicationContext applicationContext = new AnnotationConfigWebApplicationContext();
674701
applicationContext.register(configs);
@@ -1192,6 +1219,45 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
11921219

11931220
}
11941221

1222+
@Configuration
1223+
@EnableWebSecurity
1224+
static class OAuth2LoginConfigWithOAuth2Client extends CommonLambdaSecurityFilterChainConfig {
1225+
1226+
private final ClientRegistrationRepository clientRegistrationRepository = mock(
1227+
ClientRegistrationRepository.class);
1228+
1229+
private final OAuth2AuthorizedClientRepository authorizedClientRepository = mock(
1230+
OAuth2AuthorizedClientRepository.class);
1231+
1232+
@Bean
1233+
SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
1234+
// @formatter:off
1235+
http
1236+
.oauth2Login((oauth2Login) -> oauth2Login
1237+
.clientRegistrationRepository(
1238+
new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION))
1239+
.authorizedClientRepository(new HttpSessionOAuth2AuthorizedClientRepository())
1240+
)
1241+
.oauth2Client((oauth2Client) -> oauth2Client
1242+
.clientRegistrationRepository(this.clientRegistrationRepository)
1243+
.authorizedClientRepository(this.authorizedClientRepository)
1244+
);
1245+
// @formatter:on
1246+
return super.configureFilterChain(http);
1247+
}
1248+
1249+
@Bean
1250+
ClientRegistrationRepository clientRegistrationRepository() {
1251+
return this.clientRegistrationRepository;
1252+
}
1253+
1254+
@Bean
1255+
OAuth2AuthorizedClientRepository authorizedClientRepository() {
1256+
return this.authorizedClientRepository;
1257+
}
1258+
1259+
}
1260+
11951261
private abstract static class CommonSecurityFilterChainConfig {
11961262

11971263
SecurityFilterChain configureFilterChain(HttpSecurity http) throws Exception {

0 commit comments

Comments
 (0)