From a11170008db50515de95b89d0ec6c855e002b6b9 Mon Sep 17 00:00:00 2001
From: Simen Bekkhus <sbekkhus91@gmail.com>
Date: Tue, 28 Jun 2022 16:17:32 +0200
Subject: [PATCH 1/4] feat: remove session id from session data

---
 lib/fastifySession.js |  9 ++++++---
 lib/session.js        | 45 +++++++++++++++++++++++--------------------
 test/base.test.js     | 12 +++++++-----
 test/session.test.js  |  5 +++--
 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/lib/fastifySession.js b/lib/fastifySession.js
index b158eb8..023c7e0 100644
--- a/lib/fastifySession.js
+++ b/lib/fastifySession.js
@@ -69,7 +69,8 @@ function decryptSession (sessionId, options, request, done) {
           idGenerator,
           cookieOpts,
           secret,
-          session
+          session,
+          decryptedSessionId
         )
 
         restoredSession.destroy(err => {
@@ -88,7 +89,8 @@ function decryptSession (sessionId, options, request, done) {
           idGenerator,
           cookieOpts,
           secret,
-          session
+          session,
+          decryptedSessionId
         )
       } else {
         request.session = Session.restore(
@@ -96,7 +98,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..dd0debe 100644
--- a/lib/session.js
+++ b/lib/session.js
@@ -10,16 +10,17 @@ 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)
@@ -29,10 +30,8 @@ module.exports = class Session {
     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(this[sessionIdKey], secret)
     this[originalHash] = this[hash]()
   }
 
@@ -87,14 +86,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 +108,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 +130,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,10 +146,6 @@ module.exports = class Session {
     }
   }
 
-  [sign] () {
-    return cookieSignature.sign(this.sessionId, this[secretKey])
-  }
-
   [hash] () {
     const sess = this
     const str = stringify(sess, function (key, val) {
@@ -172,10 +167,18 @@ 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)
+  get sessionId () {
+    return this[sessionIdKey]
+  }
+
+  get encryptedSessionId () {
+    return this[encryptedSessionIdKey]
+  }
+
+  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)
+    restoredCookie.expires = new Date(prevSession.expires)
     restoredSession.cookie = restoredCookie
     restoredSession.expires = restoredCookie.expires
     restoredSession[originalHash] = restoredSession[hash]()
diff --git a/test/base.test.js b/test/base.test.js
index 2dfad4d..1fea32b 100644
--- a/test/base.test.js
+++ b/test/base.test.js
@@ -50,8 +50,10 @@ test('should support multiple secrets', async (t) => {
 
   const plugin = fastifyPlugin(async (fastify, opts) => {
     fastify.addHook('onRequest', (request, reply, done) => {
+      const expires = Date.now() - 1000
       request.sessionStore.set('aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e', {
-        expires: Date.now() + 1000
+        cookie: { expires },
+        expires
       }, done)
     })
   })
@@ -99,7 +101,6 @@ test('should set session cookie using the default cookie name', async (t) => {
     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: '/' }
       }, done)
     })
@@ -119,7 +120,7 @@ test('should set session cookie using the default cookie name', async (t) => {
   })
 
   t.is(statusCode, 200)
-  t.regex(cookie, /sessionId=undefined; Path=\/; HttpOnly; Secure/)
+  t.regex(cookie, /sessionId=.*\..*; Path=\/; HttpOnly; Secure/)
 })
 
 test('should create new session on expired session', async (t) => {
@@ -128,7 +129,6 @@ test('should create new session on expired session', async (t) => {
     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: '/' }
       }, done)
     })
@@ -186,8 +186,10 @@ test('should set new session cookie if expired', async (t) => {
 
   const plugin = fastifyPlugin(async (fastify, opts) => {
     fastify.addHook('onRequest', (request, reply, done) => {
+      const expires = Date.now() - 1000
       request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', {
-        expires: Date.now() + 1000
+        cookie: { expires },
+        expires
       }, done)
     })
   })
diff --git a/test/session.test.js b/test/session.test.js
index 5ea66f8..b9252b6 100644
--- a/test/session.test.js
+++ b/test/session.test.js
@@ -36,9 +36,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 port = await testServer((request, reply) => {
     t.truthy(request.session.encryptedSessionId)
+    // serialize, then deserialize to make sure it's gone
+    t.falsy(JSON.parse(JSON.stringify(request.session)).encryptedSessionId)
     reply.send(200)
   }, DEFAULT_OPTIONS)
 
@@ -224,7 +226,6 @@ test('should decryptSession with custom request object', async (t) => {
     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)
   })

From 86ac8fcc83941174907894dead72853356641e17 Mon Sep 17 00:00:00 2001
From: Simen Bekkhus <sbekkhus91@gmail.com>
Date: Wed, 29 Jun 2022 10:13:38 +0200
Subject: [PATCH 2/4] use local variable

---
 lib/session.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/session.js b/lib/session.js
index dd0debe..9ca81df 100644
--- a/lib/session.js
+++ b/lib/session.js
@@ -31,7 +31,7 @@ module.exports = class Session {
     this[requestKey] = request
     this.touch()
     this[sessionIdKey] = sessionId
-    this[encryptedSessionIdKey] = cookieSignature.sign(this[sessionIdKey], secret)
+    this[encryptedSessionIdKey] = cookieSignature.sign(sessionId, secret)
     this[originalHash] = this[hash]()
   }
 

From bd4706f61682175727d6614db75a4da90a54de42 Mon Sep 17 00:00:00 2001
From: Simen Bekkhus <sbekkhus91@gmail.com>
Date: Wed, 29 Jun 2022 10:48:41 +0200
Subject: [PATCH 3/4] do not add session id to session

---
 lib/session.js       | 2 +-
 test/base.test.js    | 2 ++
 test/session.test.js | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/session.js b/lib/session.js
index 9ca81df..8d3b10c 100644
--- a/lib/session.js
+++ b/lib/session.js
@@ -70,7 +70,7 @@ module.exports = class Session {
 
   [addDataToSession] (prevSession) {
     for (const key in prevSession) {
-      if (!['expires', 'cookie'].includes(key)) {
+      if (!['expires', 'cookie', 'sessionId', 'encryptedSessionId'].includes(key)) {
         this[key] = prevSession[key]
       }
     }
diff --git a/test/base.test.js b/test/base.test.js
index 1fea32b..c5219c8 100644
--- a/test/base.test.js
+++ b/test/base.test.js
@@ -101,6 +101,7 @@ test('should set session cookie using the default cookie name', async (t) => {
     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: '/' }
       }, done)
     })
@@ -129,6 +130,7 @@ test('should create new session on expired session', async (t) => {
     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: '/' }
       }, done)
     })
diff --git a/test/session.test.js b/test/session.test.js
index b9252b6..321c1eb 100644
--- a/test/session.test.js
+++ b/test/session.test.js
@@ -226,6 +226,7 @@ test('should decryptSession with custom request object', async (t) => {
     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)
   })

From 9e0819bee18a02a9965ef6966898193ace292dd0 Mon Sep 17 00:00:00 2001
From: Simen Bekkhus <sbekkhus91@gmail.com>
Date: Thu, 30 Jun 2022 08:54:53 +0200
Subject: [PATCH 4/4] remove top level expires

---
 README.md             |  2 +-
 lib/fastifySession.js |  2 +-
 lib/session.js        | 12 +++++-------
 test/base.test.js     | 20 +++++++-------------
 test/session.test.js  | 25 ++++---------------------
 test/store.test.js    |  2 +-
 types/types.d.ts      |  2 +-
 7 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/README.md b/README.md
index a42b0b8..07e09b8 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 023c7e0..308736a 100644
--- a/lib/fastifySession.js
+++ b/lib/fastifySession.js
@@ -63,7 +63,7 @@ function decryptSession (sessionId, options, request, done) {
         newSession(secret, request, cookieOpts, idGenerator, done)
         return
       }
