Skip to content

Commit

Permalink
fix(sign&verify)!: Remove default none support from sign and verify m…
Browse files Browse the repository at this point in the history 
…ethods, 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 18, 2022
1 parent 7e6a86b commit 1a95784
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 137 deletions.
2 changes: 2 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ encoded private key for RSA and ECDSA. In case of a private key with passphrase
`options`:

* `algorithm` (default: `HS256`)
* * `none` MUST be configured in order to create unsigned tokens.
* `expiresIn`: expressed in seconds or a string describing a time span [vercel/ms](https://github.com/vercel/ms).
> Eg: `60`, `"2 days"`, `"10h"`, `"7d"`. A numeric value is interpreted as a seconds count. If you use a string be sure you provide the time units (days, hours, etc), otherwise milliseconds unit is used by default (`"120"` is equal to `"120ms"`).
* `notBefore`: expressed in seconds or a string describing a time span [vercel/ms](https://github.com/vercel/ms).
Expand Down Expand Up @@ -138,6 +139,7 @@ As mentioned in [this comment](https://github.com/auth0/node-jsonwebtoken/issues
`options`

* `algorithms`: List of strings with the names of the allowed algorithms. For instance, `["HS256", "HS384"]`.
* `none` MUST be configured in order to verify unsigned tokens.
* `audience`: if you want to check audience (`aud`), provide a value here. The audience can be checked against a string, a regular expression or a list of strings and/or regular expressions.
> Eg: `"urn:foo"`, `/urn:f[o]{2}/`, `[/urn:f[o]{2}/, "urn:bar"]`
* `complete`: return an object with the decoded `{ payload, header, signature }` instead of only the usual content of the payload.
Expand Down
10 changes: 0 additions & 10 deletions test/async_sign.tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ describe('signing a token asynchronously', function() {
});
});

//Known bug: https://github.com/brianloveswords/node-jws/issues/62
//If you need this use case, you need to go for the non-callback-ish code style.
it.skip('should work with none algorithm where secret is falsy', function(done) {
jwt.sign({ foo: 'bar' }, undefined, { algorithm: 'none' }, function(err, token) {
expect(token).to.be.a('string');
expect(token.split('.')).to.have.length(3);
done();
});
});

it('should return error when secret is not a cert for RS256', function(done) {
//this throw an error because the secret is not a cert and RS256 requires a cert.
jwt.sign({ foo: 'bar' }, secret, { algorithm: 'RS256' }, function (err) {
Expand Down
6 changes: 3 additions & 3 deletions test/claim-aud.test.js
View file
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 1a95784

Please sign in to comment.