diff --git a/README.md b/README.md index 7e72c2a..d0d020d 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ Allows to destroy the session in the store. If you do not pass a callback, a Pro #### Session#touch() -Updates the `expires` property of the session. +Updates the `expires` property of the session's cookie. #### Session#regenerate(callback) diff --git a/lib/fastifySession.js b/lib/fastifySession.js index da15eaa..b4607b3 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -62,13 +62,14 @@ function decryptSession (sessionId, options, request, done) { newSession(secret, request, cookieOpts, idGenerator, done) return } - if (session?.expires && session.expires <= Date.now()) { + if (session.cookie?.expires && session.cookie.expires <= Date.now()) { const restoredSession = Session.restore( request, idGenerator, cookieOpts, secret, - session + session, + decryptedSessionId ) restoredSession.destroy(err => { @@ -87,7 +88,8 @@ function decryptSession (sessionId, options, request, done) { idGenerator, cookieOpts, secret, - session + session, + decryptedSessionId ) } else { request.session = Session.restore( @@ -95,7 +97,8 @@ function decryptSession (sessionId, options, request, done) { idGenerator, cookieOpts, secret, - session + session, + decryptedSessionId ) } done() diff --git a/lib/session.js b/lib/session.js index 59eca9c..c280112 100644 --- a/lib/session.js +++ b/lib/session.js @@ -10,36 +10,33 @@ const stringify = configureStringifier({ bigint: false }) const maxAge = Symbol('maxAge') const secretKey = Symbol('secretKey') -const sign = Symbol('sign') const addDataToSession = Symbol('addDataToSession') const generateId = Symbol('generateId') const requestKey = Symbol('request') const cookieOptsKey = Symbol('cookieOpts') const originalHash = Symbol('originalHash') const hash = Symbol('hash') +const sessionIdKey = Symbol('sessionId') +const encryptedSessionIdKey = Symbol('encryptedSessionId') module.exports = class Session { - constructor (request, idGenerator, cookieOpts, secret, prevSession = {}) { + constructor (request, idGenerator, cookieOpts, secret, prevSession = {}, sessionId = idGenerator(request)) { this[generateId] = idGenerator - this.expires = null this.cookie = new Cookie(cookieOpts) this[cookieOptsKey] = cookieOpts this[maxAge] = cookieOpts.maxAge this[secretKey] = secret this[addDataToSession](prevSession) this[requestKey] = request - this.touch() - if (!this.sessionId) { - this.sessionId = this[generateId](this[requestKey]) - this.encryptedSessionId = this[sign]() - } + this[sessionIdKey] = sessionId + this[encryptedSessionIdKey] = cookieSignature.sign(sessionId, secret) this[originalHash] = this[hash]() + this.touch() // Needs to happen after originalHash is set, in case maxAge forces an update } touch () { if (this[maxAge]) { - this.expires = new Date(Date.now() + this[maxAge]) - this.cookie.expires = this.expires + this.cookie.expires = new Date(Date.now() + this[maxAge]) } } @@ -71,7 +68,7 @@ module.exports = class Session { [addDataToSession] (prevSession) { for (const key in prevSession) { - if (!['expires', 'cookie'].includes(key)) { + if (!['cookie', 'sessionId', 'encryptedSessionId'].includes(key)) { this[key] = prevSession[key] } } @@ -87,14 +84,14 @@ module.exports = class Session { destroy (callback) { if (callback) { - this[requestKey].sessionStore.destroy(this.sessionId, error => { + this[requestKey].sessionStore.destroy(this[sessionIdKey], error => { this[requestKey].session = null callback(error) }) } else { return new Promise((resolve, reject) => { - this[requestKey].sessionStore.destroy(this.sessionId, error => { + this[requestKey].sessionStore.destroy(this[sessionIdKey], error => { this[requestKey].session = null if (error) { @@ -109,15 +106,15 @@ module.exports = class Session { reload (callback) { if (callback) { - this[requestKey].sessionStore.get(this.sessionId, (error, session) => { - this[requestKey].session = new Session(this[requestKey], this[generateId], this[cookieOptsKey], this[secretKey], session) + this[requestKey].sessionStore.get(this[sessionIdKey], (error, session) => { + this[requestKey].session = new Session(this[requestKey], this[generateId], this[cookieOptsKey], this[secretKey], session, this[sessionIdKey]) callback(error) }) } else { return new Promise((resolve, reject) => { - this[requestKey].sessionStore.get(this.sessionId, (error, session) => { - this[requestKey].session = new Session(this[requestKey], this[generateId], this[cookieOptsKey], this[secretKey], session) + this[requestKey].sessionStore.get(this[sessionIdKey], (error, session) => { + this[requestKey].session = new Session(this[requestKey], this[generateId], this[cookieOptsKey], this[secretKey], session, this[sessionIdKey]) if (error) { reject(error) @@ -131,12 +128,12 @@ module.exports = class Session { save (callback) { if (callback) { - this[requestKey].sessionStore.set(this.sessionId, this, error => { + this[requestKey].sessionStore.set(this[sessionIdKey], this, error => { callback(error) }) } else { return new Promise((resolve, reject) => { - this[requestKey].sessionStore.set(this.sessionId, this, error => { + this[requestKey].sessionStore.set(this[sessionIdKey], this, error => { if (error) { reject(error) } else { @@ -147,16 +144,13 @@ module.exports = class Session { } } - [sign] () { - return cookieSignature.sign(this.sessionId, this[secretKey]) - } - [hash] () { const sess = this const str = stringify(sess, function (key, val) { // ignore sess.cookie property if (this === sess && key === 'cookie') { - return + // we want `touch` to affect the hash of the session + return sess.cookie.expires?.getTime() } return val @@ -169,15 +163,28 @@ module.exports = class Session { } isModified () { - return this[originalHash] !== this[hash]() + return Boolean( + this[originalHash] !== this[hash]() || + // If maxAge is set, the session is modified. + // This is necessary because we don't read the cookie's expiration from the cookie itself. + // session.cookie.expires is initially set from Cookie(cookieOpts), so we cannot detect if it changed from maxAge alone. + this[maxAge] + ) + } + + get sessionId () { + return this[sessionIdKey] + } + + get encryptedSessionId () { + return this[encryptedSessionIdKey] } - static restore (request, idGenerator, cookieOpts, secret, prevSession) { - const restoredSession = new Session(request, idGenerator, cookieOpts, secret, prevSession) + static restore (request, idGenerator, cookieOpts, secret, prevSession, sessionId) { + const restoredSession = new Session(request, idGenerator, cookieOpts, secret, prevSession, sessionId) const restoredCookie = new Cookie(cookieOpts) restoredCookie.expires = new Date(prevSession.cookie.expires) restoredSession.cookie = restoredCookie - restoredSession.expires = restoredCookie.expires restoredSession[originalHash] = restoredSession[hash]() return restoredSession } diff --git a/test/base.test.js b/test/base.test.js index 22dbf5b..f9db942 100644 --- a/test/base.test.js +++ b/test/base.test.js @@ -2,7 +2,7 @@ const test = require('tap').test const fastifyPlugin = require('fastify-plugin') -const { DEFAULT_OPTIONS, DEFAULT_COOKIE, buildFastify } = require('./util') +const { DEFAULT_OPTIONS, DEFAULT_COOKIE, DEFAULT_COOKIE_VALUE, DEFAULT_SESSION_ID, buildFastify } = require('./util') test('should not set session cookie on post without params', async (t) => { t.plan(3) @@ -45,7 +45,7 @@ test('should set session cookie', async (t) => { }) test('should support multiple secrets', async (t) => { - t.plan(2) + t.plan(4) const options = { secret: ['geheim', 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk'] } @@ -53,7 +53,7 @@ test('should support multiple secrets', async (t) => { const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { request.sessionStore.set('aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e', { - expires: Date.now() + 1000 + test: {} }, done) }) }) @@ -64,16 +64,29 @@ test('should support multiple secrets', async (t) => { const fastify = await buildFastify(handler, options, plugin) t.teardown(() => fastify.close()) - const response = await fastify.inject({ + const sessionIdEncryptedWithOldSecret = 'aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e.eiVu2YbrcqbTUYTYaANks%2Fjn%2Bjta7QgpsxLO%2BOLN%2F4U' + const sessionIdEncryptedWithNewSecret = 'aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e.InCp31AuDa7DX%2F8rGBz8RMFiCpmUtjcF%2BS7Aco7tur8' + + const responseForOldSecret = await fastify.inject({ url: '/', headers: { 'x-forwarded-proto': 'https', - cookie: 'sessionId=aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e.eiVu2YbrcqbTUYTYaANks%2Fjn%2Bjta7QgpsxLO%2BOLN%2F4U; Path=/; HttpOnly; Secure' + cookie: `sessionId=${sessionIdEncryptedWithOldSecret}; Path=/; HttpOnly; Secure` } }) + t.equal(responseForOldSecret.statusCode, 200) + // It will be replaced with the new secret! + t.equal(responseForOldSecret.headers['set-cookie'].includes(sessionIdEncryptedWithNewSecret), true) - t.equal(response.statusCode, 200) - t.equal(response.headers['set-cookie'].includes('aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e'), false) + const responseForNewSecret = await fastify.inject({ + url: '/', + headers: { + 'x-forwarded-proto': 'https', + cookie: `sessionId=${sessionIdEncryptedWithNewSecret}; Path=/; HttpOnly; Secure` + } + }) + t.equal(responseForNewSecret.statusCode, 200) + t.equal(responseForNewSecret.headers['set-cookie'].includes(sessionIdEncryptedWithNewSecret), true) }) test('should set session cookie using the specified cookie name', async (t) => { @@ -101,9 +114,7 @@ test('should set session cookie using the default cookie name', async (t) => { t.plan(2) const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { - request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { - expires: Date.now() + 1000, - sessionId: 'Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', + request.sessionStore.set(DEFAULT_SESSION_ID, { cookie: { secure: true, httpOnly: true, path: '/' } }, done) }) @@ -124,17 +135,15 @@ test('should set session cookie using the default cookie name', async (t) => { }) t.equal(response.statusCode, 200) - t.match(response.headers['set-cookie'], /sessionId=undefined; Path=\/; HttpOnly; Secure/) + t.match(response.headers['set-cookie'], /sessionId=.*\..*; Path=\/; HttpOnly; Secure/) }) test('should create new session on expired session', async (t) => { t.plan(2) const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { - request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { - expires: Date.now() - 1000, - sessionId: 'Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', - cookie: { secure: true, httpOnly: true, path: '/' } + request.sessionStore.set(DEFAULT_SESSION_ID, { + cookie: { expires: new Date(Date.now() - 1000), secure: true, httpOnly: true, path: '/' } }, done) }) }) @@ -160,21 +169,21 @@ test('should create new session on expired session', async (t) => { t.match(response.headers['set-cookie'], /sessionId=.*\..*; Path=\/; Expires=.*; HttpOnly; Secure/) }) -test('should set session.expires if maxAge', async (t) => { - t.plan(2) +test('should set session.cookie.expires if maxAge', async (t) => { + t.plan(3) const options = { secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', cookie: { maxAge: 42 } } const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { - request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { - expires: Date.now() + 1000 + request.sessionStore.set(DEFAULT_SESSION_ID, { + test: {} }, done) }) }) function handler (request, reply) { - t.ok(request.session.expires) + t.ok(request.session.cookie.expires) reply.send(200) } const fastify = await buildFastify(handler, options, plugin) @@ -182,10 +191,12 @@ test('should set session.expires if maxAge', async (t) => { const response = await fastify.inject({ url: '/', - headers: { cookie: DEFAULT_COOKIE } + headers: { cookie: DEFAULT_COOKIE, 'x-forwarded-proto': 'https' } }) t.equal(response.statusCode, 200) + // Should be sending cookie as well + t.match(response.headers['set-cookie'], /sessionId=.*\..*; Path=\/; Expires=.*; HttpOnly; Secure/) }) test('should set new session cookie if expired', async (t) => { @@ -193,13 +204,12 @@ test('should set new session cookie if expired', async (t) => { const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { - request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { - expires: Date.now() + 1000 + request.sessionStore.set(DEFAULT_SESSION_ID, { + cookie: { expires: new Date(Date.now() - 1000) } }, done) }) }) function handler (request, reply) { - request.session.test = {} reply.send(200) } const fastify = await buildFastify(handler, DEFAULT_OPTIONS, plugin) @@ -213,7 +223,7 @@ test('should set new session cookie if expired', async (t) => { } }) - t.equal(response.headers['set-cookie'].includes('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN.B7fUDYXU9fXF9pNuL3qm4NVmSduLJ6kzCOPh5JhHGoE'), false) + t.equal(response.headers['set-cookie'].includes(DEFAULT_COOKIE_VALUE), false) t.match(response.headers['set-cookie'], /sessionId=[\w-]{32}.[\w-%]{43,57}; Path=\/; HttpOnly; Secure/) t.equal(response.statusCode, 200) }) @@ -236,7 +246,7 @@ test('should return new session cookie if does not exist in store', async (t) => }) t.equal(response.statusCode, 200) - t.equal(response.headers['set-cookie'].includes('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN.B7fUDYXU9fXF9pNuL3qm4NVmSduLJ6kzCOPh5JhHGoE'), false) + t.equal(response.headers['set-cookie'].includes(DEFAULT_COOKIE_VALUE), false) t.match(response.headers['set-cookie'], /sessionId=[\w-]{32}.[\w-%]{43,57}; Path=\/; HttpOnly; Secure/) }) @@ -267,8 +277,8 @@ test('should create new session if cookie contains invalid session', async (t) = } const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { - request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { - expires: Date.now() + 1000 + request.sessionStore.set(DEFAULT_SESSION_ID, { + test: {} }, done) }) }) @@ -278,7 +288,7 @@ test('should create new session if cookie contains invalid session', async (t) = const response = await fastify.inject({ url: '/', headers: { - cookie: 'sessionId=Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN.B7fUDYXx9fXF9pNuL3qm4NVmSduLJ6kzCOPh5JhHGoE; Path=/; HttpOnly; Secure', + cookie: 'sessionId=Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN.ohnoinvalidnooooL3qm4NVmSduLJ6kzCOPh5JhHGoE; Path=/; HttpOnly; Secure', 'x-forwarded-proto': 'https' } }) @@ -288,7 +298,7 @@ test('should create new session if cookie contains invalid session', async (t) = t.match(response.headers['set-cookie'], /sessionId=[\w-]{32}.[\w-%]{43,57}; Path=\/; HttpOnly; Secure/) }) -test('should not set session cookie if no data in session and saveUninitialized is false', async (t) => { +test('should not set session cookie if saveUninitialized is false and no data in session', async (t) => { t.plan(2) const options = { secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', @@ -305,3 +315,59 @@ test('should not set session cookie if no data in session and saveUninitialized t.equal(response.statusCode, 200) t.ok(response.headers['set-cookie'] === undefined) }) + +test('should not set session cookie if saveUninitialized is false and data in session is unmodified', async (t) => { + t.plan(2) + const options = { + secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', + saveUninitialized: false + } + const plugin = fastifyPlugin(async (fastify, opts) => { + fastify.addHook('onRequest', (request, reply, done) => { + request.sessionStore.set(DEFAULT_SESSION_ID, { + test: {} + }, done) + }) + }) + const fastify = await buildFastify((request, reply) => reply.send(200), options, plugin) + t.teardown(() => fastify.close()) + + const response = await fastify.inject({ + url: '/', + headers: { cookie: DEFAULT_COOKIE, 'x-forwarded-proto': 'https' } + }) + + t.equal(response.statusCode, 200) + t.ok(response.headers['set-cookie'] === undefined) +}) + +test('should set session cookie if saveUninitialized is false and maxAge is on', async (t) => { + t.plan(2) + const options = { + cookie: { + maxAge: 42 + }, + secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', + saveUninitialized: false + } + const plugin = fastifyPlugin(async (fastify, opts) => { + fastify.addHook('onRequest', (request, reply, done) => { + request.sessionStore.set(DEFAULT_SESSION_ID, { + // In this scenario, maxAge would have set expires in a previous request + cookie: { + expires: new Date(Date.now() + 1000) + } + }, done) + }) + }) + const fastify = await buildFastify((request, reply) => reply.send(200), options, plugin) + t.teardown(() => fastify.close()) + + const response = await fastify.inject({ + url: '/', + headers: { cookie: DEFAULT_COOKIE, 'x-forwarded-proto': 'https' } + }) + + t.equal(response.statusCode, 200) + t.ok(response.headers['set-cookie']) +}) diff --git a/test/session.test.js b/test/session.test.js index a1e5158..58fecfe 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -42,9 +42,11 @@ test('should destroy the session', async (t) => { }) test('should add session.encryptedSessionId object to request', async (t) => { - t.plan(2) + t.plan(3) const fastify = await buildFastify((request, reply) => { t.ok(request.session.encryptedSessionId) + // serialize, then deserialize to make sure it's gone + t.notOk(JSON.parse(JSON.stringify(request.session)).encryptedSessionId) reply.send(200) }, DEFAULT_OPTIONS) t.teardown(() => fastify.close()) @@ -71,25 +73,6 @@ test('should add session.cookie object to request', async (t) => { t.equal(response.statusCode, 200) }) -test('should add session.expires object to request', async (t) => { - t.plan(2) - const options = { - secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', - cookie: { maxAge: 42 } - } - const fastify = await buildFastify((request, reply) => { - t.ok(request.session.expires) - reply.send(200) - }, options) - t.teardown(() => fastify.close()) - - const response = await fastify.inject({ - url: '/' - }) - - t.equal(response.statusCode, 200) -}) - test('should add session.sessionId object to request', async (t) => { t.plan(2) const fastify = await buildFastify((request, reply) => { @@ -248,8 +231,6 @@ test('should decryptSession with custom request object', async (t) => { fastify.addHook('onRequest', (request, reply, done) => { request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { testData: 'this is a test', - expires: Date.now() + 1000, - sessionId: 'Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', cookie: { secure: true, httpOnly: true, path: '/' } }, done) }) @@ -309,7 +290,7 @@ test('should bubble up errors with destroy call if session expired', async (t) = const store = { set (id, data, cb) { cb(null) }, get (id, cb) { - cb(null, { expires: Date.now() - 1000, cookie: { expires: Date.now() - 1000 } }) + cb(null, { cookie: { expires: new Date(Date.now() - 1000) } }) }, destroy (id, cb) { cb(new Error('No can do')) } } @@ -350,7 +331,7 @@ test('should not reset session cookie expiration if rolling is false', async (t) fastify.register(fastifyCookie) fastify.register(fastifySession, options) fastify.addHook('onRequest', (request, reply, done) => { - reply.send(request.session.expires) + reply.send(request.session.cookie.expires) done() }) @@ -387,7 +368,7 @@ test('should update the expires property of the session using Session#touch() ev fastify.register(fastifySession, options) fastify.addHook('onRequest', (request, reply, done) => { request.session.touch() - reply.send(request.session.expires) + reply.send(request.session.cookie.expires) done() }) diff --git a/test/store.test.js b/test/store.test.js index 770986c..1c4af10 100644 --- a/test/store.test.js +++ b/test/store.test.js @@ -101,7 +101,7 @@ test('should set new session cookie if expired', async (t) => { const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', { - expires: Date.now() - 1000 + cookie: { expires: new Date(Date.now() - 1000) } }, done) }) }) diff --git a/test/util.js b/test/util.js index 9088b1f..591e700 100644 --- a/test/util.js +++ b/test/util.js @@ -5,7 +5,8 @@ const fastifyCookie = require('@fastify/cookie') const fastifySession = require('../lib/fastifySession') const DEFAULT_OPTIONS = { secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk' } -const DEFAULT_COOKIE_VALUE = 'sessionId=Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN.B7fUDYXU9fXF9pNuL3qm4NVmSduLJ6kzCOPh5JhHGoE' +const DEFAULT_SESSION_ID = 'Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN' +const DEFAULT_COOKIE_VALUE = `sessionId=${DEFAULT_SESSION_ID}.B7fUDYXU9fXF9pNuL3qm4NVmSduLJ6kzCOPh5JhHGoE` const DEFAULT_COOKIE = `${DEFAULT_COOKIE_VALUE}; Path=/; HttpOnly; Secure` async function buildFastify (handler, sessionOptions, plugin) { @@ -25,5 +26,6 @@ module.exports = { buildFastify, DEFAULT_COOKIE_VALUE, DEFAULT_COOKIE, - DEFAULT_OPTIONS + DEFAULT_OPTIONS, + DEFAULT_SESSION_ID } diff --git a/types/types.d.ts b/types/types.d.ts index c5c2ebf..66aca95 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -20,7 +20,7 @@ interface SessionData extends ExpressSessionData { encryptedSessionId: string; - /** Updates the `expires` property of the session. */ + /** Updates the `expires` property of the session's cookie. */ touch(): void; /**