diff --git a/.eslintrc.json b/.eslintrc.json index 572b76fd..65270721 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -10,7 +10,10 @@ "rules": { "comma-style": "error", "dot-notation": "error", - "indent": ["error", 2], + "indent": [ + "error", + 2 + ], "no-control-regex": "error", "no-div-regex": "error", "no-eval": "error", @@ -18,6 +21,19 @@ "no-invalid-regexp": "error", "no-trailing-spaces": "error", "no-undef": "error", - "no-unused-vars": "error" + "no-unused-vars": "error", + "eqeqeq": "warn", + "complexity": [ + "warn", + 15 + ], + "max-statements": [ + "warn", + 30 + ], + "max-depth": [ + "warn", + 4 + ] } } diff --git a/.github/workflows/tdd-quality.yml b/.github/workflows/tdd-quality.yml new file mode 100644 index 00000000..e96f8a4b --- /dev/null +++ b/.github/workflows/tdd-quality.yml @@ -0,0 +1,39 @@ +name: TDD quality gate + +on: + push: + branches: ['**'] + pull_request: + +permissions: + contents: read + +jobs: + quality: + strategy: + fail-fast: false + matrix: + node: [16, 18, 20, 22, 24] + + name: Lint + cobertura (Node ${{ matrix.node }}) + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Setup Node + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 + with: + node-version: ${{ matrix.node }} + + - name: Install dependencies + run: npm install + + - name: Analisis estatico (ESLint) + run: npm run lint + + - name: Pruebas y puerta de cobertura (nyc) + run: npm run coverage diff --git a/MIGRATION.md b/MIGRATION.md new file mode 100644 index 00000000..3436dff6 --- /dev/null +++ b/MIGRATION.md @@ -0,0 +1,26 @@ +# Migration notes (fork) + +## `verify()` rejects unknown options (HU-03) + +`jwt.verify()` used to ignore any option it did not read. A typo such as +`algoritms` silently dropped the algorithm allow-list. It now accepts only +the documented options: + +`algorithms`, `audience`, `issuer`, `subject`, `jwtid`, `complete`, +`ignoreExpiration`, `ignoreNotBefore`, `clockTolerance`, `maxAge`, `nonce`, +`clockTimestamp`, `allowInvalidAsymmetricKeyTypes`. + +Any other name throws (or is passed to the callback as) a +`JsonWebTokenError` with the message `"" is not an allowed option`. + +### What to change + +| You pass to `verify()` | Change it to | +|---|---| +| `algorithm: 'HS256'` | `algorithms: ['HS256']`; `algorithm` never restricted the allow-list | +| `jetid` (or another misspelled name) | the correct name, e.g. `jwtid` | +| `typ`, `exp`, `nbf` | remove them; `verify()` reads these from the token, not from the options | +| `sign()` options such as `expiresIn`, `notBefore`, `noTimestamp` | remove them, or use `maxAge` / `clockTolerance` | + +If you share one options object between `sign()` and `verify()`, build +separate objects for each call. diff --git a/README.md b/README.md index 4e20dd9c..0239c49f 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ $ npm install jsonwebtoken # Migration notes +* [Unknown `verify()` options are rejected (fork)](MIGRATION.md) * [From v8 to v9](https://github.com/auth0/node-jsonwebtoken/wiki/Migration-Notes:-v8-to-v9) * [From v7 to v8](https://github.com/auth0/node-jsonwebtoken/wiki/Migration-Notes:-v7-to-v8) @@ -162,6 +163,8 @@ As mentioned in [this comment](https://github.com/auth0/node-jsonwebtoken/issues * `nonce`: if you want to check `nonce` claim, provide a string value here. It is used on Open ID for the ID Tokens. ([Open ID implementation notes](https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes)) * `allowInvalidAsymmetricKeyTypes`: if true, allows asymmetric keys which do not match the specified algorithm. This option is intended only for backwards compatability and should be avoided. +Any other option name is rejected with a `JsonWebTokenError` (`'"" is not an allowed option'`), so a typo such as `algoritms` cannot silently disable a check. + ```js // verify a token symmetric - synchronous var decoded = jwt.verify(token, 'shhhhh'); @@ -314,6 +317,7 @@ Error object: * 'jwt issuer invalid. expected: [OPTIONS ISSUER]' * 'jwt id invalid. expected: [OPTIONS JWT ID]' * 'jwt subject invalid. expected: [OPTIONS SUBJECT]' + * '"[OPTION NAME]" is not an allowed option' - `verify()` received an undocumented option ```js jwt.verify(token, 'shhhhh', function(err, decoded) { diff --git a/lib/validateVerifyOptions.js b/lib/validateVerifyOptions.js new file mode 100644 index 00000000..f7bb88ac --- /dev/null +++ b/lib/validateVerifyOptions.js @@ -0,0 +1,42 @@ +const JsonWebTokenError = require('./JsonWebTokenError'); + +function isBoolean(value) { + return typeof value === 'boolean'; +} + +// Ordered: the first failing rule determines the reported error. +const OPTION_RULES = [ + { + name: 'clockTimestamp', + // Falsy values are accepted and fall back to the current time. + isValid: (value) => typeof value === 'number' || !value, + message: 'clockTimestamp must be a number' + }, + { name: 'clockTolerance', isValid: Number.isFinite, message: 'clockTolerance must be a finite number' }, + { name: 'ignoreExpiration', isValid: isBoolean, message: 'ignoreExpiration must be a boolean' }, + { name: 'ignoreNotBefore', isValid: isBoolean, message: 'ignoreNotBefore must be a boolean' }, + { + name: 'nonce', + isValid: (value) => typeof value === 'string' && value.trim() !== '', + message: 'nonce must be a non-empty string' + }, + { + name: 'allowInvalidAsymmetricKeyTypes', + isValid: isBoolean, + message: 'allowInvalidAsymmetricKeyTypes must be a boolean' + } +]; + +const ALLOWED_OPTIONS = [ + 'algorithms', 'audience', 'issuer', 'subject', 'jwtid', 'complete', 'maxAge' +].concat(OPTION_RULES.map((rule) => rule.name)); + +module.exports = function validateVerifyOptions(options) { + const unknownOption = Object.keys(options).find((key) => ALLOWED_OPTIONS.indexOf(key) === -1); + if (unknownOption !== undefined) { + return new JsonWebTokenError('"' + unknownOption + '" is not an allowed option'); + } + + const failedRule = OPTION_RULES.find((rule) => options[rule.name] !== undefined && !rule.isValid(options[rule.name])); + return failedRule ? new JsonWebTokenError(failedRule.message) : null; +}; diff --git a/test/claim-exp.test.js b/test/claim-exp.test.js index fbdbc522..8bd2d48b 100644 --- a/test/claim-exp.test.js +++ b/test/claim-exp.test.js @@ -134,7 +134,7 @@ describe('expires', function() { const header = { alg: 'HS256' }; const payload = { exp }; const token = jws.sign({ header, payload, secret: 'secret', encoding: 'utf8' }); - testUtils.verifyJWTHelper(token, 'secret', { exp }, (err) => { + testUtils.verifyJWTHelper(token, 'secret', {}, (err) => { testUtils.asyncCheck(done, () => { expect(err).to.be.instanceOf(jwt.JsonWebTokenError); expect(err).to.have.property('message', 'invalid exp value'); diff --git a/test/claim-jti.test.js b/test/claim-jti.test.js index 9721f7c7..212b2fd4 100644 --- a/test/claim-jti.test.js +++ b/test/claim-jti.test.js @@ -117,7 +117,7 @@ describe('jwtid', function() { it('should verify with "jti" in payload', function (done) { signWithJWTId(undefined, {jti: 'foo'}, (e1, token) => { - testUtils.verifyJWTHelper(token, 'secret', {jetid: 'foo'}, (e2, decoded) => { + testUtils.verifyJWTHelper(token, 'secret', {jwtid: 'foo'}, (e2, decoded) => { testUtils.asyncCheck(done, () => { expect(e1).to.be.null; expect(e2).to.be.null; diff --git a/test/claim-nbf.test.js b/test/claim-nbf.test.js index 72397de1..6f98c9ca 100644 --- a/test/claim-nbf.test.js +++ b/test/claim-nbf.test.js @@ -134,7 +134,7 @@ describe('not before', function() { const header = { alg: 'HS256' }; const payload = { nbf }; const token = jws.sign({ header, payload, secret: 'secret', encoding: 'utf8' }); - testUtils.verifyJWTHelper(token, 'secret', {nbf}, (err) => { + testUtils.verifyJWTHelper(token, 'secret', {}, (err) => { testUtils.asyncCheck(done, () => { expect(err).to.be.instanceOf(jwt.JsonWebTokenError); expect(err).to.have.property('message', 'invalid nbf value'); diff --git a/test/jwt.hs.tests.js b/test/jwt.hs.tests.js index 1f5ec2fa..300fac2d 100644 --- a/test/jwt.hs.tests.js +++ b/test/jwt.hs.tests.js @@ -97,7 +97,7 @@ describe('HS256', function() { it('should return an error when the token is expired', function(done) { var token = jwt.sign({ exp: 1 }, secret, { algorithm: 'HS256' }); - jwt.verify(token, secret, { algorithm: 'HS256' }, function(err, decoded) { + jwt.verify(token, secret, { algorithms: ['HS256'] }, function(err, decoded) { assert.isUndefined(decoded); assert.isNotNull(err); done(); @@ -106,7 +106,7 @@ describe('HS256', function() { it('should NOT return an error when the token is expired with "ignoreExpiration"', function(done) { var token = jwt.sign({ exp: 1, foo: 'bar' }, secret, { algorithm: 'HS256' }); - jwt.verify(token, secret, { algorithm: 'HS256', ignoreExpiration: true }, function(err, decoded) { + jwt.verify(token, secret, { algorithms: ['HS256'], ignoreExpiration: true }, function(err, decoded) { assert.ok(decoded.foo); assert.equal('bar', decoded.foo); assert.isNull(err); @@ -128,7 +128,7 @@ describe('HS256', function() { it('should return the "invalid token" error', function(done) { var malformedToken = token + ' '; // corrupt the token by adding a space - jwt.verify(malformedToken, secret, { algorithm: 'HS256', ignoreExpiration: true }, function(err) { + jwt.verify(malformedToken, secret, { algorithms: ['HS256'], ignoreExpiration: true }, function(err) { assert.isNotNull(err); assert.equal('JsonWebTokenError', err.name); assert.equal('invalid token', err.message); diff --git a/test/option-complete.test.js b/test/option-complete.test.js index 29320e8a..11eb9fe7 100644 --- a/test/option-complete.test.js +++ b/test/option-complete.test.js @@ -22,7 +22,7 @@ describe('complete option', function () { }, ].forEach((testCase) => { it(testCase.description, function (done) { - testUtils.verifyJWTHelper(signed, pub, { typ: 'JWT', complete: testCase.complete }, (err, decoded) => { + testUtils.verifyJWTHelper(signed, pub, { complete: testCase.complete }, (err, decoded) => { testUtils.asyncCheck(done, () => { expect(err).to.be.null; expect(decoded.header).to.have.property('alg', header.alg); @@ -39,7 +39,7 @@ describe('complete option', function () { }, ].forEach((testCase) => { it(testCase.description, function (done) { - testUtils.verifyJWTHelper(signed, pub, { typ: 'JWT', complete: testCase.complete }, (err, decoded) => { + testUtils.verifyJWTHelper(signed, pub, { complete: testCase.complete }, (err, decoded) => { testUtils.asyncCheck(done, () => { expect(err).to.be.null; expect(decoded.header).to.be.undefined; diff --git a/test/option-maxAge.test.js b/test/option-maxAge.test.js index 10340f46..ca7b7081 100644 --- a/test/option-maxAge.test.js +++ b/test/option-maxAge.test.js @@ -37,8 +37,8 @@ describe('maxAge option', function() { }, ].forEach((testCase) => { it(testCase.description, function (done) { - expect(jwt.verify(token, 'secret', {maxAge: '3s', algorithm: 'HS256'})).to.not.throw; - jwt.verify(token, 'secret', {maxAge: testCase.maxAge, algorithm: 'HS256'}, (err) => { + expect(jwt.verify(token, 'secret', {maxAge: '3s', algorithms: ['HS256']})).to.not.throw; + jwt.verify(token, 'secret', {maxAge: testCase.maxAge, algorithms: ['HS256']}, (err) => { expect(err).to.be.null; done(); }) @@ -54,11 +54,11 @@ describe('maxAge option', function() { {foo: 'bar'}, ].forEach((maxAge) => { it(`should error with value ${util.inspect(maxAge)}`, function (done) { - expect(() => jwt.verify(token, 'secret', {maxAge, algorithm: 'HS256'})).to.throw( + expect(() => jwt.verify(token, 'secret', {maxAge, algorithms: ['HS256']})).to.throw( jwt.JsonWebTokenError, '"maxAge" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60' ); - jwt.verify(token, 'secret', {maxAge, algorithm: 'HS256'}, (err) => { + jwt.verify(token, 'secret', {maxAge, algorithms: ['HS256']}, (err) => { expect(err).to.be.instanceOf(jwt.JsonWebTokenError); expect(err.message).to.equal( '"maxAge" should be a number of seconds or string representing a timespan eg: "1d", "20h", 60' diff --git a/test/verify-options-validation.test.js b/test/verify-options-validation.test.js new file mode 100644 index 00000000..7e51b732 --- /dev/null +++ b/test/verify-options-validation.test.js @@ -0,0 +1,92 @@ +'use strict'; + +const jwt = require('../'); +const expect = require('chai').expect; + +const SECRET = 'secret'; +const NOW = 1700000000; + +function expiredToken() { + return jwt.sign({ exp: NOW - 60 }, SECRET, { noTimestamp: true }); +} + +function notYetValidToken() { + return jwt.sign({ nbf: NOW + 600 }, SECRET, { noTimestamp: true }); +} + +describe('verify: validación de opciones (HU-01, HU-02, HU-03)', function () { + describe('HU-01 "clockTolerance" debe ser un número finito', function () { + ['abc', '3600', Infinity, -Infinity, NaN, {}, [], true].forEach(function (value) { + it('rechaza un token vencido con clockTolerance de ' + String(typeof value === 'object' ? JSON.stringify(value) : value), function () { + expect(function () { + jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, clockTolerance: value }); + }).to.throw(jwt.JsonWebTokenError, 'clockTolerance must be a finite number'); + }); + }); + + it('reporta el error por callback y no lo lanza', function (done) { + jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, clockTolerance: 'abc' }, function (err, decoded) { + expect(err).to.be.instanceOf(jwt.JsonWebTokenError); + expect(err.message).to.equal('clockTolerance must be a finite number'); + expect(decoded).to.equal(undefined); + done(); + }); + }); + + it('conserva el comportamiento con un número válido', function () { + expect(jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, clockTolerance: 120 })).to.have.property('exp', NOW - 60); + expect(function () { + jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, clockTolerance: 10 }); + }).to.throw(jwt.TokenExpiredError); + }); + }); + + describe('HU-02 "ignoreExpiration" e "ignoreNotBefore" deben ser booleanos', function () { + ['false', 'true', 0, 1, {}, []].forEach(function (value) { + it('rechaza ignoreExpiration de ' + JSON.stringify(value), function () { + expect(function () { + jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, ignoreExpiration: value }); + }).to.throw(jwt.JsonWebTokenError, 'ignoreExpiration must be a boolean'); + }); + + it('rechaza ignoreNotBefore de ' + JSON.stringify(value), function () { + expect(function () { + jwt.verify(notYetValidToken(), SECRET, { clockTimestamp: NOW, ignoreNotBefore: value }); + }).to.throw(jwt.JsonWebTokenError, 'ignoreNotBefore must be a boolean'); + }); + }); + + it('conserva el comportamiento con booleanos o sin la opción', function () { + expect(jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, ignoreExpiration: true })).to.have.property('exp'); + expect(jwt.verify(notYetValidToken(), SECRET, { clockTimestamp: NOW, ignoreNotBefore: true })).to.have.property('nbf'); + expect(function () { + jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW, ignoreExpiration: false }); + }).to.throw(jwt.TokenExpiredError); + expect(function () { + jwt.verify(expiredToken(), SECRET, { clockTimestamp: NOW }); + }).to.throw(jwt.TokenExpiredError); + }); + }); + + describe('HU-03 opciones desconocidas no se ignoran en silencio', function () { + ['algoritms', 'ignoreExpiracion', 'expiresIn'].forEach(function (name) { + it('rechaza la opción desconocida "' + name + '"', function () { + const options = {}; + options[name] = ['HS512']; + expect(function () { + jwt.verify(jwt.sign({ a: 1 }, SECRET), SECRET, options); + }).to.throw(jwt.JsonWebTokenError, '"' + name + '" is not an allowed option'); + }); + }); + + it('acepta todas las opciones documentadas de verify', function () { + const token = jwt.sign({ a: 1, nonce: 'n1' }, SECRET, { audience: 'aud', issuer: 'iss', subject: 'sub', jwtid: 'jid' }); + const decoded = jwt.verify(token, SECRET, { + algorithms: ['HS256'], audience: 'aud', issuer: 'iss', subject: 'sub', jwtid: 'jid', + complete: false, ignoreExpiration: false, ignoreNotBefore: false, clockTolerance: 5, + maxAge: '1h', nonce: 'n1', clockTimestamp: Math.floor(Date.now() / 1000), allowInvalidAsymmetricKeyTypes: false + }); + expect(decoded).to.have.property('a', 1); + }); + }); +}); diff --git a/test/verify.tests.js b/test/verify.tests.js index 88500756..78ed4334 100644 --- a/test/verify.tests.js +++ b/test/verify.tests.js @@ -23,7 +23,7 @@ describe('verify', function() { encoding: 'utf8' }); - jwt.verify(signed, pub, {typ: 'JWT'}, function(err, p) { + jwt.verify(signed, pub, {}, function(err, p) { assert.isNull(err); assert.deepEqual(p, payload); done(); @@ -42,7 +42,7 @@ describe('verify', function() { }); expect(function () { - jwt.verify(signed, 'secret', {typ: 'JWT'}); + jwt.verify(signed, 'secret', {}); }).to.throw(JsonWebTokenError, /jwt signature is required/); }); @@ -58,7 +58,7 @@ describe('verify', function() { }); expect(function () { - jwt.verify(signed, undefined, {typ: 'JWT'}); + jwt.verify(signed, undefined, {}); }).to.throw(JsonWebTokenError, /please specify "none" in "algorithms" to verify unsigned tokens/); }); @@ -73,7 +73,7 @@ describe('verify', function() { encoding: 'utf8' }); - jwt.verify(signed, null, {typ: 'JWT', algorithms: ['none']}, function(err, p) { + jwt.verify(signed, null, {algorithms: ['none']}, function(err, p) { assert.isNull(err); assert.deepEqual(p, payload); done(); @@ -83,7 +83,7 @@ describe('verify', function() { it('should not mutate options', function (done) { const header = { alg: 'HS256' }; const payload = { iat: Math.floor(Date.now() / 1000 ) }; - const options = { typ: 'JWT' }; + const options = {}; const signed = jws.sign({ header: header, payload: payload, @@ -93,7 +93,7 @@ describe('verify', function() { jwt.verify(signed, 'secret', options, function(err) { assert.isNull(err); - assert.deepEqual(Object.keys(options).length, 1); + assert.deepEqual(Object.keys(options).length, 0); done(); }); }); diff --git a/verify.js b/verify.js index cdbfdc45..e4816058 100644 --- a/verify.js +++ b/verify.js @@ -4,6 +4,7 @@ const TokenExpiredError = require('./lib/TokenExpiredError'); const decode = require('./decode'); const timespan = require('./lib/timespan'); const validateAsymmetricKey = require('./lib/validateAsymmetricKey'); +const validateVerifyOptions = require('./lib/validateVerifyOptions'); const PS_SUPPORTED = require('./lib/psSupported'); const jws = require('jws'); const {KeyObject, createSecretKey, createPublicKey} = require("crypto"); @@ -42,16 +43,9 @@ module.exports = function (jwtString, secretOrPublicKey, options, callback) { }; } - if (options.clockTimestamp && typeof options.clockTimestamp !== 'number') { - return done(new JsonWebTokenError('clockTimestamp must be a number')); - } - - if (options.nonce !== undefined && (typeof options.nonce !== 'string' || options.nonce.trim() === '')) { - return done(new JsonWebTokenError('nonce must be a non-empty string')); - } - - if (options.allowInvalidAsymmetricKeyTypes !== undefined && typeof options.allowInvalidAsymmetricKeyTypes !== 'boolean') { - return done(new JsonWebTokenError('allowInvalidAsymmetricKeyTypes must be a boolean')); + const optionsError = validateVerifyOptions(options); + if (optionsError) { + return done(optionsError); } const clockTimestamp = options.clockTimestamp || Math.floor(Date.now() / 1000);