Skip to content

fix: refresh cookie attributes for rolling sessions - #1149

Open
cpruijsen wants to merge 1 commit into
expressjs:masterfrom
cpruijsen:fix/issue-1077-314312dd
Open

cpruijsen wants to merge 1 commit into
expressjs:masterfrom
cpruijsen:fix/issue-1077-314312dd

Conversation

@cpruijsen

@cpruijsen cpruijsen commented Oct 10, 2026 •

Copy link
Copy Markdown

With rolling: true, the re-sent session cookie now takes its attributes from the current cookie option instead of the cookie data stored with the session. Before, a change to sameSite, secure, httpOnly, path or domain only reached sessions created after the change.

Addresses #1077.

In index.js, the cookie construction in store.generate moves into a createCookie(req) function, and inflate calls it when rolling is enabled. So cookie as a function and secure/sameSite: 'auto' resolve the same way as for a new session. The refresh happens before the route handler runs, so a handler can still change req.session.cookie in the same request.

Judgement call: maxAge is not refreshed

The session's own expires and originalMaxAge are kept, so a per-session req.session.cookie.maxAge (for example a "remember me" duration) is not changed. A stored originalMaxAge cannot be told apart from such an override. As a result, a change to cookie.maxAge in the options still applies only to new sessions, and the maxAge part of the issue is not covered.

The other attributes follow the current options. A per-visitor attribute that a handler set in an earlier request (for example req.session.cookie.domain) is reset to the configured value on the next request.

Alternatives, if you prefer one:

  • Replace the cookie fully, so maxAge also follows config and per-session maxAge overrides are dropped. This means deleting the lines in inflate that restore expires and originalMaxAge.
  • Refresh only behind a new option, so the current behaviour stays the default.

Not changed: the non-rolling path that re-sends a persistent cookie after a session change, and req.session.reload(), which still restores the stored attributes.

Tests

Two cases under "rolling option" in test/session.js: an existing session gets the new SameSite and keeps its original expiry, and a handler's req.session.cookie.sameSite change still applies. The first fails without the change and passes with it.

When rolling is enabled, the session identifier cookie is re-sent on
every response, but its attributes were inflated from the stored
session data, so changes to the cookie option (such as sameSite,
secure, or path) never applied to existing sessions.

Build the re-sent cookie from the current resolved cookie options when
a session is inflated with rolling enabled, while keeping the
session's own expires and originalMaxAge.

@yxshee yxshee left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked on f98bf9c (on top of master):

  • npm test: 175 passing, npm run lint: clean
  • With index.js reverted to master, "should send updated cookie attributes on existing session" fails, as expected. "should honor session cookie changes" passes on both. It's a guard that a handler can still override the refreshed cookie, and it's useful to keep.

I agree with keeping expires/originalMaxAge: dropping a per-session "remember me" maxAge on the next request would be the bigger surprise.

Heads-up for whoever merges: this and my #1151 both restructure the store.generate body (this PR pulls the cookie into createCookie, #1151 moves the generator to generateSession). The conflict is textual only. I'll rebase #1151 onto this one if it lands first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants