From b61cc3438165a15c6d34c2ec18275e08390ea26e Mon Sep 17 00:00:00 2001 From: Eduardo Diaz Date: Sat, 13 May 2017 17:28:11 +0200 Subject: [PATCH 1/2] maxAge: Fix logic with number + use seconds instead of ms --- README.md | 4 +- test/verify.tests.js | 137 +++++++++++++++++++++++-------------------- verify.js | 12 ++-- 3 files changed, 82 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 608660f..0a58f8e 100644 --- a/README.md +++ b/README.md @@ -119,8 +119,8 @@ encoded public key for RSA and ECDSA. * `ignoreNotBefore`... * `subject`: if you want to check subject (`sub`), provide a value here * `clockTolerance`: number of seconds to tolerate when checking the `nbf` and `exp` claims, to deal with small clock differences among different servers -* `maxAge`: the maximum allowed age for tokens to still be valid. Currently it is expressed in milliseconds or a string describing a time span [zeit/ms](https://github.com/zeit/ms). Eg: `1000`, `"2 days"`, `"10h"`, `"7d"`. **We advise against using milliseconds precision, though, since JWTs can only contain seconds. The maximum precision might be reduced to seconds in the future.** -* `clockTimestamp`: the time in seconds that should be used as the current time for all necessary comparisons (also against `maxAge`, so our advise is to avoid using `clockTimestamp` and a `maxAge` in milliseconds together) +* `maxAge`: the maximum allowed age for tokens to still be valid. It is expressed in seconds or a string describing a time span [zeit/ms](https://github.com/zeit/ms). Eg: `1000`, `"2 days"`, `"10h"`, `"7d"`. +* `clockTimestamp`: the time in seconds that should be used as the current time for all necessary comparisons. ```js diff --git a/test/verify.tests.js b/test/verify.tests.js index 05a1aed..aab1d24 100644 --- a/test/verify.tests.js +++ b/test/verify.tests.js @@ -68,8 +68,8 @@ describe('verify', function() { }); describe('expiration', function () { - // { foo: 'bar', iat: 1437018582, exp: 1437018583 } - var token = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJmb28iOiJiYXIiLCJpYXQiOjE0MzcwMTg1ODIsImV4cCI6MTQzNzAxODU4M30.NmMv7sXjM1dW0eALNXud8LoXknZ0mH14GtnFclwJv0s'; + // { foo: 'bar', iat: 1437018582, exp: 1437018592 } + var token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJpYXQiOjE0MzcwMTg1ODIsImV4cCI6MTQzNzAxODU5Mn0.3aR3vocmgRpG05rsI9MpR6z2T_BGtMQaPq2YR6QaroU'; var key = 'key'; var clock; @@ -78,22 +78,22 @@ describe('verify', function() { }); it('should error on expired token', function (done) { - clock = sinon.useFakeTimers(1437018650000); + clock = sinon.useFakeTimers(1437018650000); // iat + 58s, exp + 48s var options = {algorithms: ['HS256']}; jwt.verify(token, key, options, function (err, p) { assert.equal(err.name, 'TokenExpiredError'); assert.equal(err.message, 'jwt expired'); assert.equal(err.expiredAt.constructor.name, 'Date'); - assert.equal(Number(err.expiredAt), 1437018583000); + assert.equal(Number(err.expiredAt), 1437018592000); assert.isUndefined(p); done(); }); }); it('should not error on expired token within clockTolerance interval', function (done) { - clock = sinon.useFakeTimers(1437018584000); - var options = {algorithms: ['HS256'], clockTolerance: 100} + clock = sinon.useFakeTimers(1437018594000); // iat + 12s, exp + 2s + var options = {algorithms: ['HS256'], clockTolerance: 5 } jwt.verify(token, key, options, function (err, p) { assert.isNull(err); @@ -103,8 +103,8 @@ describe('verify', function() { }); it('should not error if within maxAge timespan', function (done) { - clock = sinon.useFakeTimers(1437018582500); - var options = {algorithms: ['HS256'], maxAge: '600ms'}; + clock = sinon.useFakeTimers(1437018587500); // iat + 5.5s, exp - 4.5s + var options = {algorithms: ['HS256'], maxAge: '6s'}; jwt.verify(token, key, options, function (err, p) { assert.isNull(err); @@ -114,70 +114,83 @@ describe('verify', function() { }); describe('option: maxAge', function () { - it('should error for claims issued before a certain timespan', function (done) { - clock = sinon.useFakeTimers(1437018582500); - var options = {algorithms: ['HS256'], maxAge: '321ms'}; - jwt.verify(token, key, options, function (err, p) { - assert.equal(err.name, 'TokenExpiredError'); - assert.equal(err.message, 'maxAge exceeded'); - assert.equal(err.expiredAt.constructor.name, 'Date'); - assert.equal(Number(err.expiredAt), 1437018582321); - assert.isUndefined(p); - done(); + ['3s', 3].forEach(function(maxAge) { + it(`should error for claims issued before a certain timespan (${typeof maxAge} type)`, function (done) { + clock = sinon.useFakeTimers(1437018587000); // iat + 5s, exp - 5s + var options = {algorithms: ['HS256'], maxAge: maxAge}; + + jwt.verify(token, key, options, function (err, p) { + assert.equal(err.name, 'TokenExpiredError'); + assert.equal(err.message, 'maxAge exceeded'); + assert.equal(err.expiredAt.constructor.name, 'Date'); + assert.equal(Number(err.expiredAt), 1437018585000); + assert.isUndefined(p); + done(); + }); }); }); - it('should not error for claims issued before a certain timespan but still inside clockTolerance timespan', function (done) { - clock = sinon.useFakeTimers(1437018582500); - var options = {algorithms: ['HS256'], maxAge: '321ms', clockTolerance: 100}; + ['5s', 5].forEach(function (maxAge) { + it(`should not error for claims issued before a certain timespan but still inside clockTolerance timespan (${typeof maxAge} type)`, function (done) { + clock = sinon.useFakeTimers(1437018587500); // iat + 5.5s, exp - 4.5s + var options = {algorithms: ['HS256'], maxAge: maxAge, clockTolerance: 1 }; - jwt.verify(token, key, options, function (err, p) { - assert.isNull(err); - assert.equal(p.foo, 'bar'); - done(); + jwt.verify(token, key, options, function (err, p) { + assert.isNull(err); + assert.equal(p.foo, 'bar'); + done(); + }); }); }); - it('should not error if within maxAge timespan', function (done) { - clock = sinon.useFakeTimers(1437018582500); - var options = {algorithms: ['HS256'], maxAge: '600ms'}; + ['6s', 6].forEach(function (maxAge) { + it(`should not error if within maxAge timespan (${typeof maxAge} type)`, function (done) { + clock = sinon.useFakeTimers(1437018587500);// iat + 5.5s, exp - 4.5s + var options = {algorithms: ['HS256'], maxAge: maxAge}; - jwt.verify(token, key, options, function (err, p) { - assert.isNull(err); - assert.equal(p.foo, 'bar'); - done(); + jwt.verify(token, key, options, function (err, p) { + assert.isNull(err); + assert.equal(p.foo, 'bar'); + done(); + }); }); }); - it('can be more restrictive than expiration', function (done) { - clock = sinon.useFakeTimers(1437018582900); - var options = {algorithms: ['HS256'], maxAge: '800ms'}; - jwt.verify(token, key, options, function (err, p) { - assert.equal(err.name, 'TokenExpiredError'); - assert.equal(err.message, 'maxAge exceeded'); - assert.equal(err.expiredAt.constructor.name, 'Date'); - assert.equal(Number(err.expiredAt), 1437018582800); - assert.isUndefined(p); - done(); + ['8s', 8].forEach(function (maxAge) { + it(`can be more restrictive than expiration (${typeof maxAge} type)`, function (done) { + clock = sinon.useFakeTimers(1437018591900); // iat + 9.9s, exp - 0.1s + var options = {algorithms: ['HS256'], maxAge: maxAge }; + + jwt.verify(token, key, options, function (err, p) { + assert.equal(err.name, 'TokenExpiredError'); + assert.equal(err.message, 'maxAge exceeded'); + assert.equal(err.expiredAt.constructor.name, 'Date'); + assert.equal(Number(err.expiredAt), 1437018590000); + assert.isUndefined(p); + done(); + }); }); }); - it('cannot be more permissive than expiration', function (done) { - clock = sinon.useFakeTimers(1437018583100); - var options = {algorithms: ['HS256'], maxAge: '1200ms'}; - jwt.verify(token, key, options, function (err, p) { - // maxAge not exceded, but still expired - assert.equal(err.name, 'TokenExpiredError'); - assert.equal(err.message, 'jwt expired'); - assert.equal(err.expiredAt.constructor.name, 'Date'); - assert.equal(Number(err.expiredAt), 1437018583000); - assert.isUndefined(p); - done(); + ['12s', 12].forEach(function (maxAge) { + it(`cannot be more permissive than expiration (${typeof maxAge} type)`, function (done) { + clock = sinon.useFakeTimers(1437018593000); // iat + 11s, exp + 1s + var options = {algorithms: ['HS256'], maxAge: '12s'}; + + jwt.verify(token, key, options, function (err, p) { + // maxAge not exceded, but still expired + assert.equal(err.name, 'TokenExpiredError'); + assert.equal(err.message, 'jwt expired'); + assert.equal(err.expiredAt.constructor.name, 'Date'); + assert.equal(Number(err.expiredAt), 1437018592000); + assert.isUndefined(p); + done(); + }); }); }); + it('should error if maxAge is specified but there is no iat claim', function (done) { - clock = sinon.useFakeTimers(1437018582900); var token = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJmb28iOiJiYXIifQ.0MBPd4Bru9-fK_HY3xmuDAc6N_embknmNuhdb9bKL_U'; var options = {algorithms: ['HS256'], maxAge: '1s'}; @@ -249,7 +262,7 @@ describe('verify', function() { }); describe('option: maxAge and clockTimestamp', function () { - // { foo: 'bar', iat: 1437018582, exp: 1437018800 } + // { foo: 'bar', iat: 1437018582, exp: 1437018800 } exp = iat + 218s var token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJpYXQiOjE0MzcwMTg1ODIsImV4cCI6MTQzNzAxODgwMH0.AVOsNC7TiT-XVSpCpkwB1240izzCIJ33Lp07gjnXVpA'; it('should error for claims issued before a certain timespan', function (done) { var clockTimestamp = 1437018682; @@ -265,12 +278,12 @@ describe('verify', function() { }); }); it('should not error for claims issued before a certain timespan but still inside clockTolerance timespan', function (done) { - var clockTimestamp = 1437018582; + var clockTimestamp = 1437018592; // iat + 10s var options = { algorithms: ['HS256'], clockTimestamp: clockTimestamp, - maxAge: '321ms', - clockTolerance: 100 + maxAge: '3s', + clockTolerance: 10 }; jwt.verify(token, key, options, function (err, p) { @@ -280,8 +293,8 @@ describe('verify', function() { }); }); it('should not error if within maxAge timespan', function (done) { - var clockTimestamp = 1437018582; - var options = {algorithms: ['HS256'], clockTimestamp: clockTimestamp, maxAge: '600ms'}; + var clockTimestamp = 1437018587; // iat + 5s + var options = {algorithms: ['HS256'], clockTimestamp: clockTimestamp, maxAge: '6s'}; jwt.verify(token, key, options, function (err, p) { assert.isNull(err); @@ -290,7 +303,7 @@ describe('verify', function() { }); }); it('can be more restrictive than expiration', function (done) { - var clockTimestamp = 1437018588; + var clockTimestamp = 1437018588; // iat + 6s var options = {algorithms: ['HS256'], clockTimestamp: clockTimestamp, maxAge: '5s'}; jwt.verify(token, key, options, function (err, p) { @@ -303,7 +316,7 @@ describe('verify', function() { }); }); it('cannot be more permissive than expiration', function (done) { - var clockTimestamp = 1437018900; + var clockTimestamp = 1437018900; // iat + 318s (exp: iat + 218s) var options = {algorithms: ['HS256'], clockTimestamp: clockTimestamp, maxAge: '1000y'}; jwt.verify(token, key, options, function (err, p) { diff --git a/verify.js b/verify.js index d10bfab..bbebf58 100644 --- a/verify.js +++ b/verify.js @@ -2,8 +2,8 @@ var JsonWebTokenError = require('./lib/JsonWebTokenError'); var NotBeforeError = require('./lib/NotBeforeError'); var TokenExpiredError = require('./lib/TokenExpiredError'); var decode = require('./decode'); +var timespan = require('./lib/timespan'); var jws = require('jws'); -var ms = require('ms'); var xtend = require('xtend'); module.exports = function (jwtString, secretOrPublicKey, options, callback) { @@ -165,15 +165,13 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { } if (options.maxAge) { - var maxAge = ms(options.maxAge); if (typeof payload.iat !== 'number') { return done(new JsonWebTokenError('iat required when maxAge is specified')); } - // We have to compare against either options.clockTimestamp or the currentDate _with_ millis - // to not change behaviour (version 7.2.1). Should be resolve somehow for next major. - var nowOrClockTimestamp = ((options.clockTimestamp || 0) * 1000) || Date.now(); - if (nowOrClockTimestamp - (payload.iat * 1000) > maxAge + (options.clockTolerance || 0) * 1000) { - return done(new TokenExpiredError('maxAge exceeded', new Date(payload.iat * 1000 + maxAge))); + + var maxAgeTimestamp = timespan(options.maxAge, payload.iat); + if (clockTimestamp >= maxAgeTimestamp + (options.clockTolerance || 0)) { + return done(new TokenExpiredError('maxAge exceeded', new Date(maxAgeTimestamp * 1000))); } } From 66a4f8b996c8357727ce62a84605a005b2f5eb18 Mon Sep 17 00:00:00 2001 From: Eduardo Diaz Date: Tue, 16 May 2017 11:19:16 +0200 Subject: [PATCH 2/2] maxAge: Add validation to timespan result --- test/verify.tests.js | 25 ++++++++++++++++++++----- verify.js | 3 +++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/test/verify.tests.js b/test/verify.tests.js index aab1d24..9685525 100644 --- a/test/verify.tests.js +++ b/test/verify.tests.js @@ -115,7 +115,7 @@ describe('verify', function() { describe('option: maxAge', function () { - ['3s', 3].forEach(function(maxAge) { + [String('3s'), '3s', 3].forEach(function(maxAge) { it(`should error for claims issued before a certain timespan (${typeof maxAge} type)`, function (done) { clock = sinon.useFakeTimers(1437018587000); // iat + 5s, exp - 5s var options = {algorithms: ['HS256'], maxAge: maxAge}; @@ -131,7 +131,7 @@ describe('verify', function() { }); }); - ['5s', 5].forEach(function (maxAge) { + [String('5s'), '5s', 5].forEach(function (maxAge) { it(`should not error for claims issued before a certain timespan but still inside clockTolerance timespan (${typeof maxAge} type)`, function (done) { clock = sinon.useFakeTimers(1437018587500); // iat + 5.5s, exp - 4.5s var options = {algorithms: ['HS256'], maxAge: maxAge, clockTolerance: 1 }; @@ -144,7 +144,7 @@ describe('verify', function() { }); }); - ['6s', 6].forEach(function (maxAge) { + [String('6s'), '6s', 6].forEach(function (maxAge) { it(`should not error if within maxAge timespan (${typeof maxAge} type)`, function (done) { clock = sinon.useFakeTimers(1437018587500);// iat + 5.5s, exp - 4.5s var options = {algorithms: ['HS256'], maxAge: maxAge}; @@ -157,7 +157,7 @@ describe('verify', function() { }); }); - ['8s', 8].forEach(function (maxAge) { + [String('8s'), '8s', 8].forEach(function (maxAge) { it(`can be more restrictive than expiration (${typeof maxAge} type)`, function (done) { clock = sinon.useFakeTimers(1437018591900); // iat + 9.9s, exp - 0.1s var options = {algorithms: ['HS256'], maxAge: maxAge }; @@ -173,7 +173,7 @@ describe('verify', function() { }); }); - ['12s', 12].forEach(function (maxAge) { + [String('12s'), '12s', 12].forEach(function (maxAge) { it(`cannot be more permissive than expiration (${typeof maxAge} type)`, function (done) { clock = sinon.useFakeTimers(1437018593000); // iat + 11s, exp + 1s var options = {algorithms: ['HS256'], maxAge: '12s'}; @@ -190,6 +190,20 @@ describe('verify', function() { }); }); + [new String('1s'), 'no-timespan-string'].forEach(function (maxAge){ + it(`should error if maxAge is specified with a wrong string format/type (value: ${maxAge}, type: ${typeof maxAge})`, function (done) { + clock = sinon.useFakeTimers(1437018587000); // iat + 5s, exp - 5s + var options = { algorithms: ['HS256'], maxAge: maxAge }; + + jwt.verify(token, key, options, function (err, p) { + assert.equal(err.name, 'JsonWebTokenError'); + assert.equal(err.message, '"maxAge" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60'); + assert.isUndefined(p); + done(); + }); + }); + }); + it('should error if maxAge is specified but there is no iat claim', function (done) { var token = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJmb28iOiJiYXIifQ.0MBPd4Bru9-fK_HY3xmuDAc6N_embknmNuhdb9bKL_U'; var options = {algorithms: ['HS256'], maxAge: '1s'}; @@ -201,6 +215,7 @@ describe('verify', function() { done(); }); }); + }); describe('option: clockTimestamp', function () { diff --git a/verify.js b/verify.js index bbebf58..de30a36 100644 --- a/verify.js +++ b/verify.js @@ -170,6 +170,9 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { } var maxAgeTimestamp = timespan(options.maxAge, payload.iat); + if (typeof maxAgeTimestamp === 'undefined') { + return done(new JsonWebTokenError('"maxAge" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60')); + } if (clockTimestamp >= maxAgeTimestamp + (options.clockTolerance || 0)) { return done(new TokenExpiredError('maxAge exceeded', new Date(maxAgeTimestamp * 1000))); }