Skip to content

Commit 829cac4

Browse files
siva-sai-udaygirisiva-sai-udaygiri
committed
Honor @primary for UserDetailsService and UserDetailsPasswordService in InitializeUserDetailsBeanManagerConfigurer (#17902)
Signed-off-by: Siva Sai Udayagiri <[email protected]>
1 parent 581d666 commit 829cac4

File tree

2 files changed

+71
-92
lines changed

2 files changed

+71
-92
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java‎

Lines changed: 44 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,6 @@
66
* You may obtain a copy of the License at
77
*
88
* https://www.apache.org/licenses/LICENSE-2.0
9-
*
10-
* Unless required by applicable law or agreed to in writing, software
11-
* distributed under the License is distributed on an "AS IS" BASIS,
12-
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13-
* See the License for the specific language governing permissions and
14-
* limitations under the License.
159
*/
1610

1711
package org.springframework.security.config.annotation.authentication.configuration;
@@ -21,7 +15,6 @@
2115
import org.apache.commons.logging.Log;
2216
import org.apache.commons.logging.LogFactory;
2317

24-
import org.springframework.beans.BeansException;
2518
import org.springframework.context.ApplicationContext;
2619
import org.springframework.core.Ordered;
2720
import org.springframework.core.annotation.Order;
@@ -34,15 +27,15 @@
3427
import org.springframework.security.crypto.password.PasswordEncoder;
3528

3629
/**
37-
* Lazily initializes the global authentication with a {@link UserDetailsService}. If
38-
* multiple beans of that type exist, the container's autowire rules are used to select a
39-
* single candidate (e.g. {@code @Primary}). If no single candidate can be resolved, the
40-
* configurer logs a warning and does not auto-wire. Optionally wires a
41-
* {@link PasswordEncoder}, {@link UserDetailsPasswordService}, and
42-
* {@link CompromisedPasswordChecker} when available.
30+
* Lazily initializes the global authentication with a {@link UserDetailsService}
31+
* if it is not yet configured. Honors {@code @Primary} when multiple
32+
* {@link UserDetailsService} (or {@link UserDetailsPasswordService}) beans are present.
33+
* If a single {@link PasswordEncoder} or {@link CompromisedPasswordChecker} bean is
34+
* available, those are wired as well.
4335
*
4436
* @author Rob Winch
4537
* @author Ngoc Nhan
38+
* @author You
4639
* @since 4.1
4740
*/
4841
@Order(InitializeUserDetailsBeanManagerConfigurer.DEFAULT_ORDER)
@@ -68,33 +61,54 @@ class InitializeUserDetailsManagerConfigurer extends GlobalAuthenticationConfigu
6861
@Override
6962
public void configure(AuthenticationManagerBuilder auth) {
7063
String[] beanNames = InitializeUserDetailsBeanManagerConfigurer.this.context
71-
.getBeanNamesForType(UserDetailsService.class);
64+
.getBeanNamesForType(UserDetailsService.class);
7265

66+
// If user configured an AuthenticationProvider already, warn and bail
67+
if (auth.isConfigured()) {
68+
if (beanNames.length > 0) {
69+
this.logger.warn(
70+
"Global AuthenticationManager configured with an AuthenticationProvider bean. "
71+
+ "UserDetailsService beans will not be used by Spring Security for automatically configuring username/password login. "
72+
+ "Consider removing the AuthenticationProvider bean or configure DaoAuthenticationProvider manually.");
73+
}
74+
return;
75+
}
76+
77+
// No UDS beans — nothing to do
7378
if (beanNames.length == 0) {
7479
return;
7580
}
7681

77-
if (auth.isConfigured()) {
78-
this.logger.warn("Global AuthenticationManager configured with an AuthenticationProvider bean. "
79-
+ "UserDetailsService beans will not be used by Spring Security for automatically configuring username/password login. "
80-
+ "Consider removing the AuthenticationProvider bean. "
81-
+ "Alternatively, consider using the UserDetailsService in a manually instantiated DaoAuthenticationProvider. "
82-
+ "If the current configuration is intentional, to turn off this warning, "
83-
+ "increase the logging level of 'org.springframework.security.config.annotation.authentication.configuration.InitializeUserDetailsBeanManagerConfigurer' to ERROR");
82+
/*
83+
* Try to resolve a single autowire-candidate UDS from the container.
84+
* getIfAvailable() returns:
85+
* - the bean if there is exactly one, or
86+
* - the @Primary bean if there are multiple and one is marked primary,
87+
* - otherwise null.
88+
*/
89+
UserDetailsService userDetailsService = getAutowireCandidateOrNull(UserDetailsService.class);
90+
91+
// If still ambiguous and we have multiple beans, keep current (warn + skip)
92+
if (userDetailsService == null && beanNames.length > 1) {
93+
this.logger.warn(LogMessage.format(
94+
"Found %s UserDetailsService beans, with names %s. "
95+
+ "Global Authentication Manager will not use a UserDetailsService for username/password login. "
96+
+ "Consider publishing a single (or @Primary) UserDetailsService bean.",
97+
beanNames.length, Arrays.toString(beanNames)));
8498
return;
8599
}
86100

87-
UserDetailsService userDetailsService = getBeanIfUnique(UserDetailsService.class);
101+
// If there is exactly one bean and getIfAvailable returned null (shouldn't happen),
102+
// fall back to retrieving that single bean by name.
88103
if (userDetailsService == null) {
89-
this.logger.warn(LogMessage.format("Found %s UserDetailsService beans, with names %s. "
90-
+ "Global Authentication Manager will not use a UserDetailsService for username/password login. "
91-
+ "Consider publishing a single (or primary) UserDetailsService bean.", beanNames.length,
92-
Arrays.toString(beanNames)));
93-
return;
104+
userDetailsService = InitializeUserDetailsBeanManagerConfigurer.this.context
105+
.getBean(beanNames[0], UserDetailsService.class);
94106
}
95107

96108
PasswordEncoder passwordEncoder = getBeanIfUnique(PasswordEncoder.class);
109+
// Honor @Primary for UDPS as well
97110
UserDetailsPasswordService passwordManager = getAutowireCandidateOrNull(UserDetailsPasswordService.class);
111+
// Keep "unique only" semantics for optional checker
98112
CompromisedPasswordChecker passwordChecker = getBeanIfUnique(CompromisedPasswordChecker.class);
99113

100114
DaoAuthenticationProvider provider = new DaoAuthenticationProvider(userDetailsService);
@@ -108,48 +122,20 @@ public void configure(AuthenticationManagerBuilder auth) {
108122
provider.setCompromisedPasswordChecker(passwordChecker);
109123
}
110124
provider.afterPropertiesSet();
125+
111126
auth.authenticationProvider(provider);
112127

113-
String selectedName = resolveBeanName(beanNames, userDetailsService);
114128
this.logger.info(LogMessage.format(
115-
"Global AuthenticationManager configured with UserDetailsService bean with name %s", selectedName));
129+
"Global AuthenticationManager configured with UserDetailsService bean (auto-selected)."));
116130
}
117131

