- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.2k
Honor @Primary for UserDetailsService and UserDetailsPasswordService in InitializeUserDetailsBeanManagerConfigurer #18015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2382273    to
    d4e00e4      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @siva-sai-udaygirl, for the PR! I've left some feedback inline.
|  | ||
| PasswordEncoder passwordEncoder = getBeanIfUnique(PasswordEncoder.class); | ||
| // Also resolve UDPS via container so @Primary is honored | ||
| UserDetailsPasswordService passwordManager = getAutowireCandidateOrNull(UserDetailsPasswordService.class); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this changes the existing semantics by calling getIfAvailable instead of getIfUnique. The ObjectProvider JavaDoc implies that it accounts for @Primary:
	/**
	 * Return an instance (possibly shared or independent) of the object
	 * managed by this factory.
	 * @return an instance of the bean, or {@code null} if not available or
	 * not unique (i.e. multiple candidates found **with none marked as primary**)
	 * @throws BeansException in case of creation errors
	 * @see #getObject()
	 */Can you confirm with unit tests that changing to getIfAvailable is necessary to support @Primary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @jzheaux! I added unit tests to show why we need to rely on the container’s autowire-candidate resolution to honor @primary.
• With getIfUnique(...) when there are two UserDetailsService beans and one is @primary, it still returns null (multiple names), so the global AuthenticationManager isn’t configured.
• With getIfAvailable(...) it delegates to autowire-candidate selection (same as normal injection), so the @primary bean is chosen, and the AuthenticationManager is configured.
New tests:
- whenMultipleUdsAndOneResolvableCandidate_thenPrimaryIsAutoWired
- whenMultipleUdsAndNoSingleCandidate_thenSkipAutoWiring
Location: spring-security-config/src/test/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurerTests.java
This keeps behavior the same for the single-bean case, and for multiple beans without a single resolvable candidate we still skip auto-wiring (and log). Please take another look and let me know if you’d prefer a different approach.
Signed-off-by: Siva Sai Udayagiri <[email protected]>
This commit rearranges the branches to reduce nesting Signed-off-by: Josh Cummings <[email protected]>
Given that the codebase uses getBeanIfUnique logic in many places, it is a little noisy to clarify how it works in just this location. Instead, the previous commit simplifies the branching logic, reducing the need for clarification of what could otherwise be surprising. Signed-off-by: Josh Cummings <[email protected]>
d4e00e4    to
    581d666      
    Compare
  
    …in InitializeUserDetailsBeanManagerConfigurer (spring-projects#17902) Signed-off-by: Siva Sai Udayagiri <[email protected]>
2cacbbb    to
    829cac4      
    Compare
  
    
Motivation
In scenarios where multiple
UserDetailsServiceorUserDetailsPasswordServicebeansare defined, Spring may raise
NoUniqueBeanDefinitionExceptionor select the wrongbean without explicit resolution.
Changes
@PrimarytoUserDetailsServiceandUserDetailsPasswordServicebeans inInitializeUserDetailsBeanManagerConfigurerIssue Reference
Fixes gh-17902
Testing
InitializeUserDetailsBeanManagerConfigurerTeststo validateresolution when multiple candidates are present
./gradlew checkNotes