I don't think this is a bug, and it's implemented to work twice, but I'm wondering if there's a way to do it, so I leave an issue.
Context
In CustomProvider that implements the AuthenticationProvider interface, if an exception is thrown from the authentication() method, the authentication() method of the CustomProvider is executed twice.
The example code for Custom Provider is as follows.
@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
if (authentication.getPrincipal() instanceof UserDetails) {
return new UsernamePasswordAuthenticationToken(authentication.getPrincipal(), "", authentication.getAuthorities());
}
final UserDetails user = loadUserByUsername(authentication);
final var loginResult = login(authentication);
if (isFailedLogin(loginResult)) {
increaseLoginCount(user.getUsername()); // twice increase count
log.info(loginResult.getMessage());
throw new UsernameNotFoundException(loginResult.getMessage()); // here exception
}
return new UsernamePasswordAuthenticationToken(user, "", authentication.getAuthorities());
}
The relevant logic is performed in the ProviderManager class of the security project,
Provider Manager's authenticate() method is approximately as follows.
@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
...
for (AuthenticationProvider provider : getProviders()) {
if (!provider.supports(toTest)) {
continue;
}
if (logger.isTraceEnabled()) {
logger.trace(LogMessage.format("Authenticating request with %s (%d/%d)",
provider.getClass().getSimpleName(), ++currentPosition, size));
}
try {
// first call: The result is null because CustomProvider throws an exception.
result = provider.authenticate(authentication);
...
if (result == null && this.parent != null) {
// Allow the parent to try.
try {
// second call
parentResult = this.parent.authenticate(authentication);
result = parentResult;
}
This seems to be the general behavior of ProviderManager, but I have logic in CustomProvider's authenticate() method that increases the count + 1 every time a user fails to log in, and it is called twice, increasing the count by 2.
Is there any way or solution to prevent logic for CustomProvider to retry? Or, even if it is done twice, is there a way to find out that it was done the second time in CustomProvider's authenticate() method?
Comment From: marcusdacoregio
Hi, @JuHyun419.
I believe that you can use HttpSecurity#authenticationManager to register your custom ProviderManager with no parent manager. Something like:
@Bean
SecurityFilterChain filterChain(HttpSecurity http, List<AuthenticationProvider> providers) {
http.authenticationManager(myProviderManager(providers));
// ...
return http.build();
}
private ProviderManager myProviderManager(List<AuthenticationProvider> providers) {
ProviderManager manager = new ProviderManager(providers);
return manager;
}
Does that make sense?
Comment From: JuHyun419
@marcusdacoregio Thank you for your quick response
I tried your way, but still, My CustomProvider is called twice
@Configuration
@EnableWebSecurity
@EnableMethodSecurity(
securedEnabled = true,
jsr250Enabled = true)
@RequiredArgsConstructor
public class SecurityConfig {
...
@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http, List<AuthenticationProvider> providers) throws Exception {
final var authenticationManager = authenticationManager(http);
http.authenticationManager(customProviderManager(providers));
http.cors(Customizer.withDefaults())
.httpBasic().disable()
.csrf().disable()
.formLogin().disable();
http.headers().frameOptions().sameOrigin()
.and()
.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS);
...
}
@Bean
public MyCustomProvider authenticationProvider() {
return new MyCustomProvider(authAPIService, redisService);
}
private ProviderManager customProviderManager(List<AuthenticationProvider> providers) {
return new ProviderManager(providers);
}
In ProviderManager, this.parent is still not null, so it seems to be calling twice.
Is there something wrong with my code settings?
Comment From: JuHyun419
I don't know if this is a good way, but there are other solutions.
The authentication.setAuthenticated(false); method allows count + 1 to be performed only on the second call.
like this,
// My CustomProvider
@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
...
final UserDetails user = loadUserByUsername(authentication);
final var loginResult = login(authentication);
// Cases called due to initial authentication failure processing
if (!authentication.isAuthenticated()) {
increaseLoginCount(user.getUsername());
}
if (isFailedLogin(loginResult)) {
log.error(loginResult.getMessage());
// Process false when initial authentication fails
authentication.setAuthenticated(false);
throw new UsernameNotFoundException(loginResult.getMessage());
}
return new UsernamePasswordAuthenticationToken(user, "", authentication.getAuthorities());
}
Comment From: marcusdacoregio
Hello, @JuHyun419. Can you provide a minimal, reproducible sample where I can check your configuration?
Comment From: JuHyun419
@marcusdacoregio There were some issues in the process of making sample projects to reproduce the same situation.
The versions I use are Spring Boot 2.7x and Spring Security 5.7.6, The version for creating the sample project is Spring Boot 3.1.6, Spring Security 6.1.5, and many changes exist in security 5-> security 6.1.5.
I think it will take some time to change my project version to the latest version and create a project to reproducible sample. Is it all right?
Comment From: marcusdacoregio
You can use Spring Security 5.7 if the sample is minimal and reproducible.
Comment From: spring-projects-issues
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Comment From: spring-projects-issues
Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.