chore(atlas-service): use server oidc browser setting for atlas login C…

…OMPASS-7063 (#4899) * chore(atlas-service): use server oidc browser setting for atlas login * chore(atlas-service): return spawned child from openBrowser for better error handling Co-authored-by: Anna Henningsen <[email protected]> * chore(preferences): remove contractions from preferences description Co-authored-by: Anna Henningsen <[email protected]> * chore(compass-settings): update labels based on product input * chore(preferences): remove "does not apply" now that we have an explicit label for server oidc settings More product feedback --------- Co-authored-by: Anna Henningsen <[email protected]>
mongodb-js · Sep 25, 2023 · ddf20a7 · ddf20a7
1 parent fe5652d
commit ddf20a7
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 23 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/atlas-service/src/main.spec.ts b/packages/atlas-service/src/main.spec.ts
@@ -124,7 +124,9 @@ describe('AtlasServiceMain', function () {
       expect(
         mockOidcPlugin.mongoClientOptions.authMechanismProperties
           .REQUEST_TOKEN_CALLBACK
-      ).to.have.been.calledOnce;
+        // two times because we need to explicitly request token first to show a
+        // proper error message from oidc plugin in case of failed sign in
+      ).to.have.been.calledTwice;
       expect(userInfo).to.have.property('sub', '1234');
     });
 
@@ -139,7 +141,42 @@ describe('AtlasServiceMain', function () {
       expect(
         mockOidcPlugin.mongoClientOptions.authMechanismProperties
           .REQUEST_TOKEN_CALLBACK
-      ).to.have.been.calledOnce;
+        // two times because we need to explicitly request token first to show a
+        // proper error message from oidc plugin in case of failed sign in
+      ).to.have.been.calledTwice;
+    });
+
+    it('should fail with oidc-plugin error first if auth failed', async function () {
+      AtlasService['fetch'] = sandbox.stub().resolves({
+        ok: false,
+        json: sandbox.stub().rejects(),
+        status: 401,
+        statusText: 'Unauthorized',
+      });
+      AtlasService['plugin'] = {
+        mongoClientOptions: {
+          authMechanismProperties: {
+            REQUEST_TOKEN_CALLBACK: sandbox
+              .stub()
+              .rejects(
+                new Error(
+                  'Failed to request token for some specific plugin reason'
+                )
+              ),
+            REFRESH_TOKEN_CALLBACK: sandbox.stub().rejects(),
+          },
+        },
+      } as any;
+
+      try {
+        await AtlasService.signIn();
+        expect.fail('Expected AtlasService.signIn to throw');
+      } catch (err) {
+        expect(err).to.have.property(
+          'message',
+          'Failed to request token for some specific plugin reason'
+        );
+      }
     });
   });
 

diff --git a/packages/atlas-service/src/main.ts b/packages/atlas-service/src/main.ts
@@ -42,6 +42,7 @@ import { SecretStore, SECRET_STORE_KEY } from './secret-store';
 import { AtlasUserConfigStore } from './user-config-store';
 import { OidcPluginLogger } from './oidc-plugin-logger';
 import { getActiveUser } from 'compass-preferences-model';
+import { spawn } from 'child_process';
 
 const { log, track } = createLoggerAndTelemetry('COMPASS-ATLAS-SERVICE');
 
