[EC-377] Transition Policy service into providing observables (#3259)

* Added abstractions for PolicyApiService and PolicyService * Added implementations for PolicyApiService and PolicyService * Updated all references to new PolicyApiService and PolicyService * Deleted old PolicyService abstraction and implementation * Fixed CLI import path for policy.service * Fixed main.background.ts policyApiService dependency for policyService * Ran prettier * Updated policy-api.service with the correct imports * [EC-377] Removed methods from StateService that read policies * [EC-377] Updated policy service getAll method to use observable collection * [EC-377] Added first unit tests for policy service * [EC-377] Added more unit tests for Policy Service * [EC-376] Sorted methods order in PolicyApiService * [EC-376] Removed unused clearCache method from PolicyService * [EC-376] Added upsert method to PolicyService * [EC-376] PolicyApiService putPolicy method now upserts data to PolicyService * [EC-377] Removed tests for deleted clearCache method * [EC-377] Added unit test for PolicyService.upsert * [EC-377] Updated references to state service observables * [EC-377] Removed getAll method from PolicyService and refactored components to use observable collection * [EC-377] Updated components to use concatMap instead of async subscribe * [EC-377] Removed getPolicyForOrganization from policyApiService * [EC-377] Updated policyAppliesToUser to return observable collection * [EC-377] Changed policyService.policyAppliesToUser to return observable * [EC-377] Fixed browser settings.component.ts getting vault timeout * Updated people.component.ts to get ResetPassword policy through a subscription * [EC-377] Changed passwordGenerationService.getOptions to return observable * [EC-377] Fixed CLI generate.command.ts getting enforcePasswordGeneratorPoliciesOnOptions * [EC-377] Fixed eslint errors on rxjs * [EC-377] Reverted changes on passwordGeneration.service and vaultTimeout.service * [EC-377] Removed eslint disable on web/vault/add-edit-component * [EC-377] Changed AccountData.policies to TemporaryDataEncryption * [EC-377] Updated import.component to be reactive to policyAppliesToUser$ * [EC-377] Updated importBlockedByPolicy$ * [EC-377] Fixed missing rename * [EC-377] Updated policyService.masterPasswordPolicyOptions to return observable * [EC-377] Fixed vaultTimeout imports from merge * [EC-377] Reverted call to passwordGenerationService.getOptions * [EC-377] Reverted call to enforcePasswordGeneratorPoliciesOnOptions * [EC-377] Removed unneeded ngOnDestroy * Apply suggestions from code review Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com> * [EC-377] Fixed login.component.ts and register.component.ts * [EC-377] Updated PolicyService to update vaultTimeout * [EC-377] Updated PolicyService dependencies * [EC-377] Renamed policyAppliesToUser to policyAppliesToActiveUser * [EC-377] VaultTimeoutSettings service now gets the vault timeout directly instead of using observables * [EC-377] Fixed unit tests by removing unneeded vaultTimeoutSettingsService * [EC-377] Set getDecryptedPolicies and setDecryptedPolicies as deprecated * [EC-377] Set PolicyService.getAll as deprecated and updated to use prototype.hasOwnProperty * [EC-565] Reverted unintended change to vaultTimeoutSettings that was causing a bug to not display the correct vault timeout * [EC-377] Removed unneeded destroy$ from preferences.component.ts * [EC-377] Fixed policy.service.ts import of OrganizationService Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com> Co-authored-by: mimartin12 <77340197+mimartin12@users.noreply.github.com>
bitwarden · Oct 11, 2022 · 3a298bd · 3a298bd
1 parent ea12ee2
commit 3a298bd
Show file tree

Hide file tree

Showing 35 changed files with 923 additions and 279 deletions.
diff --git a/apps/browser/src/background/commands.background.ts b/apps/browser/src/background/commands.background.ts
@@ -64,7 +64,7 @@ export default class CommandsBackground {
   }
 
   private async generatePasswordToClipboard() {
-    const options = (await this.passwordGenerationService.getOptions())[0];
+    const options = (await this.passwordGenerationService.getOptions())?.[0] ?? {};
     const password = await this.passwordGenerationService.generatePassword(options);
     this.platformUtilsService.copyToClipboard(password, { window: window });
     this.passwordGenerationService.addHistory(password);

diff --git a/apps/browser/src/background/contextMenus.background.ts b/apps/browser/src/background/contextMenus.background.ts
@@ -66,7 +66,7 @@ export default class ContextMenusBackground {
   }
 
   private async generatePasswordToClipboard() {
-    const options = (await this.passwordGenerationService.getOptions())[0];
+    const options = (await this.passwordGenerationService.getOptions())?.[0] ?? {};
     const password = await this.passwordGenerationService.generatePassword(options);
     this.platformUtilsService.copyToClipboard(password, { window: window });
     this.passwordGenerationService.addHistory(password);

diff --git a/apps/browser/src/background/notification.background.ts b/apps/browser/src/background/notification.background.ts
@@ -446,6 +446,8 @@ export default class NotificationBackground {
   }
 
   private async allowPersonalOwnership(): Promise<boolean> {
-    return !(await this.policyService.policyAppliesToUser(PolicyType.PersonalOwnership));
+    return !(await firstValueFrom(
+      this.policyService.policyAppliesToActiveUser$(PolicyType.PersonalOwnership)
+    ));
   }
 }
diff --git a/apps/cli/src/commands/export.command.ts b/apps/cli/src/commands/export.command.ts
@@ -1,5 +1,6 @@
 import * as program from "commander";
 import * as inquirer from "inquirer";
+import { firstValueFrom } from "rxjs";
 
 import { ExportFormat, ExportService } from "@bitwarden/common/abstractions/export.service";
 import { PolicyService } from "@bitwarden/common/abstractions/policy/policy.service.abstraction";
@@ -15,7 +16,9 @@ export class ExportCommand {
   async run(options: program.OptionValues): Promise<Response> {
     if (
       options.organizationid == null &&
-      (await this.policyService.policyAppliesToUser(PolicyType.DisablePersonalVaultExport))
+      (await firstValueFrom(
+        this.policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport)
+      ))
     ) {
       return Response.badRequest(
         "One or more organization policies prevents you from exporting your personal vault."

diff --git a/apps/web/src/app/accounts/login/login.component.ts b/apps/web/src/app/accounts/login/login.component.ts
@@ -1,6 +1,7 @@
-import { Component, NgZone } from "@angular/core";
+import { Component, NgZone, OnDestroy, OnInit } from "@angular/core";
 import { FormBuilder } from "@angular/forms";
 import { ActivatedRoute, Router } from "@angular/router";
+import { Subject, takeUntil } from "rxjs";
 import { first } from "rxjs/operators";
 
 import { LoginComponent as BaseLoginComponent } from "@bitwarden/angular/components/login.component";
@@ -29,13 +30,14 @@ import { RouterService, StateService } from "../../core";
   selector: "app-login",
   templateUrl: "login.component.html",
 })
-// eslint-disable-next-line rxjs-angular/prefer-takeuntil
-export class LoginComponent extends BaseLoginComponent {
+export class LoginComponent extends BaseLoginComponent implements OnInit, OnDestroy {
   showResetPasswordAutoEnrollWarning = false;
   enforcedPasswordPolicyOptions: MasterPasswordPolicyOptions;
   policies: ListResponse<PolicyResponse>;
   showPasswordless = false;
 
+  private destroy$ = new Subject<void>();
+
   constructor(
     authService: AuthService,
     router: Router,
@@ -128,12 +130,21 @@ export class LoginComponent extends BaseLoginComponent {
         this.showResetPasswordAutoEnrollWarning =
           resetPasswordPolicy[1] && resetPasswordPolicy[0].autoEnrollEnabled;
 
-        this.enforcedPasswordPolicyOptions =
-          await this.policyService.getMasterPasswordPolicyOptions(policyList);
+        this.policyService
+          .masterPasswordPolicyOptions$(policyList)
+          .pipe(takeUntil(this.destroy$))
+          .subscribe((enforcedPasswordPolicyOptions) => {
+            this.enforcedPasswordPolicyOptions = enforcedPasswordPolicyOptions;
+          });
       }
     }
   }
 
+  ngOnDestroy(): void {
+    this.destroy$.next();
+    this.destroy$.complete();
+  }
+
   async goAfterLogIn() {
     const masterPassword = this.formGroup.value.masterPassword;
 

diff --git a/apps/web/src/app/accounts/register.component.ts b/apps/web/src/app/accounts/register.component.ts
@@ -1,6 +1,7 @@
-import { Component } from "@angular/core";
+import { Component, OnDestroy, OnInit } from "@angular/core";
 import { UntypedFormBuilder } from "@angular/forms";
 import { ActivatedRoute, Router } from "@angular/router";
+import { Subject, takeUntil } from "rxjs";
 import { first } from "rxjs/operators";
 
 import { RegisterComponent as BaseRegisterComponent } from "@bitwarden/angular/components/register.component";
@@ -27,14 +28,14 @@ import { RouterService } from "../core";
   selector: "app-register",
   templateUrl: "register.component.html",
 })
-// eslint-disable-next-line rxjs-angular/prefer-takeuntil
-export class RegisterComponent extends BaseRegisterComponent {
+export class RegisterComponent extends BaseRegisterComponent implements OnInit, OnDestroy {
   email = "";
   showCreateOrgMessage = false;
   layout = "";
   enforcedPolicyOptions: MasterPasswordPolicyOptions;
 
   private policies: Policy[];
+  private destroy$ = new Subject<void>();
 
   constructor(
     formValidationErrorService: FormValidationErrorsService,
@@ -130,11 +131,19 @@ export class RegisterComponent extends BaseRegisterComponent {
     }
 
     if (this.policies != null) {
-      this.enforcedPolicyOptions = await this.policyService.getMasterPasswordPolicyOptions(
-        this.policies
-      );
+      this.policyService
+        .masterPasswordPolicyOptions$(this.policies)
+        .pipe(takeUntil(this.destroy$))
+        .subscribe((enforcedPasswordPolicyOptions) => {
+          this.enforcedPolicyOptions = enforcedPasswordPolicyOptions;
+        });
     }
 
     await super.ngOnInit();
   }
+
+  ngOnDestroy(): void {
+    this.destroy$.next();
+    this.destroy$.complete();
+  }
 }
diff --git a/apps/web/src/app/accounts/trial-initiation/trial-initiation.component.spec.ts b/apps/web/src/app/accounts/trial-initiation/trial-initiation.component.spec.ts
@@ -7,7 +7,7 @@ import { ActivatedRoute, Router } from "@angular/router";
 import { RouterTestingModule } from "@angular/router/testing";
 // eslint-disable-next-line no-restricted-imports
 import { Substitute } from "@fluffy-spoon/substitute";
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, of } from "rxjs";
 
 import { I18nPipe } from "@bitwarden/angular/pipes/i18n.pipe";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
@@ -47,7 +47,7 @@ describe("TrialInitiationComponent", () => {
     };
 
     policyServiceMock = {
-      getMasterPasswordPolicyOptions: jest.fn(),
+      masterPasswordPolicyOptions$: jest.fn(),
     };
 
     TestBed.configureTestingModule({
@@ -145,14 +145,16 @@ describe("TrialInitiationComponent", () => {
           },
         ],
       });
-      policyServiceMock.getMasterPasswordPolicyOptions.mockReturnValueOnce({
-        minComplexity: 4,
-        minLength: 10,
-        requireLower: null,
-        requireNumbers: null,
-        requireSpecial: null,
-        requireUpper: null,
-      } as MasterPasswordPolicyOptions);
+      policyServiceMock.masterPasswordPolicyOptions$.mockReturnValue(
+        of({
+          minComplexity: 4,
+          minLength: 10,
+          requireLower: null,
+          requireNumbers: null,
+          requireSpecial: null,
+          requireUpper: null,
+        } as MasterPasswordPolicyOptions)
+      );
 
       // Need to recreate component with new service mocks
       fixture = TestBed.createComponent(TrialInitiationComponent);

diff --git a/apps/web/src/app/accounts/trial-initiation/trial-initiation.component.ts b/apps/web/src/app/accounts/trial-initiation/trial-initiation.component.ts
@@ -1,9 +1,9 @@
 import { StepperSelectionEvent } from "@angular/cdk/stepper";
 import { TitleCasePipe } from "@angular/common";
-import { Component, OnInit, ViewChild } from "@angular/core";
+import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core";
 import { UntypedFormBuilder, Validators } from "@angular/forms";
 import { ActivatedRoute, Router } from "@angular/router";
-import { first } from "rxjs";
+import { first, Subject, takeUntil } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/abstractions/log.service";
@@ -24,8 +24,7 @@ import { VerticalStepperComponent } from "./vertical-stepper/vertical-stepper.co
   selector: "app-trial",
   templateUrl: "trial-initiation.component.html",
 })
-// eslint-disable-next-line rxjs-angular/prefer-takeuntil
-export class TrialInitiationComponent implements OnInit {
+export class TrialInitiationComponent implements OnInit, OnDestroy {
   email = "";
   org = "";
   orgInfoSubLabel = "";
@@ -63,6 +62,8 @@ export class TrialInitiationComponent implements OnInit {
     }
   }
 
+  private destroy$ = new Subject<void>();
+
   constructor(
     private route: ActivatedRoute,
     protected router: Router,
@@ -140,12 +141,20 @@ export class TrialInitiationComponent implements OnInit {
     }
 
     if (this.policies != null) {
-      this.enforcedPolicyOptions = await this.policyService.getMasterPasswordPolicyOptions(
-        this.policies
-      );
+      this.policyService
+        .masterPasswordPolicyOptions$(this.policies)
+        .pipe(takeUntil(this.destroy$))
+        .subscribe((enforcedPasswordPolicyOptions) => {
+          this.enforcedPolicyOptions = enforcedPasswordPolicyOptions;
+        });
     }
   }
 
+  ngOnDestroy(): void {
+    this.destroy$.next();
+    this.destroy$.complete();
+  }
+
   stepSelectionChange(event: StepperSelectionEvent) {
     // Set org info sub label
     if (event.selectedIndex === 1 && this.orgInfoFormGroup.controls.name.value === "") {

diff --git a/apps/web/src/app/core/event.service.ts b/apps/web/src/app/core/event.service.ts
@@ -1,16 +1,32 @@
-import { Injectable } from "@angular/core";
+import { Injectable, OnDestroy, OnInit } from "@angular/core";
+import { Subject, takeUntil } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { PolicyService } from "@bitwarden/common/abstractions/policy/policy.service.abstraction";
 import { DeviceType } from "@bitwarden/common/enums/deviceType";
 import { EventType } from "@bitwarden/common/enums/eventType";
 import { PolicyType } from "@bitwarden/common/enums/policyType";
+import { Policy } from "@bitwarden/common/models/domain/policy";
 import { EventResponse } from "@bitwarden/common/models/response/eventResponse";
 
 @Injectable()
-export class EventService {
+export class EventService implements OnInit, OnDestroy {
+  private destroy$ = new Subject<void>();
+  private policies: Policy[];
+
   constructor(private i18nService: I18nService, private policyService: PolicyService) {}
 
+  ngOnInit(): void {
+    this.policyService.policies$.pipe(takeUntil(this.destroy$)).subscribe((policies) => {
+      this.policies = policies;
+    });
+  }
+
+  ngOnDestroy() {
+    this.destroy$.next();
+    this.destroy$.complete();
+  }
+
   getDefaultDateFilters() {
     const d = new Date();
     const end = new Date(d.getFullYear(), d.getMonth(), d.getDate(), 23, 59);
@@ -326,8 +342,7 @@ export class EventService {
       case EventType.Policy_Updated: {
         msg = this.i18nService.t("modifiedPolicyId", this.formatPolicyId(ev));
 
-        const policies = await this.policyService.getAll();
-        const policy = policies.filter((p) => p.id === ev.policyId)[0];
+        const policy = this.policies.filter((p) => p.id === ev.policyId)[0];
         let p1 = this.getShortId(ev.policyId);
         if (policy != null) {
           p1 = PolicyType[policy.type];