From 612b15abcc1a3a87c45db0b05fc4a33ad19fc458 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Tue, 1 Oct 2024 08:49:42 -0500 Subject: [PATCH] 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 --- .../ott/JdbcOneTimeTokenService.java | 38 ++++++++++++------- .../ott/JdbcOneTimeTokenServiceTests.java | 11 ++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenService.java b/core/src/main/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenService.java index ec4a4291af..66eda461e8 100644 --- a/core/src/main/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenService.java +++ b/core/src/main/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenService.java @@ -31,6 +31,7 @@ import org.apache.commons.logging.Log; 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.concurrent.ThreadPoolTaskScheduler; 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 @@ import org.springframework.util.StringUtils; * @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 @@ public final class JdbcOneTimeTokenService implements OneTimeTokenService, Dispo } 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 final class JdbcOneTimeTokenService implements OneTimeTokenService, Dispo 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) { diff --git a/core/src/test/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenServiceTests.java b/core/src/test/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenServiceTests.java index b7d3bb86d9..028dc3ade5 100644 --- a/core/src/test/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenServiceTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenServiceTests.java @@ -175,6 +175,17 @@ public class JdbcOneTimeTokenServiceTests { 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 parameters = this.oneTimeTokenParametersMapper.apply(oneTimeToken); PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray());