From 4537773c6b1ccc1c54ff4879bc883f1d7cf8a4a5 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Wed, 12 Feb 2025 22:16:22 -0500 Subject: [PATCH 1/5] feat: enforce secret requirement for session creation --- index.js | 6 +++--- test/session.js | 16 ++-------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/index.js b/index.js index d41b2378..da2003a5 100644 --- a/index.js +++ b/index.js @@ -143,8 +143,8 @@ function session(options) { secret = [secret]; } - if (!secret) { - deprecate('req.secret; provide secret option'); + if (secret === undefined) { + throw new Error('secret is required for sessions'); } // notify user that this store is not @@ -207,7 +207,7 @@ function session(options) { // backwards compatibility for signed cookies // req.secret is passed from the cookie parser middleware - var secrets = secret || [req.secret]; + var secrets = secret; var originalHash; var originalId; diff --git a/test/session.js b/test/session.js index 7bf3e51f..af96eb89 100644 --- a/test/session.js +++ b/test/session.js @@ -35,20 +35,8 @@ describe('session()', function(){ .expect(200, done) }) - it('should error without secret', function(done){ - request(createServer({ secret: undefined })) - .get('/') - .expect(500, /secret.*required/, done) - }) - - it('should get secret from req.secret', function(done){ - function setup (req) { - req.secret = 'keyboard cat' - } - - request(createServer(setup, { secret: undefined })) - .get('/') - .expect(200, '', done) + it('should reject without secret', function(){ + assert.throws(session.bind(null, { secret: undefined }), /secret.*required/) }) it('should create a new session', function (done) { From e0bbb61169a794227694810ae556225c2e5b03b4 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Wed, 12 Feb 2025 23:34:19 -0500 Subject: [PATCH 2/5] feat: remove compatibility with cookie parser --- index.js | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/index.js b/index.js index da2003a5..64f462fb 100644 --- a/index.js +++ b/index.js @@ -112,7 +112,7 @@ function session(options) { var saveUninitializedSession = opts.saveUninitialized // get the cookie signing secret - var secret = opts.secret + var secrets = opts.secret if (typeof generateId !== 'function') { throw new TypeError('genid option must be a function'); @@ -135,16 +135,16 @@ function session(options) { // TODO: switch to "destroy" on next major var unsetDestroy = opts.unset === 'destroy' - if (Array.isArray(secret) && secret.length === 0) { + if (Array.isArray(secrets) && secrets.length === 0) { throw new TypeError('secret option array must contain one or more strings'); } - if (secret && !Array.isArray(secret)) { - secret = [secret]; + if (secrets && !Array.isArray(secrets)) { + secrets = [secrets]; } - if (secret === undefined) { - throw new Error('secret is required for sessions'); + if (secrets === undefined) { + throw new Error('secret option required for sessions'); } // notify user that this store is not @@ -199,16 +199,6 @@ function session(options) { return } - // ensure a secret is available or bail - if (!secret && !req.secret) { - next(new Error('secret option required for sessions')); - return; - } - - // backwards compatibility for signed cookies - // req.secret is passed from the cookie parser middleware - var secrets = secret; - var originalHash; var originalId; var savedHash; From a3b31f6307a68fb7c6337a7582bba416ede1cf67 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Sat, 15 Mar 2025 20:55:19 -0500 Subject: [PATCH 3/5] test: move test to describe --- test/session.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/session.js b/test/session.js index af96eb89..532672b7 100644 --- a/test/session.js +++ b/test/session.js @@ -35,10 +35,6 @@ describe('session()', function(){ .expect(200, done) }) - it('should reject without secret', function(){ - assert.throws(session.bind(null, { secret: undefined }), /secret.*required/) - }) - it('should create a new session', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store }, function (req, res) { @@ -1182,6 +1178,10 @@ describe('session()', function(){ }); describe('secret option', function () { + it('should reject without secret', function(){ + assert.throws(session.bind(null, { secret: undefined }), /secret.*required/) + }) + it('should reject empty arrays', function () { assert.throws(createServer.bind(null, { secret: [] }), /secret option array/); }) From 38e40862ab717f817ae31c8ae402ce18bd40f984 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Sat, 15 Mar 2025 21:26:28 -0500 Subject: [PATCH 4/5] fix: improve secret option validation in session function --- index.js | 2 +- test/session.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 64f462fb..835202a7 100644 --- a/index.js +++ b/index.js @@ -143,7 +143,7 @@ function session(options) { secrets = [secrets]; } - if (secrets === undefined) { + if (!secrets) { throw new Error('secret option required for sessions'); } diff --git a/test/session.js b/test/session.js index 532672b7..f8d1881f 100644 --- a/test/session.js +++ b/test/session.js @@ -1178,8 +1178,11 @@ describe('session()', function(){ }); describe('secret option', function () { - it('should reject without secret', function(){ - assert.throws(session.bind(null, { secret: undefined }), /secret.*required/) + it('should reject without secret',function () { + for (const secret of [undefined, null, '', false]) { + console.log(secret) + assert.throws(session.bind(null, { secret }), /secret option required for sessions/) + } }) it('should reject empty arrays', function () { From 7f17ead13008561c915cc5e89d480b441aef21f5 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Tue, 18 Mar 2025 12:02:43 -0500 Subject: [PATCH 5/5] Update test/session.js --- test/session.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/session.js b/test/session.js index f8d1881f..eff7e596 100644 --- a/test/session.js +++ b/test/session.js @@ -1180,7 +1180,6 @@ describe('session()', function(){ describe('secret option', function () { it('should reject without secret',function () { for (const secret of [undefined, null, '', false]) { - console.log(secret) assert.throws(session.bind(null, { secret }), /secret option required for sessions/) } })