[PS-1879] Fix Key Connector Migration Flow (#4080)

* Move OrganizationService to fullSync

* Add Tech Debt Tracking Link

* Remove Commented out code

* Add InternalOrganizationService

* Use InternalOrganization in services that get to update state

apps/browser/src/background/main.background.ts

@@ -17,7 +17,7 @@ import { KeyConnectorService as KeyConnectorServiceAbstraction } from "@bitwarde
 import { LogService as LogServiceAbstraction } from "@bitwarden/common/abstractions/log.service";
 import { MessagingService as MessagingServiceAbstraction } from "@bitwarden/common/abstractions/messaging.service";
 import { NotificationsService as NotificationsServiceAbstraction } from "@bitwarden/common/abstractions/notifications.service";
-import { OrganizationService as OrganizationServiceAbstraction } from "@bitwarden/common/abstractions/organization/organization.service.abstraction";
+import { InternalOrganizationService as InternalOrganizationServiceAbstraction } from "@bitwarden/common/abstractions/organization/organization.service.abstraction";
 import { PasswordGenerationService as PasswordGenerationServiceAbstraction } from "@bitwarden/common/abstractions/passwordGeneration.service";
 import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/abstractions/platformUtils.service";
 import { PolicyApiServiceAbstraction } from "@bitwarden/common/abstractions/policy/policy-api.service.abstraction";
@@ -153,7 +153,7 @@ export default class MainBackground {
   popupUtilsService: PopupUtilsService;
   sendService: SendServiceAbstraction;
   fileUploadService: FileUploadServiceAbstraction;
-  organizationService: OrganizationServiceAbstraction;
+  organizationService: InternalOrganizationServiceAbstraction;
   providerService: ProviderServiceAbstraction;
   keyConnectorService: KeyConnectorServiceAbstraction;
   userVerificationService: UserVerificationServiceAbstraction;
@@ -317,7 +317,7 @@ export default class MainBackground {
       this.stateService
     );
     this.syncNotifierService = new SyncNotifierService();
-    this.organizationService = new OrganizationService(this.stateService, this.syncNotifierService);
+    this.organizationService = new OrganizationService(this.stateService);
     this.policyService = new PolicyService(this.stateService, this.organizationService);
     this.policyApiService = new PolicyApiService(
       this.policyService,
@@ -409,7 +409,7 @@ export default class MainBackground {
       this.stateService,
       this.providerService,
       this.folderApiService,
-      this.syncNotifierService,
+      this.organizationService,
       logoutCallback
     );
     this.eventService = new EventService(

apps/browser/src/background/service_factories/organization-service.factory.ts

@@ -3,15 +3,10 @@ import { OrganizationService } from "@bitwarden/common/services/organization/org
 
 import { FactoryOptions, CachedServices, factory } from "./factory-options";
 import { stateServiceFactory, StateServiceInitOptions } from "./state-service.factory";
-import {
-  syncNotifierServiceFactory,
-  SyncNotifierServiceInitOptions,
-} from "./sync-notifier-service.factory";
 
 type OrganizationServiceFactoryOptions = FactoryOptions;
 
 export type OrganizationServiceInitOptions = OrganizationServiceFactoryOptions &
-  SyncNotifierServiceInitOptions &
   StateServiceInitOptions;
 
 export function organizationServiceFactory(
@@ -22,10 +17,6 @@ export function organizationServiceFactory(
     cache,
     "organizationService",
     opts,
-    async () =>
+    async () => new OrganizationService(await stateServiceFactory(cache, opts))
-      new OrganizationService(
-        await stateServiceFactory(cache, opts),
-        await syncNotifierServiceFactory(cache, opts)
-      )
   );
 }

apps/cli/src/bw.ts

@@ -240,7 +240,7 @@ export class Main {
 
     this.providerService = new ProviderService(this.stateService);
 
-    this.organizationService = new OrganizationService(this.stateService, this.syncNotifierService);
+    this.organizationService = new OrganizationService(this.stateService);
 
     this.policyService = new PolicyService(this.stateService, this.organizationService);
 
@@ -322,7 +322,7 @@ export class Main {
       this.stateService,
       this.providerService,
       this.folderApiService,
-      this.syncNotifierService,
+      this.organizationService,
       async (expired: boolean) => await this.logout()
     );
 

apps/web/webpack.config.js

@@ -273,6 +273,7 @@ const devServer =
                   https://quack.duckduckgo.com/api/email/addresses
                   https://app.anonaddy.com/api/v1/aliases
                   https://api.fastmail.com
+                  http://localhost:5000
                 ;object-src
                   'self'
                   blob:

libs/angular/src/services/jslib-services.module.ts

@@ -35,7 +35,10 @@ import { LoginService as LoginServiceAbstraction } from "@bitwarden/common/abstr
 import { MessagingService as MessagingServiceAbstraction } from "@bitwarden/common/abstractions/messaging.service";
 import { NotificationsService as NotificationsServiceAbstraction } from "@bitwarden/common/abstractions/notifications.service";
 import { OrganizationApiServiceAbstraction } from "@bitwarden/common/abstractions/organization/organization-api.service.abstraction";
-import { OrganizationService as OrganizationServiceAbstraction } from "@bitwarden/common/abstractions/organization/organization.service.abstraction";
+import {
+  InternalOrganizationService,
+  OrganizationService as OrganizationServiceAbstraction,
+} from "@bitwarden/common/abstractions/organization/organization.service.abstraction";
 import { PasswordGenerationService as PasswordGenerationServiceAbstraction } from "@bitwarden/common/abstractions/passwordGeneration.service";
 import { PasswordRepromptService as PasswordRepromptServiceAbstraction } from "@bitwarden/common/abstractions/passwordReprompt.service";
 import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/abstractions/platformUtils.service";
@@ -356,7 +359,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
         StateServiceAbstraction,
         ProviderServiceAbstraction,
         FolderApiServiceAbstraction,
-        SyncNotifierServiceAbstraction,
+        OrganizationServiceAbstraction,
         LOGOUT_CALLBACK,
       ],
     },
@@ -506,6 +509,8 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
         LogService,
         OrganizationServiceAbstraction,
         CryptoFunctionServiceAbstraction,
+        SyncNotifierServiceAbstraction,
+        MessagingServiceAbstraction,
         LOGOUT_CALLBACK,
       ],
     },
@@ -522,7 +527,11 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
     {
       provide: OrganizationServiceAbstraction,
       useClass: OrganizationService,
-      deps: [StateServiceAbstraction, SyncNotifierServiceAbstraction],
+      deps: [StateServiceAbstraction],
+    },
+    {
+      provide: InternalOrganizationService,
+      useExisting: OrganizationServiceAbstraction,
     },
     {
       provide: ProviderServiceAbstraction,

libs/common/spec/services/organization/organization.service.spec.ts

@@ -1,12 +1,9 @@
-import { MockProxy, mock, any, mockClear, matches } from "jest-mock-extended";
+import { MockProxy, mock, any, mockClear } from "jest-mock-extended";
-import { BehaviorSubject, firstValueFrom, Subject } from "rxjs";
+import { BehaviorSubject, firstValueFrom } from "rxjs";
 
 import { StateService } from "@bitwarden/common/abstractions/state.service";
-import { SyncNotifierService } from "@bitwarden/common/abstractions/sync/syncNotifier.service.abstraction";
 import { OrganizationData } from "@bitwarden/common/models/data/organization.data";
-import { SyncResponse } from "@bitwarden/common/models/response/sync.response";
 import { OrganizationService } from "@bitwarden/common/services/organization/organization.service";
-import { SyncEventArgs } from "@bitwarden/common/types/syncEventArgs";
 
 describe("Organization Service", () => {
   let organizationService: OrganizationService;
@@ -14,8 +11,6 @@ describe("Organization Service", () => {
   let stateService: MockProxy<StateService>;
   let activeAccount: BehaviorSubject<string>;
   let activeAccountUnlocked: BehaviorSubject<boolean>;
-  let syncNotifierService: MockProxy<SyncNotifierService>;
-  let sync: Subject<SyncEventArgs>;
 
   const resetStateService = async (
     customizeStateService: (stateService: MockProxy<StateService>) => void
@@ -25,7 +20,7 @@ describe("Organization Service", () => {
     stateService.activeAccount$ = activeAccount;
     stateService.activeAccountUnlocked$ = activeAccountUnlocked;
     customizeStateService(stateService);
-    organizationService = new OrganizationService(stateService, syncNotifierService);
+    organizationService = new OrganizationService(stateService);
     await new Promise((r) => setTimeout(r, 50));
   };
 
@@ -41,12 +36,7 @@ describe("Organization Service", () => {
       "1": organizationData("1", "Test Org"),
     });
 
-    sync = new Subject<SyncEventArgs>();
+    organizationService = new OrganizationService(stateService);
-
-    syncNotifierService = mock<SyncNotifierService>();
-    syncNotifierService.sync$ = sync;
-
-    organizationService = new OrganizationService(stateService, syncNotifierService);
   });
 
   afterEach(() => {
@@ -169,36 +159,6 @@ describe("Organization Service", () => {
     });
   });
 
-  describe("syncEvent works", () => {
-    it("Complete event updates data", async () => {
-      sync.next({
-        status: "Completed",
-        successfully: true,
-        data: new SyncResponse({
-          profile: {
-            organizations: [
-              {
-                id: "1",
-                name: "Updated Name",
-              },
-            ],
-          },
-        }),
-      });
-
-      await new Promise((r) => setTimeout(r, 500));
-
-      expect(stateService.setOrganizations).toHaveBeenCalledTimes(1);
-
-      expect(stateService.setOrganizations).toHaveBeenLastCalledWith(
-        matches((organizationData: { [id: string]: OrganizationData }) => {
-          const organization = organizationData["1"];
-          return organization?.name === "Updated Name";
-        })
-      );
-    });
-  });
-
   function organizationData(id: string, name: string) {
     const data = new OrganizationData({} as any);
     data.id = id;

libs/common/src/abstractions/organization/organization.service.abstraction.ts

@@ -1,6 +1,7 @@
 import { map, Observable } from "rxjs";
 
 import { Utils } from "../../misc/utils";
+import { OrganizationData } from "../../models/data/organization.data";
 import { Organization } from "../../models/domain/organization";
 import { I18nService } from "../i18n.service";
 
@@ -83,3 +84,7 @@ export abstract class OrganizationService {
   canManageSponsorships: () => Promise<boolean>;
   hasOrganizations: () => boolean;
 }
+
+export abstract class InternalOrganizationService extends OrganizationService {
+  replace: (organizations: { [id: string]: OrganizationData }) => Promise<void>;
+}

libs/common/src/services/organization/organization.service.ts

@@ -1,21 +1,16 @@
-import { BehaviorSubject, concatMap, filter } from "rxjs";
+import { BehaviorSubject, concatMap } from "rxjs";
 
-import { OrganizationService as OrganizationServiceAbstraction } from "../../abstractions/organization/organization.service.abstraction";
+import { InternalOrganizationService as InternalOrganizationServiceAbstraction } from "../../abstractions/organization/organization.service.abstraction";
 import { StateService } from "../../abstractions/state.service";
-import { SyncNotifierService } from "../../abstractions/sync/syncNotifier.service.abstraction";
 import { OrganizationData } from "../../models/data/organization.data";
 import { Organization } from "../../models/domain/organization";
-import { isSuccessfullyCompleted } from "../../types/syncEventArgs";
 
-export class OrganizationService implements OrganizationServiceAbstraction {
+export class OrganizationService implements InternalOrganizationServiceAbstraction {
   private _organizations = new BehaviorSubject<Organization[]>([]);
 
   organizations$ = this._organizations.asObservable();
 
-  constructor(
+  constructor(private stateService: StateService) {
-    private stateService: StateService,
-    private syncNotifierService: SyncNotifierService
-  ) {
     this.stateService.activeAccountUnlocked$
       .pipe(
         concatMap(async (unlocked) => {
@@ -29,28 +24,6 @@ export class OrganizationService implements OrganizationServiceAbstraction {
         })
       )
       .subscribe();
-
-    this.syncNotifierService.sync$
-      .pipe(
-        filter(isSuccessfullyCompleted),
-        concatMap(async ({ data }) => {
-          const { profile } = data;
-          const organizations: { [id: string]: OrganizationData } = {};
-          profile.organizations.forEach((o) => {
-            organizations[o.id] = new OrganizationData(o);
-          });
-
-          profile.providerOrganizations.forEach((o) => {
-            if (organizations[o.id] == null) {
-              organizations[o.id] = new OrganizationData(o);
-              organizations[o.id].isProviderUser = true;
-            }
-          });
-
-          await this.updateStateAndObservables(organizations);
-        })
-      )
-      .subscribe();
   }
 
   async getAll(userId?: string): Promise<Organization[]> {
@@ -78,7 +51,7 @@ export class OrganizationService implements OrganizationServiceAbstraction {
 
     organizations[organization.id] = organization;
 
-    await this.updateStateAndObservables(organizations);
+    await this.replace(organizations);
   }
 
   async delete(id: string): Promise<void> {
@@ -92,7 +65,7 @@ export class OrganizationService implements OrganizationServiceAbstraction {
     }
 
     delete organizations[id];
-    await this.updateStateAndObservables(organizations);
+    await this.replace(organizations);
   }
 
   get(id: string): Organization {
@@ -121,9 +94,9 @@ export class OrganizationService implements OrganizationServiceAbstraction {
     return organizations.find((organization) => organization.identifier === identifier);
   }
 
-  private async updateStateAndObservables(organizationsMap: { [id: string]: OrganizationData }) {
+  async replace(organizations: { [id: string]: OrganizationData }) {
-    await this.stateService.setOrganizations(organizationsMap);
+    await this.stateService.setOrganizations(organizations);
-    this.updateObservables(organizationsMap);
+    this.updateObservables(organizations);
   }
 
   private updateObservables(organizationsMap: { [id: string]: OrganizationData }) {

libs/common/src/services/sync/sync.service.ts

@@ -7,17 +7,18 @@ import { InternalFolderService } from "../../abstractions/folder/folder.service.
 import { KeyConnectorService } from "../../abstractions/keyConnector.service";
 import { LogService } from "../../abstractions/log.service";
 import { MessagingService } from "../../abstractions/messaging.service";
+import { InternalOrganizationService } from "../../abstractions/organization/organization.service.abstraction";
 import { InternalPolicyService } from "../../abstractions/policy/policy.service.abstraction";
 import { ProviderService } from "../../abstractions/provider.service";
 import { SendService } from "../../abstractions/send.service";
 import { SettingsService } from "../../abstractions/settings.service";
 import { StateService } from "../../abstractions/state.service";
 import { SyncService as SyncServiceAbstraction } from "../../abstractions/sync/sync.service.abstraction";
-import { SyncNotifierService } from "../../abstractions/sync/syncNotifier.service.abstraction";
 import { sequentialize } from "../../misc/sequentialize";
 import { CipherData } from "../../models/data/cipher.data";
 import { CollectionData } from "../../models/data/collection.data";
 import { FolderData } from "../../models/data/folder.data";
+import { OrganizationData } from "../../models/data/organization.data";
 import { PolicyData } from "../../models/data/policy.data";
 import { ProviderData } from "../../models/data/provider.data";
 import { SendData } from "../../models/data/send.data";
@@ -52,7 +53,7 @@ export class SyncService implements SyncServiceAbstraction {
     private stateService: StateService,
     private providerService: ProviderService,
     private folderApiService: FolderApiServiceAbstraction,
-    private syncNotifierService: SyncNotifierService,
+    private organizationService: InternalOrganizationService,
     private logoutCallback: (expired: boolean) => Promise<void>
   ) {}
 
@@ -76,10 +77,8 @@ export class SyncService implements SyncServiceAbstraction {
   @sequentialize(() => "fullSync")
   async fullSync(forceSync: boolean, allowThrowOnError = false): Promise<boolean> {
     this.syncStarted();
-    this.syncNotifierService.next({ status: "Started" });
     const isAuthenticated = await this.stateService.getIsAuthenticated();
     if (!isAuthenticated) {
-      this.syncNotifierService.next({ status: "Completed", successfully: false });
       return this.syncCompleted(false);
     }
 
@@ -95,7 +94,6 @@ export class SyncService implements SyncServiceAbstraction {
 
     if (!needsSync) {
       await this.setLastSync(now);
-      this.syncNotifierService.next({ status: "Completed", successfully: false });
       return this.syncCompleted(false);
     }
 
@@ -112,13 +110,11 @@ export class SyncService implements SyncServiceAbstraction {
       await this.syncPolicies(response.policies);
 
       await this.setLastSync(now);
-      this.syncNotifierService.next({ status: "Completed", successfully: true, data: response });
       return this.syncCompleted(true);
     } catch (e) {
       if (allowThrowOnError) {
         throw e;
       } else {
-        this.syncNotifierService.next({ status: "Completed", successfully: false });
         return this.syncCompleted(false);
       }
     }
@@ -315,11 +311,24 @@ export class SyncService implements SyncServiceAbstraction {
     await this.stateService.setForcePasswordReset(response.forcePasswordReset);
     await this.keyConnectorService.setUsesKeyConnector(response.usesKeyConnector);
 
+    const organizations: { [id: string]: OrganizationData } = {};
+    response.organizations.forEach((o) => {
+      organizations[o.id] = new OrganizationData(o);
+    });
+
     const providers: { [id: string]: ProviderData } = {};
     response.providers.forEach((p) => {
       providers[p.id] = new ProviderData(p);
     });
 
+    response.providerOrganizations.forEach((o) => {
+      if (organizations[o.id] == null) {
+        organizations[o.id] = new OrganizationData(o);
+        organizations[o.id].isProviderUser = true;
+      }
+    });
+
+    await this.organizationService.replace(organizations);
     await this.providerService.save(providers);
 
     if (await this.keyConnectorService.userNeedsMigration()) {