118-
/**
119-
* Resolve a single autowire candidate for the given type (honors
120-
* {@code @Primary}). Returns {@code null} if ambiguous or not present.
121-
*/
122132
private <T> T getAutowireCandidateOrNull(Class<T> type) {
123-
try {
124-
return InitializeUserDetailsBeanManagerConfigurer.this.context.getBeanProvider(type).getIfAvailable();
125-
}
126-
catch (BeansException ex) {
127-
return null;
128-
}
133+
return InitializeUserDetailsBeanManagerConfigurer.this.context.getBeanProvider(type).getIfAvailable();
129134
}
130135

131-
/**
132-
* Return a bean of the requested class if there's exactly one registered
133-
* component; {@code null} otherwise.
134-
*/
135136
private <T> T getBeanIfUnique(Class<T> type) {
136137
return InitializeUserDetailsBeanManagerConfigurer.this.context.getBeanProvider(type).getIfUnique();
137138
}
138-
139-
private String resolveBeanName(String[] candidates, Object instance) {
140-
for (String name : candidates) {
141-
try {
142-
Object bean = InitializeUserDetailsBeanManagerConfigurer.this.context.getBean(name);
143-
if (bean == instance) {
144-
return name;
145-
}
146-
}
147-
catch (BeansException ignored) {
148-
}
149-
}
150-
return instance.getClass().getName();
151-
}
152-
153139
}
154140

155141
}