-      if (session && session.expires && session.expires <= Date.now()) {
+      if (session && session.cookie && session.cookie.expires && session.cookie.expires <= Date.now()) {
         const restoredSession = Session.restore(
           request,
           idGenerator,
diff --git a/lib/session.js b/lib/session.js
index 8d3b10c..dd18f43 100644
--- a/lib/session.js
+++ b/lib/session.js
@@ -22,7 +22,6 @@ const encryptedSessionIdKey = Symbol('encryptedSessionId')
 module.exports = class Session {
   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
@@ -37,8 +36,7 @@ module.exports = class Session {
 
   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])
     }
   }
 
@@ -70,7 +68,7 @@ module.exports = class Session {
 
   [addDataToSession] (prevSession) {
     for (const key in prevSession) {
-      if (!['expires', 'cookie', 'sessionId', 'encryptedSessionId'].includes(key)) {
+      if (!['cookie', 'sessionId', 'encryptedSessionId'].includes(key)) {
         this[key] = prevSession[key]
       }
     }
@@ -151,7 +149,8 @@ module.exports = class Session {
     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
       }
 
       return val
@@ -178,9 +177,8 @@ module.exports = class Session {
   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.expires)
+    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 c5219c8..7a8b10b 100644
--- a/test/base.test.js
+++ b/test/base.test.js
@@ -50,10 +50,8 @@ test('should support multiple secrets', async (t) => {
 
   const plugin = fastifyPlugin(async (fastify, opts) => {
     fastify.addHook('onRequest', (request, reply, done) => {
-      const expires = Date.now() - 1000
       request.sessionStore.set('aYb4uTIhdBXCfk_ylik4QN6-u26K0u0e', {
-        cookie: { expires },
-        expires
+        cookie: { expires: Date.now() - 1000 }
       }, done)
     })
   })
@@ -100,9 +98,8 @@ test('should set session cookie using the default cookie name', 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,
         sessionId: 'Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN',
-        cookie: { secure: true, httpOnly: true, path: '/' }
+        cookie: { expires: Date.now() + 1000, secure: true, httpOnly: true, path: '/' }
       }, done)
     })
   })
@@ -129,9 +126,8 @@ 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('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', {
-        expires: Date.now() - 1000,
         sessionId: 'Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN',
-        cookie: { secure: true, httpOnly: true, path: '/' }
+        cookie: { expires: Date.now() - 1000, secure: true, httpOnly: true, path: '/' }
       }, done)
     })
   })
@@ -165,12 +161,12 @@ test('should set session.expires if maxAge', 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: Date.now() + 1000 }
       }, done)
     })
   })
   function handler (request, reply) {
-    t.truthy(request.session.expires)
+    t.truthy(request.session.cookie.expires)
     reply.send(200)
   }
   const port = await testServer(handler, options, plugin)
@@ -188,10 +184,8 @@ test('should set new session cookie if expired', async (t) => {
 
   const plugin = fastifyPlugin(async (fastify, opts) => {
     fastify.addHook('onRequest', (request, reply, done) => {
-      const expires = Date.now() - 1000
       request.sessionStore.set('Qk_XT2K7-clT-x1tVvoY6tIQ83iP72KN', {
-        cookie: { expires },
-        expires
+        cookie: { expires: Date.now() - 1000 }
       }, done)
     })
   })
@@ -262,7 +256,7 @@ 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
+        cookie: { expires: Date.now() + 1000 }
       }, done)
     })
   })
diff --git a/test/session.test.js b/test/session.test.js
index 321c1eb..90a9308 100644
--- a/test/session.test.js
+++ b/test/session.test.js
@@ -61,22 +61,6 @@ test('should add session.cookie object to request', async (t) => {
   t.is(statusCode, 200)
 })
 
-test('should add session.expires object to request', async (t) => {
-  t.plan(2)
-  const options = {
-    secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk',
-    cookie: { maxAge: 42 }
-  }
-  const port = await testServer((request, reply) => {
-    t.truthy(request.session.expires)
-    reply.send(200)
-  }, options)
-
-  const { statusCode } = await request(`http://localhost:${port}`)
-
-  t.is(statusCode, 200)
-})
-
 test('should add session.sessionId object to request', async (t) => {
   t.plan(2)
   const port = await testServer((request, reply) => {
@@ -225,9 +209,8 @@ 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: '/' }
+      cookie: { expires: Date.now() + 1000, secure: true, httpOnly: true, path: '/' }
     }, done)
   })
 
@@ -286,7 +269,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: Date.now() - 1000 } })
     },
     destroy (id, cb) { cb(new Error('No can do')) }
   }
@@ -327,7 +310,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()
   })
 
@@ -364,7 +347,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 cc73e24..30969d4 100644
--- a/test/store.test.js
+++ b/test/store.test.js
@@ -87,7 +87,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: Date.now() - 1000}
       }, done)
     })
   })
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;
 
   /**