Address PR Feedback

- Add final to implementations - Remove OneTimeTokenAuthenticationFilter and use AuthenticationFilter directly - Align method names in OneTimeToken - Create OneTimeTokenAuthenticationRequestSuccessHandler for a better control of the strategy used when a successful ott authentication request is made, defaults to RedirectOneTimeTokenAuthenticationRequestSuccessHandler
marcusdacoregio · Aug 15, 2024 · 0e43f84 · 0e43f84
1 parent 0c46878
commit 0e43f84
Show file tree

Hide file tree

Showing 15 changed files with 216 additions and 70 deletions.
diff --git a/...src/main/java/org/springframework/security/config/annotation/web/HttpSecurityBuilder.java b/...src/main/java/org/springframework/security/config/annotation/web/HttpSecurityBuilder.java
@@ -32,7 +32,6 @@
 import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
 import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
 import org.springframework.security.web.authentication.logout.LogoutFilter;
-import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationFilter;
 import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter;
 import org.springframework.security.web.authentication.preauth.x509.X509AuthenticationFilter;
 import org.springframework.security.web.authentication.rememberme.RememberMeAuthenticationFilter;
@@ -151,14 +150,14 @@ public interface HttpSecurityBuilder<H extends HttpSecurityBuilder<H>>
 	 * {@docRoot}/org/springframework/security/cas/web/CasAuthenticationFilter.html">CasAuthenticationFilter</a></li>
 	 * <li>{@link org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter}</li>
 	 * <li>{@link org.springframework.security.saml2.provider.service.web.authentication.Saml2WebSsoAuthenticationFilter}</li>
-	 * <li>{@link OneTimeTokenAuthenticationFilter}</li>
 	 * <li>{@link UsernamePasswordAuthenticationFilter}</li>
 	 * <li>{@link org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter}</li>
 	 * <li>{@link org.springframework.security.web.authentication.ui.DefaultLogoutPageGeneratingFilter}</li>
 	 * <li>{@link ConcurrentSessionFilter}</li>
 	 * <li>{@link DigestAuthenticationFilter}</li>
 	 * <li>{@link BearerTokenAuthenticationFilter}</li>
 	 * <li>{@link BasicAuthenticationFilter}</li>
+	 * <li>{@link org.springframework.security.web.authentication.AuthenticationFilter}</li>
 	 * <li>{@link RequestCacheAwareFilter}</li>
 	 * <li>{@link SecurityContextHolderAwareRequestFilter}</li>
 	 * <li>{@link JaasApiIntegrationFilter}</li>

diff --git a/.../org/springframework/security/config/annotation/web/builders/FilterOrderRegistration.java b/.../org/springframework/security/config/annotation/web/builders/FilterOrderRegistration.java
@@ -27,9 +27,9 @@
 import org.springframework.security.web.access.intercept.AuthorizationFilter;
 import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
 import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
+import org.springframework.security.web.authentication.AuthenticationFilter;
 import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
 import org.springframework.security.web.authentication.logout.LogoutFilter;
-import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationFilter;
 import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationRequestFilter;
 import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter;
 import org.springframework.security.web.authentication.preauth.x509.X509AuthenticationFilter;
