-
Couldn't load subscription status.
- 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
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.
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 <udayagirishivasai@gmail.com>
This commit rearranges the branches to reduce nesting Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>
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 <3627351+jzheaux@users.noreply.github.com>
d4e00e4 to
581d666
Compare
...in InitializeUserDetailsBeanManagerConfigurer (spring-projects#17902) Signed-off-by: Siva Sai Udayagiri <udayagirishivasai@gmail.com>
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