From feb97513923f8b2c30b7a33f06a6d59fc6413312 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 23:26:20 +0000 Subject: [PATCH 1/3] Refuse an expired session when loading it, for every store (P1-10) 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 Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f --- spec/store_spec.cr | 29 +++++++++++++++++++++++++++++ src/store.cr | 4 +++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/spec/store_spec.cr b/spec/store_spec.cr index 4ae27f1..a429519 100644 --- a/spec/store_spec.cr +++ b/spec/store_spec.cr @@ -67,6 +67,35 @@ describe Session::Store do end end + describe "expiry" do + it "does not load a cookie session whose expiry has passed" do + store = Session::CookieStore(UserSession).new + session = UserSession.new + session.username = "replayed" + session.expires_at = 1.second.ago + + cookies = HTTP::Cookies.new + cookies << HTTP::Cookie.new(name: store.session_key, value: session.session_id) + cookies << HTTP::Cookie.new(name: store.data_key, value: Session.config.encryptor.encrypt_and_sign(session.to_json)) + store.load_from(cookies) + + store.current_session?.should be_nil + store.current_session.session_id.should_not eq session.session_id + end + + it "still loads a cookie session that has not expired" do + store = Session::CookieStore(UserSession).new + session = UserSession.new + + cookies = HTTP::Cookies.new + cookies << HTTP::Cookie.new(name: store.session_key, value: session.session_id) + cookies << HTTP::Cookie.new(name: store.data_key, value: Session.config.encryptor.encrypt_and_sign(session.to_json)) + store.load_from(cookies) + + store.current_session.session_id.should eq session.session_id + end + end + describe "flash messages" do it "provides access to flash" do store = Session::MemoryStore(UserSession).new diff --git a/src/store.cr b/src/store.cr index b394884..5240fd1 100644 --- a/src/store.cr +++ b/src/store.cr @@ -137,7 +137,9 @@ module Session end if current_session_id = request_cookies[session_key]? - if session = self[current_session_id.value]? + # The expiry is checked here, for every store: a cookie's `Expires` is only advice to the + # browser, and a copied cookie carries its own signed `expires_at` forever. + if (session = self[current_session_id.value]?) && session.valid? self.current_session = session # Apply sliding expiration if enabled From 62a18bc87a4e18144c3436e157ea66455864194c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:13:28 +0000 Subject: [PATCH 2/3] Refuse a payload without expires_at; delete an expired Redis session 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 Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f --- spec/base_spec.cr | 16 +++++++++++++++ spec/store_spec.cr | 43 +++++++++++++++++++++++++++++++++++++++ src/base.cr | 11 ++++++++++ src/stores/redis_store.cr | 9 +++++++- 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/spec/base_spec.cr b/spec/base_spec.cr index 833c977..4165642 100644 --- a/spec/base_spec.cr +++ b/spec/base_spec.cr @@ -46,6 +46,22 @@ describe Session::Base do restored.expires_at.to_unix.should eq session.expires_at.to_unix end + it "is required when deserialising, so a payload without it is refused instead of given a fresh expiry" do + payload = JSON.parse(UserSession.new.to_json).as_h + payload.delete("expires_at") + + expect_raises(JSON::SerializableError, /expires_at/) do + UserSession.from_json(payload.to_json) + end + end + + it "keeps the serialised format: payloads written before it was required still load" do + json = UserSession.new.to_json + + JSON.parse(json).as_h.keys.should_not contain "expires_at_present" + UserSession.from_json(json).valid?.should be_true + end + it "reflects configured timeout" do Session.config.timeout = 2.hours session = UserSession.new diff --git a/spec/store_spec.cr b/spec/store_spec.cr index a429519..3690fbd 100644 --- a/spec/store_spec.cr +++ b/spec/store_spec.cr @@ -83,6 +83,20 @@ describe Session::Store do store.current_session.session_id.should_not eq session.session_id end + it "does not load a signed cookie session whose payload has no expires_at" do + store = Session::CookieStore(UserSession).new + session = UserSession.new + payload = JSON.parse(session.to_json).as_h + payload.delete("expires_at") + + cookies = HTTP::Cookies.new + cookies << HTTP::Cookie.new(name: store.session_key, value: session.session_id) + cookies << HTTP::Cookie.new(name: store.data_key, value: Session.config.encryptor.encrypt_and_sign(payload.to_json)) + store.load_from(cookies) + + store.current_session?.should be_nil + end + it "still loads a cookie session that has not expired" do store = Session::CookieStore(UserSession).new session = UserSession.new @@ -254,6 +268,35 @@ if REDIS_AVAILABLE store.should be_a Session::Store(UserSession) store.storage.should eq "Session::RedisStore(UserSession)" end + + it "does not load an expired session that is still in Redis, and deletes it" do + store = Session::RedisStore(UserSession).new(client: redis_client) + session = UserSession.new + session.username = "replayed" + session.expires_at = 1.second.ago + key = "session:#{session.session_id}" + redis_client.set(key, session.to_json, ex: 3600) + + cookies = HTTP::Cookies.new + cookies << HTTP::Cookie.new(name: store.session_key, value: session.session_id) + store.load_from(cookies) + + store.current_session?.should be_nil + store.current_session.session_id.should_not eq session.session_id + redis_client.get(key).should be_nil + end + + it "still loads a live session from Redis" do + store = Session::RedisStore(UserSession).new(client: redis_client) + session = UserSession.new + redis_client.set("session:#{session.session_id}", session.to_json, ex: 3600) + + cookies = HTTP::Cookies.new + cookies << HTTP::Cookie.new(name: store.session_key, value: session.session_id) + store.load_from(cookies) + + store.current_session.session_id.should eq session.session_id + end end end diff --git a/src/base.cr b/src/base.cr index 2d2da4f..a114e89 100644 --- a/src/base.cr +++ b/src/base.cr @@ -21,7 +21,11 @@ module Session getter session_id : String = UUID.random.to_s getter created_at : Time = Time.local + # Required in a stored or signed payload (see `after_initialize`): the default is for `new` only. + @[JSON::Field(presence: true)] property expires_at : Time = Session.config.timeout.from_now + @[JSON::Field(ignore: true)] + @expires_at_present : Bool = false # Messages for the next request. They belong to the session, so they are saved with it # and reach only the visitor they were meant for. @@ -70,5 +74,12 @@ module Session private def timeout Session.config.timeout.from_now end + + # A payload without `expires_at` fails to deserialise, so no store adopts it: the default would + # give it a fresh expiry. A subclass that overrides this must call `super`. + protected def after_initialize + return if @expires_at_present + raise JSON::SerializableError.new("Missing JSON attribute: expires_at", self.class.to_s, "expires_at", 0, 0, nil) + end end end diff --git a/src/stores/redis_store.cr b/src/stores/redis_store.cr index 029d1fc..78d9afb 100644 --- a/src/stores/redis_store.cr +++ b/src/stores/redis_store.cr @@ -93,7 +93,7 @@ module Session ) do with_redis_connection do |client| if data = client.get(prefixed(key)) - begin + session = begin json_data = decrypt_if_enabled(data) T.from_json(json_data) rescue ex : Session::SessionEncryptionException @@ -106,6 +106,13 @@ module Session Log.warn { "Failed to deserialize session data for key #{key}: #{ex.message}" } nil end + + # Like MemoryStore: an expired session is removed instead of returned. + if session && !session.valid? + client.del(prefixed(key)) + session = nil + end + session end end end From 085668dd0d3c6299cce3104c8c29f110d7c6cd96 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 02:07:15 +0000 Subject: [PATCH 3/3] UPGRADE: expired sessions are refused; RedisStore sessions end at expires_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 Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f --- UPGRADE.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index 80c1a50..e19cb68 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -18,6 +18,20 @@ Nothing here needs a code change. Semver impact: minor (new opt-in API, one bug - `Session::Store#current_session?` returns the request's session without creating one. - `Session::Flash#to_h` returns the messages a view shows. +## Expired sessions are refused when loaded (behavior change) + +No API change, but sessions can now end sooner than before. Semver impact: patch (security fix). + +- Every store now refuses a session whose `expires_at` has passed when it is loaded, and the request starts a fresh + session. A copied `CookieStore` cookie no longer stays signed in after the timeout. +- `RedisStore` sessions now end at their `expires_at` even for a visitor who keeps using them, when + `sliding_expiration` is `false` (the default): `expires_at` is set when the session is created (or regenerated), + so the session lasts `timeout` from then. Before, every save ran `SETEX`, which reset the Redis TTL, and nothing + read `expires_at`, so an active session lived indefinitely. To keep sessions alive while they are used, set + `config.sliding_expiration = true`: each request then moves `expires_at` to `timeout` from now. +- A stored or signed payload without `expires_at` no longer loads. Payloads written by released versions always + carry it. A `Session::Base` subclass that overrides `after_initialize` must call `super`. + ## Upgrading from the `SessionData` API to `Session::Base` Releases before v1.0.28 defined sessions with the `SessionData` module and the `SessionId(T)` wrapper. From v1.0.28 on, a single `Session::Base` class replaces both. There was no 2.0 release: this migration happens inside the 1.0.x line, and the "Before" snippets below show code that no longer compiles.