#59 — Remember Me — Persistent Login #59

New issue

Closed

opened 2026-08-18 13:13:22 +02:00 by lena · 1 comment

lena commented

2026-08-18 13:13:22 +02:00

(Migrated from git.butzei.de)

As a returning user,
I want to stay logged in across browser restarts for an extended period,
so that I don't have to re-enter my credentials every time I open the app.

Acceptance criteria:

Login sessions are persistent by default (survive browser close), not just tab-lifetime as today
A session remains valid for up to 3 months of inactivity, and is refreshed (rolling window) on each active use — so a user active at least once every 3 months is never logged out
The session cookie/token is rotated periodically during active use (not the same static value reused for the full 3 months), to limit the damage window of a leaked value — delivered as an accepted deviation: no literal cookie-value rotation (not achievable from application code, see design Decision 0/2); the underlying goal (bound a leaked cookie's exposure window) is met instead by instant full revocation via "log out of all devices". Documented in docs/SECURITY_NOTES.md.
Logging out actually destroys the server-side session (fixes existing gap: LogoutUserCommand currently only clears the UserId key, leaving a live empty session in Redis until idle-timeout)
"Log out of all devices" is available from Settings and invalidates every active session for the current user
Re-authentication (password re-entry) is required for sensitive actions even within an active remembered session: change password, delete account, transfer list ownership
Cookie remains HttpOnly, Secure, SameSite=Strict (no regression from current baseline)
Login endpoint has basic rate limiting (pre-existing known gap; becomes higher priority once a compromised password yields long-lived access)

Out of scope for this story:

Per-device session list / naming devices (only "log out everywhere," not a manageable list of individual sessions)
Optional/opt-in checkbox — default is always-on persistent login, no user-facing toggle
Multi-factor authentication

Decisions (2026-07-20): Product Owner confirmed 3 months, sliding/rolling window (not truly unbounded) is
an acceptable interpretation of "remember forever" in daily use, given automatic rotation. Persistent login is
the default behavior, not opt-in.

Blockers: None

Priority: Should — directly requested by the Product Owner as recurring daily-use friction (frequent
re-logins).

# Story: Remember Me — Persistent Login **As a** returning user, **I want to** stay logged in across browser restarts for an extended period, **so that** I don't have to re-enter my credentials every time I open the app. **Acceptance criteria:** - [x] Login sessions are persistent by default (survive browser close), not just tab-lifetime as today - [x] A session remains valid for up to 3 months of inactivity, and is refreshed (rolling window) on each active use — so a user active at least once every 3 months is never logged out - [x] The session cookie/token is rotated periodically during active use (not the same static value reused for the full 3 months), to limit the damage window of a leaked value — **delivered as an accepted deviation**: no literal cookie-value rotation (not achievable from application code, see design Decision 0/2); the underlying goal (bound a leaked cookie's exposure window) is met instead by instant full revocation via "log out of all devices". Documented in `docs/SECURITY_NOTES.md`. - [x] Logging out actually destroys the server-side session (fixes existing gap: `LogoutUserCommand` currently only clears the `UserId` key, leaving a live empty session in Redis until idle-timeout) - [x] "Log out of all devices" is available from Settings and invalidates every active session for the current user - [x] Re-authentication (password re-entry) is required for sensitive actions even within an active remembered session: change password, delete account, transfer list ownership - [x] Cookie remains HttpOnly, Secure, SameSite=Strict (no regression from current baseline) - [x] Login endpoint has basic rate limiting (pre-existing known gap; becomes higher priority once a compromised password yields long-lived access) **Out of scope for this story:** - Per-device session list / naming devices (only "log out everywhere," not a manageable list of individual sessions) - Optional/opt-in checkbox — default is always-on persistent login, no user-facing toggle - Multi-factor authentication **Decisions (2026-07-20):** Product Owner confirmed 3 months, sliding/rolling window (not truly unbounded) is an acceptable interpretation of "remember forever" in daily use, given automatic rotation. Persistent login is the default behavior, not opt-in. **Blockers:** None **Priority:** Should — directly requested by the Product Owner as recurring daily-use friction (frequent re-logins).

lena commented

2026-08-18 13:13:22 +02:00

(Migrated from git.butzei.de)

design (59_remember_me_persistent_login_design.md)

Design: `#59` — Remember Me / Persistent Login

Current state (confirmed by reading the code)

Session store: ASP.NET Core AddSession() (ISession/ISessionStore) backed by
AddStackExchangeRedisCache — no custom session abstraction.
app.UseSession(...) sets IdleTimeout = 30 days (server-side, sliding) but the cookie itself has
no MaxAge/Expires — it's a browser session cookie, wiped on browser close. That's the literal bug
this story fixes: the server would keep the Redis record alive for 30 days, but the cookie doesn't survive
a restart.
LogoutUserCommandHandler only calls SetCurrentUserIdCommand(null), which does
context.Session.Remove("UserId") — the Redis record and cookie both live on until 30-day idle timeout.
Confirmed gap, matches the story's premise.
No session-ID rotation anywhere. No per-user session index (can't enumerate/kill a user's other sessions).
ChangePasswordCommand already requires the current password (own inline check) — no changes needed there.
DeleteCurrentUserAccountCommand (username-typed confirmation only) and TransferTodoListOwnershipCommand
(no confirmation at all) have no password re-entry — net new for both.

Decisions

0. ISession.Id cannot be used as a Redis key or as a session identifier — this invalidated an
earlier draft of this design. Decompiled Microsoft.AspNetCore.Session.DistributedSession (10.0.10) to
confirm: .Id returns a random 16-byte GUID generated independently of the session's actual cache key
(_sessionKey, a private field never exposed via any public API), persisted as part of the session's own
serialized payload purely for logging/debug purposes. It has no relationship to the cache row's address and
no relationship to the cookie value. An earlier version of this design tried to use context.Session.Id to
IDistributedCache.Remove(...) a specific session's Redis row directly — that removes a key that was never
actually used for storage, silently no-ops, and leaves the real row untouched. Caught by an integration test
against the real DistributedSessionStore (not mocks) before it shipped — see the frontend/backend engineer
team memory entry for this cycle. Decompiled SessionMiddleware.Invoke as well: it mints its own session key
before calling _next, and only writes Set-Cookie when the incoming cookie was missing/invalid — reassigning
HttpContext.Session to a different ISession instance later in the pipeline does not cause a new cookie
to be written, because the cookie-writing callback captured its value before _next ran. Net effect: there is
no supported way, from application code, to (a) address/delete a specific other session's store row, or
(b) force the framework to mint a new cookie value mid-pipeline. Both of the following decisions route around
this rather than fight it.

1. Persistent + rolling cookie, refreshed by re-appending the same opaque value.
SessionOptions.Cookie.MaxAge = 90 days (SessionPolicy.MaxLifetime) makes the cookie persistent, but per
Decision 0, the framework never re-sends Set-Cookie for an already-valid incoming cookie — so on its own,
MaxAge would count down from first login only, not roll forward with use. Fix: SessionMaintenanceMiddleware
(runs after UseSession()) periodically (SessionPolicy.CookieRefreshInterval, 24h, tracked via an
IssuedAtUtc session value) re-appends context.Request.Cookies[name] — the same opaque string the
browser already sent, never decoded or reinterpreted — via Response.Cookies.Append(...) with a fresh
MaxAge. This achieves a true rolling window using only Request.Cookies/Response.Cookies, no
understanding of the cookie's internal format required.

2. No literal session-ID/token rotation — the AC's underlying goal (bound a leaked cookie's exposure
window) is met by instant full revocation instead. Per Decision 0, minting a genuinely new session ID mid-
pipeline isn't achievable without either reflecting into DistributedSession's private fields or
reimplementing ASP.NET Core's internal CookieProtection (base64 + IDataProtector, purpose string
"SessionMiddleware") by hand to self-issue a compatible cookie. Both were prototyped; both were rejected as
too fragile for what they buy (a hand-rolled reimplementation of framework-internal, version-coupled crypto
plumbing, to satisfy a defense-in-depth AC that a different, more standard mechanism already covers — see
Decision 3). Documented as an explicit accepted deviation from the story's literal AC text, not a silent gap —
see SECURITY_NOTES.md.

