fix(session): align auth cookie maxAge with Redis session TTL#3870
Open
hariom888 wants to merge 1 commit into
Open
fix(session): align auth cookie maxAge with Redis session TTL#3870hariom888 wants to merge 1 commit into
hariom888 wants to merge 1 commit into
Conversation
getAuthCookieOptions() set maxAge: 60 * 60 (1h) for the authToken and sessionId cookies, while createSession() persisted the Redis session with ex: 24 * 60 * 60 (24h). The two had drifted because they were set independently with no shared constant. After 1h the browser dropped the cookies (client appears logged out, can no longer call DELETE /api/auth/session since there's no sessionId cookie left to send), while the Redis session remained valid and visible to admin tooling for up to 23 more hours, consuming the single-session slot enforced by createSession on the user's next real login. Extract SESSION_TTL_SECONDS into lib/sessionConstants.js and use it for both the cookie maxAge and the Redis TTL, so they can't drift apart again. Add a regression test asserting the TTL passed to Redis equals the cookie's maxAge, and a route-level test asserting both cookies use SESSION_TTL_SECONDS.
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.
Problem
getAuthCookieOptions()inapp/api/auth/session/route.jssetmaxAge: 60 * 60(1 hour) for both theauthTokenandsessionIdcookies.createSession()inlib/sessionManager.jspersisted the session in Redis withex: 24 * 60 * 60(24 hours), and theuser:sessions:{userId}set was also expired after 24 hours. The two lifetimes were set independently with no shared constant, so they had drifted (1h vs 24h).Consequences:
sessionIdcookie left to callDELETE /api/auth/sessionand cleanly terminate the session.session:{sessionId}entry stays valid for up to 23 more hours, showing up as a "ghost" active session in admin tooling (app/api/admin/sessions/terminate/route.js) that the legitimate user can no longer terminate themselves, and which occupies the single-session slot enforced bycreateSession's termination logic on the user's next real login.Fix
Extract a single shared constant,
SESSION_TTL_SECONDS, inlib/sessionConstants.js, and use it for both:maxAgeingetAuthCookieOptions()ex/expire) increateSession()The cookie lifetime was raised to match the 24h Redis TTL (rather than shrinking the Redis TTL to 1h), since the single-session-per-user guarantee is meant to hold for the session's full real lifetime. The logout path's cookie-clearing (
maxAge: 0) is unaffected.Tests
lib/__tests__/sessionTtlConsistency.test.js: asserts the TTLcreateSessionpasses to Redis equalsSESSION_TTL_SECONDS— the exact "fails CI if they drift" guard requested in the issue.app/api/auth/session/__tests__/route.test.js: asserts both theauthTokenandsessionIdcookies set byPOST /api/auth/sessionhavemaxAge === SESSION_TTL_SECONDS(and explicitly not the old60 * 60).Closes #3805