Skip to content

Regenerate the session before logging in as a new user - #170

Open
aaronofinn wants to merge 3 commits into
fastify:mainfrom
aaronofinn:fix/session-not-regenerated-on-login
Open

aaronofinn wants to merge 3 commits into
fastify:mainfrom
aaronofinn:fix/session-not-regenerated-on-login

Conversation

@aaronofinn

Copy link
Copy Markdown
Contributor

Description

auth/index.ts's login handler assigns request.session.user = {...} and calls request.session.save(), but never rotates the session ID. If a valid session cookie is already present when /login is called, @fastify/session reuses that same ID, it just overwrites who the session belongs to.

If an attacker could get a session ID they already know onto a victim's browser before the victim authenticates (however that happens), the ID staying the same across login means the attacker's foreknowledge of it remains useful after the victim logs in. Regenerating the session ID at the moment authentication state changes is the standard defense, it invalidates whatever ID existed beforehand.

Checklist

@jean-michelet

Copy link
Copy Markdown
Member

We need this: fastify/session#348
@fastify/release: possible to release a patch of fastify/session plz?

@Tony133

Tony133 commented Sep 14, 2026

Copy link
Copy Markdown
Member

We need this: fastify/session#348 @fastify/release: possible to release a patch of fastify/session plz?

Done! see new release here: https://github.com/fastify/session/releases/tag/v11.1.3 🔥 👀

@jean-michelet

Copy link
Copy Markdown
Member

@Tony133 Thx!
@aaronofinn Can you plz update the version?

@aaronofinn

Copy link
Copy Markdown
Contributor Author

@jean-michelet done, can you merge when ready? I don't have access

@sonarqubecloud

Copy link
Copy Markdown

Comment thread .npmrc
@@ -1,2 +1,3 @@
ignore-scripts=true
min-release-age=7
min-release-age-exclude=@fastify/session

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, we're gonna wait.

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.

3 participants