Skip to content

Commit c7c8f08

Browse files
committed
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
1 parent f61a8de commit c7c8f08

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

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

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,21 @@
1616

1717
package org.springframework.security.authentication.ott;
1818

19+
import org.apache.commons.logging.Log;
20+
import org.apache.commons.logging.LogFactory;
21+
import org.springframework.context.MessageSource;
22+
import org.springframework.context.MessageSourceAware;
23+
import org.springframework.context.support.MessageSourceAccessor;
24+
import org.springframework.security.authentication.AccountExpiredException;
1925
import org.springframework.security.authentication.AuthenticationProvider;
2026
import org.springframework.security.authentication.BadCredentialsException;
27+
import org.springframework.security.authentication.DisabledException;
28+
import org.springframework.security.authentication.LockedException;
2129
import org.springframework.security.core.Authentication;
2230
import org.springframework.security.core.AuthenticationException;
31+
import org.springframework.security.core.SpringSecurityMessageSource;
2332
import org.springframework.security.core.userdetails.UserDetails;
33+
import org.springframework.security.core.userdetails.UserDetailsChecker;
2434
import org.springframework.security.core.userdetails.UserDetailsService;
2535
import org.springframework.security.core.userdetails.UsernameNotFoundException;
2636
import org.springframework.util.Assert;
@@ -31,14 +41,21 @@
3141
* {@link UserDetailsService} to fetch user authorities.
3242
*
3343
* @author Marcus da Coregio
44+
* @author Andrey Litvitski
3445
* @since 6.4
3546
*/
36-
public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider {
47+
public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider, MessageSourceAware {
3748

3849
private final OneTimeTokenService oneTimeTokenService;
3950

4051
private final UserDetailsService userDetailsService;
4152

53+
private final Log logger = LogFactory.getLog(getClass());
54+
55+
private UserDetailsChecker authenticationChecks = new DefaultAuthenticationChecks();
56+
57+
private MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor();
58+
4259
public OneTimeTokenAuthenticationProvider(OneTimeTokenService oneTimeTokenService,
4360
UserDetailsService userDetailsService) {
4461
Assert.notNull(oneTimeTokenService, "oneTimeTokenService cannot be null");
@@ -56,6 +73,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
5673
}
5774
try {
5875
UserDetails user = this.userDetailsService.loadUserByUsername(consumed.getUsername());
76+
this.authenticationChecks.check(user);
5977
OneTimeTokenAuthenticationToken authenticated = OneTimeTokenAuthenticationToken.authenticated(user,
6078
user.getAuthorities());
6179
authenticated.setDetails(otpAuthenticationToken.getDetails());
@@ -71,4 +89,39 @@ public boolean supports(Class<?> authentication) {
7189
return OneTimeTokenAuthenticationToken.class.isAssignableFrom(authentication);
7290
}
7391

92+
@Override
93+
public void setMessageSource(MessageSource messageSource) {
94+
this.messages = new MessageSourceAccessor(messageSource);
95+
}
96+
97+
public void setAuthenticationChecks(UserDetailsChecker authenticationChecks) {
98+
this.authenticationChecks = authenticationChecks;
99+
}
100+
101+
private class DefaultAuthenticationChecks implements UserDetailsChecker {
102+
103+
@Override
104+
public void check(UserDetails user) {
105+
if (!user.isAccountNonLocked()) {
106+
OneTimeTokenAuthenticationProvider.this.logger
107+
.debug("Failed to authenticate since user account is locked");
108+
throw new LockedException(OneTimeTokenAuthenticationProvider.this.messages
109+
.getMessage("AbstractUserDetailsAuthenticationProvider.locked", "User account is locked"));
110+
}
111+
if (!user.isEnabled()) {
112+
OneTimeTokenAuthenticationProvider.this.logger
113+
.debug("Failed to authenticate since user account is disabled");
114+
throw new DisabledException(OneTimeTokenAuthenticationProvider.this.messages
115+
.getMessage("AbstractUserDetailsAuthenticationProvider.disabled", "User is disabled"));
116+
}
117+
if (!user.isAccountNonExpired()) {
118+
OneTimeTokenAuthenticationProvider.this.logger
119+
.debug("Failed to authenticate since user account has expired");
120+
throw new AccountExpiredException(OneTimeTokenAuthenticationProvider.this.messages
121+
.getMessage("AbstractUserDetailsAuthenticationProvider.expired", "User account has expired"));
122+
}
123+
}
124+
125+
}
126+
74127
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.mockito.junit.jupiter.MockitoExtension;
2727

2828
import org.springframework.security.authentication.BadCredentialsException;
29+
import org.springframework.security.core.AuthenticationException;
2930
import org.springframework.security.core.userdetails.User;
3031
import org.springframework.security.core.userdetails.UserDetailsService;
3132
import org.springframework.security.core.userdetails.UsernameNotFoundException;
@@ -42,6 +43,7 @@
4243
* Tests for {@link OneTimeTokenAuthenticationProvider}.
4344
*
4445
* @author Max Batischev
46+
* @author Andrey Litvitski
4547
*/
4648
@ExtendWith(MockitoExtension.class)
4749
public class OneTimeTokenAuthenticationProviderTests {
@@ -79,6 +81,17 @@ void authenticateWhenAuthenticationTokenIsPresentThenAuthenticates() {
7981
assertThat(CollectionUtils.isEmpty(user.getAuthorities())).isTrue();
8082
}
8183

84+
@Test
85+
void authenticateWhenAuthenticationTokenIsPresentThenFails() {
86+
given(this.oneTimeTokenService.consume(any()))
87+
.willReturn(new DefaultOneTimeToken(TOKEN, USERNAME, Instant.now().plusSeconds(120)));
88+
given(this.userDetailsService.loadUserByUsername(anyString()))
89+
.willReturn(new User(USERNAME, PASSWORD, false, false, false, false, List.of()));
90+
OneTimeTokenAuthenticationToken token = new OneTimeTokenAuthenticationToken(TOKEN);
91+
92+
assertThatExceptionOfType(AuthenticationException.class).isThrownBy(() -> this.provider.authenticate(token));
93+
}
94+
8295
@Test
8396
void authenticateWhenOneTimeTokenIsNotFoundThenFails() {
8497
given(this.oneTimeTokenService.consume(any())).willReturn(null);

0 commit comments

Comments
 (0)