From 2ff990edd28ad9e6e6bc2c573fecb412033b5695 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Fri, 5 Apr 2024 13:10:24 -0500 Subject: [PATCH] Update policy service to clear its own state (#8564) --- .../browser/src/background/main.background.ts | 1 - apps/cli/src/bw.ts | 1 - apps/desktop/src/app/app.component.ts | 1 - apps/web/src/app/app.component.ts | 1 - .../policy/policy.service.abstraction.ts | 1 - .../services/policy/policy.service.spec.ts | 60 ------------------- .../services/policy/policy.service.ts | 9 +-- 7 files changed, 3 insertions(+), 71 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 255538de52..235ccdf1aa 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1139,7 +1139,6 @@ export default class MainBackground { this.cipherService.clear(userId), this.folderService.clear(userId), this.collectionService.clear(userId), - this.policyService.clear(userId), this.passwordGenerationService.clear(userId), this.vaultTimeoutSettingsService.clear(userId), this.vaultFilterService.clear(), diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 3815fc773b..08d43a88ef 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -702,7 +702,6 @@ export class Main { this.cipherService.clear(userId), this.folderService.clear(userId), this.collectionService.clear(userId as UserId), - this.policyService.clear(userId as UserId), this.passwordGenerationService.clear(), this.providerService.save(null, userId as UserId), ]); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 4e74135c49..71b272b897 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -583,7 +583,6 @@ export class AppComponent implements OnInit, OnDestroy { await this.collectionService.clear(userBeingLoggedOut); await this.passwordGenerationService.clear(userBeingLoggedOut); await this.vaultTimeoutSettingsService.clear(userBeingLoggedOut); - await this.policyService.clear(userBeingLoggedOut); await this.biometricStateService.logout(userBeingLoggedOut as UserId); await this.providerService.save(null, userBeingLoggedOut as UserId); diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 32f4ee67e2..628875f04a 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -274,7 +274,6 @@ export class AppComponent implements OnDestroy, OnInit { this.cipherService.clear(userId), this.folderService.clear(userId), this.collectionService.clear(userId), - this.policyService.clear(userId), this.passwordGenerationService.clear(), this.biometricStateService.logout(userId as UserId), this.paymentMethodWarningService.clear(), diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index fb805f94cd..21669f78ad 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -78,5 +78,4 @@ export abstract class PolicyService { export abstract class InternalPolicyService extends PolicyService { upsert: (policy: PolicyData) => Promise; replace: (policies: { [id: string]: PolicyData }) => Promise; - clear: (userId?: string) => Promise; } diff --git a/libs/common/src/admin-console/services/policy/policy.service.spec.ts b/libs/common/src/admin-console/services/policy/policy.service.spec.ts index 8fa79f4d1c..a1633d29ff 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.spec.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.spec.ts @@ -102,66 +102,6 @@ describe("PolicyService", () => { ]); }); - describe("clear", () => { - beforeEach(() => { - activeUserState.nextState( - arrayToRecord([ - policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { - minutes: 14, - }), - ]), - ); - }); - - it("clears state for the active user", async () => { - await policyService.clear(); - - expect(await firstValueFrom(policyService.policies$)).toEqual([]); - expect(await firstValueFrom(activeUserState.state$)).toEqual(null); - expect(stateProvider.activeUser.getFake(POLICIES).nextMock).toHaveBeenCalledWith([ - "userId", - null, - ]); - }); - - it("clears state for an inactive user", async () => { - const inactiveUserId = "someOtherUserId" as UserId; - const inactiveUserState = stateProvider.singleUser.getFake(inactiveUserId, POLICIES); - inactiveUserState.nextState( - arrayToRecord([ - policyData("10", "another-test-organization", PolicyType.PersonalOwnership, true), - ]), - ); - - await policyService.clear(inactiveUserId); - - // Active user is not affected - const expectedActiveUserPolicy: Partial = { - id: "1" as PolicyId, - organizationId: "test-organization", - type: PolicyType.MaximumVaultTimeout, - enabled: true, - data: { minutes: 14 }, - }; - expect(await firstValueFrom(policyService.policies$)).toEqual([expectedActiveUserPolicy]); - expect(await firstValueFrom(activeUserState.state$)).toEqual({ - "1": expectedActiveUserPolicy, - }); - expect(stateProvider.activeUser.getFake(POLICIES).nextMock).not.toHaveBeenCalled(); - - // Non-active user is cleared - expect( - await firstValueFrom( - policyService.getAll$(PolicyType.PersonalOwnership, "someOtherUserId" as UserId), - ), - ).toEqual([]); - expect(await firstValueFrom(inactiveUserState.state$)).toEqual(null); - expect( - stateProvider.singleUser.getFake("someOtherUserId" as UserId, POLICIES).nextMock, - ).toHaveBeenCalledWith(null); - }); - }); - describe("masterPasswordPolicyOptions", () => { it("returns default policy options", async () => { const data: any = { diff --git a/libs/common/src/admin-console/services/policy/policy.service.ts b/libs/common/src/admin-console/services/policy/policy.service.ts index d60d2e3293..0cbc7204de 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.ts @@ -1,6 +1,6 @@ import { combineLatest, firstValueFrom, map, Observable, of } from "rxjs"; -import { KeyDefinition, POLICIES_DISK, StateProvider } from "../../../platform/state"; +import { UserKeyDefinition, POLICIES_DISK, StateProvider } from "../../../platform/state"; import { PolicyId, UserId } from "../../../types/guid"; import { OrganizationService } from "../../abstractions/organization/organization.service.abstraction"; import { InternalPolicyService as InternalPolicyServiceAbstraction } from "../../abstractions/policy/policy.service.abstraction"; @@ -14,8 +14,9 @@ import { ResetPasswordPolicyOptions } from "../../models/domain/reset-password-p const policyRecordToArray = (policiesMap: { [id: string]: PolicyData }) => Object.values(policiesMap || {}).map((f) => new Policy(f)); -export const POLICIES = KeyDefinition.record(POLICIES_DISK, "policies", { +export const POLICIES = UserKeyDefinition.record(POLICIES_DISK, "policies", { deserializer: (policyData) => policyData, + clearOn: ["logout"], }); export class PolicyService implements InternalPolicyServiceAbstraction { @@ -222,10 +223,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction { await this.activeUserPolicyState.update(() => policies); } - async clear(userId?: UserId): Promise { - await this.stateProvider.setUserState(POLICIES, null, userId); - } - /** * Determines whether an orgUser is exempt from a specific policy because of their role * Generally orgUsers who can manage policies are exempt from them, but some policies are stricter