Skip to content

Commit

Permalink
fix(sign&verify)!: Remove default none support from sign and `ver…
Browse files Browse the repository at this point in the history 
…ify` methods, and require it to be explicitly configured (#851)

* fix(sign&verify)!: Remove default none support from sign and verify methods, and require it to be explicitly configured

BREAKING CHANGE: Removes fallback for none algorithm for the verify method.
  • Loading branch information
@jakelacey2012
jakelacey2012 committed Nov 29, 2022
1 parent 7e6a86b commit 8345030
Show file tree
Hide file tree
Showing 15 changed files with 153 additions and 124 deletions.
6 changes: 3 additions & 3 deletions test/claim-aud.test.js
View file
Expand Up @@ -6,7 +6,7 @@ const util = require('util');
const testUtils = require('./test-utils');

function signWithAudience(audience, payload, callback) {
const options = {algorithm: 'none'};
const options = {algorithm: 'HS256'};
if (audience !== undefined) {
options.audience = audience;
}
Expand All @@ -15,7 +15,7 @@ function signWithAudience(audience, payload, callback) {
}

function verifyWithAudience(token, audience, callback) {
testUtils.verifyJWTHelper(token, undefined, {audience}, callback);
testUtils.verifyJWTHelper(token, 'secret', {audience}, callback);
}

describe('audience', function() {
Expand Down Expand Up @@ -47,7 +47,7 @@ describe('audience', function() {

// undefined needs special treatment because {} is not the same as {aud: undefined}
it('should error with with value undefined', function (done) {
testUtils.signJWTHelper({}, 'secret', {audience: undefined, algorithm: 'none'}, (err) => {
testUtils.signJWTHelper({}, 'secret', {audience: undefined, algorithm: 'HS256'}, (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', '"audience" must be a string or array');
Expand Down
39 changes: 19 additions & 20 deletions test/claim-exp.test.js
View file
Expand Up @@ -5,12 +5,10 @@ const expect = require('chai').expect;
const sinon = require('sinon');
const util = require('util');
const testUtils = require('./test-utils');

const base64UrlEncode = testUtils.base64UrlEncode;
const noneAlgorithmHeader = 'eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0';
const jws = require('jws');

function signWithExpiresIn(expiresIn, payload, callback) {
const options = {algorithm: 'none'};
const options = {algorithm: 'HS256'};
if (expiresIn !== undefined) {
options.expiresIn = expiresIn;
}
Expand Down Expand Up @@ -49,7 +47,7 @@ describe('expires', function() {

// undefined needs special treatment because {} is not the same as {expiresIn: undefined}
it('should error with with value undefined', function (done) {
testUtils.signJWTHelper({}, undefined, {expiresIn: undefined, algorithm: 'none'}, (err) => {
testUtils.signJWTHelper({}, 'secret', {expiresIn: undefined, algorithm: 'HS256'}, (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property(
Expand Down Expand Up @@ -133,9 +131,10 @@ describe('expires', function() {
{foo: 'bar'},
].forEach((exp) => {
it(`should error with with value ${util.inspect(exp)}`, function (done) {
const encodedPayload = base64UrlEncode(JSON.stringify({exp}));
const token = `${noneAlgorithmHeader}.${encodedPayload}.`;
testUtils.verifyJWTHelper(token, undefined, {exp}, (err) => {
const header = { alg: 'HS256' };
const payload = { exp };
const token = jws.sign({ header, payload, secret: 'secret', encoding: 'utf8' });
testUtils.verifyJWTHelper(token, 'secret', { exp }, (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(jwt.JsonWebTokenError);
expect(err).to.have.property('message', 'invalid exp value');
Expand All @@ -158,7 +157,7 @@ describe('expires', function() {
it('should set correct "exp" with negative number of seconds', function(done) {
signWithExpiresIn(-10, {}, (e1, token) => {
fakeClock.tick(-10001);
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -170,7 +169,7 @@ describe('expires', function() {

it('should set correct "exp" with positive number of seconds', function(done) {
signWithExpiresIn(10, {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -183,7 +182,7 @@ describe('expires', function() {
it('should set correct "exp" with zero seconds', function(done) {
signWithExpiresIn(0, {}, (e1, token) => {
fakeClock.tick(-1);
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -196,7 +195,7 @@ describe('expires', function() {
it('should set correct "exp" with negative string timespan', function(done) {
signWithExpiresIn('-10 s', {}, (e1, token) => {
fakeClock.tick(-10001);
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -209,7 +208,7 @@ describe('expires', function() {
it('should set correct "exp" with positive string timespan', function(done) {
signWithExpiresIn('10 s', {}, (e1, token) => {
fakeClock.tick(-10001);
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -222,7 +221,7 @@ describe('expires', function() {
it('should set correct "exp" with zero string timespan', function(done) {
signWithExpiresIn('0 s', {}, (e1, token) => {
fakeClock.tick(-1);
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand Down Expand Up @@ -267,7 +266,7 @@ describe('expires', function() {

it('should set correct "exp" when "iat" is passed', function (done) {
signWithExpiresIn(-10, {iat: 80}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -279,7 +278,7 @@ describe('expires', function() {

it('should verify "exp" using "clockTimestamp"', function (done) {
signWithExpiresIn(10, {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {clockTimestamp: 69}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {clockTimestamp: 69}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -293,7 +292,7 @@ describe('expires', function() {
it('should verify "exp" using "clockTolerance"', function (done) {
signWithExpiresIn(5, {}, (e1, token) => {
fakeClock.tick(10000);
testUtils.verifyJWTHelper(token, undefined, {clockTimestamp: 6}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {clockTimestamp: 6}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -306,7 +305,7 @@ describe('expires', function() {

it('should ignore a expired token when "ignoreExpiration" is true', function (done) {
signWithExpiresIn('-10 s', {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {ignoreExpiration: true}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {ignoreExpiration: true}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -319,7 +318,7 @@ describe('expires', function() {

it('should error on verify if "exp" is at current time', function(done) {
signWithExpiresIn(undefined, {exp: 60}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {}, (e2) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.instanceOf(jwt.TokenExpiredError);
Expand All @@ -331,7 +330,7 @@ describe('expires', function() {

it('should error on verify if "exp" is before current time using clockTolerance', function (done) {
signWithExpiresIn(-5, {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {clockTolerance: 5}, (e2) => {
testUtils.verifyJWTHelper(token, 'secret', {clockTolerance: 5}, (e2) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.instanceOf(jwt.TokenExpiredError);
Expand Down
31 changes: 15 additions & 16 deletions test/claim-iat.test.js
View file
Expand Up @@ -5,24 +5,22 @@ const expect = require('chai').expect;
const sinon = require('sinon');
const util = require('util');
const testUtils = require('./test-utils');

const base64UrlEncode = testUtils.base64UrlEncode;
const noneAlgorithmHeader = 'eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0';
const jws = require('jws');

function signWithIssueAt(issueAt, options, callback) {
const payload = {};
if (issueAt !== undefined) {
payload.iat = issueAt;
}
const opts = Object.assign({algorithm: 'none'}, options);
const opts = Object.assign({algorithm: 'HS256'}, options);
// async calls require a truthy secret
// see: https://github.com/brianloveswords/node-jws/issues/62
testUtils.signJWTHelper(payload, 'secret', opts, callback);
}

function verifyWithIssueAt(token, maxAge, options, callback) {
function verifyWithIssueAt(token, maxAge, options, secret, callback) {
const opts = Object.assign({maxAge}, options);
testUtils.verifyJWTHelper(token, undefined, opts, callback);
testUtils.verifyJWTHelper(token, secret, opts, callback);
}

describe('issue at', function() {
Expand Down Expand Up @@ -50,7 +48,7 @@ describe('issue at', function() {

// undefined needs special treatment because {} is not the same as {iat: undefined}
it('should error with iat of undefined', function (done) {
testUtils.signJWTHelper({iat: undefined}, 'secret', {algorithm: 'none'}, (err) => {
testUtils.signJWTHelper({iat: undefined}, 'secret', {algorithm: 'HS256'}, (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(Error);
expect(err.message).to.equal('"iat" should be a number of seconds');
Expand All @@ -76,9 +74,10 @@ describe('issue at', function() {
{foo: 'bar'},
].forEach((iat) => {
it(`should error with iat of ${util.inspect(iat)}`, function (done) {
const encodedPayload = base64UrlEncode(JSON.stringify({iat}));
const token = `${noneAlgorithmHeader}.${encodedPayload}.`;
verifyWithIssueAt(token, '1 min', {}, (err) => {
const header = { alg: 'HS256' };
const payload = { iat };
const token = jws.sign({ header, payload, secret: 'secret', encoding: 'utf8' });
verifyWithIssueAt(token, '1 min', {}, 'secret', (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(jwt.JsonWebTokenError);
expect(err.message).to.equal('iat required when maxAge is specified');
Expand Down Expand Up @@ -188,9 +187,9 @@ describe('issue at', function() {
},
].forEach((testCase) => {
it(testCase.description, function (done) {
const token = jwt.sign({}, 'secret', {algorithm: 'none'});
const token = jwt.sign({}, 'secret', {algorithm: 'HS256'});
fakeClock.tick(testCase.clockAdvance);
verifyWithIssueAt(token, testCase.maxAge, testCase.options, (err, token) => {
verifyWithIssueAt(token, testCase.maxAge, testCase.options, 'secret', (err, token) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.null;
expect(token).to.be.a('object');
Expand Down Expand Up @@ -235,10 +234,10 @@ describe('issue at', function() {
].forEach((testCase) => {
it(testCase.description, function(done) {
const expectedExpiresAtDate = new Date(testCase.expectedExpiresAt);
const token = jwt.sign({}, 'secret', {algorithm: 'none'});
const token = jwt.sign({}, 'secret', {algorithm: 'HS256'});
fakeClock.tick(testCase.clockAdvance);

verifyWithIssueAt(token, testCase.maxAge, testCase.options, (err) => {
verifyWithIssueAt(token, testCase.maxAge, testCase.options, 'secret', (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(jwt.JsonWebTokenError);
expect(err.message).to.equal(testCase.expectedError);
Expand All @@ -252,7 +251,7 @@ describe('issue at', function() {
describe('with string payload', function () {
it('should not add iat to string', function (done) {
const payload = 'string payload';
const options = {algorithm: 'none'};
const options = {algorithm: 'HS256'};
testUtils.signJWTHelper(payload, 'secret', options, (err, token) => {
const decoded = jwt.decode(token);
testUtils.asyncCheck(done, () => {
Expand All @@ -264,7 +263,7 @@ describe('issue at', function() {

it('should not add iat to stringified object', function (done) {
const payload = '{}';
const options = {algorithm: 'none', header: {typ: 'JWT'}};
const options = {algorithm: 'HS256', header: {typ: 'JWT'}};
testUtils.signJWTHelper(payload, 'secret', options, (err, token) => {
const decoded = jwt.decode(token);
testUtils.asyncCheck(done, () => {
Expand Down
22 changes: 11 additions & 11 deletions test/claim-iss.test.js
View file
Expand Up @@ -6,7 +6,7 @@ const util = require('util');
const testUtils = require('./test-utils');

function signWithIssuer(issuer, payload, callback) {
const options = {algorithm: 'none'};
const options = {algorithm: 'HS256'};
if (issuer !== undefined) {
options.issuer = issuer;
}
Expand Down Expand Up @@ -44,7 +44,7 @@ describe('issuer', function() {

// undefined needs special treatment because {} is not the same as {issuer: undefined}
it('should error with with value undefined', function (done) {
testUtils.signJWTHelper({}, undefined, {issuer: undefined, algorithm: 'none'}, (err) => {
testUtils.signJWTHelper({}, 'secret', {issuer: undefined, algorithm: 'HS256'}, (err) => {
testUtils.asyncCheck(done, () => {
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', '"issuer" must be a string');
Expand Down Expand Up @@ -92,7 +92,7 @@ describe('issuer', function() {
describe('when signing and verifying a token', function () {
it('should not verify "iss" if verify "issuer" option not provided', function(done) {
signWithIssuer(undefined, {iss: 'foo'}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -105,7 +105,7 @@ describe('issuer', function() {
describe('with string "issuer" option', function () {
it('should verify with a string "issuer"', function (done) {
signWithIssuer('foo', {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: 'foo'}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: 'foo'}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -117,7 +117,7 @@ describe('issuer', function() {

it('should verify with a string "iss"', function (done) {
signWithIssuer(undefined, {iss: 'foo'}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: 'foo'}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: 'foo'}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -129,7 +129,7 @@ describe('issuer', function() {

it('should error if "iss" does not match verify "issuer" option', function(done) {
signWithIssuer(undefined, {iss: 'foobar'}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: 'foo'}, (e2) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: 'foo'}, (e2) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.instanceOf(jwt.JsonWebTokenError);
Expand All @@ -141,7 +141,7 @@ describe('issuer', function() {

it('should error without "iss" and with verify "issuer" option', function(done) {
signWithIssuer(undefined, {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: 'foo'}, (e2) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: 'foo'}, (e2) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.instanceOf(jwt.JsonWebTokenError);
Expand All @@ -155,7 +155,7 @@ describe('issuer', function() {
describe('with array "issuer" option', function () {
it('should verify with a string "issuer"', function (done) {
signWithIssuer('bar', {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: ['foo', 'bar']}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: ['foo', 'bar']}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -167,7 +167,7 @@ describe('issuer', function() {

it('should verify with a string "iss"', function (done) {
signWithIssuer(undefined, {iss: 'foo'}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: ['foo', 'bar']}, (e2, decoded) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: ['foo', 'bar']}, (e2, decoded) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.null;
Expand All @@ -179,7 +179,7 @@ describe('issuer', function() {

it('should error if "iss" does not match verify "issuer" option', function(done) {
signWithIssuer(undefined, {iss: 'foobar'}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: ['foo', 'bar']}, (e2) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: ['foo', 'bar']}, (e2) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.instanceOf(jwt.JsonWebTokenError);
Expand All @@ -191,7 +191,7 @@ describe('issuer', function() {

it('should error without "iss" and with verify "issuer" option', function(done) {
signWithIssuer(undefined, {}, (e1, token) => {
testUtils.verifyJWTHelper(token, undefined, {issuer: ['foo', 'bar']}, (e2) => {
testUtils.verifyJWTHelper(token, 'secret', {issuer: ['foo', 'bar']}, (e2) => {
testUtils.asyncCheck(done, () => {
expect(e1).to.be.null;
expect(e2).to.be.instanceOf(jwt.JsonWebTokenError);
Expand Down

0 comments on commit 8345030

Please sign in to comment.