JdbcOneTimeTokenService.setCleanupCron

Spring Security uses setter methods for optional member variables. Allows
for a null cleanupCron to disable the cleanup.

In a clustered environment it is likely that users do not want all nodes
to be performing a cleanup because it will cause contention on the ott
table.

Another example is if a user wants to invoke cleanUpExpiredTokens with a
different strategy all together, they might want to disable the cron job.

Issue gh-15735

Author: rwinch

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

@@ -31,6 +31,7 @@
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.DisposableBean;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.jdbc.core.ArgumentPreparedStatementSetter;
 import org.springframework.jdbc.core.JdbcOperations;
 import org.springframework.jdbc.core.PreparedStatementSetter;
@@ -40,7 +41,6 @@
 import org.springframework.scheduling.support.CronTrigger;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
-import org.springframework.util.StringUtils;
 
 /**
  *
@@ -56,7 +56,7 @@
  * @author Max Batischev
  * @since 6.4
  */
-public final class JdbcOneTimeTokenService implements OneTimeTokenService, DisposableBean {
+public final class JdbcOneTimeTokenService implements OneTimeTokenService, DisposableBean, InitializingBean {
 
 	private final Log logger = LogFactory.getLog(getClass());
 
@@ -70,7 +70,7 @@ public final class JdbcOneTimeTokenService implements OneTimeTokenService, Dispo
 
 	private ThreadPoolTaskScheduler taskScheduler;
 
-	private static final String DEFAULT_CLEANUP_CRON = "0 * * * * *";
+	private static final String DEFAULT_CLEANUP_CRON = "@hourly";
 
 	private static final String TABLE_NAME = "one_time_tokens";
 
@@ -104,23 +104,27 @@ public final class JdbcOneTimeTokenService implements OneTimeTokenService, Dispo
 	/**
 	 * Constructs a {@code JdbcOneTimeTokenService} using the provide parameters.
 	 * @param jdbcOperations the JDBC operations
-	 * @param cleanupCron cleanup cron expression
 	 */
-	public JdbcOneTimeTokenService(JdbcOperations jdbcOperations, String cleanupCron) {
-		Assert.isTrue(StringUtils.hasText(cleanupCron), "cleanupCron cannot be null orr empty");
+	public JdbcOneTimeTokenService(JdbcOperations jdbcOperations) {
 		Assert.notNull(jdbcOperations, "jdbcOperations cannot be null");
 		this.jdbcOperations = jdbcOperations;
-		this.taskScheduler = createTaskScheduler(cleanupCron);
+		this.taskScheduler = createTaskScheduler(DEFAULT_CLEANUP_CRON);
 	}
 
 	/**
-	 * Constructs a {@code JdbcOneTimeTokenService} using the provide parameters.
-	 * @param jdbcOperations the JDBC operations
+	 * Sets the chron expression used for cleaning up expired sessions. The default is to
+	 * run hourly.
+	 *
+	 * For more advanced use cases the cleanupCron may be set to null which will disable
+	 * the built-in cleanup. Users can then invoke {@link #cleanupExpiredTokens()} using
+	 * custom logic.
+	 * @param cleanupCron the chron expression passed to {@link CronTrigger} used for
+	 * determining how frequent to perform cleanup. The default is "@hourly".
+	 * @see CronTrigger
+	 * @see #cleanupExpiredTokens()
 	 */
-	public JdbcOneTimeTokenService(JdbcOperations jdbcOperations) {
-		Assert.notNull(jdbcOperations, "jdbcOperations cannot be null");
-		this.jdbcOperations = jdbcOperations;
-		this.taskScheduler = createTaskScheduler(DEFAULT_CLEANUP_CRON);
+	public void setCleanupCron(String cleanupCron) {
+		this.taskScheduler = createTaskScheduler(cleanupCron);
 	}
 
 	@Override
@@ -174,6 +178,9 @@ private void deleteOneTimeToken(OneTimeToken oneTimeToken) {
 	}
 
 	private ThreadPoolTaskScheduler createTaskScheduler(String cleanupCron) {
+		if (cleanupCron == null) {
+			return null;
+		}
 		ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler();
 		taskScheduler.setThreadNamePrefix("spring-one-time-tokens-");
 		taskScheduler.initialize();
@@ -188,6 +195,11 @@ public void cleanupExpiredTokens() {
 		this.logger.debug("Cleaned up " + deletedCount + " expired tokens");
 	}
 
+	@Override
+	public void afterPropertiesSet() throws Exception {
+		this.taskScheduler.afterPropertiesSet();
+	}
+
 	@Override
 	public void destroy() throws Exception {
 		if (this.taskScheduler != null) {

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

@@ -175,6 +175,17 @@ void cleanupExpiredTokens() {
 		assertThat(deletedOneTimeToken2).isNull();
 	}
 
+	@Test
+	void setCleanupChronWhenNullThenNoException() {
+		this.oneTimeTokenService.setCleanupCron(null);
+	}
+
+	@Test
+	void setCleanupChronWhenAlreadyNullThenNoException() {
+		this.oneTimeTokenService.setCleanupCron(null);
+		this.oneTimeTokenService.setCleanupCron(null);
+	}
+
 	private void saveToken(OneTimeToken oneTimeToken) {
 		List<SqlParameterValue> parameters = this.oneTimeTokenParametersMapper.apply(oneTimeToken);
 		PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray());