fix: always send a nonce in the auth request MONGOSH-1905 (#195)

mongodb-js · Nov 19, 2024 · a6826fb · a6826fb
1 parent 3e77a74
commit a6826fb
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 46 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -57,7 +57,7 @@
     "@mongodb-js/eslint-config-devtools": "^0.9.9",
     "@mongodb-js/mocha-config-devtools": "^1.0.0",
     "@mongodb-js/monorepo-tools": "^1.1.4",
-    "@mongodb-js/oidc-mock-provider": "^0.8.0",
+    "@mongodb-js/oidc-mock-provider": "^0.10.2",
     "@mongodb-js/prettier-config-devtools": "^1.0.1",
     "@mongodb-js/tsconfig-devtools": "^1.0.0",
     "@types/chai": "^4.2.21",

diff --git a/src/api.ts b/src/api.ts
@@ -197,6 +197,14 @@ export interface MongoDBOIDCPluginOptions {
    * broken identity providers.
    */
   passIdTokenAsAccessToken?: boolean;
+
+  /**
+   * Skip the nonce parameter in the Authorization Code request. This could
+   * be used to work with providers that don't support the nonce parameter.
+   *
+   * Default is `false`.
+   */
+  skipNonceInAuthCodeRequest?: boolean;
 }
 
 /** @public */

diff --git a/src/plugin.spec.ts b/src/plugin.spec.ts
@@ -77,6 +77,19 @@ async function delay(ms: number) {
   return await new Promise((resolve) => setTimeout(resolve, ms));
 }
 
+function testAuthCodeFlow(
+  fn: (opts: Partial<MongoDBOIDCPluginOptions>) => Mocha.Func
+): void {
+  for (const skipNonceInAuthCodeRequest of [true, false]) {
+    describe(`with skipNonceInAuthCodeRequest: ${skipNonceInAuthCodeRequest.toString()}`, function () {
+      it(
+        'can successfully authenticate with auth code flow',
+        fn({ skipNonceInAuthCodeRequest })
+      );
+    });
+  }
+}
+
 describe('OIDC plugin (local OIDC provider)', function () {
   this.timeout(90_000);
 
@@ -138,18 +151,42 @@ describe('OIDC plugin (local OIDC provider)', function () {
       plugin = createMongoDBOIDCPlugin(pluginOptions);
     });
 
-    it('can request tokens through the browser', async function () {
-      const result = await requestToken(
-        plugin,
-        provider.getMongodbOIDCDBInfo()
-      );
-      const accessTokenContents = getJWTContents(result.accessToken);
-      expect(accessTokenContents.sub).to.equal('testuser');
-      expect(accessTokenContents.client_id).to.equal(
-        provider.getMongodbOIDCDBInfo().clientId
-      );
-      verifySuccessfulAuthCodeFlowLog(await readLog());
-    });
+    testAuthCodeFlow(
+      (opts) =>
+        async function () {
+          pluginOptions = {
+            ...pluginOptions,
+            ...opts,
+          };
+          plugin = createMongoDBOIDCPlugin(pluginOptions);
+          let idToken: string | undefined;
+          plugin.logger.once('mongodb-oidc-plugin:auth-succeeded', (event) => {
+            idToken = event.tokens.idToken;
+          });
+
+          const result = await requestToken(
+            plugin,
+            provider.getMongodbOIDCDBInfo()
+          );
+          const accessTokenContents = getJWTContents(result.accessToken);
+          expect(accessTokenContents.sub).to.equal('testuser');
+          expect(accessTokenContents.client_id).to.equal(
+            provider.getMongodbOIDCDBInfo().clientId
+          );
+
+          verifySuccessfulAuthCodeFlowLog(await readLog());
+
+          expect(idToken).to.not.be.undefined;
+
+          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- we know it's non-null from the above check
+          const idTokenContents = getJWTContents(idToken!);
+          if (opts.skipNonceInAuthCodeRequest) {
+            expect(idTokenContents.nonce).to.be.undefined;
+          } else {
+            expect(idTokenContents.nonce).to.not.be.undefined;
+          }
+        }
+    );
 
     it('will re-use tokens while they are valid if no username was provided', async function () {
       const skipAuthAttemptEvent = once(
@@ -1017,18 +1054,22 @@ describe('OIDC plugin (local OIDC provider)', function () {
       };
     });
 
-    it('can successfully authenticate with Okta using auth code flow', async function () {
-      plugin = createMongoDBOIDCPlugin({
-        ...defaultOpts,
-        allowedFlows: ['auth-code'],
-        openBrowser: (opts) =>
-          oktaBrowserAuthCodeFlow({ ...opts, username, password }),
-      });
-      const result = await requestToken(plugin, metadata);
+    testAuthCodeFlow(
+      (opts) =>
+        async function () {
+          plugin = createMongoDBOIDCPlugin({
+            ...defaultOpts,
+            allowedFlows: ['auth-code'],
+            openBrowser: (opts) =>
+              oktaBrowserAuthCodeFlow({ ...opts, username, password }),
+            ...opts,
+          });
+          const result = await requestToken(plugin, metadata);
 
-      validateToken(getJWTContents(result.accessToken));
-      verifySuccessfulAuthCodeFlowLog(await readLog());
-    });
+          validateToken(getJWTContents(result.accessToken));
+          verifySuccessfulAuthCodeFlowLog(await readLog());
+        }
+    );
 
     it('can successfully authenticate with Okta using device auth flow', async function () {
       plugin = createMongoDBOIDCPlugin({
@@ -1087,18 +1128,22 @@ describe('OIDC plugin (local OIDC provider)', function () {
       };
     });
 
-    it('can successfully authenticate with Azure using auth code flow', async function () {
-      plugin = createMongoDBOIDCPlugin({
-        ...defaultOpts,
-        allowedFlows: ['auth-code'],
-        openBrowser: (opts) =>
-          azureBrowserAuthCodeFlow({ ...opts, username, password }),
-      });
-      const result = await requestToken(plugin, metadata);
+    testAuthCodeFlow(
+      (opts) =>
+        async function () {
+          plugin = createMongoDBOIDCPlugin({
+            ...defaultOpts,
+            allowedFlows: ['auth-code'],
+            openBrowser: (opts) =>
+              azureBrowserAuthCodeFlow({ ...opts, username, password }),
+            ...opts,
+          });
+          const result = await requestToken(plugin, metadata);
 
-      validateToken(getJWTContents(result.accessToken));
-      verifySuccessfulAuthCodeFlowLog(await readLog());
-    });
+          validateToken(getJWTContents(result.accessToken));
+          verifySuccessfulAuthCodeFlowLog(await readLog());
+        }
+    );
 
     it('can successfully authenticate with Azure using device auth flow', async function () {
       plugin = createMongoDBOIDCPlugin({

diff --git a/src/plugin.ts b/src/plugin.ts
@@ -657,6 +657,10 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
     let client!: BaseClient;
     let actualRedirectURI!: string;
 
+    const nonce = this.options.skipNonceInAuthCodeRequest
+      ? undefined
+      : generators.nonce();
+
     try {
       await withAbortCheck(signal, async ({ signalCheck, signalPromise }) => {
         // We mark the operations that we want to allow to result in a fallback
@@ -680,6 +684,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
           code_challenge: codeChallenge,
           code_challenge_method: 'S256',
           state: oidcStateParam,
+          nonce,
         });
         validateSecureHTTPUrl(authCodeFlowUrl, 'authCodeFlowUrl');
         const { localUrl, onAccessed: onLocalUrlAccessed } =
@@ -760,6 +765,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
     const tokenSet = await client.callback(actualRedirectURI, params, {
       code_verifier: codeVerifier,
       state: oidcStateParam,
+      nonce,
     });
     this.updateStateWithTokenSet(state, tokenSet);
   }

diff --git a/test/oidc-test-provider.ts b/test/oidc-test-provider.ts
@@ -282,10 +282,13 @@ async function waitForTitle(
 ): Promise<void> {
   await browser.waitUntil(async () => {
     const actual = (await browser.$(selector).getText()).trim();
-    let matches;
-    if (typeof expected === 'string')
+    let matches: boolean;
+    if (typeof expected === 'string') {
       matches = actual.toLowerCase() === expected.toLowerCase();
-    else matches = expected.test(actual);
+    } else {
+      matches = expected.test(actual);
+    }
+
     if (!matches) {
       throw new Error(`Wanted title "${String(expected)}", saw "${actual}"`);
     }
@@ -481,7 +484,7 @@ export async function azureBrowserDeviceAuthFlow({
   try {
     const normalizeUserCode = (str: string) => str.replace(/-/g, '');
     browser = await spawnBrowser(verificationUrl, true);
-    await waitForTitle(browser, 'Enter code', 'div[role="heading"]');
+    await waitForTitle(browser, /Enter code/, 'div[role="heading"]');
     await ensureValue(
       browser,
       'input[name="otc"]',