Merge pull request #17816 from mozilla/FXA-10528

refactor(settings): Create OAuthNative integration, rename OAuth integration to OAuthWeb

packages/fxa-settings/src/components/ChooseWhatToSync/sync-engines.ts

@@ -22,9 +22,9 @@ export type SyncEngineId = EngineConfig['id'] | WebChannelEngineConfig['id'];
 /* These sync engines are always offered to the user in Sync fx_desktop_v3
  * and other engines can be received and added with a webchannel message.
  *
- * For OAuth Sync (oauth_webchannel_v1) which includes sync mobile and sync
- * desktop on FF 123+, we do not display options by default and instead, we
- * receive the webchannel message and overwrite the options.
+ * For OAuth Sync (oauth_webchannel_v1) which includes sync mobile and
+ * oauth sync desktop, we do not display options by default and instead,
+ * we receive the webchannel message and overwrite the options.
  */
 export const defaultDesktopV3SyncEngineConfigs = [
   {

packages/fxa-settings/src/lib/integrations/integration-factory-flags.ts

@@ -57,7 +57,7 @@ export class DefaultIntegrationFlags implements IntegrationFlags {
     return this.searchParam('context') === Constants.FX_DESKTOP_V3_CONTEXT;
   }
 
-  // Sync mobile, Sync FF Desktop 123+, and supplicant pairing use this context
+  // Sync mobile, Sync FF OAuth Desktop, and supplicant pairing use this context
   isOAuthWebChannelContext() {
     return this.searchParam('context') === Constants.OAUTH_WEBCHANNEL_CONTEXT;
   }

packages/fxa-settings/src/lib/integrations/integration-factory.test.ts

@@ -8,6 +8,9 @@ import {
   Integration,
   IntegrationType,
   OAuthIntegration,
+  OAuthNativeClients,
+  OAuthNativeIntegration,
+  OAuthWebIntegration,
   PairingAuthorityIntegration,
   PairingSupplicantIntegration,
   RelierClientInfo,
@@ -25,6 +28,7 @@ type IntegrationFlagOverrides = {
   isOAuth?: boolean;
   isServiceSync?: boolean;
   isV3DesktopContext?: boolean;
+  isOAuthWebChannelContext?: boolean;
 };
 
 type FactoryCallCounts = {
@@ -67,13 +71,14 @@ describe('lib/integrations/integration-factory', () => {
     sandbox
       .stub(flags, 'isV3DesktopContext')
       .returns(!!flagOverrides.isV3DesktopContext);
+    sandbox
+      .stub(flags, 'isOAuthWebChannelContext')
+      .returns(!!flagOverrides.isOAuthWebChannelContext);
 
     urlQueryData.set('scope', 'profile');
     urlQueryData.set('client_id', '720bc80adfa6988d');
     urlQueryData.set('redirect_uri', 'https://redirect.to');
 
-    urlHashData.set('scope', 'profile');
-
     // Create a factory with current state
     const factory = new IntegrationFactory({
       window,
@@ -149,21 +154,9 @@ describe('lib/integrations/integration-factory', () => {
       expect(integration.wantsKeys()).toBeFalsy();
       expect(integration.isTrusted()).toBeTruthy();
     });
-
-    // TODO: Remove with approval.
-    //
-    // I think maybe this is feature envy, perhaps we should have some dedicated thing that checks integration state
-    // and account state to determine if features are needed. As far as I can tell the integration models
-    // themselves really shouldn't know or care about 'accounts'
-    //
-    // describe('accountNeedsPermissions', function () {
-    //   it('returns `false`', function () {
-    //     assert.isFalse(integration.accountNeedsPermissions());
-    //   });
-    // });
   });
 
-  describe('SyncDesktop creation', () => {
+  describe('SyncDesktopV3 creation', () => {
     const ACTION = 'email';
     const CONTEXT = 'fx_desktop_v3';
     const COUNTRY = 'RO';
@@ -206,48 +199,74 @@ describe('lib/integrations/integration-factory', () => {
     // TODO: Port remaining tests from content-server
   });
 
-  describe('OAuthIntegration creation', () => {
+  describe('OAuthWebIntegration creation', () => {
     let integration: OAuthIntegration;
 
     describe('OAuth redirect', () => {
       beforeEach(async () => {
         integration = await setup<OAuthIntegration>(
           { isOAuth: true },
           { initIntegration: 1, initOAuthIntegration: 1, initClientInfo: 1 },
-          (i: Integration) => i instanceof OAuthIntegration
+          (i: Integration) => i instanceof OAuthWebIntegration
         );
       });
 
       it('has correct state', async () => {
-        expect(integration.type).toEqual(IntegrationType.OAuth);
+        expect(integration.type).toEqual(IntegrationType.OAuthWeb);
         expect(integration.isSync()).toBeFalsy();
         expect(integration.wantsKeys()).toBeFalsy();
         expect(integration.isTrusted()).toBeTruthy();
       });
     });
 
-    describe('OAuth Sync', () => {
+    // TODO: Port remaining tests from content-server
+  });
+
+  describe('OAuthNativeIntegration creation', () => {
+    let integration: OAuthNativeIntegration;
+
+    describe('without sync', () => {
       beforeEach(async () => {
-        integration = await setup<OAuthIntegration>(
+        integration = await setup<OAuthNativeIntegration>(
           { isOAuth: true },
           { initIntegration: 1, initOAuthIntegration: 1, initClientInfo: 1 },
-          (i: Integration) => i instanceof OAuthIntegration
+          (i: Integration) => i instanceof OAuthNativeIntegration
+        );
+      });
+
+      it('has correct state', async () => {
+        expect(integration.type).toEqual(IntegrationType.OAuthWeb);
+        expect(integration.isSync()).toBeFalsy();
+        expect(integration.wantsKeys()).toBeFalsy();
+        expect(integration.isTrusted()).toBeTruthy();
+      });
+    });
+
+    describe('with sync', () => {
+      beforeEach(async () => {
+        integration = await setup<OAuthNativeIntegration>(
+          { isOAuth: true, isOAuthWebChannelContext: true },
+          { initIntegration: 1, initOAuthIntegration: 1, initClientInfo: 1 },
+          (i: Integration) => i instanceof OAuthNativeIntegration
         );
         await mockSearchParams({
           scope: Constants.OAUTH_OLDSYNC_SCOPE,
           context: Constants.OAUTH_WEBCHANNEL_CONTEXT,
+          clientId: OAuthNativeClients.FirefoxIOS,
         });
+        sandbox.stub(integration, 'clientInfo').get(() => ({
+          ...clientInfo,
+          clientId: OAuthNativeClients.FirefoxIOS,
+        }));
       });
 
       it('has correct state', async () => {
-        expect(integration.type).toEqual(IntegrationType.OAuth);
+        expect(integration.type).toEqual(IntegrationType.OAuthNative);
         expect(integration.isSync()).toBeTruthy();
         expect(integration.wantsKeys()).toBeTruthy();
         expect(integration.isTrusted()).toBeTruthy();
       });
     });
-
-    // TODO: Port remaining tests from content-server
   });
 
   describe('PairingSupplicantIntegration creation', () => {

packages/fxa-settings/src/lib/integrations/integration-factory.ts

@@ -3,7 +3,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import {
-  OAuthIntegration,
+  OAuthWebIntegration,
+  OAuthNativeIntegration,
   PairingAuthorityIntegration,
   PairingSupplicantIntegration,
   Integration,
@@ -12,6 +13,7 @@ import {
   WebIntegration,
   RelierClientInfo,
   RelierSubscriptionInfo,
+  OAuthIntegration,
 } from '../../models/integrations';
 import {
   ModelDataStore,
@@ -106,7 +108,11 @@ export class IntegrationFactory {
     } else if (flags.isDevicePairingAsSupplicant()) {
       return this.createPairingSupplicationIntegration(data, storageData);
     } else if (flags.isOAuth()) {
-      return this.createOAuthIntegration(data, storageData);
+      if (flags.isOAuthWebChannelContext()) {
+        return this.createOAuthNativeIntegration(data, storageData);
+      } else {
+        return this.createOAuthWebIntegration(data, storageData);
+      }
     } else if (flags.isV3DesktopContext()) {
       return this.createSyncDesktopV3Integration(data);
     } else if (flags.isServiceSync()) {
@@ -144,12 +150,31 @@ export class IntegrationFactory {
     return integration;
   }
 
-  private createOAuthIntegration(
+  private createOAuthWebIntegration(
     data: ModelDataStore,
     storageData: ModelDataStore
   ) {
     // Resolve configuration settings for oauth relier
-    const integration = new OAuthIntegration(data, storageData, config.oauth);
+    const integration = new OAuthWebIntegration(
+      data,
+      storageData,
+      config.oauth
+    );
+    this.initIntegration(integration);
+    this.initOAuthIntegration(integration, this.flags);
+    this.initClientInfo(integration);
+    return integration;
+  }
+
+  private createOAuthNativeIntegration(
+    data: ModelDataStore,
+    storageData: ModelDataStore
+  ) {
+    const integration = new OAuthNativeIntegration(
+      data,
+      storageData,
+      config.oauth
+    );
     this.initIntegration(integration);
     this.initOAuthIntegration(integration, this.flags);
     this.initClientInfo(integration);

packages/fxa-settings/src/lib/integrations/interfaces/integration-flags.ts

@@ -9,6 +9,7 @@ export interface IntegrationFlags {
   isDevicePairingAsAuthority(): boolean;
   isDevicePairingAsSupplicant(): boolean;
   isOAuth(): boolean;
+  isOAuthWebChannelContext(): boolean;
   isV3DesktopContext(): boolean;
   isOAuthSuccessFlow(): { status: boolean; clientId: string };
   isOAuthVerificationFlow(): boolean;

packages/fxa-settings/src/lib/oauth/hooks.tsx

@@ -7,8 +7,8 @@ import { useCallback } from 'react';
 import {
   Integration,
   OAuthIntegration,
+  isOAuthNativeIntegrationSync,
   isOAuthIntegration,
-  isSyncOAuthIntegration,
 } from '../../models';
 import { createEncryptedBundle } from '../crypto/scoped-keys';
 import { Constants } from '../constants';
@@ -181,7 +181,7 @@ export function useFinishOAuthFlowHandler(
   authClient: AuthClient,
   integration: Integration
 ): UseFinishOAuthFlowHandlerResult {
-  const isSyncOAuth = isSyncOAuthIntegration(integration);
+  const isSyncOAuth = isOAuthNativeIntegrationSync(integration);
 
   const finishOAuthFlowHandler: FinishOAuthFlowHandler = useCallback(
     async (accountUid, sessionToken, keyFetchToken, unwrapBKey) => {

packages/fxa-settings/src/models/integrations/base-integration.ts

@@ -5,7 +5,8 @@
 import { MozServices } from '../../lib/types';
 
 export enum IntegrationType {
-  OAuth = 'OAuth',
+  OAuthWeb = 'OAuthWeb', // OAuth for non-browser services/RPs
+  OAuthNative = 'OAuthNative', // OAuth for desktop & mobile clients
   PairingAuthority = 'PairingAuthority', // TODO
   PairingSupplicant = 'PairingSupplicant', // TODO
   SyncBasic = 'SyncBasic',
@@ -96,7 +97,19 @@ export abstract class Integration<
     this.features = { ...this.features, ...features } as T;
   }
 
-  isSync(): boolean {
+  isSync() {
+    return false;
+  }
+
+  isDesktopSync() {
+    return false;
+  }
+
+  isFirefoxMobileClient() {
+    return false;
+  }
+
+  isFirefoxDesktopClient() {
     return false;
   }
 
@@ -162,11 +175,6 @@ export abstract class Integration<
   isTrusted() {
     return true;
   }
-
-  // TODO: This seems like feature envy... Move logic elsewhere.
-  // accountNeedsPermissions(account:RelierAccount): boolean {
-  //   return false;
-  // }
 }
 
 export class BaseIntegration<

packages/fxa-settings/src/models/integrations/index.ts

@@ -5,7 +5,8 @@
 export * from './base-integration';
 export * from './channel-info';
 export * from './client-info';
-export * from './oauth-integration';
+export * from './oauth-web-integration';
+export * from './oauth-native-integration';
 export * from './pairing-authority-integration';
 export * from './pairing-supplicant-integration';
 export * from './signin-signup-info';