Skip to content

Commit

Permalink
fix: align cookie implementation with express-session (fastify#177)
Browse files Browse the repository at this point in the history
* fix: align cookie implementation with express-session

* test: fix flaky test
  • Loading branch information
climba03003 authored Oct 20, 2022
1 parent 1b3c829 commit fd7873d
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 38 deletions.
67 changes: 67 additions & 0 deletions lib/cookie.js
Original file line number Diff line number Diff line change
@@ -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
}
}
}
3 changes: 2 additions & 1 deletion lib/fastifySession.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down
35 changes: 0 additions & 35 deletions lib/getCookieOpts.js

This file was deleted.

4 changes: 2 additions & 2 deletions lib/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions test/cookie.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
})
})

0 comments on commit fd7873d

Please sign in to comment.