From 50cc36d53ea197b2a35e6a83eeaccb000e2b88d6 Mon Sep 17 00:00:00 2001 From: Max Batischev Date: Sun, 29 Sep 2024 00:06:10 +0300 Subject: [PATCH 01/10] Add support JdbcOneTimeTokenService Closes gh-15735 --- .../aot/hint/OneTimeTokenRuntimeHints.java | 40 +++ .../ott/JdbcOneTimeTokenService.java | 239 ++++++++++++++++++ .../resources/META-INF/spring/aot.factories | 4 +- .../core/ott/jdbc/one-time-tokens-schema.sql | 5 + .../hint/OneTimeTokenRuntimeHintsTests.java | 59 +++++ .../ott/JdbcOneTimeTokenServiceTests.java | 196 ++++++++++++++ 6 files changed, 542 insertions(+), 1 deletion(-) create mode 100644 core/src/main/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHints.java create mode 100644 core/src/main/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenService.java create mode 100644 core/src/main/resources/org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql create mode 100644 core/src/test/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHintsTests.java create mode 100644 core/src/test/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenServiceTests.java diff --git a/core/src/main/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHints.java b/core/src/main/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHints.java new file mode 100644 index 00000000000..5dd7ddb3ef0 --- /dev/null +++ b/core/src/main/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHints.java @@ -0,0 +1,40 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.aot.hint; + +import org.springframework.aot.hint.RuntimeHints; +import org.springframework.aot.hint.RuntimeHintsRegistrar; +import org.springframework.jdbc.core.JdbcOperations; +import org.springframework.security.authentication.ott.OneTimeToken; +import org.springframework.security.authentication.ott.OneTimeTokenService; + +/** + * + * A JDBC implementation of an {@link OneTimeTokenService} that uses a + * {@link JdbcOperations} for {@link OneTimeToken} persistence. + * + * @author Max Batischev + * @since 6.4 + */ +class OneTimeTokenRuntimeHints implements RuntimeHintsRegistrar { + + @Override + public void registerHints(RuntimeHints hints, ClassLoader classLoader) { + hints.resources().registerPattern("org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql"); + } + +} 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 new file mode 100644 index 00000000000..fe8b32e48dd --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenService.java @@ -0,0 +1,239 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.ott; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Timestamp; +import java.sql.Types; +import java.time.Clock; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.function.Function; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.jdbc.core.ArgumentPreparedStatementSetter; +import org.springframework.jdbc.core.JdbcOperations; +import org.springframework.jdbc.core.PreparedStatementSetter; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.SqlParameterValue; +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; + +/** + * + * A JDBC implementation of an {@link OneTimeTokenService} that uses a + * {@link JdbcOperations} for {@link OneTimeToken} persistence. + * + *

+ * NOTE: This {@code JdbcOneTimeTokenService} depends on the table definition + * described in + * "classpath:org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql" and + * therefore MUST be defined in the database schema. + * + * @author Max Batischev + * @since 6.4 + */ +public final class JdbcOneTimeTokenService implements OneTimeTokenService { + + private final Log logger = LogFactory.getLog(getClass()); + + private final JdbcOperations jdbcOperations; + + private Function> oneTimeTokenParametersMapper = new OneTimeTokenParametersMapper(); + + private RowMapper oneTimeTokenRowMapper = new OneTimeTokenRowMapper(); + + private Clock clock = Clock.systemUTC(); + + private ThreadPoolTaskScheduler taskScheduler; + + private static final String DEFAULT_CLEANUP_CRON = "0 * * * * *"; + + private static final String TABLE_NAME = "one_time_tokens"; + + // @formatter:off + private static final String COLUMN_NAMES = "token_value, " + + "username, " + + "expires_at"; + // @formatter:on + + // @formatter:off + private static final String SAVE_AUTHORIZED_CLIENT_SQL = "INSERT INTO " + TABLE_NAME + + " (" + COLUMN_NAMES + ") VALUES (?, ?, ?)"; + // @formatter:on + + private static final String FILTER = "token_value = ?"; + + private static final String DELETE_ONE_TIME_TOKEN_SQL = "DELETE FROM " + TABLE_NAME + " WHERE " + FILTER; + + // @formatter:off + private static final String SELECT_ONE_TIME_TOKEN_SQL = "SELECT " + COLUMN_NAMES + + " FROM " + TABLE_NAME + + " WHERE " + FILTER; + // @formatter:on + + // @formatter:off + private static final String DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY = "DELETE FROM " + + TABLE_NAME + + " WHERE expires_at < ?"; + // @formatter:on + + /** + * 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"); + Assert.notNull(jdbcOperations, "jdbcOperations cannot be null"); + this.jdbcOperations = jdbcOperations; + this.taskScheduler = createTaskScheduler(cleanupCron); + } + + /** + * Constructs a {@code JdbcOneTimeTokenService} using the provide parameters. + * @param jdbcOperations the JDBC operations + */ + public JdbcOneTimeTokenService(JdbcOperations jdbcOperations) { + Assert.notNull(jdbcOperations, "jdbcOperations cannot be null"); + this.jdbcOperations = jdbcOperations; + this.taskScheduler = createTaskScheduler(DEFAULT_CLEANUP_CRON); + } + + @Override + public OneTimeToken generate(GenerateOneTimeTokenRequest request) { + Assert.notNull(request, "generateOneTimeTokenRequest cannot be null"); + String token = UUID.randomUUID().toString(); + Instant fiveMinutesFromNow = this.clock.instant().plusSeconds(300); + OneTimeToken oneTimeToken = new DefaultOneTimeToken(token, request.getUsername(), fiveMinutesFromNow); + insertOneTimeToken(oneTimeToken); + return oneTimeToken; + } + + private void insertOneTimeToken(OneTimeToken oneTimeToken) { + List parameters = this.oneTimeTokenParametersMapper.apply(oneTimeToken); + PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); + this.jdbcOperations.update(SAVE_AUTHORIZED_CLIENT_SQL, pss); + } + + @Override + public OneTimeToken consume(OneTimeTokenAuthenticationToken authenticationToken) { + Assert.notNull(authenticationToken, "authenticationToken cannot be null"); + + List tokens = selectOneTimeToken(authenticationToken); + if (CollectionUtils.isEmpty(tokens)) { + return null; + } + OneTimeToken token = tokens.get(0); + deleteOneTimeToken(token); + if (isExpired(token)) { + return null; + } + return token; + } + + private boolean isExpired(OneTimeToken ott) { + return this.clock.instant().isAfter(ott.getExpiresAt()); + } + + private List selectOneTimeToken(OneTimeTokenAuthenticationToken authenticationToken) { + List parameters = List + .of(new SqlParameterValue(Types.VARCHAR, authenticationToken.getTokenValue())); + PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); + return this.jdbcOperations.query(SELECT_ONE_TIME_TOKEN_SQL, pss, this.oneTimeTokenRowMapper); + } + + private void deleteOneTimeToken(OneTimeToken oneTimeToken) { + List parameters = List + .of(new SqlParameterValue(Types.VARCHAR, oneTimeToken.getTokenValue())); + PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); + this.jdbcOperations.update(DELETE_ONE_TIME_TOKEN_SQL, pss); + } + + private ThreadPoolTaskScheduler createTaskScheduler(String cleanupCron) { + ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler(); + taskScheduler.setThreadNamePrefix("spring-one-time-tokens-"); + taskScheduler.initialize(); + taskScheduler.schedule(this::cleanUpExpiredTokens, new CronTrigger(cleanupCron)); + return taskScheduler; + } + + public void cleanUpExpiredTokens() { + List parameters = List.of(new SqlParameterValue(Types.TIMESTAMP, Instant.now())); + PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); + int deletedCount = this.jdbcOperations.update(DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY, pss); + this.logger.debug("Cleaned up " + deletedCount + " expired tokens"); + } + + /** + * Sets the {@link Clock} used when generating one-time token and checking token + * expiry. + * @param clock the clock + */ + public void setClock(Clock clock) { + Assert.notNull(clock, "clock cannot be null"); + this.clock = clock; + } + + /** + * The default {@code Function} that maps {@link OneTimeToken} to a {@code List} of + * {@link SqlParameterValue}. + * + * @author Max Batischev + * @since 6.4 + */ + public static class OneTimeTokenParametersMapper implements Function> { + + @Override + public List apply(OneTimeToken oneTimeToken) { + List parameters = new ArrayList<>(); + parameters.add(new SqlParameterValue(Types.VARCHAR, oneTimeToken.getTokenValue())); + parameters.add(new SqlParameterValue(Types.VARCHAR, oneTimeToken.getUsername())); + parameters.add(new SqlParameterValue(Types.TIMESTAMP, Timestamp.from(oneTimeToken.getExpiresAt()))); + return parameters; + } + + } + + /** + * The default {@link RowMapper} that maps the current row in + * {@code java.sql.ResultSet} to {@link OneTimeToken}. + * + * @author Max Batischev + * @since 6.4 + */ + public static class OneTimeTokenRowMapper implements RowMapper { + + @Override + public OneTimeToken mapRow(ResultSet rs, int rowNum) throws SQLException { + String tokenValue = rs.getString("token_value"); + String userName = rs.getString("username"); + Instant expiresAt = rs.getTimestamp("expires_at").toInstant(); + return new DefaultOneTimeToken(tokenValue, userName, expiresAt); + } + + } + +} diff --git a/core/src/main/resources/META-INF/spring/aot.factories b/core/src/main/resources/META-INF/spring/aot.factories index 2a24e540732..8596dc6a3fe 100644 --- a/core/src/main/resources/META-INF/spring/aot.factories +++ b/core/src/main/resources/META-INF/spring/aot.factories @@ -1,4 +1,6 @@ org.springframework.aot.hint.RuntimeHintsRegistrar=\ -org.springframework.security.aot.hint.CoreSecurityRuntimeHints +org.springframework.security.aot.hint.CoreSecurityRuntimeHints,\ +org.springframework.security.aot.hint.OneTimeTokenRuntimeHints + org.springframework.beans.factory.aot.BeanFactoryInitializationAotProcessor=\ org.springframework.security.aot.hint.SecurityHintsAotProcessor diff --git a/core/src/main/resources/org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql b/core/src/main/resources/org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql new file mode 100644 index 00000000000..2c471ee4042 --- /dev/null +++ b/core/src/main/resources/org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql @@ -0,0 +1,5 @@ +create table one_time_tokens( + token_value varchar(36) not null primary key, + username varchar_ignorecase(50) not null, + expires_at timestamp not null +); diff --git a/core/src/test/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHintsTests.java b/core/src/test/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHintsTests.java new file mode 100644 index 00000000000..d132e9f49af --- /dev/null +++ b/core/src/test/java/org/springframework/security/aot/hint/OneTimeTokenRuntimeHintsTests.java @@ -0,0 +1,59 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.aot.hint; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import org.springframework.aot.hint.RuntimeHints; +import org.springframework.aot.hint.RuntimeHintsRegistrar; +import org.springframework.aot.hint.predicate.RuntimeHintsPredicates; +import org.springframework.core.io.support.SpringFactoriesLoader; +import org.springframework.util.ClassUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link OneTimeTokenRuntimeHints} + * + * @author Max Batischev + */ +class OneTimeTokenRuntimeHintsTests { + + private final RuntimeHints hints = new RuntimeHints(); + + @BeforeEach + void setup() { + SpringFactoriesLoader.forResourceLocation("META-INF/spring/aot.factories") + .load(RuntimeHintsRegistrar.class) + .forEach((registrar) -> registrar.registerHints(this.hints, ClassUtils.getDefaultClassLoader())); + } + + @ParameterizedTest + @MethodSource("getOneTimeTokensSqlFiles") + void oneTimeTokensSqlFilesHasHints(String schemaFile) { + assertThat(RuntimeHintsPredicates.resource().forResource(schemaFile)).accepts(this.hints); + } + + private static Stream getOneTimeTokensSqlFiles() { + return Stream.of("org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql"); + } + +} 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 new file mode 100644 index 00000000000..9bbbe32fb6e --- /dev/null +++ b/core/src/test/java/org/springframework/security/authentication/ott/JdbcOneTimeTokenServiceTests.java @@ -0,0 +1,196 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.ott; + +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.temporal.ChronoUnit; +import java.util.List; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.jdbc.core.ArgumentPreparedStatementSetter; +import org.springframework.jdbc.core.JdbcOperations; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.PreparedStatementSetter; +import org.springframework.jdbc.core.SqlParameterValue; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; +import org.springframework.util.CollectionUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link JdbcOneTimeTokenService}. + * + * @author Max Batischev + */ +public class JdbcOneTimeTokenServiceTests { + + private static final String USERNAME = "user"; + + private static final String TOKEN_VALUE = "1234"; + + private static final String ONE_TIME_TOKEN_SQL_RESOURCE = "org/springframework/security/core/ott/jdbc/one-time-tokens-schema.sql"; + + private EmbeddedDatabase db; + + private JdbcOperations jdbcOperations; + + private JdbcOneTimeTokenService oneTimeTokenService; + + private final JdbcOneTimeTokenService.OneTimeTokenParametersMapper oneTimeTokenParametersMapper = new JdbcOneTimeTokenService.OneTimeTokenParametersMapper(); + + @BeforeEach + void setUp() { + this.db = createDb(); + this.jdbcOperations = new JdbcTemplate(this.db); + this.oneTimeTokenService = new JdbcOneTimeTokenService(this.jdbcOperations); + } + + @AfterEach + public void tearDown() { + this.db.shutdown(); + } + + private static EmbeddedDatabase createDb() { + // @formatter:off + return new EmbeddedDatabaseBuilder() + .generateUniqueName(true) + .setType(EmbeddedDatabaseType.HSQL) + .setScriptEncoding("UTF-8") + .addScript(ONE_TIME_TOKEN_SQL_RESOURCE) + .build(); + // @formatter:on + } + + @Test + void constructorWhenJdbcOperationsIsNullThenThrowIllegalArgumentException() { + // @formatter:off + assertThatIllegalArgumentException() + .isThrownBy(() -> new JdbcOneTimeTokenService(null)) + .withMessage("jdbcOperations cannot be null"); + // @formatter:on + } + + @Test + void generateWhenGenerateOneTimeTokenRequestIsNullThenThrowIllegalArgumentException() { + // @formatter:off + assertThatIllegalArgumentException() + .isThrownBy(() -> this.oneTimeTokenService.generate(null)) + .withMessage("generateOneTimeTokenRequest cannot be null"); + // @formatter:on + } + + @Test + void consumeWhenAuthenticationTokenIsNullThenThrowIllegalArgumentException() { + // @formatter:off + assertThatIllegalArgumentException() + .isThrownBy(() -> this.oneTimeTokenService.consume(null)) + .withMessage("authenticationToken cannot be null"); + // @formatter:on + } + + @Test + void generateThenTokenValueShouldBeValidUuidAndProvidedUsernameIsUsed() { + OneTimeToken oneTimeToken = this.oneTimeTokenService.generate(new GenerateOneTimeTokenRequest(USERNAME)); + + OneTimeToken persistedOneTimeToken = selectOneTimeToken(oneTimeToken.getTokenValue()); + assertThat(persistedOneTimeToken).isNotNull(); + assertThat(persistedOneTimeToken.getUsername()).isNotNull(); + assertThat(persistedOneTimeToken.getTokenValue()).isNotNull(); + assertThat(persistedOneTimeToken.getExpiresAt()).isNotNull(); + } + + @Test + void consumeWhenTokenExistsThenReturnItself() { + OneTimeToken oneTimeToken = this.oneTimeTokenService.generate(new GenerateOneTimeTokenRequest(USERNAME)); + OneTimeTokenAuthenticationToken authenticationToken = new OneTimeTokenAuthenticationToken( + oneTimeToken.getTokenValue()); + + OneTimeToken consumedOneTimeToken = this.oneTimeTokenService.consume(authenticationToken); + + assertThat(consumedOneTimeToken).isNotNull(); + assertThat(consumedOneTimeToken.getUsername()).isNotNull(); + assertThat(consumedOneTimeToken.getTokenValue()).isNotNull(); + assertThat(consumedOneTimeToken.getExpiresAt()).isNotNull(); + OneTimeToken persistedOneTimeToken = selectOneTimeToken(consumedOneTimeToken.getTokenValue()); + assertThat(persistedOneTimeToken).isNull(); + } + + @Test + void consumeWhenTokenDoesNotExistsThenReturnNull() { + OneTimeTokenAuthenticationToken authenticationToken = new OneTimeTokenAuthenticationToken(TOKEN_VALUE); + + OneTimeToken consumedOneTimeToken = this.oneTimeTokenService.consume(authenticationToken); + + assertThat(consumedOneTimeToken).isNull(); + } + + @Test + void consumeWhenTokenIsExpiredThenReturnNull() { + GenerateOneTimeTokenRequest request = new GenerateOneTimeTokenRequest(USERNAME); + OneTimeToken generated = this.oneTimeTokenService.generate(request); + 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); + assertThat(consumed).isNull(); + } + + @Test + void cleanupExpiredTokens() { + OneTimeToken token1 = new DefaultOneTimeToken("123", USERNAME, Instant.now().minusSeconds(300)); + OneTimeToken token2 = new DefaultOneTimeToken("456", USERNAME, Instant.now().minusSeconds(300)); + saveToken(token1); + saveToken(token2); + + this.oneTimeTokenService.cleanUpExpiredTokens(); + + OneTimeToken deletedOneTimeToken1 = selectOneTimeToken("123"); + OneTimeToken deletedOneTimeToken2 = selectOneTimeToken("456"); + assertThat(deletedOneTimeToken1).isNull(); + assertThat(deletedOneTimeToken2).isNull(); + } + + private void saveToken(OneTimeToken oneTimeToken) { + List parameters = this.oneTimeTokenParametersMapper.apply(oneTimeToken); + PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); + this.jdbcOperations.update("INSERT INTO one_time_tokens (token_value, username, expires_at) VALUES (?, ?, ?)", + pss); + } + + private OneTimeToken selectOneTimeToken(String tokenValue) { + // @formatter:off + List result = this.jdbcOperations.query( + "select token_value, username, expires_at from one_time_tokens where token_value = ?", + new JdbcOneTimeTokenService.OneTimeTokenRowMapper(), tokenValue); + if (CollectionUtils.isEmpty(result)) { + return null; + } + return result.get(0); + // @formatter:on + } + +} From 4f328c950365457c739dd61c053aa28ad5f7b2df Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Tue, 1 Oct 2024 08:50:41 -0500 Subject: [PATCH 02/10] destroy() shuts down the taskScheduler Issue gh-15735 --- .../authentication/ott/JdbcOneTimeTokenService.java | 10 +++++++++- .../ott/JdbcOneTimeTokenServiceTests.java | 3 ++- 2 files changed, 11 insertions(+), 2 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 fe8b32e48dd..379733d83b4 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 @@ -30,6 +30,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.DisposableBean; import org.springframework.jdbc.core.ArgumentPreparedStatementSetter; import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.PreparedStatementSetter; @@ -55,7 +56,7 @@ * @author Max Batischev * @since 6.4 */ -public final class JdbcOneTimeTokenService implements OneTimeTokenService { +public final class JdbcOneTimeTokenService implements OneTimeTokenService, DisposableBean { private final Log logger = LogFactory.getLog(getClass()); @@ -187,6 +188,13 @@ public void cleanUpExpiredTokens() { this.logger.debug("Cleaned up " + deletedCount + " expired tokens"); } + @Override + public void destroy() throws Exception { + if (this.taskScheduler != null) { + this.taskScheduler.shutdown(); + } + } + /** * Sets the {@link Clock} used when generating one-time token and checking token * expiry. 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 9bbbe32fb6e..016c4b0f496 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 @@ -68,8 +68,9 @@ void setUp() { } @AfterEach - public void tearDown() { + public void tearDown() throws Exception { this.db.shutdown(); + this.oneTimeTokenService.destroy(); } private static EmbeddedDatabase createDb() { From 4787ac254de46d4d44a73d825f08d07b70c98bac Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Tue, 1 Oct 2024 08:57:32 -0500 Subject: [PATCH 03/10] cleanUpExpiredTokens->cleanupExpiredTokens Issue gh-15735 --- .../security/authentication/ott/JdbcOneTimeTokenService.java | 4 ++-- .../authentication/ott/JdbcOneTimeTokenServiceTests.java | 2 +- 2 files changed, 3 insertions(+), 3 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 379733d83b4..ec4a4291afe 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 @@ -177,11 +177,11 @@ private ThreadPoolTaskScheduler createTaskScheduler(String cleanupCron) { ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler(); taskScheduler.setThreadNamePrefix("spring-one-time-tokens-"); taskScheduler.initialize(); - taskScheduler.schedule(this::cleanUpExpiredTokens, new CronTrigger(cleanupCron)); + taskScheduler.schedule(this::cleanupExpiredTokens, new CronTrigger(cleanupCron)); return taskScheduler; } - public void cleanUpExpiredTokens() { + public void cleanupExpiredTokens() { List parameters = List.of(new SqlParameterValue(Types.TIMESTAMP, Instant.now())); PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); int deletedCount = this.jdbcOperations.update(DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY, pss); 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 016c4b0f496..b7d3bb86d9f 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 @@ -167,7 +167,7 @@ void cleanupExpiredTokens() { saveToken(token1); saveToken(token2); - this.oneTimeTokenService.cleanUpExpiredTokens(); + this.oneTimeTokenService.cleanupExpiredTokens(); OneTimeToken deletedOneTimeToken1 = selectOneTimeToken("123"); OneTimeToken deletedOneTimeToken2 = selectOneTimeToken("456"); 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 04/10] 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 ec4a4291afe..66eda461e8b 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.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) { 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 b7d3bb86d9f..028dc3ade57 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 @@ 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 parameters = this.oneTimeTokenParametersMapper.apply(oneTimeToken); PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); From e8c71df899cefe9bc9554e3ad501fe8a2a941f61 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Tue, 1 Oct 2024 08:59:52 -0500 Subject: [PATCH 05/10] Use private Inner JdbcOneTimeTokenService classes Issue gh-15735 --- .../ott/JdbcOneTimeTokenService.java | 4 +- .../ott/JdbcOneTimeTokenServiceTests.java | 51 +++++++------------ 2 files changed, 19 insertions(+), 36 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 66eda461e8b..ee108cb1052 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 @@ -224,7 +224,7 @@ public void setClock(Clock clock) { * @author Max Batischev * @since 6.4 */ - public static class OneTimeTokenParametersMapper implements Function> { + private static class OneTimeTokenParametersMapper implements Function> { @Override public List apply(OneTimeToken oneTimeToken) { @@ -244,7 +244,7 @@ public List apply(OneTimeToken oneTimeToken) { * @author Max Batischev * @since 6.4 */ - public static class OneTimeTokenRowMapper implements RowMapper { + private static class OneTimeTokenRowMapper implements RowMapper { @Override public OneTimeToken mapRow(ResultSet rs, int rowNum) throws SQLException { 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 028dc3ade57..3a325adaca7 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 @@ -17,27 +17,25 @@ package org.springframework.security.authentication.ott; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.time.ZoneOffset; import java.time.temporal.ChronoUnit; -import java.util.List; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.jdbc.core.ArgumentPreparedStatementSetter; import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.JdbcTemplate; -import org.springframework.jdbc.core.PreparedStatementSetter; -import org.springframework.jdbc.core.SqlParameterValue; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; -import org.springframework.util.CollectionUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; /** * Tests for {@link JdbcOneTimeTokenService}. @@ -58,8 +56,6 @@ public class JdbcOneTimeTokenServiceTests { private JdbcOneTimeTokenService oneTimeTokenService; - private final JdbcOneTimeTokenService.OneTimeTokenParametersMapper oneTimeTokenParametersMapper = new JdbcOneTimeTokenService.OneTimeTokenParametersMapper(); - @BeforeEach void setUp() { this.db = createDb(); @@ -115,7 +111,8 @@ void consumeWhenAuthenticationTokenIsNullThenThrowIllegalArgumentException() { void generateThenTokenValueShouldBeValidUuidAndProvidedUsernameIsUsed() { OneTimeToken oneTimeToken = this.oneTimeTokenService.generate(new GenerateOneTimeTokenRequest(USERNAME)); - OneTimeToken persistedOneTimeToken = selectOneTimeToken(oneTimeToken.getTokenValue()); + OneTimeToken persistedOneTimeToken = this.oneTimeTokenService + .consume(new OneTimeTokenAuthenticationToken(oneTimeToken.getTokenValue())); assertThat(persistedOneTimeToken).isNotNull(); assertThat(persistedOneTimeToken.getUsername()).isNotNull(); assertThat(persistedOneTimeToken.getTokenValue()).isNotNull(); @@ -134,7 +131,8 @@ void consumeWhenTokenExistsThenReturnItself() { assertThat(consumedOneTimeToken.getUsername()).isNotNull(); assertThat(consumedOneTimeToken.getTokenValue()).isNotNull(); assertThat(consumedOneTimeToken.getExpiresAt()).isNotNull(); - OneTimeToken persistedOneTimeToken = selectOneTimeToken(consumedOneTimeToken.getTokenValue()); + OneTimeToken persistedOneTimeToken = this.oneTimeTokenService + .consume(new OneTimeTokenAuthenticationToken(consumedOneTimeToken.getTokenValue())); assertThat(persistedOneTimeToken).isNull(); } @@ -162,15 +160,19 @@ void consumeWhenTokenIsExpiredThenReturnNull() { @Test void cleanupExpiredTokens() { - OneTimeToken token1 = new DefaultOneTimeToken("123", USERNAME, Instant.now().minusSeconds(300)); - OneTimeToken token2 = new DefaultOneTimeToken("456", USERNAME, Instant.now().minusSeconds(300)); - saveToken(token1); - saveToken(token2); + Clock clock = mock(Clock.class); + Instant fiveMinutesAgo = Instant.now().minus(Duration.ofMinutes(5)); + given(clock.instant()).willReturn(fiveMinutesAgo); + this.oneTimeTokenService.setClock(clock); + OneTimeToken token1 = this.oneTimeTokenService.generate(new GenerateOneTimeTokenRequest(USERNAME)); + OneTimeToken token2 = this.oneTimeTokenService.generate(new GenerateOneTimeTokenRequest(USERNAME)); this.oneTimeTokenService.cleanupExpiredTokens(); - OneTimeToken deletedOneTimeToken1 = selectOneTimeToken("123"); - OneTimeToken deletedOneTimeToken2 = selectOneTimeToken("456"); + OneTimeToken deletedOneTimeToken1 = this.oneTimeTokenService + .consume(new OneTimeTokenAuthenticationToken(token1.getTokenValue())); + OneTimeToken deletedOneTimeToken2 = this.oneTimeTokenService + .consume(new OneTimeTokenAuthenticationToken(token2.getTokenValue())); assertThat(deletedOneTimeToken1).isNull(); assertThat(deletedOneTimeToken2).isNull(); } @@ -186,23 +188,4 @@ void setCleanupChronWhenAlreadyNullThenNoException() { this.oneTimeTokenService.setCleanupCron(null); } - private void saveToken(OneTimeToken oneTimeToken) { - List parameters = this.oneTimeTokenParametersMapper.apply(oneTimeToken); - PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); - this.jdbcOperations.update("INSERT INTO one_time_tokens (token_value, username, expires_at) VALUES (?, ?, ?)", - pss); - } - - private OneTimeToken selectOneTimeToken(String tokenValue) { - // @formatter:off - List result = this.jdbcOperations.query( - "select token_value, username, expires_at from one_time_tokens where token_value = ?", - new JdbcOneTimeTokenService.OneTimeTokenRowMapper(), tokenValue); - if (CollectionUtils.isEmpty(result)) { - return null; - } - return result.get(0); - // @formatter:on - } - } From 650ec3ba829b372da1be7043975080698c0a28ec Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:47:38 -0500 Subject: [PATCH 06/10] Use Duration for calculating validity This improves readability. Issue gh-15735 --- .../security/authentication/ott/JdbcOneTimeTokenService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 ee108cb1052..ec3b5dcdb87 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 @@ -21,6 +21,7 @@ import java.sql.Timestamp; import java.sql.Types; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.List; @@ -131,7 +132,7 @@ public void setCleanupCron(String cleanupCron) { public OneTimeToken generate(GenerateOneTimeTokenRequest request) { Assert.notNull(request, "generateOneTimeTokenRequest cannot be null"); String token = UUID.randomUUID().toString(); - Instant fiveMinutesFromNow = this.clock.instant().plusSeconds(300); + Instant fiveMinutesFromNow = this.clock.instant().plus(Duration.ofMinutes(5)); OneTimeToken oneTimeToken = new DefaultOneTimeToken(token, request.getUsername(), fiveMinutesFromNow); insertOneTimeToken(oneTimeToken); return oneTimeToken; From c4b60cd080cccfb8fdd233089574d8e9fe552897 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:48:10 -0500 Subject: [PATCH 07/10] Reduce visibility for JdbcOneTimeTokenServiceTests Issue gh-15735 --- .../authentication/ott/JdbcOneTimeTokenServiceTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 3a325adaca7..2bbca29282c 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 @@ -42,7 +42,7 @@ * * @author Max Batischev */ -public class JdbcOneTimeTokenServiceTests { +class JdbcOneTimeTokenServiceTests { private static final String USERNAME = "user"; @@ -64,7 +64,7 @@ void setUp() { } @AfterEach - public void tearDown() throws Exception { + void tearDown() throws Exception { this.db.shutdown(); this.oneTimeTokenService.destroy(); } From 7738e6c895607a095995ddb1b6f543e0589c7675 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:48:28 -0500 Subject: [PATCH 08/10] Add logger.isDebugEnabled() Issue gh-15735 --- .../security/authentication/ott/JdbcOneTimeTokenService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 ec3b5dcdb87..1b44b9ea6ed 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 @@ -193,7 +193,9 @@ public void cleanupExpiredTokens() { List parameters = List.of(new SqlParameterValue(Types.TIMESTAMP, Instant.now())); PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); int deletedCount = this.jdbcOperations.update(DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY, pss); - this.logger.debug("Cleaned up " + deletedCount + " expired tokens"); + if (logger.isDebugEnabled()) { + this.logger.debug("Cleaned up " + deletedCount + " expired tokens"); + } } @Override From c3a5ae12548101f7980356f090afb847b6c02c64 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:39:58 -0500 Subject: [PATCH 09/10] Fix logger checkstyle --- .../security/authentication/ott/JdbcOneTimeTokenService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1b44b9ea6ed..93144cd5535 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 @@ -193,7 +193,7 @@ public void cleanupExpiredTokens() { List parameters = List.of(new SqlParameterValue(Types.TIMESTAMP, Instant.now())); PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); int deletedCount = this.jdbcOperations.update(DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY, pss); - if (logger.isDebugEnabled()) { + if (this.logger.isDebugEnabled()) { this.logger.debug("Cleaned up " + deletedCount + " expired tokens"); } } From f002fedb733f49e1b36d148b4d87ed48c4770332 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:40:40 -0500 Subject: [PATCH 10/10] Document JdbcOneTimeTokenService Issue gh-15735 --- docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc b/docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc index df21b2cf3cf..5d34d0ba2d8 100644 --- a/docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc +++ b/docs/modules/ROOT/pages/servlet/authentication/onetimetoken.adoc @@ -412,6 +412,7 @@ class MagicLinkGeneratedOneTimeTokenSuccessHandler : GeneratedOneTimeTokenHandle The interface that define the common operations for generating and consuming one-time tokens is the javadoc:org.springframework.security.authentication.ott.OneTimeTokenService[]. Spring Security uses the javadoc:org.springframework.security.authentication.ott.InMemoryOneTimeTokenService[] as the default implementation of that interface, if none is provided. +For production environments consider using javadoc:org.springframework.security.authentication.ott.JdbcOneTimeTokenService[]. Some of the most common reasons to customize the `OneTimeTokenService` are, but not limited to: