Skip to content

Commit

Permalink
fix: make sure to set cookie on manual session saves (#203)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimenB authored Jul 4, 2023
1 parent 607d7bb commit d255ee7
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 3 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ Gets a value from the session

Sets a value in the session

#### Session#isModified()

Whether the session has been modified from what was loaded from the store (or created)

#### Session#isSaved()

Whether the session (and any of its potential modifications) has persisted to the store

### fastify.decryptSession(sessionId, request, cookieOptions, next)
This plugin also decorates the fastify instance with `decryptSession` in case you want to decrypt the session manually.

Expand Down
13 changes: 12 additions & 1 deletion lib/fastifySession.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,22 @@ function fastifySession (fastify, options, next) {
const cookieSessionId = getCookieSessionId(request)
const saveSession = shouldSaveSession(request, cookieSessionId, saveUninitializedSession, rollingSessions)
const isInsecureConnection = cookieOpts.secure === true && isConnectionSecure(request) === false
const sessionIdWithPrefix = hasCookiePrefix ? `${cookiePrefix}${session.encryptedSessionId}` : session.encryptedSessionId
if (!saveSession || isInsecureConnection) {
// if a session cookie is set, but has a different ID, clear it
if (cookieSessionId && cookieSessionId !== session.encryptedSessionId) {
reply.clearCookie(cookieName, { domain: cookieOpts.domain })
}

if (session.isSaved()) {
reply.setCookie(
cookieName,
sessionIdWithPrefix,
// we need to remove extra properties to align the same with `express-session`
session.cookie.toJSON()
)
}

done()
return
}
Expand All @@ -183,7 +194,7 @@ function fastifySession (fastify, options, next) {
}
reply.setCookie(
cookieName,
hasCookiePrefix ? `${cookiePrefix}${session.encryptedSessionId}` : session.encryptedSessionId,
sessionIdWithPrefix,
// we need to remove extra properties to align the same with `express-session`
session.cookie.toJSON()
)
Expand Down
8 changes: 8 additions & 0 deletions lib/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const hash = Symbol('hash')
const sessionIdKey = Symbol('sessionId')
const sessionStoreKey = Symbol('sessionStore')
const encryptedSessionIdKey = Symbol('encryptedSessionId')
const savedKey = Symbol('saved')

module.exports = class Session {
constructor (
Expand All @@ -38,6 +39,7 @@ module.exports = class Session {
prevSession[sessionIdKey] === sessionId &&
prevSession[encryptedSessionIdKey]
) || cookieSigner.sign(this.sessionId)
this[savedKey] = false
this.cookie = new Cookie(prevSession?.cookie || cookieOpts, request)

if (prevSession) {
Expand Down Expand Up @@ -173,6 +175,7 @@ module.exports = class Session {
if (error) {
callback(error)
} else {
this[savedKey] = true
this[persistedHash] = this[hash]()
callback()
}
Expand All @@ -183,6 +186,7 @@ module.exports = class Session {
if (error) {
reject(error)
} else {
this[savedKey] = true
this[persistedHash] = this[hash]()
resolve()
}
Expand Down Expand Up @@ -219,4 +223,8 @@ module.exports = class Session {
isModified () {
return this[persistedHash] !== this[hash]()
}

isSaved () {
return this[savedKey]
}
}
5 changes: 3 additions & 2 deletions test/session.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ test('when rolling is false, only save session when it changes', async t => {
})

test('when rolling is false, only save session when it changes, but not if manually saved', async t => {
t.plan(4)
t.plan(5)
let setCount = 0
const store = new Map()

Expand Down Expand Up @@ -945,11 +945,12 @@ test('when rolling is false, only save session when it changes, but not if manua
await reply.send(200)
})

const { statusCode } = await fastify.inject('/')
const { statusCode, headers } = await fastify.inject('/')

t.equal(statusCode, 200)
// we manually saved the session, so it should be called once (not once for manual save and once in `onSend`)
t.equal(setCount, 1)
t.equal(typeof headers['set-cookie'], 'string')
})

test('when rolling is true, keep saving the session', async t => {
Expand Down

0 comments on commit d255ee7

Please sign in to comment.