Fix browser PKCE crypto (#1176)

Author: AndreyVMarkelov

package-lock.json

@@ -1,6 +1,6 @@
 {
     "name": "dropbox",
-    "version": "10.37.0",
+    "version": "10.37.1",
     "registry": "npm",
     "description": "The Dropbox JavaScript SDK is a lightweight, promise based interface to the Dropbox v2 API that works in both nodejs and browser environments.",
     "main": "cjs/index.js",

package.json

@@ -18,11 +18,8 @@ const config = {
   output: {
     format: 'umd',
     sourcemap: (process.env.BUNDLE_TYPE !== 'minified'),
-    globals: {
-      crypto: 'crypto',
-    },
   },
-  external: ['es6-promise/auto', 'crypto'],
+  external: ['es6-promise/auto'],
   plugins: [
     babel(),
   ],

rollup.config.js

@@ -10,7 +10,8 @@ import { parseResponse } from './response.js';
 
 let fetch;
 let crypto;
-let Encoder;
+
+const base64EncodeBytes = (bytes) => btoa(String.fromCharCode.apply(null, bytes));
 
 // Expiration is 300 seconds but needs to be in milliseconds for Date object
 const TokenExpirationBuffer = 300 * 1000;
@@ -58,13 +59,7 @@ export default class DropboxAuth {
       fetch = typeof globalThis.fetch === 'function'
         ? globalThis.fetch.bind(globalThis)
         : undefined;
-      crypto = require('crypto'); // eslint-disable-line global-require
-    }
-
-    if (typeof TextEncoder === 'undefined') {
-      Encoder = require('util').TextEncoder; // eslint-disable-line global-require
-    } else {
-      Encoder = TextEncoder;
+      crypto = globalThis.crypto;
     }
 
     this.fetch = options.fetch || fetch;
@@ -183,35 +178,20 @@ export default class DropboxAuth {
   }
 
   generateCodeChallenge() {
-    const encoder = new Encoder();
+    const encoder = new TextEncoder();
     const codeData = encoder.encode(this.codeVerifier);
-    let codeChallenge;
-    if (isBrowserEnv() || isWorkerEnv()) {
-      return crypto.subtle.digest('SHA-256', codeData)
-        .then((digestedHash) => {
-          const base64String = btoa(String.fromCharCode.apply(null, new Uint8Array(digestedHash)));
-          codeChallenge = createBrowserSafeString(base64String).substr(0, 128);
-          this.codeChallenge = codeChallenge;
-        });
-    }
-    const digestedHash = crypto.createHash('sha256').update(codeData).digest();
-    codeChallenge = createBrowserSafeString(digestedHash);
-    this.codeChallenge = codeChallenge;
-    return Promise.resolve();
+    return crypto.subtle.digest('SHA-256', codeData)
+      .then((digestedHash) => {
+        const base64String = base64EncodeBytes(new Uint8Array(digestedHash));
+        this.codeChallenge = createBrowserSafeString(base64String).substr(0, 128);
+      });
   }
 
   generatePKCECodes() {
-    let codeVerifier;
-    if (isBrowserEnv() || isWorkerEnv()) {
-      const array = new Uint8Array(PKCELength);
-      const randomValueArray = crypto.getRandomValues(array);
-      const base64String = btoa(randomValueArray);
-      codeVerifier = createBrowserSafeString(base64String).substr(0, 128);
-    } else {
-      const randomBytes = crypto.randomBytes(PKCELength);
-      codeVerifier = createBrowserSafeString(randomBytes).substr(0, 128);
-    }
-    this.codeVerifier = codeVerifier;
+    const array = new Uint8Array(PKCELength);
+    const randomValueArray = crypto.getRandomValues(array);
+    const base64String = base64EncodeBytes(randomValueArray);
+    this.codeVerifier = createBrowserSafeString(base64String).substr(0, 128);
 
     return this.generateCodeChallenge();
   }

src/auth.js

@@ -21,6 +21,20 @@ const executeTest = (testContainer) => {
 };
 
 describe('Browser Definitions', () => {
+  describe('Node built-in dependencies', () => {
+    [
+      'es/src/auth.js',
+      'cjs/src/auth.js',
+      'dist/Dropbox-sdk.js',
+      'dist/Dropbox-sdk.min.js',
+    ].forEach((buildPath) => {
+      it(`${buildPath} does not depend on Node crypto or util`, () => {
+        const build = fs.readFileSync(path.resolve(__dirname, `../../${buildPath}`), 'utf8');
+        chai.assert.notMatch(build, /require\(['"](?:crypto|util)['"]\)/);
+      });
+    });
+  });
+
   describe('ES Build', () => {
     let html;
     let dom;

test/build/browser.js

@@ -242,8 +242,32 @@ describe('DropboxAuth', () => {
     it('saves a new code challenge on Auth obj', () => {
       const dbxAuth = new DropboxAuth();
       chai.assert.equal(dbxAuth.codeChallenge, undefined);
-      dbxAuth.generatePKCECodes();
-      chai.assert.isTrue(!!dbxAuth.codeChallenge);
+      return dbxAuth.generatePKCECodes()
+        .then(() => {
+          chai.assert.isTrue(!!dbxAuth.codeChallenge);
+        });
+    });
+
+    it('base64 encodes the random bytes used for the code verifier', () => {
+      const getRandomValuesStub = sinon.stub(globalThis.crypto, 'getRandomValues')
+        .callsFake((array) => {
+          for (let i = 0; i < array.length; i += 1) {
+            array[i] = i;
+          }
+          return array;
+        });
+      const dbxAuth = new DropboxAuth();
+
+      return dbxAuth.generatePKCECodes()
+        .then(() => {
+          chai.assert.equal(
+            dbxAuth.codeVerifier,
+            'AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0-P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5f',
+          );
+        })
+        .finally(() => {
+          getRandomValuesStub.restore();
+        });
     });
 
     it('gets called when using PKCE flow', (done) => {
@@ -257,7 +281,7 @@ describe('DropboxAuth', () => {
         .catch(done);
     });
 
-    it('generates valid code challenge from verifier (Node)', (done) => {
+    it('generates valid code challenge from verifier', (done) => {
       const dbxAuth = new DropboxAuth();
       const verifier = 'NTUsMjIsMzYsMTY4LDIyLDEzNywyNDMsOTYsMTIxLDIxNSwxNDAsMTYwLDMwLDE1LDIzMSw1NiwzMCwyMTIsMTQyLDIyMywxMzMsMTIsMjI1LDIzOCwxMDcsMjQ1LDM0';
       dbxAuth.setCodeVerifier(verifier);