From 235fb8f6eececa4c985fd34c3271346f97ed9626 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rui=20Tom=C3=A9?=
 <108268980+r-tome@users.noreply.github.com>
Date: Fri, 11 Nov 2022 15:13:27 +0000
Subject: [PATCH] [EC-739 / EC-740] Add null check to policyFilter on
 PolicyService (#4039)

* [EC-739 / EC-740] Add null check to policyFilter on PolicyService

* [EC-739 / EC-740] Add unit tests for policy filter
---
 .../spec/services/policy.service.spec.ts      | 58 ++++++++++++++++++-
 .../src/services/policy/policy.service.ts     |  9 +--
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/libs/common/spec/services/policy.service.spec.ts b/libs/common/spec/services/policy.service.spec.ts
index 232f971327..cbdbfca2aa 100644
--- a/libs/common/spec/services/policy.service.spec.ts
+++ b/libs/common/spec/services/policy.service.spec.ts
@@ -50,6 +50,12 @@ describe("PolicyService", () => {
     organizationService.getAll(null).resolves([]);
     activeAccount = new BehaviorSubject("123");
     activeAccountUnlocked = new BehaviorSubject(true);
+    stateService.getDecryptedPolicies({ userId: "user" }).resolves(null);
+    stateService.getEncryptedPolicies({ userId: "user" }).resolves({
+      "1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, {
+        minutes: 14,
+      }),
+    });
     stateService.getEncryptedPolicies().resolves({
       "1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, {
         minutes: 14,
@@ -296,7 +302,7 @@ describe("PolicyService", () => {
     });
   });
 
-  describe("policyAppliesToActiveUser", () => {
+  describe("policyAppliesToActiveUser$", () => {
     it("MasterPassword does not apply", async () => {
       const result = await firstValueFrom(
         policyService.policyAppliesToActiveUser$(PolicyType.MasterPassword)
@@ -313,6 +319,14 @@ describe("PolicyService", () => {
       expect(result).toEqual(true);
     });
 
+    it("PolicyFilter filters result", async () => {
+      const result = await firstValueFrom(
+        policyService.policyAppliesToActiveUser$(PolicyType.MaximumVaultTimeout, (p) => false)
+      );
+
+      expect(result).toEqual(false);
+    });
+
     it("DisablePersonalVaultExport does not apply", async () => {
       const result = await firstValueFrom(
         policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport)
@@ -322,6 +336,48 @@ describe("PolicyService", () => {
     });
   });
 
+  describe("policyAppliesToUser", () => {
+    it("MasterPassword does not apply", async () => {
+      const result = await policyService.policyAppliesToUser(
+        PolicyType.MasterPassword,
+        null,
+        "user"
+      );
+
+      expect(result).toEqual(false);
+    });
+
+    it("MaximumVaultTimeout applies", async () => {
+      const result = await policyService.policyAppliesToUser(
+        PolicyType.MaximumVaultTimeout,
+        null,
+        "user"
+      );
+
+      expect(result).toEqual(true);
+    });
+
+    it("PolicyFilter filters result", async () => {
+      const result = await policyService.policyAppliesToUser(
+        PolicyType.MaximumVaultTimeout,
+        (p) => false,
+        "user"
+      );
+
+      expect(result).toEqual(false);
+    });
+
+    it("DisablePersonalVaultExport does not apply", async () => {
+      const result = await policyService.policyAppliesToUser(
+        PolicyType.DisablePersonalVaultExport,
+        null,
+        "user"
+      );
+
+      expect(result).toEqual(false);
+    });
+  });
+
   function policyData(
     id: string,
     organizationId: string,
diff --git a/libs/common/src/services/policy/policy.service.ts b/libs/common/src/services/policy/policy.service.ts
index 9f084eb4d6..3dfe345633 100644
--- a/libs/common/src/services/policy/policy.service.ts
+++ b/libs/common/src/services/policy/policy.service.ts
@@ -124,10 +124,7 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
     );
   }
 
-  policyAppliesToActiveUser$(
-    policyType: PolicyType,
-    policyFilter: (policy: Policy) => boolean = (p) => true
-  ) {
+  policyAppliesToActiveUser$(policyType: PolicyType, policyFilter?: (policy: Policy) => boolean) {
     return this.policies$.pipe(
       concatMap(async (policies) => {
         const userId = await this.stateService.getUserId();
@@ -257,12 +254,12 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
   private async checkPoliciesThatApplyToUser(
     policies: Policy[],
     policyType: PolicyType,
-    policyFilter: (policy: Policy) => boolean = (p) => true,
+    policyFilter?: (policy: Policy) => boolean,
     userId?: string
   ) {
     const organizations = await this.organizationService.getAll(userId);
     const filteredPolicies = policies.filter(
-      (p) => p.type === policyType && p.enabled && policyFilter(p)
+      (p) => p.type === policyType && p.enabled && (policyFilter == null || policyFilter(p))
     );
     const policySet = new Set(filteredPolicies.map((p) => p.organizationId));