Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit b2bd58d

Browse files
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> 1 Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
1 parent f61a8de commit b2bd58d

File tree

2 files changed

+68
-1
lines changed

2 files changed

+68
-1
lines changed

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

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

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

19+
import org.apache.commons.logging.Log;
20+
import org.apache.commons.logging.LogFactory;
21+
22+
import org.springframework.context.MessageSource;
23+
import org.springframework.context.MessageSourceAware;
24+
import org.springframework.context.support.MessageSourceAccessor;
25+
import org.springframework.security.authentication.AccountExpiredException;
1926
import org.springframework.security.authentication.AuthenticationProvider;
2027
import org.springframework.security.authentication.BadCredentialsException;
28+
import org.springframework.security.authentication.DisabledException;
29+
import org.springframework.security.authentication.LockedException;
2130
import org.springframework.security.core.Authentication;
2231
import org.springframework.security.core.AuthenticationException;
32+
import org.springframework.security.core.SpringSecurityMessageSource;
2333
import org.springframework.security.core.userdetails.UserDetails;
34+
import org.springframework.security.core.userdetails.UserDetailsChecker;
2435
import org.springframework.security.core.userdetails.UserDetailsService;
2536
import org.springframework.security.core.userdetails.UsernameNotFoundException;
2637
import org.springframework.util.Assert;
@@ -31,14 +42,21 @@
3142
* {@link UserDetailsService} to fetch user authorities.
3243
*
3344
* @author Marcus da Coregio
45+
* @author Andrey Litvitski
3446
* @since 6.4
3547
*/
36-
public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider {
48+
public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider, MessageSourceAware {
3749

3850
private final OneTimeTokenService oneTimeTokenService;
3951

4052
private final UserDetailsService userDetailsService;
4153

54+
private final Log logger = LogFactory.getLog(getClass());
55+
56+
private UserDetailsChecker authenticationChecks = new DefaultAuthenticationChecks();
57+
58+
private MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor();
59+
4260
public OneTimeTokenAuthenticationProvider(OneTimeTokenService oneTimeTokenService,
4361
UserDetailsService userDetailsService) {
4462
Assert.notNull(oneTimeTokenService, "oneTimeTokenService cannot be null");
@@ -56,6 +74,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
5674
}
5775
try {
5876
UserDetails user = this.userDetailsService.loadUserByUsername(consumed.getUsername());
77+
this.authenticationChecks.check(user);
5978
OneTimeTokenAuthenticationToken authenticated = OneTimeTokenAuthenticationToken.authenticated(user,
6079
user.getAuthorities());
6180
authenticated.setDetails(otpAuthenticationToken.getDetails());
@@ -71,4 +90,39 @@ public boolean supports(Class<?> authentication) {
7190
return OneTimeTokenAuthenticationToken.class.isAssignableFrom(authentication);
7291
}
7392

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

‎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)

AltStyle によって変換されたページ (->オリジナル) /