Skip to content

Commit f5b5ebb

Browse files
feat: enforce secret requirement for session creation (#1025)
* feat: enforce secret requirement for session creation * feat: remove compatibility with cookie parser * test: move test to describe * fix: improve secret option validation in session function * Update test/session.js --------- Co-authored-by: Ulises Gascón <ulisesgascongonzalez@gmail.com>
1 parent 76bc233 commit f5b5ebb

File tree

2 files changed

+12
-32
lines changed

2 files changed

+12
-32
lines changed

index.js

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ function session(options) {
101101
var saveUninitializedSession = opts.saveUninitialized
102102

103103
// get the cookie signing secret
104-
var secret = opts.secret
104+
var secrets = opts.secret
105105

106106
if (typeof generateId !== 'function') {
107107
throw new TypeError('genid option must be a function');
@@ -124,16 +124,16 @@ function session(options) {
124124
// TODO: switch to "destroy" on next major
125125
var unsetDestroy = opts.unset === 'destroy'
126126

127-
if (Array.isArray(secret) && secret.length === 0) {
127+
if (Array.isArray(secrets) && secrets.length === 0) {
128128
throw new TypeError('secret option array must contain one or more strings');
129129
}
130130

131-
if (secret && !Array.isArray(secret)) {
132-
secret = [secret];
131+
if (secrets && !Array.isArray(secrets)) {
132+
secrets = [secrets];
133133
}
134134

135-
if (!secret) {
136-
deprecate('req.secret; provide secret option');
135+
if (!secrets) {
136+
throw new Error('secret option required for sessions');
137137
}
138138

139139
// notify user that this store is not
@@ -188,16 +188,6 @@ function session(options) {
188188
return
189189
}
190190

191-
// ensure a secret is available or bail
192-
if (!secret && !req.secret) {
193-
next(new Error('secret option required for sessions'));
194-
return;
195-
}
196-
197-
// backwards compatibility for signed cookies
198-
// req.secret is passed from the cookie parser middleware
199-
var secrets = secret || [req.secret];
200-
201191
var originalHash;
202192
var originalId;
203193
var savedHash;

test/session.js

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,6 @@ describe('session()', function(){
3535
.expect(200, done)
3636
})
3737

38-
it('should error without secret', function(done){
39-
request(createServer({ secret: undefined }))
40-
.get('/')
41-
.expect(500, /secret.*required/, done)
42-
})
43-
44-
it('should get secret from req.secret', function(done){
45-
function setup (req) {
46-
req.secret = 'keyboard cat'
47-
}
48-
49-
request(createServer(setup, { secret: undefined }))
50-
.get('/')
51-
.expect(200, '', done)
52-
})
53-
5438
it('should create a new session', function (done) {
5539
var store = new session.MemoryStore()
5640
var server = createServer({ store: store }, function (req, res) {
@@ -1194,6 +1178,12 @@ describe('session()', function(){
11941178
});
11951179

11961180
describe('secret option', function () {
1181+
it('should reject without secret',function () {
1182+
for (const secret of [undefined, null, '', false]) {
1183+
assert.throws(session.bind(null, { secret }), /secret option required for sessions/)
1184+
}
1185+
})
1186+
11971187
it('should reject empty arrays', function () {
11981188
assert.throws(createServer.bind(null, { secret: [] }), /secret option array/);
11991189
})

0 commit comments

Comments
 (0)