From a3539b094137856dde330d7c21e3afbe97586ab6 Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Thu, 21 Oct 2021 13:21:41 -0600 Subject: [PATCH] feature: add additional (standardized) error messages --- CHANGELOG.md | 11 ++++ README.md | 59 ++++++++++++++++- keyfetch.js | 76 +++++++++++---------- lib/errors.js | 179 ++++++++++++++++++++++++++++++++++---------------- 4 files changed, 234 insertions(+), 91 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..76720ac --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,11 @@ +# v3.0.0 + +**Breaking Change**: Standardize error `message`s (now they're more client-friendly). + +# v2.1.0 + +Feature: Add `code`, `status`, and `details` to errors. + +# v2.0.0 + +**Breaking Change**: require `issuers` array (rather than `["*"]` by default). diff --git a/README.md b/README.md index 46a1444..a4b34b2 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Works great for - [x] `jsonwebtoken` (Auth0) - [x] OIDC (OpenID Connect) -- [x] .well-known/jwks.json (Auth0) +- [x] .well-known/jwks.json (Auth0, Okta) - [x] Other JWKs URLs Crypto Support @@ -19,8 +19,19 @@ Crypto Support - [x] JWT verification - [x] RSA (all variants) - [x] EC / ECDSA (NIST variants P-256, P-384) +- [x] Sane error codes - [ ] esoteric variants (excluded to keep the code featherweight and secure) +# Table of Contents + +- Install +- Usage +- API + - Auth0 / Okta + - OIDC +- Errors +- Change Log + # Install ```bash @@ -248,3 +259,49 @@ keyfetch.init({ There is no background task to cleanup expired keys as of yet. For now you can limit the number of keys fetched by having a simple whitelist. + +# Errors + +`JSON.stringify()`d errors look like this: + +```js +{ + code: "INVALID_JWT", + status: 401, + details: [ "jwt.claims.exp = 1634804500", "DEBUG: helpful message" ] + message: "token's 'exp' has passed or could not parsed: 1634804500" +} +``` + +SemVer Compatibility: + +- `code` & `status` will remain the same. +- The `message` property of an error is **NOT** included in the semver compatibility guarantee (we intend to make them more client-friendly), neither is `detail` at this time (but it will be once we decide on what it should be). + +For backwards compatibility with v1, the non-stringified `message` is the same as what it was in v1 (and the v2 message is `client_message`, which replaces `message` in v3). Don't rely on it. Rely on `code`. + +| Hint | Code | Status | Message (truncated) | +| ------------------- | --------------- | ------ | ------------------------------------------------ | +| (developer error) | DEVELOPER_ERROR | 500 | test... | +| (bad gateway) | BAD_GATEWAY | 502 | The token could not be verified because our s... | +| (insecure issuer) | MALFORMED_JWT | 400 | 'test' is NOT secure. Set env 'KEYFETCH_ALLOW... | +| (parse error) | MALFORMED_JWT | 400 | could not parse jwt: 'test'... | +| (no issuer) | MALFORMED_JWT | 400 | 'iss' is not defined... | +| (malformed exp) | MALFORMED_JWT | 400 | token's 'exp' has passed or could not parsed:... | +| (expired) | INVALID_JWT | 401 | token's 'exp' has passed or could not parsed:... | +| (inactive) | INVALID_JWT | 401 | token's 'nbf' has not been reached or could n... | +| (bad signature) | INVALID_JWT | 401 | token signature verification was unsuccessful... | +| (jwk not found old) | INVALID_JWT | 401 | Retrieved a list of keys, but none of them ma... | +| (jwk not found) | INVALID_JWT | 401 | No JWK found by kid or thumbprint 'test'... | +| (no jwkws uri) | INVALID_JWT | 401 | Failed to retrieve openid configuration... | +| (unknown issuer) | INVALID_JWT | 401 | token was issued by an untrusted issuer: 'tes... | +| (failed claims) | INVALID_JWT | 401 | token did not match on one or more authorizat... | + +# Change Log + +Minor Breaking changes (with a major version bump): + +- v3.0.0 - reworked error messages (also available in v2.1.0 as `client_message`) +- v2.0.0 - changes from the default `issuers = ["*"]` to requiring that an issuer (or public jwk for verification) is specified + +See other changes in [CHANGELOG.md](./CHANGELOG.md). diff --git a/keyfetch.js b/keyfetch.js index 2999fed..df302e2 100644 --- a/keyfetch.js +++ b/keyfetch.js @@ -19,7 +19,7 @@ async function requestAsync(req) { // differentiate potentially temporary server errors from 404 if (!resp.ok && (resp.statusCode >= 500 || resp.statusCode < 200)) { - throw Errors.BAD_GATEWAY(); + throw Errors.BAD_GATEWAY({ response: resp }); } return resp; @@ -37,6 +37,8 @@ function checkMinDefaultMax(opts, key, n, d, x) { } } +keyfetch._errors = Errors; + keyfetch._clear = function () { keyCache = {}; }; @@ -46,14 +48,15 @@ keyfetch.init = function (opts) { staletime = checkMinDefaultMax(opts, "staletime", 1 * 60, staletime, 31 * 24 * 60 * 60); }; keyfetch._oidc = async function (iss) { + var url = normalizeIss(iss) + "/.well-known/openid-configuration"; var resp = await requestAsync({ - url: normalizeIss(iss) + "/.well-known/openid-configuration", + url: url, json: true }); var oidcConf = resp.body; if (!oidcConf.jwks_uri) { - throw Errors.OIDC_CONFIG_NOT_FOUND(); + throw Errors.NO_JWKS_URI(url); } return oidcConf; }; @@ -193,7 +196,7 @@ keyfetch._setCache = function (iss, cacheable) { function normalizeIss(iss) { if (!iss) { - throw Errors.TOKEN_NO_ISSUER(); + throw Errors.NO_ISSUER(); } // We definitely don't want false negatives stemming @@ -217,7 +220,7 @@ keyfetch.jwt.decode = function (jwt) { obj.claims = JSON.parse(Buffer.from(obj.payload, "base64")); return obj; } catch (e) { - var err = Errors.TOKEN_PARSE_ERROR(jwt); + var err = Errors.PARSE_ERROR(jwt); err.details = e.message; throw err; } @@ -227,7 +230,7 @@ keyfetch.jwt.verify = async function (jwt, opts) { opts = {}; } - var decoded; + var jws; var exp; var nbf; var active; @@ -249,60 +252,68 @@ keyfetch.jwt.verify = async function (jwt, opts) { } var claims = opts.claims || {}; if (!jwt || "string" === typeof jwt) { - decoded = keyfetch.jwt.decode(jwt); + jws = keyfetch.jwt.decode(jwt); } else { - decoded = jwt; + jws = jwt; } - if (!decoded.claims.iss || !issuers.some(isTrustedIssuer(decoded.claims.iss))) { + if (!jws.claims.iss || !issuers.some(isTrustedIssuer(jws.claims.iss))) { if (!(opts.jwk || opts.jwks)) { - throw Errors.ISSUER_NOT_TRUSTED(decoded.claims.iss || ""); + throw Errors.UNKNOWN_ISSUER(jws.claims.iss || ""); } } // Note claims.iss validates more strictly than opts.issuers (requires exact match) - if ( - !Object.keys(claims).every(function (key) { - if (claims[key] === decoded.claims[key]) { + var failedClaims = Object.keys(claims) + .filter(function (key) { + if (claims[key] !== jws.claims[key]) { return true; } }) - ) { - throw Errors.CLAIMS_MISMATCH(Object.keys(claims)); + .map(function (key) { + return "jwt.claims." + key + " = " + JSON.stringify(jws.claims[key]); + }); + if (failedClaims.length) { + throw Errors.FAILED_CLAIMS(failedClaims, Object.keys(claims)); } - exp = decoded.claims.exp; + exp = jws.claims.exp; if (exp && false !== opts.exp) { now = Date.now(); // TODO document that opts.exp can be used as leeway? Or introduce opts.leeway? + // fair, but not necessary + exp = parseInt(exp, 10); + if (isNaN(exp)) { + throw Errors.MALFORMED_EXP(JSON.stringify(jws.claims.exp)); + } then = (opts.exp || 0) + parseInt(exp, 10); active = then - now / 1000 > 0; // expiration was on the token or, if not, such a token is not allowed if (!active) { - throw Errors.TOKEN_EXPIRED(exp); + throw Errors.EXPIRED(exp); } } - nbf = decoded.claims.nbf; + nbf = jws.claims.nbf; if (nbf) { active = parseInt(nbf, 10) - Date.now() / 1000 <= 0; if (!active) { - throw Errors.TOKEN_INACTIVE(nbf); + throw Errors.INACTIVE(nbf); } } if (opts.jwks || opts.jwk) { return overrideLookup(opts.jwks || [opts.jwk]); } - var kid = decoded.header.kid; + var kid = jws.header.kid; var iss; var fetcher; var fetchOne; if (!opts.strategy || "oidc" === opts.strategy) { - iss = decoded.claims.iss; + iss = jws.claims.iss; fetcher = keyfetch.oidcJwks; fetchOne = keyfetch.oidcJwk; } else if ("auth0" === opts.strategy || "well-known" === opts.strategy) { - iss = decoded.claims.iss; + iss = jws.claims.iss; fetcher = keyfetch.wellKnownJwks; fetchOne = keyfetch.wellKnownJwk; } else { @@ -317,10 +328,10 @@ keyfetch.jwt.verify = async function (jwt, opts) { return fetcher(iss).then(verifyAny); function verifyOne(hit) { - if (true === keyfetch.jws.verify(decoded, hit)) { - return decoded; + if (true === keyfetch.jws.verify(jws, hit)) { + return jws; } - throw Errors.TOKEN_INVALID_SIGNATURE(); + throw Errors.BAD_SIGNATURE(jws.protected + "." + jws.payload + "." + jws.signature); } function verifyAny(hits) { @@ -330,20 +341,19 @@ keyfetch.jwt.verify = async function (jwt, opts) { if (kid !== hit.jwk.kid && kid !== hit.thumbprint) { return; } - if (true === keyfetch.jws.verify(decoded, hit)) { - return true; - } - throw Errors.TOKEN_INVALID_SIGNATURE(); - } else { - if (true === keyfetch.jws.verify(decoded, hit)) { + if (true === keyfetch.jws.verify(jws, hit)) { return true; } + throw Errors.BAD_SIGNATURE(); + } + if (true === keyfetch.jws.verify(jws, hit)) { + return true; } }) ) { - return decoded; + return jws; } - throw Errors.TOKEN_UNKNOWN_SIGNER(); + throw Errors.JWK_NOT_FOUND_OLD(kid); } function overrideLookup(jwks) { diff --git a/lib/errors.js b/lib/errors.js index 971a1fb..26cf543 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -19,30 +19,46 @@ * }} opts * @returns {AuthError} */ -function create(msg, { status = 401, code = "", details }) { +function create(old, msg, code, status, details) { /** @type AuthError */ //@ts-ignore - var err = new Error(msg); - err.message = err.message; - err.status = status; + var err = new Error(old); + err.client_message = msg; err.code = code; + err.status = status; if (details) { err.details = details; } err.source = "keyfetch"; + err.toJSON = toJSON; + err.toString = toString; return err; } +function toJSON() { + /*jshint validthis:true*/ + return { + message: this.client_message, + status: this.status, + code: this.code, + details: this.details + }; +} +function toString() { + /*jshint validthis:true*/ + return this.stack + "\n" + JSON.stringify(this); +} + // DEVELOPER_ERROR - a good token won't make a difference var E_DEVELOPER = "DEVELOPER_ERROR"; // BAD_GATEWAY - there may be a temporary error fetching the public or or whatever var E_BAD_GATEWAY = "BAD_GATEWAY"; -// MALFORMED_TOKEN - the token could not be verified - not parsable, missing claims, etc +// MALFORMED_JWT - the token could not be verified - not parsable, missing claims, etc var E_MALFORMED = "MALFORMED_JWT"; -// INVALID_TOKEN - the token's properties don't meet requirements - iss, claims, sig, exp +// INVALID_JWT - the token's properties don't meet requirements - iss, claims, sig, exp var E_INVALID = "INVALID_JWT"; module.exports = { @@ -54,12 +70,20 @@ module.exports = { * @param {string} msg * @returns {AuthError} */ - DEVELOPER_ERROR: function (msg) { - return create(msg, { status: 500, code: E_DEVELOPER }); + DEVELOPER_ERROR: function (old, msg, details) { + return create(old, msg, E_DEVELOPER, 500, details); }, - BAD_GATEWAY: function (/*err*/) { - var msg = "The server encountered a network error or a bad gateway."; - return create(msg, { status: 502, code: E_BAD_GATEWAY }); + BAD_GATEWAY: function (err) { + var msg = + "The token could not be verified because our server encountered a network error (or a bad gateway) when connecting to its issuing server."; + var details = []; + if (err.message) { + details.push("error.message = " + err.message); + } + if (err.response && err.response.statusCode) { + details.push("response.statusCode = " + err.response.statusCode); + } + return create(msg, msg, E_BAD_GATEWAY, 502); }, // @@ -71,25 +95,46 @@ module.exports = { * @returns {AuthError} */ INSECURE_ISSUER: function (iss) { - var msg = + var old = "'" + iss + "' is NOT secure. Set env 'KEYFETCH_ALLOW_INSECURE_HTTP=true' to allow for testing. (iss)"; - return create(msg, { status: 400, code: E_MALFORMED }); + var details = [ + "jwt.claims.iss = " + JSON.stringify(iss), + "DEBUG: Set ENV 'KEYFETCH_ALLOW_INSECURE_HTTP=true' to allow insecure issuers (for testing)." + ]; + var msg = + 'The token could not be verified because our server could connect to its issuing server ("iss") securely.'; + return create(old, msg, E_MALFORMED, 400, details); }, /** * @param {string} jwt * @returns {AuthError} */ - TOKEN_PARSE_ERROR: function (jwt) { - var msg = "could not parse jwt: '" + jwt + "'"; - return create(msg, { status: 400, code: E_MALFORMED }); + PARSE_ERROR: function (jwt) { + var old = "could not parse jwt: '" + jwt + "'"; + var msg = "The auth token is malformed."; + var details = ["jwt = " + JSON.stringify(jwt)]; + return create(old, msg, E_MALFORMED, 400, details); }, /** * @param {string} iss * @returns {AuthError} */ - TOKEN_NO_ISSUER: function (iss) { - var msg = "'iss' is not defined"; - return create(msg, { status: 400, code: E_MALFORMED }); + NO_ISSUER: function (iss) { + var old = "'iss' is not defined"; + var msg = 'The token could not be verified because it doesn\'t specify an issuer ("iss").'; + var details = ["jwt.claims.iss = " + JSON.stringify(iss)]; + return create(old, msg, E_MALFORMED, 400, details); + }, + + /** + * @param {string} iss + * @returns {AuthError} + */ + MALFORMED_EXP: function (exp) { + var old = "token's 'exp' has passed or could not parsed: '" + exp + "'"; + var msg = 'The auth token could not be verified because it\'s expiration date ("exp") could not be read'; + var details = ["jwt.claims.exp = " + JSON.stringify(exp)]; + return create(old, msg, E_MALFORMED, 400, details); }, // @@ -100,77 +145,97 @@ module.exports = { * @param {number} exp * @returns {AuthError} */ - TOKEN_EXPIRED: function (exp) { - //var msg = "The auth token is expired. (exp='" + exp + "')"; - var msg = "token's 'exp' has passed or could not parsed: '" + exp + "'"; - return create(msg, { code: E_INVALID }); + EXPIRED: function (exp) { + var old = "token's 'exp' has passed or could not parsed: '" + exp + "'"; + // var msg = "The auth token did not pass verification because it is expired.not properly signed."; + var msg = "The auth token is expired. To try again, go to the main page and sign in."; + var details = ["jwt.claims.exp = " + JSON.stringify(exp)]; + return create(old, msg, E_INVALID, 401, details); }, /** * @param {number} nbf * @returns {AuthError} */ - TOKEN_INACTIVE: function (nbf) { - //var msg = "The auth token is not active yet. (nbf='" + nbf + "')"; - var msg = "token's 'nbf' has not been reached or could not parsed: '" + nbf + "'"; - return create(msg, { code: E_INVALID }); + INACTIVE: function (nbf) { + var old = "token's 'nbf' has not been reached or could not parsed: '" + nbf + "'"; + var msg = "The auth token isn't valid yet. It's activation date (\"nbf\") is in the future."; + var details = ["jwt.claims.nbf = " + JSON.stringify(nbf)]; + return create(old, msg, E_INVALID, 401, details); }, /** @returns {AuthError} */ - TOKEN_INVALID_SIGNATURE: function () { - //var msg = "The auth token is not properly signed and could not be verified."; - var msg = "token signature verification was unsuccessful"; - return create(msg, { code: E_INVALID }); + BAD_SIGNATURE: function (jwt) { + var old = "token signature verification was unsuccessful"; + var msg = "The auth token did not pass verification because it is not properly signed."; + var details = ["jwt = " + JSON.stringify(jwt)]; + return create(old, msg, E_INVALID, 401, details); }, - /** @returns {AuthError} */ - TOKEN_UNKNOWN_SIGNER: function () { - var msg = "Retrieved a list of keys, but none of them matched the 'kid' (key id) of the token."; - return create(msg, { code: E_INVALID }); + /** + * @param {string} kid + * @returns {AuthError} + */ + JWK_NOT_FOUND_OLD: function (kid) { + var old = "Retrieved a list of keys, but none of them matched the 'kid' (key id) of the token."; + var msg = + 'The auth token did not pass verification because our server couldn\'t find a mutually trusted verification key ("jwk").'; + var details = ["jws.header.kid = " + JSON.stringify(kid)]; + return create(old, msg, E_INVALID, 401, details); }, /** * @param {string} id * @returns {AuthError} */ JWK_NOT_FOUND: function (id) { - var msg = "No JWK found by kid or thumbprint '" + id + "'"; - return create(msg, { code: E_INVALID }); + // TODO Distinguish between when it's a kid vs thumbprint. + var old = "No JWK found by kid or thumbprint '" + id + "'"; + var msg = + 'The auth token did not pass verification because our server couldn\'t find a mutually trusted verification key ("jwk").'; + var details = ["jws.header.kid = " + JSON.stringify(id)]; + return create(old, msg, E_INVALID, 401, details); }, /** @returns {AuthError} */ - OIDC_CONFIG_NOT_FOUND: function () { - //var msg = "Failed to retrieve OpenID configuration for token issuer"; - var msg = "Failed to retrieve openid configuration"; - return create(msg, { code: E_INVALID }); + NO_JWKWS_URI: function (url) { + var old = "Failed to retrieve openid configuration"; + var msg = + 'The auth token did not pass verification because its issuing server did not list any verification keys ("jwks").'; + var details = ["OpenID Provider Configuration: " + JSON.stringify(url)]; + return create(old, msg, E_INVALID, 401, details); }, /** * @param {string} iss * @returns {AuthError} */ - ISSUER_NOT_TRUSTED: function (iss) { - var msg = "token was issued by an untrusted issuer: '" + iss + "'"; - return create(msg, { code: E_INVALID }); + UNKNOWN_ISSUER: function (iss) { + var old = "token was issued by an untrusted issuer: '" + iss + "'"; + var msg = "The auth token did not pass verification because it wasn't issued by a server that we trust."; + var details = ["jwt.claims.iss = " + JSON.stringify(iss)]; + return create(old, msg, E_INVALID, 401, details); }, /** - * @param {Array} claimNames + * @param {Array} details * @returns {AuthError} */ - CLAIMS_MISMATCH: function (claimNames) { - var msg = "token did not match on one or more authorization claims: '" + claimNames + "'"; - return create(msg, { code: E_INVALID }); + FAILED_CLAIMS: function (details, claimNames) { + var old = "token did not match on one or more authorization claims: '" + claimNames + "'"; + var msg = + 'The auth token did not pass verification because it failed some of the verification criteria ("claims").'; + return create(old, msg, E_INVALID, 401, details); } }; +var Errors = module.exports; // for README if (require.main === module) { - console.info("| Name | Status | Message (truncated) |"); - console.info("| ---- | ------ | ------------------- |"); + console.info("| Hint | Code | Status | Message (truncated) |"); + console.info("| ---- | ---- | ------ | ------------------- |"); Object.keys(module.exports).forEach(function (k) { //@ts-ignore var E = module.exports[k]; - var e = E(); + var e = E("test"); var code = e.code; - var msg = e.message; - if ("E_" + k !== e.code) { - code = k; - msg = e.details || msg; - } - console.info(`| ${code} | ${e.status} | ${msg.slice(0, 45)}... |`); + var msg = e.message.slice(0, 45); + var hint = k.toLowerCase().replace(/_/g, " "); + console.info(`| (${hint}) | ${code} | ${e.status} | ${msg}... |`); }); + console.log(Errors.MALFORMED_EXP()); + console.log(JSON.stringify(Errors.MALFORMED_EXP(), null, 2)); }