diff --git a/lib/cookie.js b/lib/cookie.js index 93cde8b..e0b2150 100644 --- a/lib/cookie.js +++ b/lib/cookie.js @@ -36,11 +36,11 @@ module.exports = class Cookie { } function getExpires (cookieOpts) { - let expires = null + if (cookieOpts.maxAge) { + return new Date(Date.now() + cookieOpts.maxAge) + } if (cookieOpts.expires) { - expires = cookieOpts.expires - } else if (cookieOpts.maxAge) { - expires = new Date(Date.now() + cookieOpts.maxAge) + return cookieOpts.expires } - return expires + return null } diff --git a/lib/fastifySession.js b/lib/fastifySession.js index cf93989..efd0912 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -68,7 +68,8 @@ function decryptSession (sessionId, options, request, done) { 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 60953e6..73444e3 100644 --- a/lib/session.js +++ b/lib/session.js @@ -8,36 +8,36 @@ const { configure: configureStringifier } = require('safe-stable-stringify') 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)) { + // Copy over values from the previous session + for (const key in prevSession) { + this[key] = prevSession[key] + } + + this[requestKey] = request this[generateId] = idGenerator - 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.cookie = new Cookie(cookieOpts) + this[sessionIdKey] = sessionId + this[encryptedSessionIdKey] = cookieSignature.sign(sessionId, secret) this[originalHash] = this[hash]() } touch () { - if (this[maxAge]) { - this.cookie.expires = new Date(Date.now() + this[maxAge]) + if (this[cookieOptsKey].maxAge) { + this.cookie.expires = new Date(Date.now() + this[cookieOptsKey].maxAge) } } @@ -67,14 +67,6 @@ module.exports = class Session { } } - [addDataToSession] (prevSession) { - for (const key in prevSession) { - if (key !== 'cookie') { - this[key] = prevSession[key] - } - } - } - get (key) { return this[key] } @@ -85,14 +77,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) { @@ -107,15 +99,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) @@ -129,12 +121,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 { @@ -145,8 +137,12 @@ module.exports = class Session { } } - [sign] () { - return cookieSignature.sign(this.sessionId, this[secretKey]) + get sessionId() { + return this[sessionIdKey] + } + + get encryptedSessionId() { + return this[encryptedSessionIdKey] } [hash] () { @@ -170,10 +166,13 @@ module.exports = class Session { return this[originalHash] !== this[hash]() } - 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) + if (prevSession.cookie.expires) { + // Need to parse as Date + restoredCookie.expires = new Date(prevSession.cookie.expires) + } restoredSession.cookie = restoredCookie restoredSession[originalHash] = restoredSession[hash]() return restoredSession diff --git a/test/base.test.js b/test/base.test.js index aba1323..5039b32 100644 --- a/test/base.test.js +++ b/test/base.test.js @@ -2,6 +2,7 @@ const test = require('tap').test const fastifyPlugin = require('fastify-plugin') +const cookieSignature = require('cookie-signature') const { DEFAULT_OPTIONS, DEFAULT_COOKIE, DEFAULT_SESSION_ID, DEFAULT_SECRET, DEFAULT_ENCRYPTED_SESSION_ID, buildFastify } = require('./util') test('should not set session cookie on post without params', async (t) => { @@ -45,15 +46,21 @@ test('should set session cookie', async (t) => { }) test('should support multiple secrets', async (t) => { - t.plan(2) + t.plan(4) + const newSecret = 'geheim' const options = { - secret: ['geheim', DEFAULT_SECRET] + secret: [newSecret, DEFAULT_SECRET] } + const sessionId = 'aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e' + const sessionIdSignedWithOldSecret = cookieSignature.sign(sessionId, DEFAULT_SECRET) + const sessionIdSignedWithNewSecret = cookieSignature.sign(sessionId, newSecret) + const htmlEncodedSessionIdSignedWithNewSecret = 'aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e.InCp31AuDa7DX%2F8rGBz8RMFiCpmUtjcF%2BS7Aco7tur8' + const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { - request.sessionStore.set('aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e', { - expires: Date.now() + 1000 + request.sessionStore.set(sessionId, { + test: {} }, done) }) }) @@ -64,16 +71,27 @@ test('should support multiple secrets', async (t) => { const fastify = await buildFastify(handler, options, plugin) t.teardown(() => fastify.close()) - const response = await fastify.inject({ + 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=${sessionIdSignedWithOldSecret}; Path=/; HttpOnly; Secure` } }) - t.equal(response.statusCode, 200) - t.equal(response.headers['set-cookie'].includes('aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e'), false) + t.equal(responseForOldSecret.statusCode, 200) + // It will be replaced with the new secret! + t.equal(responseForOldSecret.headers['set-cookie'].includes(htmlEncodedSessionIdSignedWithNewSecret), true) + + const responseForNewSecret = await fastify.inject({ + url: '/', + headers: { + 'x-forwarded-proto': 'https', + cookie: `sessionId=${sessionIdSignedWithNewSecret}; Path=/; HttpOnly; Secure` + } + }) + t.equal(responseForNewSecret.statusCode, 200) + t.equal(responseForNewSecret.headers['set-cookie'].includes(htmlEncodedSessionIdSignedWithNewSecret), true) }) test('should set session cookie using the specified cookie name', async (t) => { @@ -99,20 +117,11 @@ test('should set session cookie using the specified cookie name', async (t) => { 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(DEFAULT_SESSION_ID, { - expires: Date.now() + 1000, - sessionId: DEFAULT_SESSION_ID, - cookie: { secure: true, httpOnly: true, path: '/' } - }, done) - }) - }) function handler (request, reply) { request.session.test = {} reply.send(200) } - const fastify = await buildFastify(handler, DEFAULT_OPTIONS, plugin) + const fastify = await buildFastify(handler, DEFAULT_OPTIONS) t.teardown(() => fastify.close()) const response = await fastify.inject({ @@ -124,7 +133,7 @@ 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=[\w-]{32}.[\w-%]{43,57}; Path=\/; HttpOnly; Secure/) }) test('should create new session on expired session', async (t) => { @@ -132,7 +141,6 @@ test('should create new session on expired session', async (t) => { const plugin = fastifyPlugin(async (fastify, opts) => { fastify.addHook('onRequest', (request, reply, done) => { request.sessionStore.set(DEFAULT_SESSION_ID, { - sessionId: DEFAULT_SESSION_ID, cookie: { secure: true, httpOnly: true, path: '/', expires: Date.now() - 1000 } }, done) }) @@ -188,7 +196,7 @@ test('should set new session cookie if expired', async (t) => { fastify.addHook('onRequest', (request, reply, done) => { request.sessionStore.set(DEFAULT_SESSION_ID, { cookie: { - expires: Date.now() + 1000 + expires: Date.now() - 1000 } }, done) }) diff --git a/test/session.test.js b/test/session.test.js index e96a81d..3e6386c 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -42,9 +42,10 @@ 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) + t.equal(JSON.parse(JSON.stringify(request.session)).encryptedSessionId, undefined) reply.send(200) }, DEFAULT_OPTIONS) t.teardown(() => fastify.close()) @@ -229,7 +230,6 @@ test('should decryptSession with custom request object', async (t) => { fastify.addHook('onRequest', (request, reply, done) => { request.sessionStore.set(DEFAULT_SESSION_ID, { testData: 'this is a test', - sessionId: DEFAULT_SESSION_ID, cookie: { secure: true, httpOnly: true, path: '/', expires: Date.now() + 1000 } }, done) }) @@ -357,22 +357,18 @@ test('should update the expires property of the session using Session#touch() ev t.plan(3) const fastify = Fastify() - - const options = { + fastify.register(fastifyCookie) + fastify.register(fastifySession, { secret: DEFAULT_SECRET, rolling: false, cookie: { secure: false, maxAge: 10000 } - } - fastify.register(fastifyCookie) - fastify.register(fastifySession, options) + }) fastify.addHook('onRequest', (request, reply, done) => { request.session.touch() reply.send(request.session.cookie.expires) done() }) - fastify.get('/', (request, reply) => reply.send(200)) - fastify.get('/check', (request, reply) => reply.send(200)) await fastify.listen({ port: 0 }) t.teardown(() => { fastify.close() }) @@ -384,14 +380,48 @@ test('should update the expires property of the session using Session#touch() ev await new Promise(resolve => setTimeout(resolve, 1)) const response2 = await fastify.inject({ - url: '/check', + url: '/', headers: { Cookie: response1.headers['set-cookie'] } }) t.equal(response2.statusCode, 200) - t.ok(response1.body !== response2.body) }) +test('will not update expires property of the session using Session#touch() if maxAge is not set', async (t) => { + t.plan(4) + + const fastify = Fastify() + fastify.register(fastifyCookie) + fastify.register(fastifySession, { + secret: DEFAULT_SECRET, + rolling: false, + cookie: { secure: false } + }) + fastify.addHook('onRequest', (request, reply, done) => { + request.session.touch() + reply.send(request.session.cookie.expires) + done() + }) + + fastify.get('/', (request, reply) => reply.send(200)) + await fastify.listen({ port: 0 }) + t.teardown(() => { fastify.close() }) + + const response1 = await fastify.inject({ + url: '/' + }) + t.equal(response1.statusCode, 200) + await new Promise(resolve => setTimeout(resolve, 1)) + t.equal(response1.body, 'null') + + const response2 = await fastify.inject({ + url: '/', + headers: { Cookie: response1.headers['set-cookie'] } + }) + t.equal(response2.statusCode, 200) + t.equal(response2.body, 'null') +}) + test('should use custom sessionId generator if available (with request)', async (t) => { const fastify = Fastify() fastify.register(fastifyCookie)