3. "Log out of all devices" via a per-user revocation timestamp (the standard "security stamp" pattern —
the same one ASP.NET Core Identity itself uses), not session enumeration. Per Decision 0, there's no way to
list or address a user's other sessions from application code, so a session-ID registry (an earlier draft
of this design) can't actually delete anything it tracks. Instead: UserEntity.SessionsRevokedBeforeUtc
(new nullable DateTimeOffset column, migration AddSessionsRevokedBeforeUtcToUser). Every session already
carries its own IssuedAtUtc (Decision 1). SessionMaintenanceMiddleware checks
GetSessionsRevokedBeforeUtcQuery for the current user on every authenticated request; if IssuedAtUtc is
older than the stored revocation timestamp, it Clear()s the session (empties it — it authenticates as
nobody from then on) and continues the pipeline as logged-out, no separate deletion needed. This is also
what actually delivers on the leaked-cookie-exposure-window goal AC #3 was reaching for: a suspected-leaked
cookie can be fully invalidated on demand, without waiting for or depending on periodic rotation.
LogoutAllDevicesCommand (WebApi) = RevokeAllSessionsForCurrentUserCommand (core project, DB write, sets
the timestamp to now, already requires an authenticated session via its own AuthorizeIsCurrentUserAuthenticatedQuery
— LogoutAllDevicesCommandHandler needs no separate auth wiring) followed by DestroyCurrentSessionCommand
so the calling device is also logged out immediately in the same response, not left to notice on its next
request.

