feat(security): complete remember-me support (#351)

[devondragon]

Summary

Completes remember-me ("stay signed in") support, which was previously wired into the filter chain but non-functional in practice (#351): no cookie configuration, no persistent token store, no revocation, no docs, no tests.

What changed

Configuration surface (WebSecurityConfig)

• New properties, defaulting to Spring Security's own defaults: user.security.rememberMe.tokenValiditySeconds (1209600 = 14 days), rememberMeParameter / rememberMeCookieName (remember-me), and useSecureCookie (unset = Secure whenever the issuing request used HTTPS).
• An optional PersistentTokenRepository is injected via ObjectProvider and wired with .tokenRepository(...) when present.
• The signing key is excluded from the Lombok-generated toString so it can never leak through bean logging.

Opt-in persistent token store (UserSecurityBeansAutoConfiguration + db-scripts/)

• user.security.rememberMe.usePersistentTokens=true creates a JdbcTokenRepositoryImpl backed by the consumer's DataSource, guarded by @ConditionalOnMissingBean so a consumer-defined repository wins.
• persistent_logins DDL added to the schema script (username widened to 255 chars — it holds emails).

Token revocation (SessionInvalidationService)

• Both invalidateUserSessions (admin sign-out-everywhere, account disable/delete) and invalidateSessionsAfterPasswordChange now remove the user's persistent tokens. This ships in the same change as the store because persistent tokens — unlike hash-based cookies — do not embed the password hash and would otherwise survive a password change.
• Failure isolation: revocation runs inside password-change transactions and after-commit deletion callbacks, so a repository failure is logged (loud, actionable ERROR) and swallowed rather than rolling back or misreporting the primary account operation. A systematically missing persistent_logins table still surfaces at the first remember-me login itself (Spring's createNewToken is not wrapped), so this cannot hide misconfiguration.
• Hash-based mode (the default) cannot be admin-revoked — there is no server-side state; the cookie is a self-contained signature. This is documented as a limitation (CONFIG.md) rather than worked around; password changes invalidate those cookies inherently. Consumers needing "sign out everywhere, now" for remember-me should use persistent tokens.

Documentation

• CONFIG.md: full property reference, the two mandatory setup steps (properties AND the login form posting the remember-me parameter — without it no cookie is ever issued), hash-based vs. persistent trade-offs, persistent_logins requirement, and secure-cookie behavior behind TLS-terminating proxies (server.forward-headers-strategy).
• README bullet rewritten (previously implied the feature worked out of the box); config metadata JSON updated; MIGRATION.md 5.2.x note for the two constructor signature changes (affects subclasses only).

Tests (all passing, full suite green)

• Hash-based: cookie issued only with the parameter; session-less cookie re-auth produces a RememberMeAuthenticationToken with a DSUserDetails principal; auto-login publishes InteractiveAuthenticationSuccessEvent; cookie rejected after password change. Deliberately no test for admin revocation of hash-based tokens (not implementable, per the ticket).
• Persistent: bean gating (absent unset/false, present true, consumer override wins); token row stored keyed by email; both invalidation paths remove tokens and the old cookie stops authenticating.
• Non-default parameter/cookie names, validity, and forced Secure flag all bind and take effect.
• Unit: removeUserTokens on both paths, no-op without the bean, repository failure swallowed without breaking invalidation.

Review

Ticket-grounded review ran (correctness/architecture, security, testing agents, all findings independently verified): 0 Critical, 1 High, 3 Medium, 3 Low — all fixed on this branch (the High was the failure-isolation issue above). Scope check clean against all five acceptance criteria. The Codex cross-model pass did not return in time and is not included.

Out of scope

The reference login form checkbox lives in the demo app: devondragon/SpringUserFrameworkDemoApp#79.

Closes#351

[Copilot]

Pull request overview

This PR completes end-to-end Spring Security remember-me (“stay signed in”) support in the framework by adding cookie/token configuration, an opt-in persistent token store, server-side token revocation on session invalidation and password changes, and accompanying documentation and test coverage.

Changes:

• Extended remember-me configuration in WebSecurityConfig (validity, parameter/cookie names, secure-cookie behavior) and wired an optional PersistentTokenRepository.
• Added opt-in JDBC-backed persistent token storage plus revocation via SessionInvalidationService.
• Added schema DDL, docs, and integration/unit tests covering both hash-based and persistent-token modes.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/main/java/com/digitalsanctuary/spring/user/security/WebSecurityConfig.java	Adds remember-me property bindings and optionally enables persistent-token mode when a PersistentTokenRepository is present.
src/main/java/com/digitalsanctuary/spring/user/security/UserSecurityBeansAutoConfiguration.java	Provides an opt-in JdbcTokenRepositoryImpl bean gated by usePersistentTokens=true.
src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java	Revokes persistent remember-me tokens during session invalidation and password-change invalidation paths (failure-isolated).
src/main/java/com/digitalsanctuary/spring/user/listener/AuthenticationEventListener.java	Clarifies comment about principal type handling for remember-me vs basic auth.
db-scripts/mariadb-schema.sql	Adds persistent_logins DDL for persistent-token remember-me.
src/main/resources/META-INF/additional-spring-configuration-metadata.json	Documents new remember-me properties for IDE metadata/autocomplete.
src/main/resources/config/dsspringuserconfig.properties	Adds property reference entries and guidance for remember-me setup and proxy/TLS considerations.
CONFIG.md	Adds full remember-me documentation, including required setup steps and hash vs persistent trade-offs/revocation semantics.
README.md	Updates feature bullet to reflect required configuration + login form parameter requirement.
MIGRATION.md	Adds 5.2.x migration note about remember-me completion and constructor signature changes for direct instantiation/subclassing.
src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java	Adds unit tests for persistent-token revocation behavior and failure isolation; updates manual constructor usage.
src/test/java/com/digitalsanctuary/spring/user/security/RememberMeIntegrationTest.java	Adds hash-based remember-me integration tests through the real form-login flow.
src/test/java/com/digitalsanctuary/spring/user/security/RememberMePersistentTokenIntegrationTest.java	Adds persistent-token integration tests including DB storage and revocation behavior.
src/test/java/com/digitalsanctuary/spring/user/security/RememberMeCustomConfigIntegrationTest.java	Verifies non-default parameter/cookie names, validity, and Secure flag binding.
src/test/java/com/digitalsanctuary/spring/user/security/CoreBeanOverrideTest.java	Verifies conditional creation/override behavior and annotations for the persistent token repository bean.

Suppressed comments (1)

src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java:268

• This test’s name and @DisplayName still claim the default warn-threshold is 1000, but the assertions/comments now correctly state that manually instantiating SessionInvalidationService (outside a Spring context) leaves warnThreshold at 0. Renaming the test avoids misleading future readers about the actual behavior under test.

 @Test
@DisplayName("uses default threshold of 1000")
void usesDefaultThresholdOf1000() {
// Given - create a new service without setting threshold (should use default)
SessionInvalidationService newService = new SessionInvalidationService(sessionRegistry, persistentTokenRepositoryProvider);
// Verify the default value is set correctly via reflection
Integer threshold = (Integer) ReflectionTestUtils.getField(newService, "warnThreshold");
// Then - default should be 0 (unset by Spring) since we're not using Spring context
// In production, Spring will inject the default value of 1000
assertThat(threshold).isEqualTo(0);
}

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[claude]

Review

Solid piece of work — this closes a real gap (remember-me was previously enabled-but-inert), and the implementation, docs, and tests are all consistent with each other. I read through WebSecurityConfig, UserSecurityBeansAutoConfiguration, SessionInvalidationService, the DDL, and all four new/changed test files. A few notes, nothing blocking.

Code quality / correctness

• Design is sound.ObjectProvider<PersistentTokenRepository> is the right tool for "switch behavior if a bean exists, no-op otherwise" — it composes cleanly with @ConditionalOnMissingBean on the consumer-override path, and SessionInvalidationService/WebSecurityConfig both resolve it lazily rather than requiring it.
• Revocation keying is consistent: DSUserDetails.getUsername() returns user.getEmail(), and revokeRememberMeTokens removes tokens by user.getEmail() — these actually match, so persistent-token revocation will hit the right rows.
• persistent_logins.username widened to VARCHAR(255) for emails, with an index on it backing the removeUserTokens lookup — good, avoids a full scan on every invalidation.
• Excluding the signing key from Lombok's toString (@ToString.Exclude) is a nice catch given WebSecurityConfig is @Data and could otherwise leak the key through bean/context logging.

Minor nits (non-blocking)

1. Broad catch in SessionInvalidationService.revokeRememberMeTokens (catch (RuntimeException ex)): JdbcTokenRepositoryImpl/PersistentTokenRepository failures surface as org.springframework.dao.DataAccessException, which is itself a RuntimeException. Catching RuntimeException broadly will also silently swallow an unrelated bug (e.g. an NPE) in this method and log it as if it were a repository failure, which could make a real defect here harder to notice. Narrowing to DataAccessException would preserve the "never break the primary operation" goal without masking programmer errors.
2. Import order in UserSecurityBeansAutoConfiguration.java: import javax.sql.DataSource; was inserted between two org.springframework.* imports (line 6), breaking alphabetical order per the project's import convention. (The file already had some pre-existing disorder, e.g. org.springframework.http.MediaType mid-block, so this isn't a new problem, but worth a pass while touching the file.)
3. usePersistentTokens=true doesn't require rememberMeEnabled=true. The persistentTokenRepository bean is gated only on usePersistentTokens, so a consumer who sets usePersistentTokens=true but forgets rememberMe.enabled=true gets a live PersistentTokenRepository bean and a removeUserTokens DB call on every session invalidation / password change, even though remember-me itself is off. Harmless (empty table, indexed no-op query) but slightly wasteful and mildly confusing if someone inspects the beans. Not worth blocking on.

Security

• Hash-based vs. persistent trade-offs are accurately documented, including the important caveat that hash-based cookies cannot be admin-revoked — that's called out clearly in both CONFIG.md and the class Javadoc, so consumers won't be surprised.
• useSecureCookie defaulting to "secure when the request was HTTPS" with an explicit callout about server.forward-headers-strategy behind a TLS-terminating proxy is exactly the kind of footgun worth documenting — good catch.
• Revocation on password change for persistent tokens closes a real gap (persistent tokens don't embed the password hash, unlike hash-based cookies), and the failure-isolation approach (log + swallow, since a missing table would already fail loudly at first remember-me login via createNewToken) is a reasonable, well-reasoned tradeoff for not breaking password-change/account-deletion on a secondary cleanup step.

Test coverage

• Good breadth: real formLogin path for hash-based issuance/rejection, persistent-token store + both revocation paths, non-default parameter/cookie/validity/secure-flag binding, bean-gating (absent/false/true/consumer-override), and unit coverage of the swallow-on-failure behavior in SessionInvalidationServiceTest.
• Deliberately skipping a "revoke hash-based tokens" test with a documented reason (not implementable) is the right call rather than a hollow test.
• One gap: no test asserts that removeUserTokens is skipped/inert when usePersistentTokens=true but rememberMe.enabled=false (relates to nit https://github.com/devondragon/SpringUserFramework/issues/2 #3 above) — minor, and arguably not worth a dedicated test given the behavior is harmless.

Docs

CONFIG.md, MIGRATION.md, and the properties file are all consistent with the code (property names, defaults, and the two-constructor-parameter migration note all check out against the actual diff). The "your login form must also post the parameter, not just the properties" callout is exactly the kind of thing that would otherwise generate support questions.

Nice work — this is a thorough, well-scoped completion of #351.

[claude]

Review: Remember-Me support (#352)

Went through the full diff (WebSecurityConfig, UserSecurityBeansAutoConfiguration, SessionInvalidationService, schema, docs, and all five new/updated test classes). This is a well-executed, thoroughly documented, and well-tested change. Nice work closing out #351.

Strengths

• Correctness of the tricky part. The revocation semantics are exactly right: persistent tokens don't embed the password hash, so SessionInvalidationService explicitly wipes them on both invalidateUserSessions and invalidateSessionsAfterPasswordChange, while hash-based cookies are left alone because they self-invalidate on password change (the signature embeds the hash). The docs are honest about the resulting limitation (hash-based mode can't be admin-revoked) rather than papering over it.
• Failure isolation in revokeRememberMeTokens (SessionInvalidationService.java:224-241) — running inside password-change transactions / after-commit deletion callbacks and swallowing the exception (with a loud ERROR log) so a flaky/misconfigured token store can't roll back or misreport the primary account operation is the right call, and it's backed by a dedicated test (repositoryFailureDoesNotBreakInvalidation).
• Opt-in, backoff-friendly wiring.persistentTokenRepository() is gated by both @ConditionalOnProperty and @ConditionalOnMissingBean, so consumers can supply their own PersistentTokenRepository and it wins — verified by CoreBeanOverrideTest.consumerPersistentTokenRepositoryWins.
• Secret hygiene. Excluding rememberMeKey from the Lombok @ToString (WebSecurityConfig.java:107-108) is a nice, easy-to-miss detail — @Data-generated toString() on a @Configuration bean is a classic way to leak a signing key into logs.
• Test coverage is genuinely comprehensive, not just happy-path: disabled-by-default stays inert even when the request parameter is posted (regression guard against silently turning on 14-day cookies for every consumer on upgrade), custom parameter/cookie-name/validity/secure-flag overrides actually bind, hash-based cookie rejection after password change, and persistent-mode revocation on both invalidation paths with an actual old-cookie-now-rejected assertion. The docstrings on the test classes explaining why each test exists (not just what it does) are a good practice.
• Docs: CONFIG.md clearly calls out the two easy-to-miss setup steps (properties and the login form's remember-me checkbox), the TLS-terminating-proxy caveat for useSecureCookie, and MIGRATION.md correctly flags the constructor signature change as breaking only for subclasses/direct instantiation.
• Schema addition follows existing conventions in mariadb-schema.sql (e.g. widening username to VARCHAR(255) for email, indexed the same way user_account.email already is).

Minor observations (non-blocking)

1. persistentTokenRepository() bean creation is independent of rememberMe.enabled. If a consumer sets usePersistentTokens=true but forgets rememberMe.enabled=true (or the key), the JdbcTokenRepositoryImpl bean is still created and sits unused, requiring the persistent_logins table to exist for no functional benefit. Harmless (no eager DB connection since createTableOnStartup defaults to false), but a log.warn or a doc note about the two flags being independent could save someone a "why isn't this working" moment.
2. revokeRememberMeTokens catches bare RuntimeException (SessionInvalidationService.java:236). The isolation intent is well-documented and justified for genuine repository failures, but a bare RuntimeException catch will also swallow an unrelated programming bug (e.g. an NPE) inside removeUserTokens, logging it as if it were a persistence failure. Narrowing to DataAccessException (what JdbcTokenRepositoryImpl actually throws) would keep the isolation guarantee while letting genuine bugs surface. Not a big deal given the ERROR-level logging either way.
3. Property naminguser.security.rememberMe.rememberMeParameter / rememberMeCookieName reads slightly redundant (mirrors Spring's own builder method names, so understandable, and consistent with the existing rememberMe.key/rememberMe.enabled prefix pattern) — just flagging in case a shorter user.security.rememberMe.parameter / .cookieName was considered and rejected for a reason worth noting, otherwise no action needed.

Security

No concerns. Signing key excluded from logging, createTableOnStartup correctly left false (no surprise DDL), cookie Secure defaulting to Spring's own HTTPS-aware behavior with the reverse-proxy caveat clearly documented, and token revocation correctly wired into both admin-initiated and password-change invalidation paths.

Overall: solid implementation, no bugs found, test and doc coverage are above the project's own bar. The two "minor observations" above are optional polish, not blockers.