Skip to content

Commit

Permalink
solution for issue expressjs#557
Browse files Browse the repository at this point in the history
  • Loading branch information
@justinwatkinsact
justinwatkinsact committed Jun 18, 2018
1 parent 104725e commit 0d77c83
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 9 deletions.
16 changes: 16 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,22 @@ app.use(session({
}))
```

##### maxDuration

The maximum amount of time that a session can stay open *even* if there are continuous
requests that keep the session alive. This is to minimize the total footprint of a session
replay attack in the case where a session identifier is stolen. This will treat the session as
expired and generate a new session. Rolling sessions do not update this behavior.

This comment has been minimized.

Sign in to view

Copy link
@jfstephe

jfstephe May 9, 2019

Seconds rather than milliseconds would be more inconsistent with the MaxAge option.
PS Thanks for taking the time to do the PR! 👍

The default is none.

```js
app.use(session({
maxDuration: 28800, // duration in seconds (this would be 8 hours)
secret: 'keyboard cat'
}))
```

##### name

The name of the session ID cookie to set in the response (and read from in the
Expand Down
48 changes: 39 additions & 9 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ exports.MemoryStore = MemoryStore;
*/

var warning = 'Warning: connect.session() MemoryStore is not\n'
+ 'designed for a production environment, as it will leak\n'
+ 'designed for a production environment, as it will leak\n'
+ 'memory, and will not scale past a single process.';

/**
Expand All @@ -63,7 +63,7 @@ var warning = 'Warning: connect.session() MemoryStore is not\n'

/* istanbul ignore next */
var defer = typeof setImmediate === 'function'
? setImmediate
? setImmediate
: function(fn){ process.nextTick(fn.bind.apply(fn, arguments)) }

/**
Expand All @@ -80,6 +80,7 @@ var defer = typeof setImmediate === 'function'
* @param {String|Array} [options.secret] Secret for signing session ID
* @param {Object} [options.store=MemoryStore] Session store
* @param {String} [options.unset]
* @param {Number} [options.maxDuration] Sets the maximum total age in seconds for a session to minimize session replay duration
* @return {Function} middleware
* @public
*/
Expand Down Expand Up @@ -143,6 +144,10 @@ function session(options) {
secret = [secret];
}

if (opts.maxDuration && typeof opts.maxDuration !== 'number') {
throw new TypeError('maxDuration needs to be specified as a number');
}

if (!secret) {
deprecate('req.secret; provide secret option');
}
Expand All @@ -163,6 +168,10 @@ function session(options) {
if (cookieOptions.secure === 'auto') {
req.session.cookie.secure = issecure(req, trustProxy);
}

if (opts.maxDuration > 0) {
req.session.cookie.createdAt = new Date();
}
};

var storeImplementsTouch = typeof store.touch === 'function';
Expand Down Expand Up @@ -281,9 +290,9 @@ function session(options) {

if (!isNaN(contentLength) && contentLength > 0) {
// measure chunk
chunk = !Buffer.isBuffer(chunk)
chunk = !Buffer.isBuffer(chunk)
? Buffer.from(chunk, encoding)
: chunk;
: chunk;
encoding = undefined;

if (chunk.length !== 0) {
Expand Down Expand Up @@ -421,7 +430,7 @@ function session(options) {
}

return !saveUninitializedSession && cookieId !== req.sessionID
? isModified(req.session)
? isModified(req.session)
: !isSaved(req.session)
}

Expand All @@ -444,10 +453,28 @@ function session(options) {
}

return cookieId != req.sessionID
? saveUninitializedSession || isModified(req.session)
? saveUninitializedSession || isModified(req.session)
: rollingSessions || req.session.cookie.expires != null && isModified(req.session);
}

// if opts.maxDuration is set, check to see if the createdAt value of the cookie is has
// expired.
function hasReachedMaxDuration (sess, opts) {
if (!opts || !opts.maxDuration || opts.maxDuration <= 0) {
return false;
}
if (!sess || !sess.cookie || !sess.cookie.createdAt) {
debug('session should be timed out, but the createdAt value is not saved')
return true;
}
var createdDate = new Date(sess.cookie.createdAt);
var nowDate = new Date();
if ((nowDate.getTime() - createdDate.getTime()) / 1000 < opts.maxDuration) {
return false;
}
return true;
}

// generate a session if the browser doesn't send a sessionID
if (!req.sessionID) {
debug('no SID sent, generating session');
Expand All @@ -469,11 +496,14 @@ function session(options) {
}

generate();
// no session
// no session
} else if (!sess) {
debug('no session found');
generate();
// populate req.session
// populate req.session
} else if (hasReachedMaxDuration(sess, opts)) {
debug('session has reached the max duration');
generate();
} else {
debug('session found');
store.createSession(req, sess);
Expand Down Expand Up @@ -617,7 +647,7 @@ function issecure(req, trustProxy) {
var header = req.headers['x-forwarded-proto'] || '';
var index = header.indexOf(',');
var proto = index !== -1
? header.substr(0, index).toLowerCase().trim()
? header.substr(0, index).toLowerCase().trim()
: header.toLowerCase().trim()

return proto === 'https';
Expand Down
1 change: 1 addition & 0 deletions session/cookie.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ Cookie.prototype = {
, domain: this.domain
, path: this.path
, sameSite: this.sameSite
, createdAt: this.createdAt
}
},

Expand Down
83 changes: 83 additions & 0 deletions test/session.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,89 @@ describe('session()', function(){
});
});

it('should keep session in maxDuration', function(done) {
var store = new session.MemoryStore();
var server = createServer({store: store, maxDuration: 10}, function(req, res) {
req.session.user = 'bob';
res.write('hello, world');
res.end();
});

request(server)
.get('/')
.expect(shouldSetCookie('connect.sid'))
.expect(200, function(err, res) {
if (err) return done(err);
var originalExpires = expires(res);
setTimeout(function() {
request(server)
.get('/')
.set('Cookie', cookie(res))
.expect(shouldNotHaveHeader('Set-Cookie'))
.expect(200, done);
}, 100);
});
});

it('should destroy session after maxDuration', function(done) {
var store = new session.MemoryStore();
var server = createServer({ store: store, maxDuration: .1 }, function(req, res) {
req.session.user = 'bob';
res.write('hello, world');
res.end();
});

request(server)
.get('/')
.expect(shouldSetCookie('connect.sid'))
.expect(200, function (err, res) {
if (err) return done(err);
var originalCookie = cookie(res);
setTimeout(function () {
request(server)
.get('/')
.set('Cookie', cookie(res))
.expect(shouldSetCookie('connect.sid'))
.expect(function (res) { assert.notEqual(originalCookie, cookie(res)); })
.expect(200, done);
}, 200);
});
});

it('should destroy session after maxDuration even with rolling sessions', function (done) {
var store = new session.MemoryStore();
var server = createServer({ store: store, maxDuration: .1, rolling: true }, function(req, res) {
req.session.user = 'bob';
res.write('hello, world');
res.end();
});

request(server)
.get('/')
.expect(shouldSetCookie('connect.sid'))
.expect(200, function (err, res) {
if (err) return done(err);
var originalCookie = cookie(res);
setTimeout(function () {
request(server)
.get('/')
.set('Cookie', cookie(res))
.expect(shouldSetCookie('connect.sid'))
.expect(200, function (err, res) {
if (err) return done(err);
setTimeout(function () {
request(server)
.get('/')
.set('Cookie', cookie(res))
.expect(shouldSetCookie('connect.sid'))
.expect(function (res) { assert.notEqual(originalCookie, cookie(res)); })
.expect(200, done);
}, 100);
});
}, 100);
});
});

describe('when response ended', function () {
it('should have saved session', function (done) {
var saved = false
Expand Down

0 comments on commit 0d77c83

Please sign in to comment.