From fd7873d8f65e95532911870f8127afd18fa199ef Mon Sep 17 00:00:00 2001 From: KaKa Date: Thu, 20 Oct 2022 09:46:59 +0800 Subject: [PATCH] fix: align cookie implementation with express-session (#177) * fix: align cookie implementation with express-session * test: fix flaky test --- lib/cookie.js | 67 +++++++++++++++++++++++++++++++++++++++++++ lib/fastifySession.js | 3 +- lib/getCookieOpts.js | 35 ---------------------- lib/session.js | 4 +-- test/cookie.test.js | 54 ++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 38 deletions(-) create mode 100644 lib/cookie.js delete mode 100644 lib/getCookieOpts.js diff --git a/lib/cookie.js b/lib/cookie.js new file mode 100644 index 0000000..d157bd6 --- /dev/null +++ b/lib/cookie.js @@ -0,0 +1,67 @@ +'use strict' + +const isConnectionSecure = require('./isConnectionSecure') + +module.exports = class Cookie { + constructor (cookie, request) { + const originalMaxAge = cookie.originalMaxAge || cookie.maxAge || null + this.path = cookie.path || '/' + this.secure = cookie.secure ?? null + this.sameSite = cookie.sameSite || null + this.domain = cookie.domain || null + this.httpOnly = cookie.httpOnly !== undefined ? cookie.httpOnly : true + this._expires = null + + if (originalMaxAge) { + this.maxAge = originalMaxAge + } else if (cookie.expires) { + this.expires = new Date(cookie.expires) + this.originalMaxAge = null + } else { + this.originalMaxAge = originalMaxAge + } + + if (this.secure === 'auto') { + if (isConnectionSecure(request)) { + this.secure = true + } else { + this.sameSite = 'Lax' + this.secure = false + } + } + } + + set expires (date) { + this._expires = date + } + + get expires () { + return this._expires + } + + set maxAge (ms) { + this.expires = new Date(Date.now() + ms) + // we force the same originalMaxAge to match old behavior + this.originalMaxAge = ms + } + + get maxAge () { + if (this.expires instanceof Date) { + return this.expires.valueOf() - Date.now() + } else { + return null + } + } + + toJSON () { + return { + expires: this._expires, + originalMaxAge: this.originalMaxAge, + sameSite: this.sameSite, + secure: this.secure, + path: this.path, + httpOnly: this.httpOnly, + domain: this.domain + } + } +} diff --git a/lib/fastifySession.js b/lib/fastifySession.js index d546941..b34f6ce 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -184,7 +184,8 @@ function fastifySession (fastify, options, next) { reply.setCookie( cookieName, hasCookiePrefix ? `${cookiePrefix}${session.encryptedSessionId}` : session.encryptedSessionId, - session.cookie + // we need to remove extra properties to align the same with `express-session` + session.cookie.toJSON() ) done() }) diff --git a/lib/getCookieOpts.js b/lib/getCookieOpts.js deleted file mode 100644 index a951369..0000000 --- a/lib/getCookieOpts.js +++ /dev/null @@ -1,35 +0,0 @@ -'use strict' - -const isConnectionSecure = require('./isConnectionSecure') - -module.exports = function getCookieOpts (cookieOpts, request) { - const originalMaxAge = cookieOpts.originalMaxAge || cookieOpts.maxAge || null - let secure = cookieOpts.secure ?? null - let sameSite = cookieOpts.sameSite || null - let expires = null - - if (originalMaxAge) { - expires = new Date(Date.now() + originalMaxAge) - } else if (cookieOpts.expires) { - expires = new Date(cookieOpts.expires) - } - - if (secure === 'auto') { - if (isConnectionSecure(request)) { - secure = true - } else { - sameSite = 'Lax' - secure = false - } - } - - return { - expires, - originalMaxAge, - sameSite, - secure, - path: cookieOpts.path || '/', - httpOnly: cookieOpts.httpOnly !== undefined ? cookieOpts.httpOnly : true, - domain: cookieOpts.domain || null - } -} diff --git a/lib/session.js b/lib/session.js index da05050..55c7dd5 100644 --- a/lib/session.js +++ b/lib/session.js @@ -2,7 +2,7 @@ const crypto = require('crypto') -const getCookieOpts = require('./getCookieOpts') +const Cookie = require('./cookie') const { configure: configureStringifier } = require('safe-stable-stringify') const stringify = configureStringifier({ bigint: false }) @@ -38,7 +38,7 @@ module.exports = class Session { prevSession[sessionIdKey] === sessionId && prevSession[encryptedSessionIdKey] ) || cookieSigner.sign(this.sessionId) - this.cookie = getCookieOpts(prevSession?.cookie || cookieOpts, request) + this.cookie = new Cookie(prevSession?.cookie || cookieOpts, request) if (prevSession) { // Copy over values from the previous session diff --git a/test/cookie.test.js b/test/cookie.test.js index bc3ad02..a24c0bc 100644 --- a/test/cookie.test.js +++ b/test/cookie.test.js @@ -5,6 +5,7 @@ const Fastify = require('fastify') const fastifyCookie = require('@fastify/cookie') const fastifySession = require('../lib/fastifySession') const fastifyPlugin = require('fastify-plugin') +const Cookie = require('../lib/cookie') const { DEFAULT_OPTIONS, DEFAULT_COOKIE, DEFAULT_SECRET, buildFastify, DEFAULT_SESSION_ID } = require('./util') test('should set session cookie', async (t) => { @@ -502,3 +503,56 @@ test('when cookie secure is set to false then store secure as false', async t => t.equal(typeof response.headers['set-cookie'], 'string') t.match(response.headers['set-cookie'], /^sessionId=[\w-]{32}.[\w-%]{43,135}; Path=\/; HttpOnly$/) }) + +test('Cookie', t => { + t.plan(4) + + const cookie = new Cookie({}) + + t.test('properties', t => { + t.plan(9) + + t.equal('expires' in cookie, true) + t.equal('originalMaxAge' in cookie, true) + t.equal('sameSite' in cookie, true) + t.equal('secure' in cookie, true) + t.equal('path' in cookie, true) + t.equal('httpOnly' in cookie, true) + t.equal('domain' in cookie, true) + t.equal('_expires' in cookie, true) + t.equal('maxAge' in cookie, true) + }) + + t.test('toJSON', t => { + t.plan(9) + + const json = cookie.toJSON() + + t.equal('expires' in json, true) + t.equal('originalMaxAge' in json, true) + t.equal('sameSite' in json, true) + t.equal('secure' in json, true) + t.equal('path' in json, true) + t.equal('httpOnly' in json, true) + t.equal('domain' in json, true) + + t.equal('_expires' in json, false) + t.equal('maxAge' in json, false) + }) + + t.test('maxAge calculated from expires', t => { + t.plan(2) + + const cookie = new Cookie({ expires: new Date(Date.now() + 1000) }) + t.equal(cookie.maxAge <= 1000, true) + t.equal(cookie.originalMaxAge, null) + }) + + t.test('maxAge set by maxAge', t => { + t.plan(2) + + const cookie = new Cookie({ maxAge: 1000 }) + t.equal(cookie.maxAge, 1000) + t.equal(cookie.originalMaxAge, 1000) + }) +})