From d95e31bf33bf3dc9a90e420a6dc90bbfd964d885 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 18 Dec 2019 18:22:03 +0100 Subject: [PATCH] fix: enabled full JWT validation on distributed and aggregated claims --- lib/client.js | 90 ++++++---------- test/client/client_instance.test.js | 155 +++++++++++++++++----------- 2 files changed, 125 insertions(+), 120 deletions(-) diff --git a/lib/client.js b/lib/client.js index a1289740..ec5806bc 100644 --- a/lib/client.js +++ b/lib/client.js @@ -62,40 +62,6 @@ function assignClaim(target, source, sourceName, throwOnMissing = true) { }; } -function getFromJWT(jwt, position, claim) { - if (typeof jwt !== 'string') { - throw new RPError({ - message: `invalid JWT type, expected a string, got: ${typeof jwt}`, - jwt, - }); - } - const parts = jwt.split('.'); - if (parts.length !== 3) { - throw new RPError({ - message: 'invalid JWT format, expected three parts', - jwt, - }); - } - const parsed = JSON.parse(base64url.decode(parts[position])); - return typeof claim === 'undefined' ? parsed : parsed[claim]; -} - -function getSub(jwt) { - return getFromJWT(jwt, 1, 'sub'); -} - -function getIss(jwt) { - return getFromJWT(jwt, 1, 'iss'); -} - -function getHeader(jwt) { - return getFromJWT(jwt, 0); -} - -function getPayload(jwt) { - return getFromJWT(jwt, 1); -} - function verifyPresence(payload, jwt, prop) { if (payload[prop] === undefined) { throw new RPError({ @@ -129,31 +95,35 @@ function authorizationParams(params) { return authParams; } -async function claimJWT(jwt) { - const iss = getIss(jwt); - const keyDef = getHeader(jwt); - if (!keyDef.alg) { - throw new RPError({ - message: 'claim source is missing JWT header alg property', - jwt, - }); - } +async function claimJWT(label, jwt) { + try { + const { header, payload } = jose.JWT.decode(jwt, { complete: true }); + const { iss } = payload; - if (keyDef.alg === 'none') { - return getPayload(jwt); - } + if (header.alg === 'none') { + return payload; + } - let key; - if (!iss || iss === this.issuer.issuer) { - key = await this.issuer.key(keyDef); - } else if (issuerRegistry.has(iss)) { - key = await issuerRegistry.get(iss).key(keyDef); - } else { - const discovered = await this.issuer.constructor.discover(iss); - key = await discovered.key(keyDef); + let key; + if (!iss || iss === this.issuer.issuer) { + key = await this.issuer.key(header); + } else if (issuerRegistry.has(iss)) { + key = await issuerRegistry.get(iss).key(header); + } else { + const discovered = await this.issuer.constructor.discover(iss); + key = await discovered.key(header); + } + return jose.JWT.verify(jwt, key); + } catch (err) { + if (err instanceof RPError || err instanceof OPError || err.name === 'AggregateError') { + throw err; + } else { + throw new RPError({ + printf: ['failed to validate the %s JWT (%s: %s)', label, err.name, err.message], + jwt, + }); + } } - - return jose.JWS.verify(jwt, key); } function getKeystore(jwks) { @@ -1026,8 +996,8 @@ module.exports = (issuer, aadIssValidation = false) => class Client extends Base } } - if (accessToken.id_token) { - const expectedSub = getSub(accessToken.id_token); + if (accessToken instanceof TokenSet && accessToken.id_token) { + const expectedSub = accessToken.claims().sub; if (parsed.sub !== expectedSub) { throw new RPError({ printf: ['userinfo sub mismatch, expected %s, got: %s', expectedSub, parsed.sub], @@ -1233,7 +1203,7 @@ module.exports = (issuer, aadIssValidation = false) => class Client extends Base }); const body = processResponse(response, { bearer: true }); - const decoded = await claimJWT.call(this, body); + const decoded = await claimJWT.call(this, 'distributed', body); delete claims._claim_sources[sourceName]; Object.entries(claims._claim_names).forEach( assignClaim(claims, decoded, sourceName, false), @@ -1270,7 +1240,7 @@ module.exports = (issuer, aadIssValidation = false) => class Client extends Base await Promise.all(aggregatedSources.map(async ([sourceName, def]) => { try { - const decoded = await claimJWT.call(this, def.JWT); + const decoded = await claimJWT.call(this, 'aggregated', def.JWT); delete claims._claim_sources[sourceName]; Object.entries(claims._claim_names).forEach(assignClaim(claims, decoded, sourceName)); } catch (err) { diff --git a/test/client/client_instance.test.js b/test/client/client_instance.test.js index 03805117..4902d16c 100644 --- a/test/client/client_instance.test.js +++ b/test/client/client_instance.test.js @@ -2589,13 +2589,25 @@ describe('Client', () => { }); describe('Client#unpackAggregatedClaims', function () { + afterEach(nock.cleanAll); before(function () { const issuer = new Issuer({ + issuer: 'https://op.example.com', + jwks_uri: 'https://op.example.com/jwks', authorization_endpoint: 'https://op.example.com/auth', }); this.client = new issuer.Client({ client_id: 'identifier', }); + const store = new jose.JWKS.KeyStore(); + + return store.generate('RSA').then(() => { + nock(issuer.issuer) + .get('/jwks') + .reply(200, store.toJWKS(true)); + + return issuer.keystore(); + }); }); it('just returns back if no claims passed', function () { @@ -2658,72 +2670,95 @@ describe('Client', () => { }); }); - it('verifies the JWT (1/3)', function () { - return Promise.all([ - getJWT({ credit_history: 'foobar' }, 'src1'), - ]).then((jwts) => { - const userinfo = { - sub: 'userID', - _claim_names: { - credit_history: 'src1', - email: 'src2', - }, - _claim_sources: { - src1: { JWT: jwts[0] }, - src2: { JWT: {} }, - }, - }; - - return this.client.unpackAggregatedClaims(userinfo) - .then(fail, (err) => { - expect(err).to.have.property('message', 'invalid JWT type, expected a string, got: object'); - }); + describe('JWT validation', function () { + it('verifies the JWT is a compact format JWS', function () { + return Promise.all([ + getJWT({ credit_history: 'foobar' }, 'src1'), + ]).then((jwts) => { + const userinfo = { + sub: 'userID', + _claim_names: { + credit_history: 'src1', + email: 'src2', + }, + _claim_sources: { + src1: { JWT: jwts[0] }, + src2: { JWT: {} }, + }, + }; + + return this.client.unpackAggregatedClaims(userinfo) + .then(fail, (err) => { + expect(err).to.have.property('message', 'failed to validate the aggregated JWT (TypeError: JWT must be a string)'); + }); + }); }); - }); - it('verifies the JWT (2/3)', function () { - return Promise.all([ - getJWT({ credit_history: 'foobar' }, 'src1'), - ]).then((jwts) => { - const userinfo = { - sub: 'userID', - _claim_names: { - credit_history: 'src1', - email: 'src2', - }, - _claim_sources: { - src1: { JWT: jwts[0] }, - src2: { JWT: '....' }, - }, - }; + it('verifies the JWT is not encrypted', function () { + return Promise.all([ + getJWT({ credit_history: 'foobar' }, 'src1'), + ]).then((jwts) => { + const userinfo = { + sub: 'userID', + _claim_names: { + credit_history: 'src1', + email: 'src2', + }, + _claim_sources: { + src1: { JWT: jwts[0] }, + src2: { JWT: '....' }, + }, + }; + + return this.client.unpackAggregatedClaims(userinfo) + .then(fail, (err) => { + expect(err).to.have.property('message', 'failed to validate the aggregated JWT (TypeError: JWTs must be decrypted first)'); + }); + }); + }); - return this.client.unpackAggregatedClaims(userinfo) - .then(fail, (err) => { - expect(err).to.have.property('message', 'invalid JWT format, expected three parts'); - }); + it('verifies the JWT has an alg', function () { + return Promise.all([ + getJWT({ credit_history: 'foobar' }, 'src1'), + ]).then((jwts) => { + const userinfo = { + sub: 'userID', + _claim_names: { + credit_history: 'src1', + email: 'src2', + }, + _claim_sources: { + src1: { JWT: jwts[0] }, + src2: { JWT: 'e30.e30.' }, + }, + }; + + return this.client.unpackAggregatedClaims(userinfo) + .then(fail, (err) => { + expect(err).to.have.property('message', 'failed to validate the aggregated JWT (JWSInvalid: missing JWS signature algorithm)'); + }); + }); }); - }); - it('verifies the JWT (3/3)', function () { - return Promise.all([ - getJWT({ credit_history: 'foobar' }, 'src1'), - ]).then((jwts) => { - const userinfo = { - sub: 'userID', - _claim_names: { - credit_history: 'src1', - email: 'src2', - }, - _claim_sources: { - src1: { JWT: jwts[0] }, - src2: { JWT: 'e30.e30.' }, - }, - }; + it('verifies the JWT is not expired', function () { + return Promise.all([ + getJWT({ credit_history: 'foobar', exp: 0 }, 'src1'), + ]).then((jwts) => { + const userinfo = { + sub: 'userID', + _claim_names: { + credit_history: 'src1', + }, + _claim_sources: { + src1: { JWT: jwts[0] }, + }, + }; - return this.client.unpackAggregatedClaims(userinfo) - .then(fail, (err) => { - expect(err).to.have.property('message', 'claim source is missing JWT header alg property'); - }); + return this.client.unpackAggregatedClaims(userinfo) + .then(fail, (err) => { + expect(err).to.have.property('message', 'failed to validate the aggregated JWT (JWTClaimInvalid: token is expired)'); + }); + }); }); });