Misconfigured OAuth2LoginAuthenticationFilter when combining OAuth2 login and OAuth2 client configuration

[BWohlbrecht]

Describe the bug
When configuring the SecurityFilterChain with both oAuth2Login and oAuth2Client sections, the resulting OAuth2LoginAuthenticationFilter is configured with the ClientRegistrationRepository and the OAuth2AuthorizedClientRepository specified in the oAuth2Client section instead of the corresponding instances in the oAuth2Login section.

The same seems to apply to other configured filters.

To Reproduce
Configure a SecurityFilterChain bean with oAuth2Login providing a custom ClientRegistrationRepository and OAuth2AuthorizedClientRepository. The correct beans are injected in the OAuth2LoginAuthenticationFilter on creation.

Then add the oAuth2Client section with different beans for ClientRegistrationRepository and OAuth2AuthorizedClientRepository.
Now OAuth2LoginAuthenticationFilter is constructed with the beans from the oAuth2Client section despite other instances being provided in the oAuth2Login section.

Expected behavior
The ClientRegistrationRepository and OAuth2AuthorizedClientRepository in the oAuth2Login section should always take precedent when constructing the OAuth2LoginAuthenticationFilter even when different instances are provided in the oAuth2Client section.

This is important for use cases where some ClientRegistrations are dedicated to login and others for authorizing use of a web API.

Sample
https://github.com/BWohlbrecht/spring-security-oauth2-sample

[BWohlbrecht]

Just to make it clear:

Using this bean definition for the SecurityFilterChain

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http,
		ClientRegistrationRepository loginRegistrationRepository,
		OAuth2AuthorizedClientRepository loginClientRepository
	) throws Exception {
	return http
		.oauth2Login(login -> {
			login
				.clientRegistrationRepository(loginRegistrationRepository)
				.authorizedClientRepository(loginClientRepository);
		})
		.build();
}

the OAuth2LoginAuthenticationFilter is constructed correctly with the supplied ClientRegistrationRepository and OAuth2AuthorizedClientRepository.

However when adding the following section

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http,
		ClientRegistrationRepository loginRegistrationRepository,
		OAuth2AuthorizedClientRepository loginClientRepository,

		ClientRegistrationRepository authorizationRegistrationRepository,
		OAuth2AuthorizedClientRepository authorizationClientRepository
	) throws Exception {
	return http
		.oauth2Login(login -> {
			login
				.clientRegistrationRepository(loginRegistrationRepository)
				.authorizedClientRepository(loginClientRepository);
		})
		.oauth2Client(client -> {
			client
				.clientRegistrationRepository(authorizationRegistrationRepository)
				.authorizedClientRepository(authorizationClientRepository);
		})
		.build();
}

the OAuth2LoginAuthenticationFilter is constructed with the ClientRegistrationRepository and OAuth2AuthorizedClientRepository supplied in the oAuth2Client section, while the instances from the oAuth2Login section are ignored.

[sjohnr]

Thanks for reporting this @BWohlbrecht. It looks like both configurers are set up to share objects via builder.setSharedObject(...) which causes them to always resolve the same instances (whichever is set last). We'll look at a fix to use distinct instances, but I will check on why it is using shared objects before proceeding and report back.