From 683d8a9b31ad6327948f84268bd2c8e4350779d1 Mon Sep 17 00:00:00 2001 From: Tim Oram Date: Wed, 10 Oct 2018 16:07:34 -0230 Subject: [PATCH] Create and implement async/sync test helpers (#523) It is difficult to write tests that ensure that both the asynchronous and synchronous calls to the sign and verify functions had the same result. These helpers ensure that the calls are the same and return the common result. As a proof of concept, the iat claim tests have been updated to use the new helpers. --- test/claim-iat.test.js | 96 +++++++++++++--------------------- test/test-utils.js | 114 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 60 deletions(-) diff --git a/test/claim-iat.test.js b/test/claim-iat.test.js index 01358e0..a1c63ba 100644 --- a/test/claim-iat.test.js +++ b/test/claim-iat.test.js @@ -9,16 +9,7 @@ const testUtils = require('./test-utils'); const base64UrlEncode = testUtils.base64UrlEncode; const noneAlgorithmHeader = 'eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0'; -function signWithIssueAtSync(issueAt, options) { - const payload = {}; - if (issueAt !== undefined) { - payload.iat = issueAt; - } - const opts = Object.assign({algorithm: 'none'}, options); - return jwt.sign(payload, undefined, opts); -} - -function signWithIssueAtAsync(issueAt, options, cb) { +function signWithIssueAt(issueAt, options, callback) { const payload = {}; if (issueAt !== undefined) { payload.iat = issueAt; @@ -26,17 +17,12 @@ function signWithIssueAtAsync(issueAt, options, cb) { const opts = Object.assign({algorithm: 'none'}, options); // async calls require a truthy secret // see: https://github.com/brianloveswords/node-jws/issues/62 - return jwt.sign(payload, 'secret', opts, cb); + testUtils.signJWTHelper(payload, 'secret', opts, callback); } -function verifyWithIssueAtSync(token, maxAge, options) { +function verifyWithIssueAt(token, maxAge, options, callback) { const opts = Object.assign({maxAge}, options); - return jwt.verify(token, undefined, opts) -} - -function verifyWithIssueAtAsync(token, maxAge, options, cb) { - const opts = Object.assign({maxAge}, options); - return jwt.verify(token, undefined, opts, cb) + testUtils.verifyJWTHelper(token, undefined, opts, callback); } describe('issue at', function() { @@ -53,22 +39,22 @@ describe('issue at', function() { {foo: 'bar'}, ].forEach((iat) => { it(`should error with iat of ${util.inspect(iat)}`, function (done) { - expect(() => signWithIssueAtSync(iat, {})).to.throw('"iat" should be a number of seconds'); - signWithIssueAtAsync(iat, {}, (err) => { - expect(err.message).to.equal('"iat" should be a number of seconds'); - 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'); + }); }); }); }); // undefined needs special treatment because {} is not the same as {iat: undefined} it('should error with iat of undefined', function (done) { - expect(() => jwt.sign({iat: undefined}, undefined, {algorithm: 'none'})).to.throw( - '"iat" should be a number of seconds' - ); - jwt.sign({iat: undefined}, undefined, {algorithm: 'none'}, (err) => { - expect(err.message).to.equal('"iat" should be a number of seconds'); - done(); + testUtils.signJWTHelper({iat: undefined}, 'secret', {algorithm: 'none'}, (err) => { + testUtils.asyncCheck(done, () => { + expect(err).to.be.instanceOf(Error); + expect(err.message).to.equal('"iat" should be a number of seconds'); + }); }); }); }); @@ -92,14 +78,11 @@ describe('issue at', function() { it(`should error with iat of ${util.inspect(iat)}`, function (done) { const encodedPayload = base64UrlEncode(JSON.stringify({iat})); const token = `${noneAlgorithmHeader}.${encodedPayload}.`; - expect(() => verifyWithIssueAtSync(token, '1 min', {})).to.throw( - jwt.JsonWebTokenError, 'iat required when maxAge is specified' - ); - - verifyWithIssueAtAsync(token, '1 min', {}, (err) => { - expect(err).to.be.instanceOf(jwt.JsonWebTokenError); - expect(err.message).to.equal('iat required when maxAge is specified'); - done(); + verifyWithIssueAt(token, '1 min', {}, (err) => { + testUtils.asyncCheck(done, () => { + expect(err).to.be.instanceOf(jwt.JsonWebTokenError); + expect(err.message).to.equal('iat required when maxAge is specified'); + }); }); }); }) @@ -163,25 +146,17 @@ describe('issue at', function() { }, ].forEach((testCase) => { it(testCase.description, function (done) { - const token = signWithIssueAtSync(testCase.iat, testCase.options); - expect(jwt.decode(token).iat).to.equal(testCase.expectedIssueAt); - signWithIssueAtAsync(testCase.iat, testCase.options, (err, token) => { - // node-jsw catches the error from expect, so we have to wrap it in try/catch and use done(error) - try { + signWithIssueAt(testCase.iat, testCase.options, (err, token) => { + testUtils.asyncCheck(done, () => { expect(err).to.be.null; expect(jwt.decode(token).iat).to.equal(testCase.expectedIssueAt); - done(); - } - catch (e) { - done(e); - } + }); }); }); }); }); describe('when verifying a token', function() { - let token; let fakeClock; beforeEach(function() { @@ -213,10 +188,14 @@ describe('issue at', function() { }, ].forEach((testCase) => { it(testCase.description, function (done) { - const token = signWithIssueAtSync(undefined, {}); + const token = jwt.sign({}, 'secret', {algorithm: 'none'}); fakeClock.tick(testCase.clockAdvance); - expect(verifyWithIssueAtSync(token, testCase.maxAge, testCase.options)).to.not.throw; - verifyWithIssueAtAsync(token, testCase.maxAge, testCase.options, done) + verifyWithIssueAt(token, testCase.maxAge, testCase.options, (err, token) => { + testUtils.asyncCheck(done, () => { + expect(err).to.be.null; + expect(token).to.be.a('object'); + }); + }); }); }); @@ -256,16 +235,15 @@ describe('issue at', function() { ].forEach((testCase) => { it(testCase.description, function(done) { const expectedExpiresAtDate = new Date(testCase.expectedExpiresAt); - token = signWithIssueAtSync(undefined, {}); + const token = jwt.sign({}, 'secret', {algorithm: 'none'}); fakeClock.tick(testCase.clockAdvance); - expect(() => verifyWithIssueAtSync(token, testCase.maxAge, {})) - .to.throw(jwt.TokenExpiredError, testCase.expectedError) - .to.have.property('expiredAt').that.deep.equals(expectedExpiresAtDate); - verifyWithIssueAtAsync(token, testCase.maxAge, {}, (err) => { - expect(err).to.be.instanceOf(jwt.TokenExpiredError); - expect(err.message).to.equal(testCase.expectedError); - expect(err.expiredAt).to.deep.equal(expectedExpiresAtDate); - done(); + + verifyWithIssueAt(token, testCase.maxAge, testCase.options, (err) => { + testUtils.asyncCheck(done, () => { + expect(err).to.be.instanceOf(jwt.JsonWebTokenError); + expect(err.message).to.equal(testCase.expectedError); + expect(err.expiredAt).to.deep.equal(expectedExpiresAtDate); + }); }); }); }); diff --git a/test/test-utils.js b/test/test-utils.js index f49889e..aa115da 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -1,13 +1,125 @@ 'use strict'; +const jwt = require('../'); +const expect = require('chai').expect; +const sinon = require('sinon'); + +/** + * Correctly report errors that occur in an asynchronous callback + * @param {function(err): void} done The mocha callback + * @param {function(): void} testFunction The assertions function + */ +function asyncCheck(done, testFunction) { + try { + testFunction(); + done(); + } + catch(err) { + done(err); + } +} + +/** + * Assert that two errors are equal + * @param e1 {Error} The first error + * @param e2 {Error} The second error + */ +// chai does not do deep equality on errors: https://github.com/chaijs/chai/issues/1009 +function expectEqualError(e1, e2) { + // message and name are not always enumerable, so manually reference them + expect(e1.message, 'Async/Sync Error equality: message').to.equal(e2.message); + expect(e1.name, 'Async/Sync Error equality: name').to.equal(e2.name); + + // compare other enumerable error properties + for(const propertyName in e1) { + expect(e1[propertyName], `Async/Sync Error equality: ${propertyName}`).to.deep.equal(e2[propertyName]); + } +} + +/** + * Base64-url encode a string + * @param str {string} The string to encode + * @returns {string} The encoded string + */ function base64UrlEncode(str) { return Buffer.from(str).toString('base64') - .replace(/\=/g, "") + .replace(/[=]/g, "") .replace(/\+/g, "-") .replace(/\//g, "_") ; } +/** + * Verify a JWT, ensuring that the asynchronous and synchronous calls to `verify` have the same result + * @param {string} jwtString The JWT as a string + * @param {string} secretOrPrivateKey The shared secret or private key + * @param {object} options Verify options + * @param {function(err, token):void} callback + */ +function verifyJWTHelper(jwtString, secretOrPrivateKey, options, callback) { + // freeze the time to ensure the clock remains stable across the async and sync calls + const fakeClock = sinon.useFakeTimers({now: Date.now()}); + let error; + let syncVerified; + try { + syncVerified = jwt.verify(jwtString, secretOrPrivateKey, options); + } + catch (err) { + error = err; + } + jwt.verify(jwtString, secretOrPrivateKey, options, (err, asyncVerifiedToken) => { + try { + if (error) { + expectEqualError(err, error); + callback(err); + } + else { + expect(syncVerified, 'Async/Sync token equality').to.deep.equal(asyncVerifiedToken); + callback(null, syncVerified); + } + } + finally { + if (fakeClock) { + fakeClock.restore(); + } + } + }); +} + +/** + * Sign a payload to create a JWT, ensuring that the asynchronous and synchronous calls to `sign` have the same result + * @param {object} payload The JWT payload + * @param {string} secretOrPrivateKey The shared secret or private key + * @param {object} options Sign options + * @param {function(err, token):void} callback + */ +function signJWTHelper(payload, secretOrPrivateKey, options, callback) { + // freeze the time to ensure the clock remains stable across the async and sync calls + const fakeClock = sinon.useFakeTimers({now: Date.now()}); + let error; + let syncSigned; + try { + syncSigned = jwt.sign(payload, secretOrPrivateKey, options); + } + catch (err) { + error = err; + } + jwt.sign(payload, secretOrPrivateKey, options, (err, asyncSigned) => { + fakeClock.restore(); + if (error) { + expectEqualError(err, error); + callback(err); + } + else { + expect(syncSigned, 'Async/Sync token equality').to.equal(asyncSigned); + callback(null, syncSigned); + } + }); +} + module.exports = { + asyncCheck, base64UrlEncode, + signJWTHelper, + verifyJWTHelper, };