[AC-1139] Moved override logic from syncService to organizationService

apps/web/src/app/billing/organizations/sm-subscribe-standalone.component.ts

@@ -9,6 +9,8 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga
 import { SecretsManagerSubscribeRequest } from "@bitwarden/common/billing/models/request/sm-subscribe.request";
 import { BillingCustomerDiscount } from "@bitwarden/common/billing/models/response/organization-subscription.response";
 import { PlanResponse } from "@bitwarden/common/billing/models/response/plan.response";
+import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 
@@ -32,7 +34,8 @@ export class SecretsManagerSubscribeStandaloneComponent {
     private platformUtilsService: PlatformUtilsService,
     private i18nService: I18nService,
     private organizationApiService: OrganizationApiServiceAbstraction,
-    private organizationService: InternalOrganizationServiceAbstraction
+    private organizationService: InternalOrganizationServiceAbstraction,
+    private configService: ConfigServiceAbstraction,
   ) {}
 
   submit = async () => {
@@ -46,13 +49,17 @@ export class SecretsManagerSubscribeStandaloneComponent {
 
     const profileOrganization = await this.organizationApiService.subscribeToSecretsManager(
       this.organization.id,
-      request
+      request,
     );
     const organizationData = new OrganizationData(profileOrganization, {
       isMember: this.organization.isMember,
       isProviderUser: this.organization.isProviderUser,
     });
-    await this.organizationService.upsert(organizationData);
+    const flexibleCollectionsEnabled = await this.configService.getFeatureFlag(
+      FeatureFlag.FlexibleCollections,
+      false,
+    );
+    await this.organizationService.upsert(organizationData, flexibleCollectionsEnabled);
 
     /*
       Because subscribing to Secrets Manager automatically provides access to Secrets Manager for the
@@ -63,7 +70,7 @@ export class SecretsManagerSubscribeStandaloneComponent {
     this.platformUtilsService.showToast(
       "success",
       null,
-      this.i18nService.t("subscribedToSecretsManager")
+      this.i18nService.t("subscribedToSecretsManager"),
     );
 
     this.onSubscribe.emit();

libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts

@@ -53,13 +53,15 @@ export function getOrganizationById(id: string) {
 
 export function canAccessAdmin(i18nService: I18nService) {
   return map<Organization[], Organization[]>((orgs) =>
-    orgs.filter(canAccessOrgAdmin).sort(Utils.getSortFunction(i18nService, "name"))
+    orgs.filter(canAccessOrgAdmin).sort(Utils.getSortFunction(i18nService, "name")),
   );
 }
 
 export function canAccessImportExport(i18nService: I18nService) {
   return map<Organization[], Organization[]>((orgs) =>
-    orgs.filter((org) => org.canAccessImportExport).sort(Utils.getSortFunction(i18nService, "name"))
+    orgs
+      .filter((org) => org.canAccessImportExport)
+      .sort(Utils.getSortFunction(i18nService, "name")),
   );
 }
 
@@ -93,6 +95,12 @@ export abstract class OrganizationService {
 }
 
 export abstract class InternalOrganizationServiceAbstraction extends OrganizationService {
-  replace: (organizations: { [id: string]: OrganizationData }) => Promise<void>;
+  replace: (
-  upsert: (OrganizationData: OrganizationData | OrganizationData[]) => Promise<void>;
+    organizations: { [id: string]: OrganizationData },
+    flexibleCollectionsEnabled: boolean,
+  ) => Promise<void>;
+  upsert: (
+    OrganizationData: OrganizationData | OrganizationData[],
+    flexibleCollectionsEnabled: boolean,
+  ) => Promise<void>;
 }

libs/common/src/admin-console/services/organization/organization.service.spec.ts

@@ -14,7 +14,7 @@ describe("Organization Service", () => {
   let activeAccountUnlocked: BehaviorSubject<boolean>;
 
   const resetStateService = async (
-    customizeStateService: (stateService: MockProxy<StateService>) => void
+    customizeStateService: (stateService: MockProxy<StateService>) => void,
   ) => {
     mockClear(stateService);
     stateService = mock<StateService>();
@@ -110,7 +110,7 @@ describe("Organization Service", () => {
   });
 
   it("upsert", async () => {
-    await organizationService.upsert(organizationData("2", "Test 2"));
+    await organizationService.upsert(organizationData("2", "Test 2"), false);
 
     expect(await firstValueFrom(organizationService.organizations$)).toEqual([
       {
@@ -146,7 +146,7 @@ describe("Organization Service", () => {
 
   describe("delete", () => {
     it("exists", async () => {
-      await organizationService.delete("1");
+      await organizationService.delete("1", false);
 
       expect(stateService.getOrganizations).toHaveBeenCalledTimes(2);
 
@@ -154,7 +154,7 @@ describe("Organization Service", () => {
     });
 
     it("does not exist", async () => {
-      organizationService.delete("1");
+      organizationService.delete("1", false);
 
       expect(stateService.getOrganizations).toHaveBeenCalledTimes(2);
     });

libs/common/src/admin-console/services/organization/organization.service.ts

@@ -5,6 +5,7 @@ import {
   InternalOrganizationServiceAbstraction,
   isMember,
 } from "../../abstractions/organization/organization.service.abstraction";
+import { OrganizationUserType } from "../../enums";
 import { OrganizationData } from "../../models/data/organization.data";
 import { Organization } from "../../models/domain/organization";
 
@@ -25,7 +26,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
 
           const data = await this.stateService.getOrganizations();
           this.updateObservables(data);
-        })
+        }),
       )
       .subscribe();
   }
@@ -42,7 +43,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
   async canManageSponsorships(): Promise<boolean> {
     const organizations = this._organizations.getValue();
     return organizations.some(
-      (o) => o.familySponsorshipAvailable || o.familySponsorshipFriendlyName !== null
+      (o) => o.familySponsorshipAvailable || o.familySponsorshipFriendlyName !== null,
     );
   }
 
@@ -51,7 +52,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
     return organizations.length > 0;
   }
 
-  async upsert(organization: OrganizationData): Promise<void> {
+  async upsert(organization: OrganizationData, flexibleCollectionsEnabled: boolean): Promise<void> {
     let organizations = await this.stateService.getOrganizations();
     if (organizations == null) {
       organizations = {};
@@ -59,10 +60,10 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
 
     organizations[organization.id] = organization;
 
-    await this.replace(organizations);
+    await this.replace(organizations, flexibleCollectionsEnabled);
   }
 
-  async delete(id: string): Promise<void> {
+  async delete(id: string, flexibleCollectionsEnabled: boolean): Promise<void> {
     const organizations = await this.stateService.getOrganizations();
     if (organizations == null) {
       return;
@@ -73,7 +74,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
     }
 
     delete organizations[id];
-    await this.replace(organizations);
+    await this.replace(organizations, flexibleCollectionsEnabled);
   }
 
   get(id: string): Organization {
@@ -102,7 +103,24 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
     return organizations.find((organization) => organization.identifier === identifier);
   }
 
-  async replace(organizations: { [id: string]: OrganizationData }) {
+  async replace(
+    organizations: { [id: string]: OrganizationData },
+    flexibleCollectionsEnabled: boolean,
+  ) {
+    // If Flexible Collections is enabled, treat Managers as Users and ignore deprecated permissions
+    if (flexibleCollectionsEnabled) {
+      Object.values(organizations).forEach((o) => {
+        if (o.type === OrganizationUserType.Manager) {
+          o.type = OrganizationUserType.User;
+        }
+
+        if (o.permissions != null) {
+          o.permissions.editAssignedCollections = false;
+          o.permissions.deleteAssignedCollections = false;
+        }
+      });
+    }
+
     await this.stateService.setOrganizations(organizations);
     this.updateObservables(organizations);
   }

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

@@ -62,7 +62,7 @@ export class SyncService implements SyncServiceAbstraction {
     private organizationService: InternalOrganizationServiceAbstraction,
     private sendApiService: SendApiService,
     private configService: ConfigServiceAbstraction,
-    private logoutCallback: (expired: boolean) => Promise<void>
+    private logoutCallback: (expired: boolean) => Promise<void>,
   ) {}
 
   async getLastSync(): Promise<Date> {
@@ -321,7 +321,11 @@ export class SyncService implements SyncServiceAbstraction {
 
     await this.setForceSetPasswordReasonIfNeeded(response);
 
-    await this.syncProfileOrganizations(response);
+    const flexibleCollectionsEnabled = await this.configService.getFeatureFlag(
+      FeatureFlag.FlexibleCollections,
+      false,
+    );
+    await this.syncProfileOrganizations(response, flexibleCollectionsEnabled);
 
     const providers: { [id: string]: ProviderData } = {};
     response.providers.forEach((p) => {
@@ -342,7 +346,7 @@ export class SyncService implements SyncServiceAbstraction {
     // The `forcePasswordReset` flag indicates an admin has reset the user's password and must be updated
     if (profileResponse.forcePasswordReset) {
       await this.stateService.setForceSetPasswordReason(
-        ForceSetPasswordReason.AdminForcePasswordReset
+        ForceSetPasswordReason.AdminForcePasswordReset,
       );
     }
 
@@ -372,12 +376,15 @@ export class SyncService implements SyncServiceAbstraction {
       // TDE user w/out MP went from having no password reset permission to having it.
       // Must set the force password reset reason so the auth guard will redirect to the set password page.
       await this.stateService.setForceSetPasswordReason(
-        ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission
+        ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission,
       );
     }
   }
 
-  private async syncProfileOrganizations(response: ProfileResponse) {
+  private async syncProfileOrganizations(
+    response: ProfileResponse,
+    flexibleCollectionsEnabled: boolean,
+  ) {
     const organizations: { [id: string]: OrganizationData } = {};
     response.organizations.forEach((o) => {
       organizations[o.id] = new OrganizationData(o, {
@@ -397,21 +404,7 @@ export class SyncService implements SyncServiceAbstraction {
       }
     });
 
-    // If Flexible Collections is enabled, treat Managers as Users and ignore deprecated permissions
+    await this.organizationService.replace(organizations, flexibleCollectionsEnabled);
-    if (await this.configService.getFeatureFlag(FeatureFlag.FlexibleCollections)) {
-      Object.values(organizations).forEach((o) => {
-        if (o.type === OrganizationUserType.Manager) {
-          o.type = OrganizationUserType.User;
-        }
-
-        if (o.permissions != null) {
-          o.permissions.editAssignedCollections = false;
-          o.permissions.deleteAssignedCollections = false;
-        }
-      });
-    }
-
-    await this.organizationService.replace(organizations);
   }
 
   private async syncFolders(response: FolderResponse[]) {