2b. Fix the logout gap with a new DestroyCurrentSessionCommand (WebApi-only, same split as
SetCurrentUserIdCommand). Session.Clear() (not just Remove("UserId"), the pre-#59 bug) +
CommitAsync() empties the session's entire record, and Response.Cookies.Delete(...) drops the browser
cookie. LogoutUserCommandHandler and DeleteCurrentUserAccountCommandHandler both switch from
SetCurrentUserIdCommand(null) to this. Per Decision 0 there's still no way to force-delete the underlying
store row, but an emptied, cookie-less session is functionally dead — it idles out of the store on the
existing TTL with zero exposure (nothing sensitive left in it).

4. Step-up re-auth for sensitive actions. New AuthorizeCurrentPasswordQuery(Password ConfirmPassword)
in Features/Authorization/ (same shape as the existing AuthorizeTodoListOwnerAccessForCurrentUserQuery —
resolves current user, loads the entity, hashes+compares) — throws AuthenticationException (401) on
mismatch, matching ChangePasswordCommandHandler's existing inline check. Wired via WithAuthorization
(chainable — confirmed IHandlerRegistrationConfig.WithAuthorization returns itself, so it composes with an
existing authorization query) on:

DeleteCurrentUserAccountCommand — adds Password ConfirmPassword alongside the existing
ConfirmedUserName (the username-typed confirmation stays as the "are you sure" UX guard;
the password is the actual security control).
TransferTodoListOwnershipCommand — adds Password ConfirmPassword, chained after the existing
AuthorizeTodoListOwnerAccessForCurrentUserQuery.
ChangePasswordCommand needs no change — already has its own current-password check.

5. Rate limiting on login — closes the [OPEN] item in SECURITY_NOTES.md. ASP.NET Core's built-in
AddRateLimiter, a fixed-window policy named "login" (5 attempts / minute, partitioned by remote IP,
QueueLimit = 0, 429 on rejection). Applied only to the LoginUserCommand endpoint inside
MapRequest<TRequest,TResult>() (same spot that already special-cases individual request types for
endpoint mapping), not globally.

Out of scope (per story)

Per-device session naming/listing, opt-in toggle (always-on), MFA.

Testing approach

No Docker in this sandbox, so DB-backed (DbTestContext) tests can't run locally — pre-existing, documented
limitation; the new GetSessionsRevokedBeforeUtcQueryHandler/RevokeAllSessionsForCurrentUserCommandHandler/
AuthorizeCurrentPasswordQueryHandler tests follow that project's existing DbTestContext pattern and were
verified to compile and to fail only with DockerUnavailableException, same as the rest of that suite.
Everything session/cookie-related in the WebApi project runs locally (no DB needed): TestContext-based
mocked-ISession tests for the simple handlers, and a small hand-written FakeSession : ISession (a
dictionary-backed real implementation, not a mock) for SessionMaintenanceMiddlewareTests — needed because
Decision 0's investigation showed real session-store behavior (not just mocked call verification) is what
actually matters here, and an earlier attempt using the real DistributedSessionStore against a real
MemoryDistributedCache is what surfaced the .Id-is-not-a-key bug in the first place.

**design** (`59_remember_me_persistent_login_design.md`) # Design: `#59` — Remember Me / Persistent Login ## Current state (confirmed by reading the code) - Session store: ASP.NET Core `AddSession()` (`ISession`/`ISessionStore`) backed by `AddStackExchangeRedisCache` — no custom session abstraction. - `app.UseSession(...)` sets `IdleTimeout = 30 days` (server-side, sliding) but the cookie itself has **no `MaxAge`/`Expires`** — it's a browser session cookie, wiped on browser close. That's the literal bug this story fixes: the server would keep the Redis record alive for 30 days, but the cookie doesn't survive a restart. - `LogoutUserCommandHandler` only calls `SetCurrentUserIdCommand(null)`, which does `context.Session.Remove("UserId")` — the Redis record and cookie both live on until 30-day idle timeout. Confirmed gap, matches the story's premise. - No session-ID rotation anywhere. No per-user session index (can't enumerate/kill a user's other sessions). - `ChangePasswordCommand` already requires the current password (own inline check) — no changes needed there. `DeleteCurrentUserAccountCommand` (username-typed confirmation only) and `TransferTodoListOwnershipCommand` (no confirmation at all) have no password re-entry — net new for both. ## Decisions **0. `ISession.Id` cannot be used as a Redis key or as a session identifier — this invalidated an earlier draft of this design.** Decompiled `Microsoft.AspNetCore.Session.DistributedSession` (10.0.10) to confirm: `.Id` returns a random 16-byte GUID generated independently of the session's actual cache key (`_sessionKey`, a private field never exposed via any public API), persisted as part of the session's own serialized payload purely for logging/debug purposes. It has no relationship to the cache row's address and no relationship to the cookie value. An earlier version of this design tried to use `context.Session.Id` to `IDistributedCache.Remove(...)` a specific session's Redis row directly — that removes a key that was never actually used for storage, silently no-ops, and leaves the real row untouched. Caught by an integration test against the real `DistributedSessionStore` (not mocks) before it shipped — see the frontend/backend engineer team memory entry for this cycle. Decompiled `SessionMiddleware.Invoke` as well: it mints its own session key before calling `_next`, and only writes `Set-Cookie` when the incoming cookie was missing/invalid — reassigning `HttpContext.Session` to a different `ISession` instance later in the pipeline does **not** cause a new cookie to be written, because the cookie-writing callback captured its value before `_next` ran. Net effect: there is no supported way, from application code, to (a) address/delete a specific *other* session's store row, or (b) force the framework to mint a new cookie value mid-pipeline. Both of the following decisions route around this rather than fight it. **1. Persistent + rolling cookie, refreshed by re-appending the same opaque value.** `SessionOptions.Cookie.MaxAge = 90 days` (`SessionPolicy.MaxLifetime`) makes the cookie persistent, but per Decision 0, the framework never re-sends `Set-Cookie` for an already-valid incoming cookie — so on its own, `MaxAge` would count down from first login only, not roll forward with use. Fix: `SessionMaintenanceMiddleware` (runs after `UseSession()`) periodically (`SessionPolicy.CookieRefreshInterval`, 24h, tracked via an `IssuedAtUtc` session value) re-appends `context.Request.Cookies[name]` — the *same opaque string* the browser already sent, never decoded or reinterpreted — via `Response.Cookies.Append(...)` with a fresh `MaxAge`. This achieves a true rolling window using only `Request.Cookies`/`Response.Cookies`, no understanding of the cookie's internal format required. **2. No literal session-ID/token rotation — the AC's underlying goal (bound a leaked cookie's exposure window) is met by instant full revocation instead.** Per Decision 0, minting a genuinely new session ID mid- pipeline isn't achievable without either reflecting into `DistributedSession`'s private fields or reimplementing ASP.NET Core's internal `CookieProtection` (base64 + `IDataProtector`, purpose string `"SessionMiddleware"`) by hand to self-issue a compatible cookie. Both were prototyped; both were rejected as too fragile for what they buy (a hand-rolled reimplementation of framework-internal, version-coupled crypto plumbing, to satisfy a defense-in-depth AC that a different, more standard mechanism already covers — see Decision 3). Documented as an explicit accepted deviation from the story's literal AC text, not a silent gap — see `SECURITY_NOTES.md`. **3. "Log out of all devices" via a per-user revocation timestamp (the standard "security stamp" pattern — the same one ASP.NET Core Identity itself uses), not session enumeration.** Per Decision 0, there's no way to list or address a user's *other* sessions from application code, so a session-ID registry (an earlier draft of this design) can't actually delete anything it tracks. Instead: `UserEntity.SessionsRevokedBeforeUtc` (new nullable `DateTimeOffset` column, migration `AddSessionsRevokedBeforeUtcToUser`). Every session already carries its own `IssuedAtUtc` (Decision 1). `SessionMaintenanceMiddleware` checks `GetSessionsRevokedBeforeUtcQuery` for the current user on every authenticated request; if `IssuedAtUtc` is older than the stored revocation timestamp, it `Clear()`s the session (empties it — it authenticates as nobody from then on) and continues the pipeline as logged-out, no separate deletion needed. This is also what actually delivers on the leaked-cookie-exposure-window goal AC `#3` was reaching for: a suspected-leaked cookie can be fully invalidated on demand, without waiting for or depending on periodic rotation. `LogoutAllDevicesCommand` (WebApi) = `RevokeAllSessionsForCurrentUserCommand` (core project, DB write, sets the timestamp to now, already requires an authenticated session via its own `AuthorizeIsCurrentUserAuthenticatedQuery` — `LogoutAllDevicesCommandHandler` needs no separate auth wiring) followed by `DestroyCurrentSessionCommand` so the calling device is also logged out immediately in the same response, not left to notice on its next request. **2b. Fix the logout gap with a new `DestroyCurrentSessionCommand` (WebApi-only, same split as `SetCurrentUserIdCommand`).** `Session.Clear()` (not just `Remove("UserId")`, the pre-`#59` bug) + `CommitAsync()` empties the session's entire record, and `Response.Cookies.Delete(...)` drops the browser cookie. `LogoutUserCommandHandler` and `DeleteCurrentUserAccountCommandHandler` both switch from `SetCurrentUserIdCommand(null)` to this. Per Decision 0 there's still no way to force-delete the underlying store row, but an emptied, cookie-less session is functionally dead — it idles out of the store on the existing TTL with zero exposure (nothing sensitive left in it). **4. Step-up re-auth for sensitive actions.** New `AuthorizeCurrentPasswordQuery(Password ConfirmPassword)` in `Features/Authorization/` (same shape as the existing `AuthorizeTodoListOwnerAccessForCurrentUserQuery` — resolves current user, loads the entity, hashes+compares) — throws `AuthenticationException` (401) on mismatch, matching `ChangePasswordCommandHandler`'s existing inline check. Wired via `WithAuthorization` (chainable — confirmed `IHandlerRegistrationConfig.WithAuthorization` returns itself, so it composes with an existing authorization query) on: - `DeleteCurrentUserAccountCommand` — adds `Password ConfirmPassword` alongside the existing `ConfirmedUserName` (the username-typed confirmation stays as the "are you sure" UX guard; the password is the actual security control). - `TransferTodoListOwnershipCommand` — adds `Password ConfirmPassword`, chained after the existing `AuthorizeTodoListOwnerAccessForCurrentUserQuery`. `ChangePasswordCommand` needs no change — already has its own current-password check. **5. Rate limiting on login** — closes the `[OPEN]` item in `SECURITY_NOTES.md`. ASP.NET Core's built-in `AddRateLimiter`, a fixed-window policy named `"login"` (5 attempts / minute, partitioned by remote IP, `QueueLimit = 0`, 429 on rejection). Applied only to the `LoginUserCommand` endpoint inside `MapRequest<TRequest,TResult>()` (same spot that already special-cases individual request types for endpoint mapping), not globally. ## Out of scope (per story) Per-device session naming/listing, opt-in toggle (always-on), MFA. ## Testing approach No Docker in this sandbox, so DB-backed (`DbTestContext`) tests can't run locally — pre-existing, documented limitation; the new `GetSessionsRevokedBeforeUtcQueryHandler`/`RevokeAllSessionsForCurrentUserCommandHandler`/ `AuthorizeCurrentPasswordQueryHandler` tests follow that project's existing `DbTestContext` pattern and were verified to compile and to fail only with `DockerUnavailableException`, same as the rest of that suite. Everything session/cookie-related in the WebApi project runs locally (no DB needed): `TestContext`-based mocked-`ISession` tests for the simple handlers, and a small hand-written `FakeSession : ISession` (a dictionary-backed real implementation, not a mock) for `SessionMaintenanceMiddlewareTests` — needed because Decision 0's investigation showed real session-store behavior (not just mocked call verification) is what actually matters here, and an earlier attempt using the real `DistributedSessionStore` against a real `MemoryDistributedCache` is what surfaced the `.Id`-is-not-a-key bug in the first place.