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

Copy link
Copy Markdown

Title: fix: refresh cookie attributes for rolling sessions

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.
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.

1 participant