‎config/src/test/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurerTests.java‎

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,6 @@
66
* You may obtain a copy of the License at
77
*
88
* https://www.apache.org/licenses/LICENSE-2.0
9-
*
10-
* Unless required by applicable law or agreed to in writing, software
11-
* distributed under the License is distributed on an "AS IS" BASIS,
12-
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13-
* See the License for the specific language governing permissions and
14-
* limitations under the License.
159
*/
1610

1711
package org.springframework.security.config.annotation.authentication.configuration;
@@ -44,9 +38,7 @@ class InitializeUserDetailsBeanManagerConfigurerTests {
4438
private static ObjectPostProcessor<Object> opp() {
4539
return new ObjectPostProcessor<>() {
4640
@Override
47-
public <O> O postProcess(O object) {
48-
return object;
49-
}
41+
public <O> O postProcess(O object) { return object; }
5042
};
5143
}
5244

@@ -62,42 +54,43 @@ void whenMultipleUdsAndOneResolvableCandidate_thenPrimaryIsAutoWired() throws Ex
6254
User.withUsername("alice").passwordEncoder(encoder::encode).password("pw").roles("USER").build());
6355
InMemoryUserDetailsManager secondary = new InMemoryUserDetailsManager();
6456

65-
ObjectProvider<UserDetailsService> udsProvider = (ObjectProvider<UserDetailsService>) mock(
66-
ObjectProvider.class);
57+
ObjectProvider<UserDetailsService> udsProvider =
58+
(ObjectProvider<UserDetailsService>) mock(ObjectProvider.class);
6759
given(ctx.getBeanProvider(UserDetailsService.class)).willReturn(udsProvider);
68-
given(udsProvider.getIfUnique()).willReturn(primary); // container picks single
69-
// candidate
60+
given(udsProvider.getIfAvailable()).willReturn(primary); // container picks single candidate
7061

7162
// resolveBeanName(..) path
7263
given(ctx.getBean("udsA")).willReturn(secondary);
7364
given(ctx.getBean("udsB")).willReturn(primary);
7465

75-
ObjectProvider<PasswordEncoder> peProvider = (ObjectProvider<PasswordEncoder>) mock(ObjectProvider.class);
66+
ObjectProvider<PasswordEncoder> peProvider =
67+
(ObjectProvider<PasswordEncoder>) mock(ObjectProvider.class);
7668
given(ctx.getBeanProvider(PasswordEncoder.class)).willReturn(peProvider);
7769
given(peProvider.getIfUnique()).willReturn(encoder);
7870

7971
// Stub optional providers to avoid NPEs
80-
ObjectProvider<UserDetailsPasswordService> udpsProvider = (ObjectProvider<UserDetailsPasswordService>) mock(
81-
ObjectProvider.class);
72+
ObjectProvider<UserDetailsPasswordService> udpsProvider =
73+
(ObjectProvider<UserDetailsPasswordService>) mock(ObjectProvider.class);
8274
given(ctx.getBeanProvider(UserDetailsPasswordService.class)).willReturn(udpsProvider);
8375
given(udpsProvider.getIfAvailable()).willReturn(null);
8476

85-
ObjectProvider<CompromisedPasswordChecker> cpcProvider = (ObjectProvider<CompromisedPasswordChecker>) mock(
86-
ObjectProvider.class);
77+
ObjectProvider<CompromisedPasswordChecker> cpcProvider =
78+
(ObjectProvider<CompromisedPasswordChecker>) mock(ObjectProvider.class);
8779
given(ctx.getBeanProvider(CompromisedPasswordChecker.class)).willReturn(cpcProvider);
8880
given(cpcProvider.getIfUnique()).willReturn(null);
8981

9082
AuthenticationManagerBuilder builder = new AuthenticationManagerBuilder(opp());
91-
new InitializeUserDetailsBeanManagerConfigurer(ctx).new InitializeUserDetailsManagerConfigurer()
92-
.configure(builder);
83+
new InitializeUserDetailsBeanManagerConfigurer(ctx)
84+
.new InitializeUserDetailsManagerConfigurer()
85+
.configure(builder);
9386

9487
AuthenticationManager manager = builder.build();
9588