@@ -99,7 +99,6 @@ final class FilterOrderRegistration {
 		this.filterToOrder.put(
 				"org.springframework.security.saml2.provider.service.web.authentication.Saml2WebSsoAuthenticationFilter",
 				order.next());
-		put(OneTimeTokenAuthenticationFilter.class, order.next());
 		put(UsernamePasswordAuthenticationFilter.class, order.next());
 		order.next(); // gh-8105
 		put(DefaultLoginPageGeneratingFilter.class, order.next());
@@ -111,6 +110,7 @@ final class FilterOrderRegistration {
 				"org.springframework.security.oauth2.server.resource.web.authentication.BearerTokenAuthenticationFilter",
 				order.next());
 		put(BasicAuthenticationFilter.class, order.next());
+		put(AuthenticationFilter.class, order.next());
 		put(RequestCacheAwareFilter.class, order.next());
 		put(SecurityContextHolderAwareRequestFilter.class, order.next());
 		put(JaasApiIntegrationFilter.class, order.next());

diff --git a/...framework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java b/...framework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java
@@ -40,14 +40,18 @@
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.web.authentication.AuthenticationConverter;
 import org.springframework.security.web.authentication.AuthenticationFailureHandler;
+import org.springframework.security.web.authentication.AuthenticationFilter;
 import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
 import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler;
 import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler;
 import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationConverter;
-import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationFilter;
 import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationRequestFilter;
+import org.springframework.security.web.authentication.ott.OneTimeTokenAuthenticationRequestSuccessHandler;
+import org.springframework.security.web.authentication.ott.RedirectOneTimeTokenAuthenticationRequestSuccessHandler;
 import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter;
 import org.springframework.security.web.authentication.ui.DefaultOneTimeTokenSubmitPageGeneratingFilter;
+import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
+import org.springframework.security.web.context.SecurityContextRepository;
 import org.springframework.security.web.csrf.CsrfToken;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -79,7 +83,8 @@ public final class OneTimeTokenLoginConfigurer<H extends HttpSecurityBuilder<H>>
 
 	private String authenticationRequestUrl = "/ott/authenticate";
 
-	private String authenticationRequestRedirectUrl = "/login/ott";
+	private OneTimeTokenAuthenticationRequestSuccessHandler authenticationRequestSuccessHandler = new RedirectOneTimeTokenAuthenticationRequestSuccessHandler(
+			"/login/ott");
 
 	private AuthenticationProvider authenticationProvider;
 
@@ -118,18 +123,27 @@ public void configure(H http) {
 
 	private void configureOttAuthenticationFilter(H http) {
 		AuthenticationManager authenticationManager = http.getSharedObject(AuthenticationManager.class);
-		OneTimeTokenAuthenticationFilter oneTimeTokenAuthenticationFilter = new OneTimeTokenAuthenticationFilter(
-				authenticationManager, this.authenticationConverter);
+		AuthenticationFilter oneTimeTokenAuthenticationFilter = new AuthenticationFilter(authenticationManager,
+				this.authenticationConverter);
+		oneTimeTokenAuthenticationFilter.setSecurityContextRepository(getSecurityContextRepository(http));
 		oneTimeTokenAuthenticationFilter.setRequestMatcher(antMatcher(HttpMethod.POST, this.loginProcessingUrl));
 		oneTimeTokenAuthenticationFilter.setFailureHandler(getAuthenticationFailureHandler());
 		oneTimeTokenAuthenticationFilter.setSuccessHandler(this.authenticationSuccessHandler);
 		http.addFilter(postProcess(oneTimeTokenAuthenticationFilter));
 	}
 
+	private SecurityContextRepository getSecurityContextRepository(H http) {
+		SecurityContextRepository securityContextRepository = http.getSharedObject(SecurityContextRepository.class);
+		if (securityContextRepository != null) {
+			return securityContextRepository;
+		}
+		return new HttpSessionSecurityContextRepository();
+	}
+
 	private void configureOttAuthenticationRequestFilter(H http) {
 		OneTimeTokenAuthenticationRequestFilter authenticationRequestFilter = new OneTimeTokenAuthenticationRequestFilter(
 				getOneTimeTokenService(http), getOneTimeTokenSender(http));
-		authenticationRequestFilter.setRedirectUrl(this.authenticationRequestRedirectUrl);
+		authenticationRequestFilter.setAuthenticationRequestSuccessHandler(this.authenticationRequestSuccessHandler);
 		authenticationRequestFilter.setRequestMatcher(antMatcher(HttpMethod.POST, this.authenticationRequestUrl));
 		http.addFilter(postProcess(authenticationRequestFilter));
 	}
@@ -177,15 +191,15 @@ public OneTimeTokenLoginConfigurer<H> authenticationRequestUrl(String authentica
 	}
 
 	/**
-	 * Specifies the URL that the user-agent will be redirected after a successful
-	 * One-Time Token authentication. Defaults to {@code POST /login/ott}. If you are
-	 * using the default submit page make sure that you also configure
-	 * {@link #submitPageUrl(String)} to this same URL.
-	 * @param authenticationRequestRedirectUrl
+	 * Specifies strategy to be used for successful one-time token authentication
+	 * requests. By default, a redirect will be performed to {@code POST /login/ott} using
+	 * the {@link RedirectOneTimeTokenAuthenticationRequestSuccessHandler}.
+	 * @param authenticationRequestSuccessHandler
 	 */
-	public OneTimeTokenLoginConfigurer<H> authenticationRequestRedirectUrl(String authenticationRequestRedirectUrl) {
-		Assert.hasText(authenticationRequestRedirectUrl, "authenticationRequestRedirectUrl cannot be null or empty");
-		this.authenticationRequestRedirectUrl = authenticationRequestRedirectUrl;
+	public OneTimeTokenLoginConfigurer<H> authenticationRequestSuccessHandler(
+			OneTimeTokenAuthenticationRequestSuccessHandler authenticationRequestSuccessHandler) {
+		Assert.notNull(authenticationRequestSuccessHandler, "authenticationRequestSuccessHandler cannot be null");
+		this.authenticationRequestSuccessHandler = authenticationRequestSuccessHandler;
 		return this;
 	}
 

diff --git a/...work/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurerTests.java b/...work/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurerTests.java
@@ -34,6 +34,8 @@
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.provisioning.InMemoryUserDetailsManager;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.authentication.SimpleUrlAuthenticationSuccessHandler;
+import org.springframework.security.web.authentication.ott.RedirectOneTimeTokenAuthenticationRequestSuccessHandler;
 import org.springframework.test.web.servlet.MockMvc;
 
 import static org.assertj.core.api.Assertions.assertThatException;
@@ -58,7 +60,7 @@ void oneTimeTokenWhenCorrectTokenThenCanAuthenticate() throws Exception {
 		this.mvc.perform(post("/ott/authenticate").param("username", "user").with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/login/ott"));
 
-		String token = TestOneTimeTokenSender.lastToken.getToken();
+		String token = TestOneTimeTokenSender.lastToken.getTokenValue();
 
 		this.mvc.perform(post("/login/ott").param("token", token).with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/"), authenticated());
@@ -68,12 +70,12 @@ void oneTimeTokenWhenCorrectTokenThenCanAuthenticate() throws Exception {
 	void oneTimeTokenWhenDifferentAuthenticationUrlsThenCanAuthenticate() throws Exception {
 		this.spring.register(OneTimeTokenDifferentAuthenticationUrlsConfig.class).autowire();
 		this.mvc.perform(post("/authenticationrequesturl").param("username", "user").with(csrf()))
-			.andExpectAll(status().isFound(), redirectedUrl("/login/ott"));
+			.andExpectAll(status().isFound(), redirectedUrl("/redirected"));
 
-		String token = TestOneTimeTokenSender.lastToken.getToken();
+		String token = TestOneTimeTokenSender.lastToken.getTokenValue();
 
 		this.mvc.perform(post("/loginprocessingurl").param("token", token).with(csrf()))
-			.andExpectAll(status().isFound(), redirectedUrl("/"), authenticated());
+			.andExpectAll(status().isFound(), redirectedUrl("/authenticated"), authenticated());
 	}
 
 	@Test
@@ -82,7 +84,7 @@ void oneTimeTokenWhenCorrectTokenUsedTwiceThenSecondTimeFails() throws Exception
 		this.mvc.perform(post("/ott/authenticate").param("username", "user").with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/login/ott"));
 
-		String token = TestOneTimeTokenSender.lastToken.getToken();
+		String token = TestOneTimeTokenSender.lastToken.getTokenValue();
 
 		this.mvc.perform(post("/login/ott").param("token", token).with(csrf()))
 			.andExpectAll(status().isFound(), redirectedUrl("/"), authenticated());
@@ -169,7 +171,9 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
 					)
 					.oneTimeTokenLogin((ott) -> ott
 							.authenticationRequestUrl("/authenticationrequesturl")
+							.authenticationRequestSuccessHandler(new RedirectOneTimeTokenAuthenticationRequestSuccessHandler("/redirected"))
 							.loginProcessingUrl("/loginprocessingurl")
+							.authenticationSuccessHandler(new SimpleUrlAuthenticationSuccessHandler("/authenticated"))
 					);
 			// @formatter:on
 			return http.build();

diff --git a/core/src/main/java/org/springframework/security/authentication/ott/DefaultOneTimeToken.java b/core/src/main/java/org/springframework/security/authentication/ott/DefaultOneTimeToken.java
@@ -44,7 +44,7 @@ public DefaultOneTimeToken(String token, String username, Instant expireAt) {
 	}
 
 	@Override
-	public String getToken() {
+	public String getTokenValue() {
 		return this.token;
 	}
 
@@ -53,7 +53,7 @@ public String getUsername() {
 		return this.username;
 	}
 
-	public Instant getExpireAt() {
+	public Instant getExpiresAt() {
 		return this.expireAt;
 	}
 

diff --git a/...ain/java/org/springframework/security/authentication/ott/InMemoryOneTimeTokenService.java b/...ain/java/org/springframework/security/authentication/ott/InMemoryOneTimeTokenService.java
@@ -34,7 +34,7 @@
  * @author Marcus da Coregio
  * @since 6.4
  */
-public class InMemoryOneTimeTokenService implements OneTimeTokenService {
+public final class InMemoryOneTimeTokenService implements OneTimeTokenService {
 
 	private final Map<String, OneTimeToken> oneTimeTokenByToken = new ConcurrentHashMap<>();
 
@@ -72,7 +72,7 @@ private void cleanExpiredTokensIfNeeded() {
 	}
 
 	private boolean isExpired(OneTimeToken ott) {
-		return this.clock.instant().isAfter(ott.getExpireAt());
+		return this.clock.instant().isAfter(ott.getExpiresAt());
 	}
 
 	void setClock(Clock clock) {

diff --git a/core/src/main/java/org/springframework/security/authentication/ott/OneTimeToken.java b/core/src/main/java/org/springframework/security/authentication/ott/OneTimeToken.java
@@ -27,9 +27,9 @@
 public interface OneTimeToken {
 
 	/**
-	 * @return the one-time token, never {@code null}
+	 * @return the one-time token value, never {@code null}
 	 */
-	String getToken();
+	String getTokenValue();
 
 	/**
 	 * @return the username associated with this token, never {@code null}
@@ -39,6 +39,6 @@ public interface OneTimeToken {
 	/**
 	 * @return the expiration time of the token
 	 */
-	Instant getExpireAt();
+	Instant getExpiresAt();
 
 }
diff --git a/...a/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java b/...a/org/springframework/security/authentication/ott/OneTimeTokenAuthenticationProvider.java
@@ -31,7 +31,7 @@
  * @author Marcus da Coregio
  * @since 6.4
  */
-public class OneTimeTokenAuthenticationProvider implements AuthenticationProvider {
+public final class OneTimeTokenAuthenticationProvider implements AuthenticationProvider {
 
 	private final OneTimeTokenService oneTimeTokenService;
 

diff --git a/...ava/org/springframework/security/authentication/ott/InMemoryOneTimeTokenServiceTests.java b/...ava/org/springframework/security/authentication/ott/InMemoryOneTimeTokenServiceTests.java
@@ -43,7 +43,7 @@ class InMemoryOneTimeTokenServiceTests {
 	void generateThenTokenValueShouldBeValidUuidAndProvidedUsernameIsUsed() {
 		OneTimeTokenAuthenticationRequest request = new OneTimeTokenAuthenticationRequest("user");
 		OneTimeToken oneTimeToken = this.oneTimeTokenService.generate(request);
-		assertThatNoException().isThrownBy(() -> UUID.fromString(oneTimeToken.getToken()));
+		assertThatNoException().isThrownBy(() -> UUID.fromString(oneTimeToken.getTokenValue()));
 		assertThat(request.getUsername()).isEqualTo("user");
 	}
 
@@ -58,18 +58,20 @@ void consumeWhenTokenDoesNotExistsThenNull() {
 	void consumeWhenTokenExistsThenReturnItself() {
 		OneTimeTokenAuthenticationRequest request = new OneTimeTokenAuthenticationRequest("user");
 		OneTimeToken generated = this.oneTimeTokenService.generate(request);
-		OneTimeTokenAuthenticationToken authenticationToken = new OneTimeTokenAuthenticationToken(generated.getToken());
+		OneTimeTokenAuthenticationToken authenticationToken = new OneTimeTokenAuthenticationToken(
+				generated.getTokenValue());
 		OneTimeToken consumed = this.oneTimeTokenService.consume(authenticationToken);
-		assertThat(consumed.getToken()).isEqualTo(generated.getToken());
+		assertThat(consumed.getTokenValue()).isEqualTo(generated.getTokenValue());
 		assertThat(consumed.getUsername()).isEqualTo(generated.getUsername());
-		assertThat(consumed.getExpireAt()).isEqualTo(generated.getExpireAt());
+		assertThat(consumed.getExpiresAt()).isEqualTo(generated.getExpiresAt());
 	}
 
 	@Test
 	void consumeWhenTokenIsExpiredThenReturnNull() {
 		OneTimeTokenAuthenticationRequest request = new OneTimeTokenAuthenticationRequest("user");
 		OneTimeToken generated = this.oneTimeTokenService.generate(request);
-		OneTimeTokenAuthenticationToken authenticationToken = new OneTimeTokenAuthenticationToken(generated.getToken());
+		OneTimeTokenAuthenticationToken authenticationToken = new OneTimeTokenAuthenticationToken(
+				generated.getTokenValue());
 		Clock tenMinutesFromNow = Clock.fixed(Instant.now().plus(10, ChronoUnit.MINUTES), ZoneOffset.UTC);
 		this.oneTimeTokenService.setClock(tenMinutesFromNow);
 		OneTimeToken consumed = this.oneTimeTokenService.consume(authenticationToken);
@@ -88,12 +90,12 @@ void generateWhenMoreThan100TokensThenClearExpired() {
 
 		assertThat(toExpire)
 			.extracting(
-					(token) -> this.oneTimeTokenService.consume(new OneTimeTokenAuthenticationToken(token.getToken())))
+					(token) -> this.oneTimeTokenService.consume(new OneTimeTokenAuthenticationToken(token.getTokenValue())))
 			.containsOnlyNulls();
 
 		assertThat(toKeep)
 			.extracting(
-					(token) -> this.oneTimeTokenService.consume(new OneTimeTokenAuthenticationToken(token.getToken())))
+					(token) -> this.oneTimeTokenService.consume(new OneTimeTokenAuthenticationToken(token.getTokenValue())))
 			.noneMatch(Objects::isNull);
 		// @formatter:on
 	}

diff --git a/docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc b/docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc
@@ -28,7 +28,7 @@ In summary, One-Time Tokens (OTT) provide a way to authenticate users without ad
 
 The One-Time Token Login works in two major steps.
 
-1. User request a token by submitting their user identifier, usually the username, and the token is delivered to them, via e-mail, SMS, Magic Link, etc.
+1. User requests a token by submitting their user identifier, usually the username, and the token is delivered to them, often as a Magic Link, via e-mail, SMS, etc.
 2. User submits the token to the application and, if valid, the user gets logged in.
 
 [[default-pages]]