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 <andrey1010102008@gmail.com>
This commit is contained in:
committed by
Josh Cummings
parent
26b20b2dd2
commit
64fa98da1a
+55
-1
@@ -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<GrantedAuthority> 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"));
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+13
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user