diff --git a/packages/@webex/plugin-authorization-browser-first-party/src/authorization.js b/packages/@webex/plugin-authorization-browser-first-party/src/authorization.js index 93386c1fe79..dc4831c6c78 100644 --- a/packages/@webex/plugin-authorization-browser-first-party/src/authorization.js +++ b/packages/@webex/plugin-authorization-browser-first-party/src/authorization.js @@ -21,10 +21,6 @@ import uuid from 'uuid'; import base64url from 'crypto-js/enc-base64url'; import CryptoJS from 'crypto-js'; -// Necessary to require lodash this way in order to stub -// methods in the unit test -const lodash = require('lodash'); - const OAUTH2_CSRF_TOKEN = 'oauth2-csrf-token'; const OAUTH2_CODE_VERIFIER = 'oauth2-code-verifier'; @@ -887,7 +883,8 @@ const Authorization = WebexPlugin.extend({ * during authorization code exchange; removes it once consumed. * * Implementation details: - * - Creates a 128 character string using base64url safe alphabet. + * - Creates a 128 character string using a cryptographically secure random + * source and the base64url safe alphabet. * - Computes SHA256 hash, encodes to base64url (no padding). * * @instance @@ -900,10 +897,14 @@ const Authorization = WebexPlugin.extend({ // eslint-disable-next-line no-underscore-dangle const safeCharacterMap = base64url._safe_map; + const randomValues = new Uint8Array(128); + + this.webex.getWindow().crypto.getRandomValues(randomValues); - const codeVerifier = lodash - .times(128, () => safeCharacterMap[lodash.random(0, safeCharacterMap.length - 1)]) - .join(''); + const codeVerifier = Array.from( + randomValues, + (randomValue) => safeCharacterMap[randomValue & (safeCharacterMap.length - 1)] + ).join(''); const codeChallenge = CryptoJS.SHA256(codeVerifier).toString(base64url); diff --git a/packages/@webex/plugin-authorization-browser-first-party/test/unit/spec/authorization.js b/packages/@webex/plugin-authorization-browser-first-party/test/unit/spec/authorization.js index 0bf3de9c4a9..70ae684c6c9 100644 --- a/packages/@webex/plugin-authorization-browser-first-party/test/unit/spec/authorization.js +++ b/packages/@webex/plugin-authorization-browser-first-party/test/unit/spec/authorization.js @@ -25,9 +25,13 @@ describe('plugin-authorization-browser-first-party', () => { href = 'https://example.com', csrfToken = undefined, pkceVerifier = undefined, - config = {} + config = {}, + getRandomValues = sinon.stub().callsFake((randomValues) => randomValues.fill(0)) ) { const mockWindow = { + crypto: { + getRandomValues, + }, history: { replaceState(a, b, location) { mockWindow.location.href = location; @@ -1305,34 +1309,84 @@ describe('plugin-authorization-browser-first-party', () => { }); describe('#_generateCodeChallenge', () => { - const expectedCodeChallenge = 'code challenge'; // eslint-disable-next-line no-underscore-dangle const safeCharacterMap = CryptoJS.enc.Base64url._safe_map; - const expectedVerifier = times(128, () => safeCharacterMap[0]).join(''); + function makeWebexWithRandomValues(fillRandomValues) { + const getRandomValuesStub = sinon.stub().callsFake((randomValues) => { + fillRandomValues(randomValues); - it('generates a challenge code and stores it in session storage', () => { - const webex = makeWebex('http://example.com'); + return randomValues; + }); + const webex = makeWebex( + 'http://example.com', + undefined, + undefined, + {}, + getRandomValuesStub + ); + + getRandomValuesStub.resetHistory(); + webex.getWindow().sessionStorage.setItem.resetHistory(); + + return {getRandomValuesStub, webex}; + } - const toStringStub = sinon.stub().returns(expectedCodeChallenge); - const randomStub = sinon.stub(lodash, 'random').returns(0); - const sha256Stub = sinon.stub(CryptoJS, 'SHA256').returns({ - toString: toStringStub, + it('uses a 128-byte CSPRNG and does not use insecure random generators', () => { + const {getRandomValuesStub, webex} = makeWebexWithRandomValues((randomValues) => { + randomValues.fill(0); }); + const mathRandomStub = sinon.stub(Math, 'random'); + const lodashRandomStub = sinon.stub(lodash, 'random'); + + // eslint-disable-next-line no-underscore-dangle + webex.authorization._generateCodeChallenge(); + + const generatedRandomValues = getRandomValuesStub.firstCall.args[0]; + + assert.calledOnceWithExactly(getRandomValuesStub, generatedRandomValues); + assert.instanceOf(generatedRandomValues, Uint8Array); + assert.lengthOf(generatedRandomValues, 128); + assert.notCalled(mathRandomStub); + assert.notCalled(lodashRandomStub); + }); + + it('generates a 128-character verifier from the base64url-safe alphabet', () => { + const {webex} = makeWebexWithRandomValues((randomValues) => { + randomValues.set(times(128, (index) => index)); + }); + const expectedVerifier = times( + 128, + (index) => safeCharacterMap[index & (safeCharacterMap.length - 1)] + ).join(''); + + // eslint-disable-next-line no-underscore-dangle + webex.authorization._generateCodeChallenge(); + + const storedVerifier = webex.getWindow().sessionStorage.setItem.firstCall.args[1]; + + assert.match(storedVerifier, /^[A-Za-z0-9_-]{128}$/); + assert.equal(storedVerifier, expectedVerifier); + }); + + it('stores the verifier and returns its SHA-256 base64url challenge', () => { + const {webex} = makeWebexWithRandomValues((randomValues) => { + randomValues.fill(42); + }); + const expectedVerifier = safeCharacterMap[42 & (safeCharacterMap.length - 1)].repeat(128); + const expectedChallenge = CryptoJS.SHA256(expectedVerifier).toString( + CryptoJS.enc.Base64url + ); // eslint-disable-next-line no-underscore-dangle const codeChallenge = webex.authorization._generateCodeChallenge(); - assert.equal(codeChallenge, expectedCodeChallenge); - assert.calledWith(sha256Stub, expectedVerifier); - assert.calledWith(toStringStub, CryptoJS.enc.Base64url); - assert.callCount(randomStub, 128); - assert.calledWith(randomStub, 0, safeCharacterMap.length - 1); - assert.calledWith( + assert.calledOnceWithExactly( webex.getWindow().sessionStorage.setItem, 'oauth2-code-verifier', expectedVerifier ); + assert.equal(codeChallenge, expectedChallenge); }); });