fix(session): cap session expiry at absolute_timeout - #205
Merged
Merged
Conversation
A sliding renewal, or a session_ttl longer than absolute_timeout, set expires_at past created_at + absolute_timeout. The backend TTL and the JWT exp follow expires_at, so the stored record and the token claimed a longer life than the configuration allows until the next read rejected the session. Cap expires_at at creation and on renewal, and skip the renewal once the expiry already sits at the cap, so such a session does not get a new token on every request. Closes #164
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.
Closes #164
Problem
With
absolute_timeoutset, a sliding renewal setexpires_attonow + session_ttleven when that was pastcreated_at + absolute_timeout. The backend TTL and the JWTexpboth come fromexpires_at, so they overshot the cap too. Repro: withsession_ttl=100,sliding_threshold=0.5andabsolute_timeout=120, a read at 60 s renewsexpires_atto 40 s past the cap. The same thing happened at creation whensession_ttl > absolute_timeout.Fix
SessionManager._expiry_for(session)returnsnow + session_ttl, capped atcreated_at + absolute_timeout. It is used at creation and on sliding renewal.expires_atlater. Otherwise, once the expiry sits at the cap and less than the threshold remains, every request would get a re-issued token with the sameexp.absolute_timeoutcheck inget_sessionstays. It still catches records stored before the cap existed, or beforeabsolute_timeoutwas lowered.Behaviour change
The backend now drops the record at the cap. A token presented after it therefore raises
SessionNotFoundErrorinstead ofSessionExpiredError, which is what already happens after an ordinarysession_ttlexpiry. The library does not tell these two apart, but applications catchingSessionExpiredErrorspecifically will see the difference. The CHANGELOG entry notes this.Tests
test_sliding_renewal_stops_at_absolute_timeout(JWT): renewedexpires_at, JWTexpand backend expiry all sit at the cap.test_no_renewal_once_expiry_reaches_absolute_timeout: no second token at the cap.test_new_session_expiry_is_capped_by_absolute_timeout.test_absolute_timeout_raises_session_expired_errorcould no longer pass as written, because its record now expires in the backend. It now covers the "record stored before the cap" case withoutsleep.mypy --strict, full suite against live Redis and Memcached (CACHEX_REQUIRE_LIVE_SERVERS=1): 861 passed.session/manager.pycoverage stays at 100%.Docs
docs/SESSION.mdanddocs/JWT_CLAIMS.md, plus their zh-TW versions, now state the cap.