From 297e4e35a8c0b11654c661f762421e2bb99cfa16 Mon Sep 17 00:00:00 2001 From: Eladio Mora Date: Sat, 23 May 2020 21:34:17 -0300 Subject: [PATCH 1/2] fix: missing cookie expires= when default maxAge is used --- lib/context.js | 1 + test/cookie.test.js | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/context.js b/lib/context.js index 1bb41a8..589b623 100644 --- a/lib/context.js +++ b/lib/context.js @@ -309,6 +309,7 @@ class ContextSession { // set expire for check json._expire = maxAge + Date.now(); json._maxAge = maxAge; + if (!opts.maxAge) opts.maxAge = maxAge; } // save to external store diff --git a/test/cookie.test.js b/test/cookie.test.js index 568d2b1..eea3e41 100644 --- a/test/cookie.test.js +++ b/test/cookie.test.js @@ -713,8 +713,12 @@ describe('Koa Session Cookie', () => { request(app.listen()) .get('/') .expect(res => { - const cookie = res.headers['set-cookie'].join('|'); - assert(cookie.includes('path=/; samesite=none; httponly')); + const cookies = res.headers['set-cookie']; + cookies.length.should.equal(2); + for (const cookie of cookies) { + assert(cookie.includes('samesite=none; httponly')); + assert(cookie.includes('path=/;')); + } }) .expect('bar') .expect(200, done); @@ -731,8 +735,12 @@ describe('Koa Session Cookie', () => { request(app.listen()) .get('/') .expect(res => { - const cookie = res.headers['set-cookie'].join('|'); - assert(cookie.includes('path=/; samesite=lax; httponly')); + const cookies = res.headers['set-cookie']; + cookies.length.should.equal(2); + for (const cookie of cookies) { + assert(cookie.includes('samesite=lax; httponly')); + assert(cookie.includes('path=/;')); + } }) .expect('bar') .expect(200, done); From 2f6926f9999534873f2c0b608a67f610ceb3d115 Mon Sep 17 00:00:00 2001 From: Eladio Mora Date: Sat, 23 May 2020 21:52:59 -0300 Subject: [PATCH 2/2] refactor: separate cookie maxAge from external store ttl in 2 options --- lib/context.js | 27 +++++++++--- test/store.test.js | 101 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 6 deletions(-) diff --git a/lib/context.js b/lib/context.js index 589b623..8e3fb82 100644 --- a/lib/context.js +++ b/lib/context.js @@ -299,7 +299,19 @@ class ContextSession { const externalKey = this.externalKey; let json = this.session.toJSON(); // set expire for check - let maxAge = opts.maxAge ? opts.maxAge : ONE_DAY; + // maxAge refers to cookie value, while ttl refers to store value + const maxAge = opts.maxAge ? opts.maxAge : ONE_DAY; + // If ttl was not provided, take value from maxAge + 10s to ensure + // store expires after cookie, unless maxAge is "session" + // then use default ONE_DAY + let ttl = opts.ttl; + if (!ttl) { + if (typeof maxAge === 'number') { + ttl = maxAge + 10000; + } else { + ttl = ONE_DAY; + } + } if (maxAge === 'session') { // do not set _expire in json if maxAge is set to 'session' // also delete maxAge from options @@ -311,15 +323,18 @@ class ContextSession { json._maxAge = maxAge; if (!opts.maxAge) opts.maxAge = maxAge; } + if (ttl === 'permanent') { + // Unset ttl if permanent so external stores can conditionally + // store with ttl or not + ttl = undefined; + } else { + if (!opts.ttl) opts.ttl = ttl; + } // save to external store if (externalKey) { debug('save %j to external key %s', json, externalKey); - if (typeof maxAge === 'number') { - // ensure store expired after cookie - maxAge += 10000; - } - await this.store.set(externalKey, json, maxAge, { + await this.store.set(externalKey, json, ttl, { changed, rolling: opts.rolling, }); diff --git a/test/store.test.js b/test/store.test.js index d1bbf93..70b1611 100644 --- a/test/store.test.js +++ b/test/store.test.js @@ -805,6 +805,107 @@ describe('Koa Session External Store', () => { .expect({ foo: 'bar' }); }); }); + + describe('when ussing ttl', () => { + let app; + let currentStoreOpts; + let currentSessionOpts; + const ONE_DAY = 86400000; + + const testMW = async ctx => { + ctx.session = { foo: 'bar' }; + ctx.status = 204; + currentStoreOpts = ctx.session._sessCtx.externalKey; + currentSessionOpts = ctx.session._sessCtx.opts; + }; + + it('should use given ttl without affecting maxAge', async () => { + const originalTTL = 43200000; + app = App({ ttl: originalTTL }); + app.use(testMW); + const res = await request(app.callback()) + .get('/') + .expect({}); + + res.headers['set-cookie'].should.have.length(2); + const cookie = res.headers['set-cookie'].join(';'); + cookie.should.containEql('expires='); + const storeKey = await store.get(currentStoreOpts); + storeKey._maxAge.should.equal(ONE_DAY); + currentSessionOpts.ttl.should.equal(originalTTL); + currentSessionOpts.maxAge.should.equal(ONE_DAY); + }); + + it('should use maxAge value if ttl is not provided', async () => { + const maxAge = 10000; + app = App({ maxAge }); + app.use(testMW); + const res = await request(app.callback()) + .get('/') + .expect({}); + + res.headers['set-cookie'].should.have.length(2); + const cookie = res.headers['set-cookie'].join(';'); + cookie.should.containEql('expires='); + const storeKey = await store.get(currentStoreOpts); + storeKey._maxAge.should.equal(maxAge); + currentSessionOpts.ttl.should.equal(maxAge + 10000); + currentSessionOpts.maxAge.should.equal(maxAge); + }); + + it('should use default ONE_DAY if ttl is not provided and maxAge is session', async () => { + const maxAge = 'session'; + app = App({ maxAge }); + app.use(testMW); + const res = await request(app.callback()) + .get('/') + .expect({}); + + res.headers['set-cookie'].should.have.length(2); + const cookie = res.headers['set-cookie'].join(';'); + cookie.should.not.containEql('expires='); + const storeKey = await store.get(currentStoreOpts); + should.not.exist(storeKey._maxAge); + currentSessionOpts.ttl.should.equal(ONE_DAY); + should.not.exists(currentSessionOpts.maxAge); + }); + + it('should leave store ttl as undefined if ttl is permanent', async () => { + app = App({ ttl: 'permanent' }); + app.use(testMW); + const res = await request(app.callback()) + .get('/') + .expect({}); + + res.headers['set-cookie'].should.have.length(2); + const cookie = res.headers['set-cookie'].join(';'); + cookie.should.containEql('expires='); + const storeKey = await store.get(currentStoreOpts); + storeKey._maxAge.should.equal(ONE_DAY); + currentSessionOpts.ttl.should.equal('permanent'); + currentSessionOpts.maxAge.should.equal(ONE_DAY); + }); + + it('should allow changing ttl on request basis', async () => { + app = App({ ttl: 'permanent' }); + app.use((ctx, next) => { + ctx.sessionOptions.ttl = 100000; + next(); + }); + app.use(testMW); + const res = await request(app.callback()) + .get('/') + .expect({}); + + res.headers['set-cookie'].should.have.length(2); + const cookie = res.headers['set-cookie'].join(';'); + cookie.should.containEql('expires='); + const storeKey = await store.get(currentStoreOpts); + storeKey._maxAge.should.equal(ONE_DAY); + currentSessionOpts.ttl.should.equal(100000); + currentSessionOpts.maxAge.should.equal(ONE_DAY); + }); + }); }); function App(options) {