From 64fa98da1a6d2a242fd266bb67d2342faf66937f Mon Sep 17 00:00:00 2001 From: Andrey Litvitski Date: Sat, 2 Aug 2025 16:28:10 +0300 Subject: [PATCH] Validate account status in OneTimeTokenAuthenticationProvider The main problem is that OneTimeTokenAuthenticationProvider does not extend from AbstractUserDetailsAuthenticationProvider, which has a preauthentication check for user details. However, we do not need to extend from it because it does not fit the context of the class. In this regard, I decided to add my own checker to this commit, which performs a preauthentication check before authorizing the account, similar to how it is done in AbstractUserDetailsAuthenticationProvider. I also added a test to OneTimeTokenAuthenticationProviderTests that identifies this problem. Closes gh-17655 Signed-off-by: Andrey Litvitski --- .../OneTimeTokenAuthenticationProvider.java | 56 ++++++++++++++++++- ...eTimeTokenAuthenticationProviderTests.java | 13 +++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java b/core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java index b72e4f8e8c..462c991026 100644 --- a/core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java @@ -19,13 +19,24 @@ package org.springframework.security.authentication.ott; import java.util.Collection; import java.util.HashSet; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceAware; +import org.springframework.context.support.MessageSourceAccessor; +import org.springframework.security.authentication.AccountExpiredException; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.DisabledException; +import org.springframework.security.authentication.LockedException; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.SpringSecurityMessageSource; import org.springframework.security.core.authority.FactorGrantedAuthority; import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsChecker; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.util.Assert; @@ -36,9 +47,10 @@ import org.springframework.util.Assert; * {@link UserDetailsService} to fetch user authorities. * * @author Marcus da Coregio + * @author Andrey Litvitski * @since 6.4 */ -public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider { +public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider, MessageSourceAware { private static final String AUTHORITY = FactorGrantedAuthority.OTT_AUTHORITY; @@ -46,6 +58,12 @@ public final class OneTimeTokenAuthenticationProvider implements AuthenticationP private final UserDetailsService userDetailsService; + private final Log logger = LogFactory.getLog(getClass()); + + private UserDetailsChecker authenticationChecks = new DefaultAuthenticationChecks(); + + private MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); + public OneTimeTokenAuthenticationProvider(OneTimeTokenService oneTimeTokenService, UserDetailsService userDetailsService) { Assert.notNull(oneTimeTokenService, "oneTimeTokenService cannot be null"); @@ -63,6 +81,7 @@ public final class OneTimeTokenAuthenticationProvider implements AuthenticationP } try { UserDetails user = this.userDetailsService.loadUserByUsername(consumed.getUsername()); + this.authenticationChecks.check(user); Collection authorities = new HashSet<>(user.getAuthorities()); authorities.add(FactorGrantedAuthority.fromAuthority(AUTHORITY)); OneTimeTokenAuthentication authenticated = new OneTimeTokenAuthentication(user, authorities); @@ -79,4 +98,39 @@ public final class OneTimeTokenAuthenticationProvider implements AuthenticationP return OneTimeTokenAuthenticationToken.class.isAssignableFrom(authentication); } + @Override + public void setMessageSource(MessageSource messageSource) { + this.messages = new MessageSourceAccessor(messageSource); + } + + public void setAuthenticationChecks(UserDetailsChecker authenticationChecks) { + this.authenticationChecks = authenticationChecks; + } + + private class DefaultAuthenticationChecks implements UserDetailsChecker { + + @Override + public void check(UserDetails user) { + if (!user.isAccountNonLocked()) { + OneTimeTokenAuthenticationProvider.this.logger + .debug("Failed to authenticate since user account is locked"); + throw new LockedException(OneTimeTokenAuthenticationProvider.this.messages + .getMessage("AbstractUserDetailsAuthenticationProvider.locked", "User account is locked")); + } + if (!user.isEnabled()) { + OneTimeTokenAuthenticationProvider.this.logger + .debug("Failed to authenticate since user account is disabled"); + throw new DisabledException(OneTimeTokenAuthenticationProvider.this.messages + .getMessage("AbstractUserDetailsAuthenticationProvider.disabled", "User is disabled")); + } + if (!user.isAccountNonExpired()) { + OneTimeTokenAuthenticationProvider.this.logger + .debug("Failed to authenticate since user account has expired"); + throw new AccountExpiredException(OneTimeTokenAuthenticationProvider.this.messages + .getMessage("AbstractUserDetailsAuthenticationProvider.expired", "User account has expired")); + } + } + + } + } diff --git a/core/src/test/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProviderTests.java index f3bc8ce322..a5db5f5883 100644 --- a/core/src/test/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProviderTests.java @@ -28,6 +28,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.SecurityAssertions; import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.authority.FactorGrantedAuthority; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetailsService; @@ -45,6 +46,7 @@ import static org.mockito.BDDMockito.given; * Tests for {@link OneTimeTokenAuthenticationProvider}. * * @author Max Batischev + * @author Andrey Litvitski */ @ExtendWith(MockitoExtension.class) public class OneTimeTokenAuthenticationProviderTests { @@ -81,6 +83,17 @@ public class OneTimeTokenAuthenticationProviderTests { assertThat(CollectionUtils.isEmpty(user.getAuthorities())).isTrue(); } + @Test + void authenticateWhenAuthenticationTokenIsPresentThenFails() { + given(this.oneTimeTokenService.consume(any())) + .willReturn(new DefaultOneTimeToken(TOKEN, USERNAME, Instant.now().plusSeconds(120))); + given(this.userDetailsService.loadUserByUsername(anyString())) + .willReturn(new User(USERNAME, PASSWORD, false, false, false, false, List.of())); + OneTimeTokenAuthenticationToken token = new OneTimeTokenAuthenticationToken(TOKEN); + + assertThatExceptionOfType(AuthenticationException.class).isThrownBy(() -> this.provider.authenticate(token)); + } + @Test void authenticateWhenOneTimeTokenIsNotFoundThenFails() { given(this.oneTimeTokenService.consume(any())).willReturn(null);