Reject non-finite exp/nbf/iat values and tighten validation

Author: AzmeerX

sign.js

@@ -5,7 +5,6 @@ const jws = require('jws');
 const includes = require('lodash.includes');
 const isBoolean = require('lodash.isboolean');
 const isInteger = require('lodash.isinteger');
-const isNumber = require('lodash.isnumber');
 const isPlainObject = require('lodash.isplainobject');
 const isString = require('lodash.isstring');
 const once = require('lodash.once');
@@ -34,9 +33,9 @@ const sign_options_schema = {
 };
 
 const registered_claims_schema = {
-  iat: { isValid: isNumber, message: '"iat" should be a number of seconds' },
-  exp: { isValid: isNumber, message: '"exp" should be a number of seconds' },
-  nbf: { isValid: isNumber, message: '"nbf" should be a number of seconds' }
+  iat: { isValid: function(value) { return typeof value === 'number' && Number.isFinite(value); }, message: '"iat" should be a number of seconds' },
+  exp: { isValid: function(value) { return typeof value === 'number' && Number.isFinite(value); }, message: '"exp" should be a number of seconds' },
+  nbf: { isValid: function(value) { return typeof value === 'number' && Number.isFinite(value); }, message: '"nbf" should be a number of seconds' }
 };
 
 function validate(schema, allowUnknown, object, parameterName) {

test/claim-exp.test.js

@@ -231,35 +231,29 @@ describe('expires', function() {
       });
     });
 
-    // TODO an exp of -Infinity should fail validation
-    it('should set null "exp" when given -Infinity', function (done) {
-      signWithExpiresIn(undefined, {exp: -Infinity}, (err, token) => {
-        const decoded = jwt.decode(token);
+    it('should throw error when "exp" is -Infinity', function (done) {
+      signWithExpiresIn(undefined, {exp: -Infinity}, (err) => {
         testUtils.asyncCheck(done, () => {
-          expect(err).to.be.null;
-          expect(decoded).to.have.property('exp', null);
+          expect(err).to.be.instanceOf(Error);
+          expect(err).to.have.property('message', '"exp" should be a number of seconds');
         });
       });
     });
 
-    // TODO an exp of Infinity should fail validation
-    it('should set null "exp" when given value Infinity', function (done) {
-      signWithExpiresIn(undefined, {exp: Infinity}, (err, token) => {
-        const decoded = jwt.decode(token);
+    it('should throw error when "exp" is Infinity', function (done) {
+      signWithExpiresIn(undefined, {exp: Infinity}, (err) => {
         testUtils.asyncCheck(done, () => {
-          expect(err).to.be.null;
-          expect(decoded).to.have.property('exp', null);
+          expect(err).to.be.instanceOf(Error);
+          expect(err).to.have.property('message', '"exp" should be a number of seconds');
         });
       });
     });
 
-    // TODO an exp of NaN should fail validation
-    it('should set null "exp" when given value NaN', function (done) {
-      signWithExpiresIn(undefined, {exp: NaN}, (err, token) => {
-        const decoded = jwt.decode(token);
+    it('should throw error when "exp" is NaN', function (done) {
+      signWithExpiresIn(undefined, {exp: NaN}, (err) => {
         testUtils.asyncCheck(done, () => {
-          expect(err).to.be.null;
-          expect(decoded).to.have.property('exp', null);
+          expect(err).to.be.instanceOf(Error);
+          expect(err).to.have.property('message', '"exp" should be a number of seconds');
         });
       });
     });

test/claim-iat.test.js

@@ -110,27 +110,6 @@ describe('issue at', function() {
         expectedIssueAt: 100,
         options: {}
       },
-      // TODO an iat of -Infinity should fail validation
-      {
-        description: 'should set null "iat" when given -Infinity',
-        iat: -Infinity,
-        expectedIssueAt: null,
-        options: {}
-      },
-      // TODO an iat of Infinity should fail validation
-      {
-        description: 'should set null "iat" when given Infinity',
-        iat: Infinity,
-        expectedIssueAt: null,
-        options: {}
-      },
-      // TODO an iat of NaN should fail validation
-      {
-        description: 'should set to current time for "iat" when given value NaN',
-        iat: NaN,
-        expectedIssueAt: 60,
-        options: {}
-      },
       {
         description: 'should remove default "iat" with "noTimestamp" option',
         iat: undefined,
@@ -153,6 +132,21 @@ describe('issue at', function() {
         });
       });
     });
+
+    [
+      -Infinity,
+      Infinity,
+      NaN,
+    ].forEach((iat) => {
+      it(`should error when "iat" is ${util.inspect(iat)}`, function (done) {
+        signWithIssueAt(iat, {}, (err) => {
+          testUtils.asyncCheck(done, () => {
+            expect(err).to.be.instanceOf(Error);
+            expect(err.message).to.equal('"iat" should be a number of seconds');
+          });
+        });
+      });
+    });
   });
 
   describe('when verifying a token', function() {

test/claim-nbf.test.js

@@ -228,35 +228,13 @@ describe('not before', function() {
       });
     });
 
-    // TODO an nbf of -Infinity should fail validation
-    it('should set null "nbf" when given -Infinity', function (done) {
-      signWithNotBefore(undefined, {nbf: -Infinity}, (err, token) => {
-        const decoded = jwt.decode(token);
-        testUtils.asyncCheck(done, () => {
-          expect(err).to.be.null;
-          expect(decoded).to.have.property('nbf', null);
-        });
-      });
-    });
-
-    // TODO an nbf of Infinity should fail validation
-    it('should set null "nbf" when given value Infinity', function (done) {
-      signWithNotBefore(undefined, {nbf: Infinity}, (err, token) => {
-        const decoded = jwt.decode(token);
-        testUtils.asyncCheck(done, () => {
-          expect(err).to.be.null;
-          expect(decoded).to.have.property('nbf', null);
-        });
-      });
-    });
-
-    // TODO an nbf of NaN should fail validation
-    it('should set null "nbf" when given value NaN', function (done) {
-      signWithNotBefore(undefined, {nbf: NaN}, (err, token) => {
-        const decoded = jwt.decode(token);
-        testUtils.asyncCheck(done, () => {
-          expect(err).to.be.null;
-          expect(decoded).to.have.property('nbf', null);
+    [-Infinity, Infinity, NaN].forEach((nbf) => {
+      it(`should error when "nbf" is ${util.inspect(nbf)}`, function (done) {
+        signWithNotBefore(undefined, {nbf}, (err) => {
+          testUtils.asyncCheck(done, () => {
+            expect(err).to.be.instanceOf(Error);
+            expect(err).to.have.property('message', '"nbf" should be a number of seconds');
+          });
         });
       });
     });