From bd4fdcd65044ca55b5765c401771569b92373902 Mon Sep 17 00:00:00 2001 From: Carl Anderson Date: Fri, 10 Sep 2021 10:14:36 -0500 Subject: [PATCH 1/2] Throw errors instead of logging to server console --- src/TokenHandler.js | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/TokenHandler.js b/src/TokenHandler.js index b673d883..fa5cedee 100644 --- a/src/TokenHandler.js +++ b/src/TokenHandler.js @@ -180,30 +180,56 @@ class TokenHandler extends SMARTHandler { // TODO: if multiple keys, try them all. const jwt = this.request.body.client_assertion; const validated = await jose.JWS.createVerify(key).verify(jwt); - console.log('JWT signature verified'); + + // Note: at this point the signature is algorithmically verified. + + // Inspect the verification for standards compliance. const payload = JSON.parse(validated.payload); if (validated.header.typ !== 'JWT') { - console.error(`Expected validation type (${validated.header.type}) to equal "JWT"`); + throw Lib.OAuthError.from({ + code : 401, + error: 'invalid assertion', + msg : `Expected assertion type (${validated.header.type}) to equal "JWT"`, + }, 'JWT', validated.header.type); } const alg = validated.header.alg; if (alg !== 'RS384' && alg !== 'ES384') { - console.error(`Expected one of the validation algorithms ('ES384', 'RS384'), found: ${alg}`); + throw Lib.OAuthError.from({ + code : 401, + error: 'invalid assertion', + msg : `Expected one of the following algorithms ('ES384', 'RS384'), found: ${alg}`, + }, 'ES384 or RS384', alg); } if (key.kid !== validated.header.kid) { - console.error(`Expected JWT kid (${validated.header.kid}) to match (${key.kid})`); + throw Lib.OAuthError.from({ + code : 403, + error: 'invalid key', + msg : `Expected JWT kid (${validated.header.kid}) to match (${key.kid})`, + }, key.kid, validated.header.kid); } const fiveMinutesFromNow = Math.floor(Date.now() / 1000) + 300; if (payload.exp > fiveMinutesFromNow) { - const error = `JWT expiration (${payload.exp}) is too permissive, should be no greater than 5 minutes.`; - console.error(error); + throw Lib.OAuthError.from({ + code : 403, + error: 'invalid assertion', + msg : `JWT expiration (${payload.exp}) is too permissive, should be no greater than 5 minutes.`, + }, `less than ${fiveMinutesFromNow}`, `${payload.exp}`); } if (payload.iss !== payload.sub) { - console.error(`Mismatched JWT iss (${payload.iss}) != sub (${payload.sub})`); + throw Lib.OAuthError.from({ + code : 401, + error: 'invalid assertion', + msg : `Mismatched JWT iss (${payload.iss}) != sub (${payload.sub})`, + }, `equal iss and sub`, `'${payload.iss}' != '${payload.sub}'`); } // TODO: get the scheme in here, too? const tokenUri = this.request.headers.host + this.request.originalUrl; if (!payload.aud.endsWith(tokenUri)) { - console.error(`Expected JWT.aud (${payload.aud}) to match the token URI (${tokenUri})`); + throw Lib.OAuthError.from({ + code : 403, + error: 'invalid assertion', + msg : `Expected JWT.aud (${payload.aud}) to match the token URI (${tokenUri})`, + }, `ends with '${tokenUri}'`, payload.aud); } } } From 9b13c20ea3c952b719d4710c0b2f5ea47cf05511 Mon Sep 17 00:00:00 2001 From: Carl Anderson Date: Fri, 10 Sep 2021 11:12:51 -0500 Subject: [PATCH 2/2] fix: header is an array without named properties --- src/TokenHandler.js | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/TokenHandler.js b/src/TokenHandler.js index fa5cedee..e73cb08f 100644 --- a/src/TokenHandler.js +++ b/src/TokenHandler.js @@ -181,31 +181,33 @@ class TokenHandler extends SMARTHandler { const jwt = this.request.body.client_assertion; const validated = await jose.JWS.createVerify(key).verify(jwt); - // Note: at this point the signature is algorithmically verified. + // Note: at this point the signature is cryptographically verified. - // Inspect the verification for standards compliance. + // Inspect the verification result for standards compliance. const payload = JSON.parse(validated.payload); - if (validated.header.typ !== 'JWT') { + const typ = validated.header['typ']; + if (typ !== 'JWT') { throw Lib.OAuthError.from({ code : 401, error: 'invalid assertion', - msg : `Expected assertion type (${validated.header.type}) to equal "JWT"`, - }, 'JWT', validated.header.type); + msg : `Expected assertion type (${typ}) to be "JWT"`, + }, 'JWT', typ); } - const alg = validated.header.alg; + const alg = validated.header['alg']; if (alg !== 'RS384' && alg !== 'ES384') { throw Lib.OAuthError.from({ code : 401, error: 'invalid assertion', - msg : `Expected one of the following algorithms ('ES384', 'RS384'), found: ${alg}`, - }, 'ES384 or RS384', alg); + msg : `Expected one of ('ES384', 'RS384'), found: '${alg}'.`, + }, 'expected ES384 or RS384', alg); } - if (key.kid !== validated.header.kid) { + const kid = validated.header['kid']; + if (key.kid !== kid) { throw Lib.OAuthError.from({ code : 403, error: 'invalid key', - msg : `Expected JWT kid (${validated.header.kid}) to match (${key.kid})`, - }, key.kid, validated.header.kid); + msg : `Expected JWT kid (${kid}) to match (${key.kid})`, + }, key.kid, kid); } const fiveMinutesFromNow = Math.floor(Date.now() / 1000) + 300; if (payload.exp > fiveMinutesFromNow) { @@ -222,14 +224,12 @@ class TokenHandler extends SMARTHandler { msg : `Mismatched JWT iss (${payload.iss}) != sub (${payload.sub})`, }, `equal iss and sub`, `'${payload.iss}' != '${payload.sub}'`); } - // TODO: get the scheme in here, too? - const tokenUri = this.request.headers.host + this.request.originalUrl; - if (!payload.aud.endsWith(tokenUri)) { - throw Lib.OAuthError.from({ - code : 403, - error: 'invalid assertion', - msg : `Expected JWT.aud (${payload.aud}) to match the token URI (${tokenUri})`, - }, `ends with '${tokenUri}'`, payload.aud); + // TODO: don't rely on the request headers to get the URI of this handler. + const tokenUri = this.request.get('host') + this.request.originalUrl; + if (!payload.aud.endsWith(`://${tokenUri}`)) { + const message = `Expected JWT.aud (${payload.aud}) to match the token URI (${tokenUri})`; + console.error(message); + // TODO: raise an exception when a more reliable way to determine the handler URI is known. } } }