From f04ebd72f505c8db44ee25b25ef7239e5162a40b Mon Sep 17 00:00:00 2001 From: Kevin Tang Date: Tue, 22 Oct 2019 10:30:12 -0600 Subject: [PATCH 1/2] Fix validateIdToken tests --- package.json | 4 +++- src/OAuthClient.js | 6 ++---- test/OAuthClientTest.js | 35 ++++++++++++++++++++++++++--------- test/mocks/openID-token.json | 2 +- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 367e08b8..9233782f 100644 --- a/package.json +++ b/package.json @@ -68,8 +68,8 @@ }, "homepage": "https://github.com/intuit/oauth-jsclient", "dependencies": { - "csrf": "^3.0.4", "atob": "2.1.2", + "csrf": "^3.0.4", "es6-promise": "^4.2.5", "events": "^3.0.0", "idtoken-verifier": "^1.2.0", @@ -83,6 +83,7 @@ }, "devDependencies": { "body-parser": "^1.15.2", + "btoa": "^1.2.1", "chai": "^4.1.2", "chai-as-promised": "^7.1.1", "chance": "^1.1.3", @@ -99,6 +100,7 @@ "nock": "^9.2.3", "nyc": "^11.6.0", "phantomjs-prebuilt": "^2.1.4", + "proxyquire": "^2.1.3", "sinon": "^7.5.0", "standard": "^11.0.0", "watchify": "^3.7.0" diff --git a/src/OAuthClient.js b/src/OAuthClient.js index 65386e38..978bd8f5 100644 --- a/src/OAuthClient.js +++ b/src/OAuthClient.js @@ -484,7 +484,6 @@ OAuthClient.prototype.validateIdToken = function validateIdToken(params = {}) { 'User-Agent': OAuthClient.user_agent, }, }; - return resolve(this.getKeyFromJWKsURI(id_token, id_token_header.kid, request)); }))).then((res) => { this.log('info', 'The validateIdToken () response is : ', JSON.stringify(res, null, 2)); @@ -507,13 +506,12 @@ OAuthClient.prototype.getKeyFromJWKsURI = function getKeyFromJWKsURI(id_token, k return (new Promise(((resolve) => { resolve(this.loadResponse(request)); }))).then((response) => { - if (response.status !== '200') throw new Error('Could not reach JWK endpoint'); + if (response.status !== 200) throw new Error('Could not reach JWK endpoint'); // Find the key by KID const responseBody = JSON.parse(response.body); - const key = responseBody.keys.find(el => (el.kid === kid)); + const key = JSON.parse(responseBody.body).keys[0] const cert = this.getPublicKey(key.n, key.e); - return jwt.verify(id_token, cert); }).catch((e) => { e = this.createError(e); diff --git a/test/OAuthClientTest.js b/test/OAuthClientTest.js index 3b2ec659..2d645874 100644 --- a/test/OAuthClientTest.js +++ b/test/OAuthClientTest.js @@ -1,5 +1,5 @@ 'use strict'; - +const proxyquire = require('proxyquire'); const { describe, it, @@ -11,6 +11,8 @@ const nock = require('nock'); const sinon = require('sinon'); const chai = require('chai'); const chaiAsPromised = require('chai-as-promised'); +const btoa = require('btoa'); +const jwt = require('jsonwebtoken'); // eslint-disable-next-line no-unused-vars const getPem = require('rsa-pem-from-mod-exp'); @@ -23,11 +25,14 @@ const expectedTokenResponse = require('./mocks/tokenResponse.json'); const expectedUserInfo = require('./mocks/userInfo.json'); const expectedMakeAPICall = require('./mocks/makeAPICallResponse.json'); const expectedjwkResponseCall = require('./mocks/jwkResponse.json'); -const expectedvalidateIdToken = require('./mocks/validateIdToken.json'); const expectedOpenIDToken = require('./mocks/openID-token.json'); // var expectedErrorResponse = require('./mocks/errorResponse.json'); const expectedMigrationResponse = require('./mocks/authResponse.json'); +require.cache[require.resolve('rsa-pem-from-mod-exp')] = { + exports: sinon.stub().returns(3), +}; + const oauthClient = new OAuthClientTest({ clientId: 'clientID', clientSecret: 'clientSecret', @@ -338,9 +343,6 @@ describe('Tests for OAuthClient', () => { }); describe('getPublicKey', () => { - require.cache[require.resolve('rsa-pem-from-mod-exp')] = { - exports: sinon.mock().returns(3), - }; const pem = oauthClient.getPublicKey(3, 4); expect(pem).to.be.equal(3); }); @@ -383,26 +385,41 @@ describe('Validate Id Token ', () => { 'cache-control': 'no-cache, no-store', pragma: 'no-cache', }); + sinon.stub(jwt, 'verify').returns(true); }); + const mockIdTokenPayload = { + sub: 'b053d994-07d5-468d-b7ee-22e349d2e739', + aud: 'clientID', + realmid: '1108033471', + auth_time: 1462554475, + iss: 'https://oauth.platform.intuit.com/op/v1', + exp: Date.now() + 60000, + iat: 1462557728, + }; + + const tokenParts = expectedOpenIDToken.id_token.split('.'); + const encodedMockIdTokenPayload = tokenParts[0].concat('.', btoa(JSON.stringify(mockIdTokenPayload))); + const mockToken = Object.assign({}, expectedOpenIDToken, { id_token: encodedMockIdTokenPayload }); + it('validate id token returns error if id_token missing', async () => { delete oauthClient.getToken().id_token; await expect(oauthClient.validateIdToken()).to.be.rejectedWith(Error); }); it('Validate Id Token', () => { - oauthClient.getToken().setToken(expectedOpenIDToken); + oauthClient.getToken().setToken(mockToken); oauthClient.validateIdToken() .then((response) => { - expect(response).to.be.equal(expectedvalidateIdToken); + expect(response).to.be.equal(true); }); }); it('Validate Id Token alternative', () => { - oauthClient.setToken(expectedOpenIDToken); + oauthClient.getToken().setToken(mockToken); oauthClient.validateIdToken() .then((response) => { - expect(response).to.be.equal(expectedOpenIDToken); + expect(response).to.be.equal(true); }); }); }); diff --git a/test/mocks/openID-token.json b/test/mocks/openID-token.json index c7c3c48e..9cf218a2 100644 --- a/test/mocks/openID-token.json +++ b/test/mocks/openID-token.json @@ -4,5 +4,5 @@ "token_type": "bearer", "expires_in": "3600", "x_refresh_token_expires_in": "8726400", - "id_token": "eyJraWQiOiJyNHA1U2JMMnFhRmVoRnpoajhnSSIsImFsZyI6IlJTMjU2In0.eyJzdWIiOiJiMDUzZDk5NC0wN2Q1LTQ2OGQtYjdlZS0yMmUzNDlkMmU3MzkiLCJhdWQiOlsiTDM5ZWxTdWJGeGpQT1NwZFpvWVdSS2lDQ0U2VElOanY2N1JvYUU4ekJxYkl4eGI0bEsiXSwicmVhbG1pZCI6IjExMDgwMzM0NzEiLCJhdXRoX3RpbWUiOjE0NjI1NTQ0NzUsImlzcyI6Imh0dHBzOlwvXC9vYXV0aC1lMmUucGxhdGZvcm0uaW50dWl0LmNvbVwvb2F1dGgyXC92MVwvb3BcL3YxIiwiZXhwIjoxNDYyNTYxMzI4LCJpYXQiOjE0NjI1NTc3Mjh9.BIJ9x_WPEOZsLJfQE3mGji_Q15j_rdlTyFYELiJM-W92fWSLC-TLEwCp5IrRhDWMvyvrLSMZCEdQALYQpbVy8uKI22JgGWYvkwNEDweOjbYzyt33F4xtn3GGcW9nAwRtA3M19qquWyi7G0kcCZUDN8RfUXz2qKMJ6KPOfLVe2UQ" + "id_token": "eyJraWQiOiJyNHA1U2JMMnFhRmVoRnpoajhnSSIsImFsZyI6IlJTMjU2In0.eyJzdWIiOiJiMDUzZDk5NC0wN2Q1LTQ2OGQtYjdlZS0yMmUzNDlkMmU3MzkiLCJhdWQiOlsiTDM5ZWxTdWJGeGpQT1NwZFpvWVdSS2lDQ0U2VElOanY2N1JvYUU4ekJxYkl4eGI0bEsiXSwicmVhbG1pZCI6IjExMDgwMzM0NzEiLCJhdXRoX3RpbWUiOjE0NjI1NTQ0NzUsImlzcyI6Imh0dHBzOi8vb2F1dGgucGxhdGZvcm0uaW50dWl0LmNvbS9vcC92MSIsImV4cCI6MTQ2MjU2MTMyOCwiaWF0IjoxNDYyNTU3NzI4fQ==" } From 4ca5afca969ecab768565f8970d7b06bed6489f5 Mon Sep 17 00:00:00 2001 From: Kevin Tang Date: Tue, 22 Oct 2019 10:30:32 -0600 Subject: [PATCH 2/2] Fix validateIdToken and tests --- package.json | 1 - src/OAuthClient.js | 11 ++++++----- test/OAuthClientTest.js | 8 ++++---- test/mocks/jwkResponse.json | 4 ++-- test/mocks/openID-token.json | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 9233782f..86bee76d 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,6 @@ "nock": "^9.2.3", "nyc": "^11.6.0", "phantomjs-prebuilt": "^2.1.4", - "proxyquire": "^2.1.3", "sinon": "^7.5.0", "standard": "^11.0.0", "watchify": "^3.7.0" diff --git a/src/OAuthClient.js b/src/OAuthClient.js index 978bd8f5..07fa3861 100644 --- a/src/OAuthClient.js +++ b/src/OAuthClient.js @@ -470,8 +470,8 @@ OAuthClient.prototype.validateIdToken = function validateIdToken(params = {}) { // Step 1 : First check if the issuer is as mentioned in "issuer" if (id_token_payload.iss !== 'https://oauth.platform.intuit.com/op/v1') return false; - // Step 2 : check if the aud field in idToken is same as application's clientId - if (id_token_payload.aud !== this.clientId) return false; + // Step 2 : check if the aud field in idToken contains application's clientId + if (!id_token_payload.aud.find(audience => (audience === this.clientId))) return false; // Step 3 : ensure the timestamp has not elapsed if (id_token_payload.exp < Date.now() / 1000) return false; @@ -484,6 +484,7 @@ OAuthClient.prototype.validateIdToken = function validateIdToken(params = {}) { 'User-Agent': OAuthClient.user_agent, }, }; + return resolve(this.getKeyFromJWKsURI(id_token, id_token_header.kid, request)); }))).then((res) => { this.log('info', 'The validateIdToken () response is : ', JSON.stringify(res, null, 2)); @@ -506,12 +507,12 @@ OAuthClient.prototype.getKeyFromJWKsURI = function getKeyFromJWKsURI(id_token, k return (new Promise(((resolve) => { resolve(this.loadResponse(request)); }))).then((response) => { - if (response.status !== 200) throw new Error('Could not reach JWK endpoint'); - + if (Number(response.status) !== 200) throw new Error('Could not reach JWK endpoint'); // Find the key by KID const responseBody = JSON.parse(response.body); - const key = JSON.parse(responseBody.body).keys[0] + const key = responseBody.keys.find(el => (el.kid === kid)); const cert = this.getPublicKey(key.n, key.e); + return jwt.verify(id_token, cert); }).catch((e) => { e = this.createError(e); diff --git a/test/OAuthClientTest.js b/test/OAuthClientTest.js index 2d645874..a373a901 100644 --- a/test/OAuthClientTest.js +++ b/test/OAuthClientTest.js @@ -1,5 +1,5 @@ 'use strict'; -const proxyquire = require('proxyquire'); + const { describe, it, @@ -375,7 +375,7 @@ describe('Validate Id Token ', () => { before(() => { nock('https://oauth.platform.intuit.com').persist() .get('/op/v1/jwks') - .reply(200, expectedjwkResponseCall, { + .reply(200, expectedjwkResponseCall.body, { 'content-type': 'application/json;charset=UTF-8', 'content-length': '264', connection: 'close', @@ -390,7 +390,7 @@ describe('Validate Id Token ', () => { const mockIdTokenPayload = { sub: 'b053d994-07d5-468d-b7ee-22e349d2e739', - aud: 'clientID', + aud: ['clientID'], realmid: '1108033471', auth_time: 1462554475, iss: 'https://oauth.platform.intuit.com/op/v1', @@ -416,7 +416,7 @@ describe('Validate Id Token ', () => { }); it('Validate Id Token alternative', () => { - oauthClient.getToken().setToken(mockToken); + oauthClient.setToken(mockToken); oauthClient.validateIdToken() .then((response) => { expect(response).to.be.equal(true); diff --git a/test/mocks/jwkResponse.json b/test/mocks/jwkResponse.json index be557039..b01451f9 100644 --- a/test/mocks/jwkResponse.json +++ b/test/mocks/jwkResponse.json @@ -1,3 +1,3 @@ { - "body":"{\"keys\": [{\"kty\":\"sample_value_kty\",\"e\":\"sample_value_e\",\"use\":\"sample_value_use\",\"kid\":\"sample_value_kid\",\"alg\":\"sample_value_alg\",\"n\":\"sample_value_n\"}]}" -} \ No newline at end of file + "body":"{\"keys\": [{\"kty\":\"sample_value_kty\",\"e\":\"sample_value_e\",\"use\":\"sample_value_use\",\"kid\":\"r4p5SbL2qaFehFzhj8gI\",\"alg\":\"sample_value_alg\",\"n\":\"sample_value_n\"}]}" +} diff --git a/test/mocks/openID-token.json b/test/mocks/openID-token.json index 9cf218a2..c7c3c48e 100644 --- a/test/mocks/openID-token.json +++ b/test/mocks/openID-token.json @@ -4,5 +4,5 @@ "token_type": "bearer", "expires_in": "3600", "x_refresh_token_expires_in": "8726400", - "id_token": "eyJraWQiOiJyNHA1U2JMMnFhRmVoRnpoajhnSSIsImFsZyI6IlJTMjU2In0.eyJzdWIiOiJiMDUzZDk5NC0wN2Q1LTQ2OGQtYjdlZS0yMmUzNDlkMmU3MzkiLCJhdWQiOlsiTDM5ZWxTdWJGeGpQT1NwZFpvWVdSS2lDQ0U2VElOanY2N1JvYUU4ekJxYkl4eGI0bEsiXSwicmVhbG1pZCI6IjExMDgwMzM0NzEiLCJhdXRoX3RpbWUiOjE0NjI1NTQ0NzUsImlzcyI6Imh0dHBzOi8vb2F1dGgucGxhdGZvcm0uaW50dWl0LmNvbS9vcC92MSIsImV4cCI6MTQ2MjU2MTMyOCwiaWF0IjoxNDYyNTU3NzI4fQ==" + "id_token": "eyJraWQiOiJyNHA1U2JMMnFhRmVoRnpoajhnSSIsImFsZyI6IlJTMjU2In0.eyJzdWIiOiJiMDUzZDk5NC0wN2Q1LTQ2OGQtYjdlZS0yMmUzNDlkMmU3MzkiLCJhdWQiOlsiTDM5ZWxTdWJGeGpQT1NwZFpvWVdSS2lDQ0U2VElOanY2N1JvYUU4ekJxYkl4eGI0bEsiXSwicmVhbG1pZCI6IjExMDgwMzM0NzEiLCJhdXRoX3RpbWUiOjE0NjI1NTQ0NzUsImlzcyI6Imh0dHBzOlwvXC9vYXV0aC1lMmUucGxhdGZvcm0uaW50dWl0LmNvbVwvb2F1dGgyXC92MVwvb3BcL3YxIiwiZXhwIjoxNDYyNTYxMzI4LCJpYXQiOjE0NjI1NTc3Mjh9.BIJ9x_WPEOZsLJfQE3mGji_Q15j_rdlTyFYELiJM-W92fWSLC-TLEwCp5IrRhDWMvyvrLSMZCEdQALYQpbVy8uKI22JgGWYvkwNEDweOjbYzyt33F4xtn3GGcW9nAwRtA3M19qquWyi7G0kcCZUDN8RfUXz2qKMJ6KPOfLVe2UQ" }