@@ -172,8 +173,24 @@ export class AtlasService {
 
   private static config: AtlasServiceConfig;
 
-  private static openExternal(...args: Parameters<typeof shell.openExternal>) {
-    return shell?.openExternal(...args);
+  private static openExternal(url: string) {
+    const { browserCommandForOIDCAuth } = preferences.getPreferences();
+    if (browserCommandForOIDCAuth) {
+      // NB: While it's possible to pass `openBrowser.command` option directly
+      // to oidc-plugin properties, it's not possible to do dynamically. To
+      // avoid recreating oidc-plugin every time user changes
+      // `browserCommandForOIDCAuth` preference (which will cause loosing
+      // internal plugin auth state), we copy oidc-plugin `openBrowser.command`
+      // option handling to our openExternal method
+      const child = spawn(browserCommandForOIDCAuth, [url], {
+        shell: true,
+        stdio: 'ignore',
+        detached: true,
+      });
+      child.unref();
+      return child;
+    }
+    return shell.openExternal(url);
   }
 
   private static getAllowedAuthFlows(): AuthFlowType[] {
@@ -335,6 +352,10 @@ export class AtlasService {
         log.info(mongoLogId(1_001_000_218), 'AtlasService', 'Starting sign in');
 
         try {
+          // We first request oauth token just so we can get a proper auth error
+          // from oidc-plugin. If we only run getUserInfo, the only thing users
+          // will see is "401 unauthorized" as the reason for sign in failure
+          await this.requestOAuthToken({ signal });
           const userInfo = await this.getUserInfo({ signal });
           log.info(
             mongoLogId(1_001_000_219),

diff --git a/packages/compass-preferences-model/src/preferences.ts b/packages/compass-preferences-model/src/preferences.ts
@@ -549,7 +549,7 @@ export const storedUserPreferencesProps: Required<{
     global: true,
     description: {
       short: 'Show Device Auth Flow Checkbox',
-      long: 'Show a checkbox on the connection form to enable device auth flow authentication. This enables a less secure authentication flow that can be used as a fallback when browser-based authentication is unavailable.',
+      long: 'Show a checkbox on the connection form to enable device auth flow authentication for MongoDB server OIDC Authentication. This enables a less secure authentication flow that can be used as a fallback when browser-based authentication is unavailable.',
     },
     validator: z.boolean().default(false),
     type: 'boolean',
@@ -562,8 +562,8 @@ export const storedUserPreferencesProps: Required<{
     cli: true,
     global: true,
     description: {
-      short: 'Browser command to use for OIDC Authentication',
-      long: 'Specify a shell command that is run to start the browser for authenticating with the OIDC identity provider. Leave this empty for default browser.',
+      short: 'Browser command to use for authentication',
+      long: 'Specify a shell command that is run to start the browser for authenticating with the OIDC identity provider for the server connection or when logging in to your Atlas Cloud account. Leave this empty for default browser.',
     },
     validator: z.string().optional(),
     type: 'string',
@@ -577,7 +577,7 @@ export const storedUserPreferencesProps: Required<{
     global: true,
     description: {
       short: 'Stay logged in with OIDC',
-      long: 'Remain logged in when using the MONGODB-OIDC authentication mechanism. Access tokens are encrypted using the system keychain before being stored.',
+      long: 'Remain logged in when using the MONGODB-OIDC authentication mechanism for MongoDB server connection. Access tokens are encrypted using the system keychain before being stored.',
     },
     validator: z.boolean().default(true),
     type: 'boolean',

diff --git a/packages/compass-settings/src/components/modal.tsx b/packages/compass-settings/src/components/modal.tsx
@@ -27,7 +27,7 @@ type Settings = {
 
 type SettingsModalProps = {
   isOpen: boolean;
-  isOIDCEnabled: boolean;
+  isOIDCTabEnabled: boolean;
   onMount?: () => void;
   onClose: () => void;
   onSave: () => void;
@@ -59,7 +59,7 @@ export const SettingsModal: React.FunctionComponent<SettingsModalProps> = ({
   onMount,
   onClose,
   onSave,
-  isOIDCEnabled,
+  isOIDCTabEnabled,
   hasChangedSettings,
 }) => {
   const onMountRef = useRef(onMount);
@@ -74,7 +74,7 @@ export const SettingsModal: React.FunctionComponent<SettingsModalProps> = ({
     { name: 'Privacy', component: PrivacySettings },
   ];
 
-  if (isOIDCEnabled) {
+  if (isOIDCTabEnabled) {
     settings.push({
       name: 'OIDC (Preview)',
       component: OIDCSettings,
@@ -133,7 +133,12 @@ export default connect(
     return {
       isOpen:
         state.settings.isModalOpen && state.settings.loadingState === 'ready',
-      isOIDCEnabled: !!state.settings.settings.enableOidc,
+      isOIDCTabEnabled:
+        !!state.settings.settings.enableOidc ||
+        // because oidc options overlap with atlas login used for ai feature
+        !!state.settings.settings.enableAIWithoutRolloutAccess ||
+        !!state.settings.settings.enableAIExperience ||
+        !!state.settings.settings.enableAIFeatures,
       hasChangedSettings: state.settings.updatedFields.length > 0,
     };
   },

diff --git a/packages/compass-settings/src/components/settings/oidc-settings.tsx b/packages/compass-settings/src/components/settings/oidc-settings.tsx
@@ -1,19 +1,18 @@
 import React from 'react';
 import SettingsList from './settings-list';
 
-const oidcFields = [
-  'browserCommandForOIDCAuth',
-  'showOIDCDeviceAuthFlow',
-  'persistOIDCTokens',
-] as const;
-
 export const OIDCSettings: React.FunctionComponent = () => {
   return (
     <div data-testid="oidc-settings">
       <div>
-        Change the behavior of the OIDC authentication mechanism in Compass.
+        Change the behavior of the OIDC authentication mechanism for server
+        connection and Atlas Login in Compass.
+      </div>
+      <SettingsList fields={['browserCommandForOIDCAuth']} />
+      <div>
+        <strong>MongoDB server OIDC Authentication options</strong>
       </div>
-      <SettingsList fields={oidcFields} />
+      <SettingsList fields={['showOIDCDeviceAuthFlow', 'persistOIDCTokens']} />
     </div>
   );
 };