Repository navigation
Refuse an expired session when loading it, for every store (P1-10) - #8
Merged
eliasjpr merged 3 commits intoOct 27, 2026
Merged
Conversation
A cookie's Expires attribute is only advice to the browser. A copied CookieStore pair carried its own signed expires_at, which nothing read, so it kept a visitor signed in past the timeout. load_from now adopts a session only while it is valid; an expired one starts a fresh session. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f
Up to standards ✅🟢 Issues
|
A signed or stored session without expires_at was given a fresh expiry by the property default, so a payload missing the field was adopted as live. expires_at is now required when deserialising (JSON presence plus after_initialize): such a payload fails to load and no store adopts it. Payloads written by released versions always carry the field, and the serialised format is unchanged. RedisStore#[]? now deletes an expired session it reads, as MemoryStore does, and a Redis spec covers an expired key that is still present. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f
…ires_at Without sliding_expiration, a RedisStore session now ends timeout after it was created even while it is in use: before, SETEX reset the Redis TTL on every save and nothing read expires_at. sliding_expiration = true keeps the old behaviour. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f
This was referenced Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Store#load_fromadopts a session only whilesession.valid?. An expired one is ignored and the request starts a fresh session.expires_atis required when a session is deserialised (JSON::Field(presence: true)plusafter_initialize): a signed or stored payload without it fails to load, so no store adopts it, instead of the property default giving it a fresh expiry. Payloads written by released versions always carry the field; the serialised format is unchanged (noexpires_at_presentkey). A subclass that overridesafter_initializemust callsuper.RedisStore#[]?deletes an expired session it reads, asMemoryStorealready did.Why (gate-2 audit P1-10)
A cookie's
Expiresattribute is only advice to the browser. A copiedCookieStorepair carries its own signedexpires_at, and nothing read it on the way in, so a replayed cookie kept a visitor signed in long after the timeout.I reproduced this against azu master (0.8.1) and session master with
require "session/azu":timeout = 2.seconds, then the same cookies replayed 3 s later still answeredcontext.session.user_id == 42. With this change they get a new, empty session.The check sits in
load_from, so it covers every store (MemoryStorealready refused expired entries;RedisStoredid not look atexpires_ateither).Tests
spec/store_spec.cr: an expired, correctly signed cookie session is not loaded (red before the fix), a live one still is; a signed cookie payload withoutexpires_atis not loaded (red before the follow-up); with Redis (REDIS_AVAILABLE), an expired session still present in Redis is not loaded and its key is deleted (the delete assertion was red before the follow-up), and a live one still loads.spec/base_spec.cr: deserialising withoutexpires_atraisesJSON::SerializableError; the serialised payload still round-trips.crystal specwith a local Redis: 528 examples, 0 failures (Crystal 1.22.1).crystal tool format --check src spec scripts examples: clean.scripts/check_docs.cr(README, UPGRADE, docs): 157 snippets, 0 failed.Behaviour change
This is a user-visible behaviour change, documented in UPGRADE.md:
RedisStoresession now ends atexpires_at(creation, or the last regeneration, plustimeout) even while the visitor keeps using it, whensliding_expirationisfalse(the default). Before, every save ranSETEX, which reset the Redis TTL, and nothing readexpires_at, so an active session lived indefinitely.config.sliding_expiration = truekeeps active sessions alive (each request movesexpires_attotimeoutfrom now).CookieStorecookie past itsexpires_atnow gets a fresh, empty session.expires_atno longer loads.Not in this PR
An absolute session lifetime and server-side logout/revocation for
CookieStore(a revocation counter or generation claim) are design decisions, listed as open questions.Semver: patch (security fix; no API change, but the behaviour change above is visible to users). No minor/major release marker in the commits.
Generated by Claude Code