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 <[email protected]>

1

Signed-off-by: Andrey Litvitski <[email protected]>

Author: therepanic

core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java

@@ -16,11 +16,22 @@
 
 package org.springframework.security.authentication.ott;
 
+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.SpringSecurityMessageSource;
 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;
@@ -31,14 +42,21 @@
  * {@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 final OneTimeTokenService oneTimeTokenService;
 
 	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");
@@ -56,6 +74,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
 		}
 		try {
 			UserDetails user = this.userDetailsService.loadUserByUsername(consumed.getUsername());
+			this.authenticationChecks.check(user);
 			OneTimeTokenAuthenticationToken authenticated = OneTimeTokenAuthenticationToken.authenticated(user,
 					user.getAuthorities());
 			authenticated.setDetails(otpAuthenticationToken.getDetails());
@@ -71,4 +90,39 @@ public boolean supports(Class<?> authentication) {
 		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"));
+			}
+		}
+
+	}
+
 }

core/src/test/java/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProviderTests.java

@@ -26,6 +26,7 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 
 import org.springframework.security.authentication.BadCredentialsException;
+import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.userdetails.User;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
@@ -42,6 +43,7 @@
  * Tests for {@link OneTimeTokenAuthenticationProvider}.
  *
  * @author Max Batischev
+ * @author Andrey Litvitski
  */
 @ExtendWith(MockitoExtension.class)
 public class OneTimeTokenAuthenticationProviderTests {
@@ -79,6 +81,17 @@ void authenticateWhenAuthenticationTokenIsPresentThenAuthenticates() {
 		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);