From e9012422fdde985145053bb0006e4815535ae8e0 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Fri, 25 Mar 2022 11:16:12 +0100 Subject: [PATCH 1/4] feat: add `destroy`, `save` and `reload` to session instance --- README.md | 30 +++++++++++++++++++++--------- lib/fastifySession.js | 38 +++++++++++++++++--------------------- lib/session.js | 38 ++++++++++++++++++++++++++++++++++---- test/session.test.js | 29 ++++++++++++++++++++++------- types/types.d.ts | 14 ++++++++++---- types/types.test-d.ts | 6 ++++-- 6 files changed, 108 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index fc775f4..bdbcadd 100644 --- a/README.md +++ b/README.md @@ -36,12 +36,11 @@ app.addHook('preHandler', (request, reply, next) => { }) ``` **NOTE**: For all unencrypted (HTTP) connections, you need to set the `secure` cookie option to `false`. See below for all cookie options and their details. -The `sessionStore` decorator of the `request` allows to get, save and delete sessions. +The `session` object has methods that allow you to get, save, reload and delete sessions. ```js app.register(fastifySession, {secret: 'a secret with minimum length of 32 characters'}); app.addHook('preHandler', (request, reply, next) => { - const session = request.session; - request.sessionStore.destroy(session.sessionId, next); + request.session.destroy(next); }) ``` @@ -103,7 +102,7 @@ idGenerator: (request) => { Allows to access or modify the session data. -#### request.destroySession(callback) +#### Session#destroy(callback) Allows to destroy the session in the store @@ -111,16 +110,29 @@ Allows to destroy the session in the store Updates the `expires` property of the session. -#### Session#regenerate() +#### Session#regenerate(callback) -Regenerates the session by generating a new `sessionId`. +Regenerates the session by generating a new `sessionId` and persist it to the store. ```js -fastify.get('/regenerate', (request, reply) => { - request.session.regenerate(); - reply.send(request.session.sessionId); +fastify.get('/regenerate', (request, reply, done) => { + request.session.regenerate(error => { + if (error) { + done(error); + return; + } + reply.send(request.session.sessionId); + }); }); ``` +#### Session#reload(callback) + +Reloads the session data from the store and re-populates the `request.session` object. + +#### Session#save(callback) + +Save the session back to the store, replacing the contents on the store with the contents in memory. + #### Session#get(key) Gets a value from the session diff --git a/lib/fastifySession.js b/lib/fastifySession.js index 420d2b1..e27d898 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -28,7 +28,6 @@ function session (fastify, options, next) { }) fastify.decorateRequest('sessionStore', { getter: () => options.store }) fastify.decorateRequest('session', null) - fastify.decorateRequest('destroySession', destroySession) fastify.addHook('onRequest', onRequest(options)) fastify.addHook('onSend', onSend(options)) next() @@ -65,7 +64,22 @@ function decryptSession (sessionId, options, request, done) { return } if (session && session.expires && session.expires <= Date.now()) { - options.store.destroy(sessionId, getDestroyCallback(secret, request, done, cookieOpts, idGenerator)) + const restoredSession = Session.restore( + request, + idGenerator, + cookieOpts, + secret, + session + ) + + restoredSession.destroy(err => { + if (err) { + done(err) + return + } + + restoredSession.regenerate(done) + }) return } if (options.rolling) { @@ -118,7 +132,7 @@ function onSend (options) { done() return } - options.store.set(session.sessionId, session, (err) => { + session.save((err) => { if (err) { done(err) return @@ -133,29 +147,11 @@ function onSend (options) { } } -function getDestroyCallback (secret, request, done, cookieOpts, idGenerator) { - return function destroyCallback (err) { - if (err) { - done(err) - return - } - newSession(secret, request, cookieOpts, idGenerator, done) - } -} - function newSession (secret, request, cookieOpts, idGenerator, done) { request.session = new Session(request, idGenerator, cookieOpts, secret) done() } -function destroySession (done) { - const request = this - request.sessionStore.destroy(request.session.sessionId, (err) => { - request.session = null - done(err) - }) -} - function checkOptions (options) { if (!options.secret) { return new Error('the secret option is required!') diff --git a/lib/session.js b/lib/session.js index 8c933db..b309eb8 100644 --- a/lib/session.js +++ b/lib/session.js @@ -9,19 +9,22 @@ const sign = Symbol('sign') const addDataToSession = Symbol('addDataToSession') const generateId = Symbol('generateId') const requestKey = Symbol('request') +const cookieOptsKey = Symbol('cookieOpts') module.exports = class Session { constructor (request, idGenerator, cookieOpts, secret, prevSession = {}) { 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.regenerate() + this.sessionId = this[generateId](this[requestKey]) + this.encryptedSessionId = this[sign]() } } @@ -32,9 +35,14 @@ module.exports = class Session { } } - regenerate () { - this.sessionId = this[generateId](this[requestKey]) - this.encryptedSessionId = this[sign]() + regenerate (callback) { + const session = new Session(this[requestKey], this[generateId], this[cookieOptsKey], this[secretKey]) + + this[requestKey].sessionStore.set(session.sessionId, session, error => { + this[requestKey].session = session + + callback(error) + }) } [addDataToSession] (prevSession) { @@ -53,6 +61,28 @@ module.exports = class Session { this[key] = value } + destroy (callback) { + this[requestKey].sessionStore.destroy(this.sessionId, error => { + this[requestKey].session = null + + callback(error) + }) + } + + reload (callback) { + this[requestKey].sessionStore.get(this.sessionId, (error, session) => { + this[requestKey].session = new Session(this[requestKey], this[generateId], this[cookieOptsKey], this[secretKey], session) + + callback(error) + }) + } + + save (callback) { + this[requestKey].sessionStore.set(this.sessionId, this, error => { + callback(error) + }) + } + [sign] () { return cookieSignature.sign(this.sessionId, this[secretKey]) } diff --git a/test/session.test.js b/test/session.test.js index 3cb183d..2926953 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -21,7 +21,7 @@ test('should add session object to request', async (t) => { test('should destroy the session', async (t) => { t.plan(3) const port = await testServer((request, reply) => { - request.destroySession((err) => { + request.session.destroy((err) => { t.falsy(err) t.is(request.session, null) reply.send(200) @@ -167,8 +167,13 @@ test('should generate new sessionId', async (t) => { fastify.register(fastifySession, options) fastify.get('/', (request, reply) => { oldSessionId = request.session.sessionId - request.session.regenerate() - reply.send(200) + request.session.regenerate(error => { + if (error) { + reply.status(500).send('Error ' + error) + } else { + reply.send(200) + } + }) }) fastify.get('/check', (request, reply) => { t.not(request.session.sessionId, oldSessionId) @@ -361,8 +366,13 @@ test('should use custom sessionId generator if available (with request)', async }) fastify.get('/login', (request, reply) => { request.session.returningVisitor = true - request.session.regenerate() - reply.status(200).send('OK ' + request.session.sessionId) + request.session.regenerate(error => { + if (error) { + reply.status(500).send('Error ' + error) + } else { + reply.status(200).send('OK ' + request.session.sessionId) + } + }) }) await fastify.listen(0) fastify.server.unref() @@ -417,8 +427,13 @@ test('should use custom sessionId generator if available (with request and rolli }) fastify.get('/login', (request, reply) => { request.session.returningVisitor = true - request.session.regenerate() - reply.status(200).send('OK ' + request.session.sessionId) + request.session.regenerate(error => { + if (error) { + reply.status(500).send('Error ' + error) + } else { + reply.status(200).send('OK ' + request.session.sessionId) + } + }) }) await fastify.listen(0) fastify.server.unref() diff --git a/types/types.d.ts b/types/types.d.ts index 7961f26..ae9b068 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -10,9 +10,6 @@ declare module 'fastify' { /** A session store. */ sessionStore: Readonly; - - /** Allows to destroy the session in the store. */ - destroySession(callback: (err: Error) => void): void; } interface Session extends SessionData {} @@ -29,7 +26,16 @@ interface SessionData extends ExpressSessionData { /** * Regenerates the session by generating a new `sessionId`. */ - regenerate(): void; + regenerate(callback: (err?: Error) => void): void; + + /** Allows to destroy the session in the store. */ + destroy(callback: (err?: Error) => void): void; + + /** Reloads the session data from the store and re-populates the request.session object. */ + reload(callback: (err?: Error) => void): void; + + /** Save the session back to the store, replacing the contents on the store with the contents in memory. */ + save(callback: (err?: Error) => void): void; /** sets values in the session. */ set(key: string, value: unknown): void; diff --git a/types/types.test-d.ts b/types/types.test-d.ts index febd047..5bc86c4 100644 --- a/types/types.test-d.ts +++ b/types/types.test-d.ts @@ -60,7 +60,7 @@ app.route({ method: 'GET', url: '/', preHandler(req, _rep, next) { - req.destroySession(next); + req.session.destroy(next); }, async handler(request, reply) { expectType(request); @@ -69,7 +69,6 @@ app.route({ expectError((request.sessionStore = null)); expectError(request.session.doesNotExist()); expectType<{ id: number } | undefined>(request.session.user); - request.session.regenerate(); request.sessionStore.set('session-set-test', request.session, () => {}) request.sessionStore.get('', (err, session) => { expectType(err); @@ -79,5 +78,8 @@ app.route({ expectType(request.session.set('foo', 'bar')); expectType(request.session.get('foo')); expectType(request.session.touch()); + expectType(request.session.reload(() => {})); + expectType(request.session.destroy(() => {})); + expectType(request.session.regenerate(() => {})); } }); From a3466f5bb1bb376cd01beb48490d3d9de81a6776 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 28 Mar 2022 09:51:35 +0200 Subject: [PATCH 2/4] tests --- test/session.test.js | 88 +++++++++++++++++++++++++++++++++++++++++++ types/types.test-d.ts | 2 +- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/test/session.test.js b/test/session.test.js index 2926953..6a82d98 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -276,6 +276,47 @@ test('should decryptSession with custom cookie options', async (t) => { }) }) +test('should bubble up errors with destroy call if session expired', async (t) => { + t.plan(3) + const fastify = Fastify() + const sessionStore = new Map() + const store = { + set (id, data, cb) { + sessionStore.set(id, { ...data, expires: Date.now() - 1000 }) + cb(null) + }, + get (id, cb) { cb(null, sessionStore.get(id)) }, + destroy (id, cb) { cb(new Error('No can do')) } + } + + const options = { + secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', + store, + cookie: { secure: false } + } + + fastify.register(fastifyCookie) + fastify.register(fastifySession, options) + + fastify.get('/', (request, reply) => { + reply.send(200) + }) + await fastify.listen(0) + fastify.server.unref() + + const { statusCode: statusCode1, cookie: setCookie } = await request({ + url: 'http://localhost:' + fastify.server.address().port + }) + t.is(statusCode1, 200) + const { sessionId } = fastify.parseCookie(setCookie) + const { statusCode: statusCode2, body } = await request({ + url: 'http://localhost:' + fastify.server.address().port, + headers: { cookie: `sessionId=${sessionId};` } + }) + t.is(statusCode2, 500) + t.is(JSON.parse(body).message, 'No can do') +}) + test('should not reset session cookie expiration if rolling is false', async (t) => { t.plan(3) @@ -459,3 +500,50 @@ test('should use custom sessionId generator if available (with request and rolli t.is(response3.statusCode, 200) t.true(sessionBody3.startsWith('returningVisitor-')) }) + +test('should reload the session', async (t) => { + t.plan(4) + const port = await testServer((request, reply) => { + request.session.someData = 'some-data' + t.is(request.session.someData, 'some-data') + + request.session.reload((err) => { + t.falsy(err) + + t.is(request.session.someData, undefined) + + reply.send(200) + }) + }, DEFAULT_OPTIONS) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) + +test('should save the session', async (t) => { + t.plan(6) + const port = await testServer((request, reply) => { + request.session.someData = 'some-data' + t.is(request.session.someData, 'some-data') + + request.session.save((err) => { + t.falsy(err) + + t.is(request.session.someData, 'some-data') + + // unlike previous test, here the session data remains after a save + request.session.reload((err) => { + t.falsy(err) + + t.is(request.session.someData, 'some-data') + + reply.send(200) + }) + }) + }, DEFAULT_OPTIONS) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) diff --git a/types/types.test-d.ts b/types/types.test-d.ts index 5bc86c4..26a211c 100644 --- a/types/types.test-d.ts +++ b/types/types.test-d.ts @@ -60,7 +60,7 @@ app.route({ method: 'GET', url: '/', preHandler(req, _rep, next) { - req.session.destroy(next); + expectType(req.session.destroy(next)); }, async handler(request, reply) { expectType(request); From b16fb4e01be146073772b63aadea85898501dfa4 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 28 Mar 2022 10:00:59 +0200 Subject: [PATCH 3/4] somewhat simpler test --- test/session.test.js | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/test/session.test.js b/test/session.test.js index 6a82d98..1632281 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -277,15 +277,13 @@ test('should decryptSession with custom cookie options', async (t) => { }) test('should bubble up errors with destroy call if session expired', async (t) => { - t.plan(3) + t.plan(2) const fastify = Fastify() - const sessionStore = new Map() const store = { - set (id, data, cb) { - sessionStore.set(id, { ...data, expires: Date.now() - 1000 }) - cb(null) + set (id, data, cb) { cb(null) }, + get (id, cb) { + cb(null, { expires: Date.now() - 1000, cookie: { expires: Date.now() - 1000 } }) }, - get (id, cb) { cb(null, sessionStore.get(id)) }, destroy (id, cb) { cb(new Error('No can do')) } } @@ -304,16 +302,11 @@ test('should bubble up errors with destroy call if session expired', async (t) = await fastify.listen(0) fastify.server.unref() - const { statusCode: statusCode1, cookie: setCookie } = await request({ - url: 'http://localhost:' + fastify.server.address().port - }) - t.is(statusCode1, 200) - const { sessionId } = fastify.parseCookie(setCookie) - const { statusCode: statusCode2, body } = await request({ + const { statusCode, body } = await request({ url: 'http://localhost:' + fastify.server.address().port, - headers: { cookie: `sessionId=${sessionId};` } + headers: { cookie: 'sessionId=_TuQsCBgxtHB3bu6wsRpTXfjqR5sK-q_.3mu5mErW+QI7w+Q0V2fZtrztSvqIpYgsnnC8LQf6ERY;' } }) - t.is(statusCode2, 500) + t.is(statusCode, 500) t.is(JSON.parse(body).message, 'No can do') }) From 7867cc3e044372fe58cc31e19b9401fb9fdd149c Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 28 Mar 2022 14:05:04 +0200 Subject: [PATCH 4/4] Update types/types.test-d.ts --- types/types.test-d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/types/types.test-d.ts b/types/types.test-d.ts index 26a211c..b4ce8dc 100644 --- a/types/types.test-d.ts +++ b/types/types.test-d.ts @@ -81,5 +81,6 @@ app.route({ expectType(request.session.reload(() => {})); expectType(request.session.destroy(() => {})); expectType(request.session.regenerate(() => {})); + expectType(request.session.save(() => {})); } });