Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enforce secret requirement for session creation #1025

Open
wants to merge 8 commits into
base: v2
Choose a base branch
from
22 changes: 6 additions & 16 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,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');
Expand All @@ -124,16 +124,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) {
deprecate('req.secret; provide secret option');
if (!secrets) {
throw new Error('secret option required for sessions');
}

// notify user that this store is not
Expand Down Expand Up @@ -188,16 +188,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 || [req.secret];

var originalHash;
var originalId;
var savedHash;
Expand Down
22 changes: 6 additions & 16 deletions test/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,6 @@ 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 create a new session', function (done) {
var store = new session.MemoryStore()
var server = createServer({ store: store }, function (req, res) {
Expand Down Expand Up @@ -1194,6 +1178,12 @@ describe('session()', function(){
});

describe('secret option', function () {
it('should reject without secret',function () {
for (const secret of [undefined, null, '', false]) {
assert.throws(session.bind(null, { secret }), /secret option required for sessions/)
}
})

it('should reject empty arrays', function () {
assert.throws(createServer.bind(null, { secret: [] }), /secret option array/);
})
Expand Down