9689
// DaoAuthenticationProvider registered
9790
assertThat(manager).isInstanceOf(ProviderManager.class);
9891
List<?> providers = ((ProviderManager) manager).getProviders();
9992
assertThat(providers)
100-
.anySatisfy((p) -> assertThat(p.getClass().getSimpleName()).isEqualTo("DaoAuthenticationProvider"));
93+
.anySatisfy((p) -> assertThat(p.getClass().getSimpleName()).isEqualTo("DaoAuthenticationProvider"));
10194

10295
// Auth works with the primary UDS + encoder
10396
var auth = manager.authenticate(new UsernamePasswordAuthenticationToken("alice", "pw"));
@@ -110,30 +103,31 @@ void whenMultipleUdsAndNoSingleCandidate_thenSkipAutoWiring() throws Exception {
110103
ApplicationContext ctx = mock(ApplicationContext.class);
111104
given(ctx.getBeanNamesForType(UserDetailsService.class)).willReturn(new String[] { "udsA", "udsB" });
112105

113-
ObjectProvider<UserDetailsService> udsProvider = (ObjectProvider<UserDetailsService>) mock(
114-
ObjectProvider.class);
106+
ObjectProvider<UserDetailsService> udsProvider =
107+
(ObjectProvider<UserDetailsService>) mock(ObjectProvider.class);
115108
given(ctx.getBeanProvider(UserDetailsService.class)).willReturn(udsProvider);
116-
given(udsProvider.getIfAvailable()).willReturn(null); // ambiguous → no single
117-
// candidate
109+
given(udsProvider.getIfAvailable()).willReturn(null); // ambiguous → no single candidate
118110

119111
// Also stub other providers to null
120-
ObjectProvider<PasswordEncoder> peProvider = (ObjectProvider<PasswordEncoder>) mock(ObjectProvider.class);
112+
ObjectProvider<PasswordEncoder> peProvider =
113+
(ObjectProvider<PasswordEncoder>) mock(ObjectProvider.class);
121114
given(ctx.getBeanProvider(PasswordEncoder.class)).willReturn(peProvider);
122115
given(peProvider.getIfUnique()).willReturn(null);
123116

124-
ObjectProvider<UserDetailsPasswordService> udpsProvider = (ObjectProvider<UserDetailsPasswordService>) mock(
125-
ObjectProvider.class);
117+
ObjectProvider<UserDetailsPasswordService> udpsProvider =
118+
(ObjectProvider<UserDetailsPasswordService>) mock(ObjectProvider.class);
126119
given(ctx.getBeanProvider(UserDetailsPasswordService.class)).willReturn(udpsProvider);
127120
given(udpsProvider.getIfAvailable()).willReturn(null);
128121

129-
ObjectProvider<CompromisedPasswordChecker> cpcProvider = (ObjectProvider<CompromisedPasswordChecker>) mock(
130-
ObjectProvider.class);
122+
ObjectProvider<CompromisedPasswordChecker> cpcProvider =
123+
(ObjectProvider<CompromisedPasswordChecker>) mock(ObjectProvider.class);
131124
given(ctx.getBeanProvider(CompromisedPasswordChecker.class)).willReturn(cpcProvider);
132125
given(cpcProvider.getIfUnique()).willReturn(null);
133126

134127
AuthenticationManagerBuilder builder = new AuthenticationManagerBuilder(opp());
135-
new InitializeUserDetailsBeanManagerConfigurer(ctx).new InitializeUserDetailsManagerConfigurer()
136-
.configure(builder);
128+
new InitializeUserDetailsBeanManagerConfigurer(ctx)
129+
.new InitializeUserDetailsManagerConfigurer()
130+
.configure(builder);
137131

138132
AuthenticationManager manager = builder.build();
139133

@@ -143,11 +137,10 @@ void whenMultipleUdsAndNoSingleCandidate_thenSkipAutoWiring() throws Exception {
143137
}
144138
else if (manager instanceof ProviderManager pm) {
145139
assertThat(pm.getProviders())
146-
.noneMatch((p) -> p.getClass().getSimpleName().equals("DaoAuthenticationProvider"));
140+
.noneMatch((p) -> p.getClass().getSimpleName().equals("DaoAuthenticationProvider"));
147141
}
148142
else {
149143
assertThat(manager.getClass().getSimpleName()).isNotEqualTo("ProviderManager");
150144
}
151145
}
152-
153146
}

0 commit comments

Comments
 (0)