diff --git a/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts b/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts index efdb743196..b00693bd56 100644 --- a/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts +++ b/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts @@ -1,4 +1,5 @@ import { PolicyService as AbstractPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; import { CachedServices, @@ -9,11 +10,6 @@ import { stateProviderFactory, StateProviderInitOptions, } from "../../../platform/background/service-factories/state-provider.factory"; -import { - stateServiceFactory as stateServiceFactory, - StateServiceInitOptions, -} from "../../../platform/background/service-factories/state-service.factory"; -import { BrowserPolicyService } from "../../services/browser-policy.service"; import { organizationServiceFactory, @@ -23,7 +19,6 @@ import { type PolicyServiceFactoryOptions = FactoryOptions; export type PolicyServiceInitOptions = PolicyServiceFactoryOptions & - StateServiceInitOptions & StateProviderInitOptions & OrganizationServiceInitOptions; @@ -36,8 +31,7 @@ export function policyServiceFactory( "policyService", opts, async () => - new BrowserPolicyService( - await stateServiceFactory(cache, opts), + new PolicyService( await stateProviderFactory(cache, opts), await organizationServiceFactory(cache, opts), ), diff --git a/apps/browser/src/admin-console/services/browser-policy.service.ts b/apps/browser/src/admin-console/services/browser-policy.service.ts deleted file mode 100644 index 2022cfec58..0000000000 --- a/apps/browser/src/admin-console/services/browser-policy.service.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { BehaviorSubject } from "rxjs"; -import { Jsonify } from "type-fest"; - -import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; - -import { browserSession, sessionSync } from "../../platform/decorators/session-sync-observable"; - -@browserSession -export class BrowserPolicyService extends PolicyService { - @sessionSync({ - initializer: (obj: Jsonify) => Object.assign(new Policy(), obj), - initializeAs: "array", - }) - protected _policies: BehaviorSubject; -} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index ce0c67efa1..c5608dc830 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -22,6 +22,7 @@ import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abs import { InternalPolicyService as InternalPolicyServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { ProviderService as ProviderServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; +import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -175,7 +176,6 @@ import { } from "@bitwarden/vault-export-core"; import { BrowserOrganizationService } from "../admin-console/services/browser-organization.service"; -import { BrowserPolicyService } from "../admin-console/services/browser-policy.service"; import ContextMenusBackground from "../autofill/background/context-menus.background"; import NotificationBackground from "../autofill/background/notification.background"; import OverlayBackground from "../autofill/background/overlay.background"; @@ -501,11 +501,7 @@ export default class MainBackground { this.stateService, this.stateProvider, ); - this.policyService = new BrowserPolicyService( - this.stateService, - this.stateProvider, - this.organizationService, - ); + this.policyService = new PolicyService(this.stateProvider, this.organizationService); this.autofillSettingsService = new AutofillSettingsService( this.stateProvider, this.policyService, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 2e6dfaee7a..40d68eb630 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -102,7 +102,6 @@ import { ImportServiceAbstraction } from "@bitwarden/importer/core"; import { VaultExportServiceAbstraction } from "@bitwarden/vault-export-core"; import { BrowserOrganizationService } from "../../admin-console/services/browser-organization.service"; -import { BrowserPolicyService } from "../../admin-console/services/browser-policy.service"; import { UnauthGuardService } from "../../auth/popup/services"; import { AutofillService } from "../../autofill/services/abstractions/autofill.service"; import MainBackground from "../../background/main.background"; @@ -293,17 +292,6 @@ function getBgService(service: keyof MainBackground) { useFactory: getBgService("eventCollectionService"), deps: [], }, - { - provide: PolicyService, - useFactory: ( - stateService: StateServiceAbstraction, - stateProvider: StateProvider, - organizationService: OrganizationService, - ) => { - return new BrowserPolicyService(stateService, stateProvider, organizationService); - }, - deps: [StateServiceAbstraction, StateProvider, OrganizationService], - }, { provide: PlatformUtilsService, useFactory: getBgService("platformUtilsService"), diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index e46937f71f..581d37c9e6 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -394,11 +394,7 @@ export class Main { this.organizationUserService = new OrganizationUserServiceImplementation(this.apiService); - this.policyService = new PolicyService( - this.stateService, - this.stateProvider, - this.organizationService, - ); + this.policyService = new PolicyService(this.stateProvider, this.organizationService); this.policyApiService = new PolicyApiService(this.policyService, this.apiService); @@ -653,7 +649,7 @@ export class Main { this.cipherService.clear(userId), this.folderService.clear(userId), this.collectionService.clear(userId as UserId), - this.policyService.clear(userId), + this.policyService.clear(userId as UserId), this.passwordGenerationService.clear(), this.providerService.save(null, userId as UserId), ]); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index be3a65dcf2..5c83bc880c 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -678,7 +678,7 @@ import { ModalService } from "./modal.service"; { provide: PolicyServiceAbstraction, useClass: PolicyService, - deps: [StateServiceAbstraction, StateProvider, OrganizationServiceAbstraction], + deps: [StateProvider, OrganizationServiceAbstraction], }, { provide: InternalPolicyService, diff --git a/libs/angular/src/tools/send/add-edit.component.ts b/libs/angular/src/tools/send/add-edit.component.ts index 9d213cfca5..2e4b2cd57b 100644 --- a/libs/angular/src/tools/send/add-edit.component.ts +++ b/libs/angular/src/tools/send/add-edit.component.ts @@ -1,7 +1,7 @@ import { DatePipe } from "@angular/common"; import { Directive, EventEmitter, Input, OnDestroy, OnInit, Output } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; -import { Subject, takeUntil } from "rxjs"; +import { Subject, map, takeUntil } from "rxjs"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; @@ -151,8 +151,11 @@ export class AddEditComponent implements OnInit, OnDestroy { }); this.policyService - .policyAppliesToActiveUser$(PolicyType.SendOptions, (p) => p.data.disableHideEmail) - .pipe(takeUntil(this.destroy$)) + .getAll$(PolicyType.SendOptions) + .pipe( + map((policies) => policies?.some((p) => p.data.disableHideEmail)), + takeUntil(this.destroy$), + ) .subscribe((policyAppliesToActiveUser) => { if ( (this.disableHideEmail = policyAppliesToActiveUser) && 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 a9f544ad77..1a85fd79fc 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 @@ -1,6 +1,7 @@ import { Observable } from "rxjs"; import { ListResponse } from "../../../models/response/list.response"; +import { UserId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyData } from "../../models/data/policy.data"; import { MasterPasswordPolicyOptions } from "../../models/domain/master-password-policy-options"; @@ -10,9 +11,9 @@ import { PolicyResponse } from "../../models/response/policy.response"; export abstract class PolicyService { /** - * All {@link Policy} objects for the active user (from sync data). - * May include policies that are disabled or otherwise do not apply to the user. - * @see {@link get$} or {@link policyAppliesToActiveUser$} if you want to know when a policy applies to a user. + * All policies for the active user from sync data. + * May include policies that are disabled or otherwise do not apply to the user. Be careful using this! + * Consider using {@link get$} or {@link getAll$} instead, which will only return policies that should be enforced against the user. */ policies$: Observable; @@ -20,37 +21,33 @@ export abstract class PolicyService { * @returns the first {@link Policy} found that applies to the active user. * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). * @param policyType the {@link PolicyType} to search for - * @param policyFilter Optional predicate to apply when filtering policies + * @see {@link getAll$} if you need all policies of a given type */ - get$: (policyType: PolicyType, policyFilter?: (policy: Policy) => boolean) => Observable; + get$: (policyType: PolicyType) => Observable; + + /** + * @returns all {@link Policy} objects of a given type that apply to the specified user (or the active user if not specified). + * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). + * @param policyType the {@link PolicyType} to search for + */ + getAll$: (policyType: PolicyType, userId?: UserId) => Observable; /** * All {@link Policy} objects for the specified user (from sync data). * May include policies that are disabled or otherwise do not apply to the user. - * @see {@link policyAppliesToUser} if you want to know when a policy applies to the user. - * @deprecated Use {@link policies$} instead + * Consider using {@link getAll$} instead, which will only return policies that should be enforced against the user. */ - getAll: (type?: PolicyType, userId?: string) => Promise; + getAll: (policyType: PolicyType) => Promise; /** - * @returns true if the {@link PolicyType} applies to the current user, otherwise false. - * @remarks A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). + * @returns true if a policy of the specified type applies to the active user, otherwise false. + * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). + * This does not take into account the policy's configuration - if that is important, use {@link getAll$} to get the + * {@link Policy} objects and then filter by Policy.data. */ - policyAppliesToActiveUser$: ( - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - ) => Observable; + policyAppliesToActiveUser$: (policyType: PolicyType) => Observable; - /** - * @returns true if the {@link PolicyType} applies to the specified user, otherwise false. - * @remarks A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). - * @see {@link policyAppliesToActiveUser$} if you only want to know about the current user. - */ - policyAppliesToUser: ( - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - userId?: string, - ) => Promise; + policyAppliesToUser: (policyType: PolicyType) => Promise; // Policy specific interfaces @@ -93,7 +90,7 @@ export abstract class PolicyService { } export abstract class InternalPolicyService extends PolicyService { - upsert: (policy: PolicyData) => Promise; + upsert: (policy: PolicyData) => Promise; replace: (policies: { [id: string]: PolicyData }) => Promise; - clear: (userId?: string) => Promise; + clear: (userId?: string) => Promise; } diff --git a/libs/common/src/admin-console/models/data/policy.data.ts b/libs/common/src/admin-console/models/data/policy.data.ts index 21ed810952..35846f2072 100644 --- a/libs/common/src/admin-console/models/data/policy.data.ts +++ b/libs/common/src/admin-console/models/data/policy.data.ts @@ -1,8 +1,9 @@ +import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyResponse } from "../response/policy.response"; export class PolicyData { - id: string; + id: PolicyId; organizationId: string; type: PolicyType; data: Record; diff --git a/libs/common/src/admin-console/models/domain/policy.ts b/libs/common/src/admin-console/models/domain/policy.ts index 70e9e53704..65af09b588 100644 --- a/libs/common/src/admin-console/models/domain/policy.ts +++ b/libs/common/src/admin-console/models/domain/policy.ts @@ -1,9 +1,10 @@ import Domain from "../../../platform/models/domain/domain-base"; +import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyData } from "../data/policy.data"; export class Policy extends Domain { - id: string; + id: PolicyId; organizationId: string; type: PolicyType; data: any; diff --git a/libs/common/src/admin-console/models/response/policy.response.ts b/libs/common/src/admin-console/models/response/policy.response.ts index c3c4a5752b..25a1f208a0 100644 --- a/libs/common/src/admin-console/models/response/policy.response.ts +++ b/libs/common/src/admin-console/models/response/policy.response.ts @@ -1,8 +1,9 @@ import { BaseResponse } from "../../../models/response/base.response"; +import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; export class PolicyResponse extends BaseResponse { - id: string; + id: PolicyId; organizationId: string; type: PolicyType; data: any; 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 9d585114e0..672a53d34e 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 @@ -1,5 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom } from "rxjs"; +import { firstValueFrom, of } from "rxjs"; import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { FakeActiveUserState } from "../../../../spec/fake-state"; @@ -19,71 +19,51 @@ import { ResetPasswordPolicyOptions } from "../../../admin-console/models/domain import { PolicyResponse } from "../../../admin-console/models/response/policy.response"; import { POLICIES, PolicyService } from "../../../admin-console/services/policy/policy.service"; import { ListResponse } from "../../../models/response/list.response"; -import { CryptoService } from "../../../platform/abstractions/crypto.service"; -import { EncryptService } from "../../../platform/abstractions/encrypt.service"; -import { ContainerService } from "../../../platform/services/container.service"; -import { StateService } from "../../../platform/services/state.service"; import { PolicyId, UserId } from "../../../types/guid"; describe("PolicyService", () => { - let policyService: PolicyService; - - let cryptoService: MockProxy; - let stateService: MockProxy; let stateProvider: FakeStateProvider; let organizationService: MockProxy; - let encryptService: MockProxy; - let activeAccount: BehaviorSubject; - let activeAccountUnlocked: BehaviorSubject; + let activeUserState: FakeActiveUserState>; + + let policyService: PolicyService; beforeEach(() => { - stateService = mock(); - const accountService = mockAccountServiceWith("userId" as UserId); stateProvider = new FakeStateProvider(accountService); organizationService = mock(); - organizationService.getAll - .calledWith("user") - .mockResolvedValue([ - new Organization( - organizationData( - "test-organization", - true, - true, - OrganizationUserStatusType.Accepted, - false, - ), - ), - ]); - organizationService.getAll.calledWith(undefined).mockResolvedValue([]); - organizationService.getAll.calledWith(null).mockResolvedValue([]); - activeAccount = new BehaviorSubject("123"); - activeAccountUnlocked = new BehaviorSubject(true); - stateService.getDecryptedPolicies.calledWith({ userId: "user" }).mockResolvedValue(null); - stateService.getEncryptedPolicies.calledWith({ userId: "user" }).mockResolvedValue({ - "1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { - minutes: 14, - }), - }); - stateService.getEncryptedPolicies.mockResolvedValue({ - "1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { - minutes: 14, - }), - }); - stateService.activeAccount$ = activeAccount; - stateService.activeAccountUnlocked$ = activeAccountUnlocked; - stateService.getUserId.mockResolvedValue("user"); - (window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService); - policyService = new PolicyService(stateService, stateProvider, organizationService); - }); + activeUserState = stateProvider.activeUser.getFake(POLICIES); + organizationService.organizations$ = of([ + // User + organization("org1", true, true, OrganizationUserStatusType.Confirmed, false), + // Owner + organization( + "org2", + true, + true, + OrganizationUserStatusType.Confirmed, + false, + OrganizationUserType.Owner, + ), + // Does not use policies + organization("org3", true, false, OrganizationUserStatusType.Confirmed, false), + // Another User + organization("org4", true, true, OrganizationUserStatusType.Confirmed, false), + // Another User + organization("org5", true, true, OrganizationUserStatusType.Confirmed, false), + ]); - afterEach(() => { - activeAccount.complete(); - activeAccountUnlocked.complete(); + policyService = new PolicyService(stateProvider, organizationService); }); it("upsert", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { minutes: 14 }), + ]), + ); + await policyService.upsert(policyData("99", "test-organization", PolicyType.DisableSend, true)); expect(await firstValueFrom(policyService.policies$)).toEqual([ @@ -104,6 +84,12 @@ describe("PolicyService", () => { }); it("replace", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { minutes: 14 }), + ]), + ); + await policyService.replace({ "2": policyData("2", "test-organization", PolicyType.DisableSend, true), }); @@ -118,37 +104,63 @@ describe("PolicyService", () => { ]); }); - it("locking should clear", async () => { - activeAccountUnlocked.next(false); - // Sleep for 100ms to avoid timing issues - await new Promise((r) => setTimeout(r, 100)); - - expect((await firstValueFrom(policyService.policies$)).length).toBe(0); - }); - describe("clear", () => { - it("null userId", async () => { + 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(stateService.setEncryptedPolicies).toBeCalledTimes(1); - - expect((await firstValueFrom(policyService.policies$)).length).toBe(0); + expect(await firstValueFrom(policyService.policies$)).toEqual([]); + expect(await firstValueFrom(activeUserState.state$)).toEqual(null); + expect(stateProvider.activeUser.getFake(POLICIES).nextMock).toHaveBeenCalledWith([ + "userId", + null, + ]); }); - it("matching userId", async () => { - await policyService.clear("user"); + 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), + ]), + ); - expect(stateService.setEncryptedPolicies).toBeCalledTimes(1); + await policyService.clear(inactiveUserId); - expect((await firstValueFrom(policyService.policies$)).length).toBe(0); - }); + // 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(); - it("mismatching userId", async () => { - await policyService.clear("12"); - - expect(stateService.setEncryptedPolicies).toBeCalledTimes(1); - - expect((await firstValueFrom(policyService.policies$)).length).toBe(1); + // 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); }); }); @@ -313,300 +325,260 @@ describe("PolicyService", () => { }); }); + describe("get$", () => { + it("returns the specified PolicyType", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org1", PolicyType.ActivateAutofill, true), + policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.get$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual({ + id: "policy2", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }); + }); + + it("does not return disabled policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org1", PolicyType.ActivateAutofill, true), + policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, false), + ]), + ); + + const result = await firstValueFrom( + policyService.get$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toBeNull(); + }); + + it("does not return policies that do not apply to the user because the user's role is exempt", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org1", PolicyType.ActivateAutofill, true), + policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, false), + ]), + ); + + const result = await firstValueFrom( + policyService.get$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toBeNull(); + }); + + it("does not return policies for organizations that do not use policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org3", PolicyType.ActivateAutofill, true), + policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom(policyService.get$(PolicyType.ActivateAutofill)); + + expect(result).toBeNull(); + }); + }); + + describe("getAll$", () => { + it("returns the specified PolicyTypes", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy3", + organizationId: "org5", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy4", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + + it("does not return disabled policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, false), // disabled + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy4", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + + it("does not return policies that do not apply to the user because the user's role is exempt", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), + policyData("policy4", "org2", PolicyType.DisablePersonalVaultExport, true), // owner + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy3", + organizationId: "org5", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + + it("does not return policies for organizations that do not use policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org3", PolicyType.DisablePersonalVaultExport, true), // does not use policies + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy4", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + }); + describe("policyAppliesToActiveUser$", () => { - it("MasterPassword does not apply", async () => { - const result = await firstValueFrom( - policyService.policyAppliesToActiveUser$(PolicyType.MasterPassword), + it("returns true when the policyType applies to the user", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), ); - expect(result).toEqual(false); - }); - - it("MaximumVaultTimeout applies", async () => { - const result = await firstValueFrom( - policyService.policyAppliesToActiveUser$(PolicyType.MaximumVaultTimeout), - ); - - 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), ); - expect(result).toEqual(false); + expect(result).toBe(true); }); - }); - describe("policyAppliesToUser", () => { - it("MasterPassword does not apply", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.MasterPassword, - null, - "user", + it("returns false when policyType is disabled", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, false), // disabled + ]), ); - expect(result).toEqual(false); - }); - - it("MaximumVaultTimeout applies", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.MaximumVaultTimeout, - null, - "user", + const result = await firstValueFrom( + policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), ); - expect(result).toEqual(true); + expect(result).toBe(false); }); - it("PolicyFilter filters result", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.MaximumVaultTimeout, - (p) => false, - "user", + it("returns false when the policyType does not apply to the user because the user's role is exempt", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy4", "org2", PolicyType.DisablePersonalVaultExport, true), // owner + ]), ); - expect(result).toEqual(false); - }); - - it("DisablePersonalVaultExport does not apply", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.DisablePersonalVaultExport, - null, - "user", + const result = await firstValueFrom( + policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), ); - expect(result).toEqual(false); - }); - }); - - // TODO: remove this nesting once fully migrated to StateProvider - describe("stateProvider methods", () => { - let policyState$: FakeActiveUserState>; - - beforeEach(() => { - policyState$ = stateProvider.activeUser.getFake(POLICIES); - organizationService.organizations$ = new BehaviorSubject([ - // User - organization("org1", true, true, OrganizationUserStatusType.Confirmed, false), - // Owner - organization( - "org2", - true, - true, - OrganizationUserStatusType.Confirmed, - false, - OrganizationUserType.Owner, - ), - // Does not use policies - organization("org3", true, false, OrganizationUserStatusType.Confirmed, false), - // Another User - organization("org4", true, true, OrganizationUserStatusType.Confirmed, false), - // Another User - organization("org5", true, true, OrganizationUserStatusType.Confirmed, false), - ]); + expect(result).toBe(false); }); - describe("get_vNext$", () => { - it("returns the specified PolicyType", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org1", PolicyType.ActivateAutofill, true), - policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); + it("returns false for organizations that do not use policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org3", PolicyType.DisablePersonalVaultExport, true), // does not use policies + ]), + ); - const result = await firstValueFrom( - policyService.get_vNext$(PolicyType.DisablePersonalVaultExport), - ); + const result = await firstValueFrom( + policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), + ); - expect(result).toEqual({ - id: "policy2", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }); - }); - - it("does not return disabled policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org1", PolicyType.ActivateAutofill, true), - policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, false), - ]), - ); - - const result = await firstValueFrom( - policyService.get_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toBeNull(); - }); - - it("does not return policies that do not apply to the user because the user's role is exempt", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org1", PolicyType.ActivateAutofill, true), - policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, false), - ]), - ); - - const result = await firstValueFrom( - policyService.get_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toBeNull(); - }); - - it("does not return policies for organizations that do not use policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org3", PolicyType.ActivateAutofill, true), - policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom(policyService.get_vNext$(PolicyType.ActivateAutofill)); - - expect(result).toBeNull(); - }); - }); - - describe("getAll_vNext$", () => { - it("returns the specified PolicyTypes", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), - policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy3", - organizationId: "org5", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy4", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); - - it("does not return disabled policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, false), // disabled - policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy4", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); - - it("does not return policies that do not apply to the user because the user's role is exempt", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), - policyData("policy4", "org2", PolicyType.DisablePersonalVaultExport, true), // owner - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy3", - organizationId: "org5", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); - - it("does not return policies for organizations that do not use policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org3", PolicyType.DisablePersonalVaultExport, true), // does not use policies - policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy4", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); + expect(result).toBe(false); }); }); @@ -618,7 +590,7 @@ describe("PolicyService", () => { data?: any, ) { const policyData = new PolicyData({} as any); - policyData.id = id; + policyData.id = id as PolicyId; policyData.organizationId = organizationId; policyData.type = type; policyData.enabled = enabled; 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 2f8c8ed802..4672d9b810 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.ts @@ -1,8 +1,6 @@ -import { BehaviorSubject, combineLatest, concatMap, map, Observable, of } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, of } from "rxjs"; import { ListResponse } from "../../../models/response/list.response"; -import { StateService } from "../../../platform/abstractions/state.service"; -import { Utils } from "../../../platform/misc/utils"; import { KeyDefinition, POLICIES_DISK, StateProvider } from "../../../platform/state"; import { PolicyId, UserId } from "../../../types/guid"; import { OrganizationService } from "../../abstractions/organization/organization.service.abstraction"; @@ -23,42 +21,19 @@ export const POLICIES = KeyDefinition.record(POLICIES_DISK }); export class PolicyService implements InternalPolicyServiceAbstraction { - protected _policies: BehaviorSubject = new BehaviorSubject([]); - - policies$ = this._policies.asObservable(); - private activeUserPolicyState = this.stateProvider.getActive(POLICIES); - activeUserPolicies$ = this.activeUserPolicyState.state$.pipe( + private activeUserPolicies$ = this.activeUserPolicyState.state$.pipe( map((policyData) => policyRecordToArray(policyData)), ); + policies$ = this.activeUserPolicies$; + constructor( - protected stateService: StateService, private stateProvider: StateProvider, private organizationService: OrganizationService, - ) { - this.stateService.activeAccountUnlocked$ - .pipe( - concatMap(async (unlocked) => { - if (Utils.global.bitwardenContainerService == null) { - return; - } + ) {} - if (!unlocked) { - this._policies.next([]); - return; - } - - const data = await this.stateService.getEncryptedPolicies(); - - await this.updateObservables(data); - }), - ) - .subscribe(); - } - - // --- StateProvider methods - not yet wired up - get_vNext$(policyType: PolicyType) { + get$(policyType: PolicyType) { const filteredPolicies$ = this.activeUserPolicies$.pipe( map((policies) => policies.filter((p) => p.type === policyType)), ); @@ -71,7 +46,7 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); } - getAll_vNext$(policyType: PolicyType, userId?: UserId) { + getAll$(policyType: PolicyType, userId?: UserId) { const filteredPolicies$ = this.stateProvider.getUserState$(POLICIES, userId).pipe( map((policyData) => policyRecordToArray(policyData)), map((policies) => policies.filter((p) => p.type === policyType)), @@ -82,8 +57,18 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); } - policyAppliesToActiveUser_vNext$(policyType: PolicyType) { - return this.get_vNext$(policyType).pipe(map((policy) => policy != null)); + async getAll(policyType: PolicyType) { + return await firstValueFrom( + this.policies$.pipe(map((policies) => policies.filter((p) => p.type === policyType))), + ); + } + + policyAppliesToActiveUser$(policyType: PolicyType) { + return this.get$(policyType).pipe(map((policy) => policy != null)); + } + + async policyAppliesToUser(policyType: PolicyType) { + return await firstValueFrom(this.policyAppliesToActiveUser$(policyType)); } private enforcedPolicyFilter(policies: Policy[], organizations: Organization[]) { @@ -105,45 +90,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); }); } - // --- End StateProvider methods - - get$(policyType: PolicyType, policyFilter?: (policy: Policy) => boolean): Observable { - return this.policies$.pipe( - concatMap(async (policies) => { - const userId = await this.stateService.getUserId(); - const appliesToCurrentUser = await this.checkPoliciesThatApplyToUser( - policies, - policyType, - policyFilter, - userId, - ); - if (appliesToCurrentUser) { - return policies.find((policy) => policy.type === policyType && policy.enabled); - } - }), - ); - } - - async getAll(type?: PolicyType, userId?: string): Promise { - let response: Policy[] = []; - const decryptedPolicies = await this.stateService.getDecryptedPolicies({ userId: userId }); - if (decryptedPolicies != null) { - response = decryptedPolicies; - } else { - const diskPolicies = await this.stateService.getEncryptedPolicies({ userId: userId }); - for (const id in diskPolicies) { - if (Object.prototype.hasOwnProperty.call(diskPolicies, id)) { - response.push(new Policy(diskPolicies[id])); - } - } - await this.stateService.setDecryptedPolicies(response, { userId: userId }); - } - if (type != null) { - return response.filter((policy) => policy.type === type); - } else { - return response; - } - } masterPasswordPolicyOptions$(policies?: Policy[]): Observable { const observable = policies ? of(policies) : this.policies$; @@ -205,15 +151,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); } - policyAppliesToActiveUser$(policyType: PolicyType, policyFilter?: (policy: Policy) => boolean) { - return this.policies$.pipe( - concatMap(async (policies) => { - const userId = await this.stateService.getUserId(); - return await this.checkPoliciesThatApplyToUser(policies, policyType, policyFilter, userId); - }), - ); - } - evaluateMasterPassword( passwordStrength: number, newPassword: string, @@ -288,68 +225,20 @@ export class PolicyService implements InternalPolicyServiceAbstraction { return policiesResponse.data.map((response) => this.mapPolicyFromResponse(response)); } - async policyAppliesToUser( - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - userId?: string, - ) { - const policies = await this.getAll(policyType, userId); - - return this.checkPoliciesThatApplyToUser(policies, policyType, policyFilter, userId); - } - - async upsert(policy: PolicyData): Promise { - let policies = await this.stateService.getEncryptedPolicies(); - if (policies == null) { - policies = {}; - } - - policies[policy.id] = policy; - - await this.updateObservables(policies); - await this.stateService.setDecryptedPolicies(null); - await this.stateService.setEncryptedPolicies(policies); + async upsert(policy: PolicyData): Promise { + await this.activeUserPolicyState.update((policies) => { + policies ??= {}; + policies[policy.id] = policy; + return policies; + }); } async replace(policies: { [id: string]: PolicyData }): Promise { - await this.updateObservables(policies); - await this.stateService.setDecryptedPolicies(null); - await this.stateService.setEncryptedPolicies(policies); + await this.activeUserPolicyState.update(() => policies); } - async clear(userId?: string): Promise { - if (userId == null || userId == (await this.stateService.getUserId())) { - this._policies.next([]); - } - await this.stateService.setDecryptedPolicies(null, { userId: userId }); - await this.stateService.setEncryptedPolicies(null, { userId: userId }); - } - - private async updateObservables(policiesMap: { [id: string]: PolicyData }) { - const policies = Object.values(policiesMap || {}).map((f) => new Policy(f)); - - this._policies.next(policies); - } - - private async checkPoliciesThatApplyToUser( - policies: Policy[], - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - userId?: string, - ) { - const organizations = await this.organizationService.getAll(userId); - const filteredPolicies = policies.filter( - (p) => p.type === policyType && p.enabled && (policyFilter == null || policyFilter(p)), - ); - const policySet = new Set(filteredPolicies.map((p) => p.organizationId)); - - return organizations.some( - (o) => - o.status >= OrganizationUserStatusType.Accepted && - o.usePolicies && - policySet.has(o.id) && - !this.isExemptFromPolicy(policyType, o), - ); + async clear(userId?: UserId): Promise { + await this.stateProvider.setUserState(POLICIES, null, userId); } /** diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 18b2f79483..4061769461 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -1,8 +1,6 @@ import { Observable } from "rxjs"; import { OrganizationData } from "../../admin-console/models/data/organization.data"; -import { PolicyData } from "../../admin-console/models/data/policy.data"; -import { Policy } from "../../admin-console/models/domain/policy"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { ForceSetPasswordReason } from "../../auth/models/domain/force-set-password-reason"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; @@ -181,14 +179,6 @@ export abstract class StateService { * @deprecated For migration purposes only, use setDecryptedUserKeyPin instead */ setDecryptedPinProtected: (value: EncString, options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this, use PolicyService - */ - getDecryptedPolicies: (options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this, use PolicyService - */ - setDecryptedPolicies: (value: Policy[], options?: StorageOptions) => Promise; /** * @deprecated Do not call this directly, use SendService */ @@ -279,17 +269,6 @@ export abstract class StateService { * @deprecated For migration purposes only, use setEncryptedUserKeyPin instead */ setEncryptedPinProtected: (value: string, options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this directly, use PolicyService - */ - getEncryptedPolicies: (options?: StorageOptions) => Promise<{ [id: string]: PolicyData }>; - /** - * @deprecated Do not call this directly, use PolicyService - */ - setEncryptedPolicies: ( - value: { [id: string]: PolicyData }, - options?: StorageOptions, - ) => Promise; /** * @deprecated Do not call this directly, use SendService */ diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 2b2024270f..32b1de9c8f 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -1,8 +1,6 @@ import { Jsonify } from "type-fest"; import { OrganizationData } from "../../../admin-console/models/data/organization.data"; -import { PolicyData } from "../../../admin-console/models/data/policy.data"; -import { Policy } from "../../../admin-console/models/domain/policy"; import { AdminAuthRequestStorable } from "../../../auth/models/domain/admin-auth-req-storable"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { KeyConnectorUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; @@ -87,7 +85,6 @@ export class AccountData { >(); localData?: any; sends?: DataEncryptionPair = new DataEncryptionPair(); - policies?: DataEncryptionPair = new DataEncryptionPair(); passwordGenerationHistory?: EncryptionPair< GeneratedPasswordHistory[], GeneratedPasswordHistory[] diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 67c71c610d..357bd27173 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -2,8 +2,6 @@ import { BehaviorSubject, Observable, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { OrganizationData } from "../../admin-console/models/data/organization.data"; -import { PolicyData } from "../../admin-console/models/data/policy.data"; -import { Policy } from "../../admin-console/models/domain/policy"; import { AccountService } from "../../auth/abstractions/account.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; @@ -799,24 +797,6 @@ export class StateService< ); } - @withPrototypeForArrayMembers(Policy) - async getDecryptedPolicies(options?: StorageOptions): Promise { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions())) - )?.data?.policies?.decrypted; - } - - async setDecryptedPolicies(value: Policy[], options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - account.data.policies.decrypted = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - } - @withPrototypeForArrayMembers(SendView) async getDecryptedSends(options?: StorageOptions): Promise { return ( @@ -1350,27 +1330,6 @@ export class StateService< ); } - @withPrototypeForObjectValues(PolicyData) - async getEncryptedPolicies(options?: StorageOptions): Promise<{ [id: string]: PolicyData }> { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.data?.policies?.encrypted; - } - - async setEncryptedPolicies( - value: { [id: string]: PolicyData }, - options?: StorageOptions, - ): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.data.policies.encrypted = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - @withPrototypeForObjectValues(SendData) async getEncryptedSends(options?: StorageOptions): Promise<{ [id: string]: SendData }> { return ( diff --git a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts index f1881080d9..7eee456775 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts @@ -110,9 +110,8 @@ describe("VaultTimeoutSettingsService", () => { stateService.getAccountDecryptionOptions.mockResolvedValue( new AccountDecryptionOptions({ hasMasterPassword: true }), ); - policyService.policyAppliesToUser.mockResolvedValue(policy === null ? false : true); - policyService.getAll.mockResolvedValue( - policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[]), + policyService.getAll$.mockReturnValue( + of(policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[])), ); stateService.getVaultTimeoutAction.mockResolvedValue(userPreference); @@ -140,9 +139,8 @@ describe("VaultTimeoutSettingsService", () => { stateService.getAccountDecryptionOptions.mockResolvedValue( new AccountDecryptionOptions({ hasMasterPassword: false }), ); - policyService.policyAppliesToUser.mockResolvedValue(policy === null ? false : true); - policyService.getAll.mockResolvedValue( - policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[]), + policyService.getAll$.mockReturnValue( + of(policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[])), ); stateService.getVaultTimeoutAction.mockResolvedValue(userPreference); diff --git a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts index e84d561fe6..0d0eb508cb 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts @@ -84,18 +84,18 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return await biometricUnlockPromise; } - async getVaultTimeout(userId?: string): Promise { + async getVaultTimeout(userId?: UserId): Promise { const vaultTimeout = await this.stateService.getVaultTimeout({ userId }); + const policies = await firstValueFrom( + this.policyService.getAll$(PolicyType.MaximumVaultTimeout, userId), + ); - if ( - await this.policyService.policyAppliesToUser(PolicyType.MaximumVaultTimeout, null, userId) - ) { - const policy = await this.policyService.getAll(PolicyType.MaximumVaultTimeout, userId); + if (policies?.length) { // Remove negative values, and ensure it's smaller than maximum allowed value according to policy - let timeout = Math.min(vaultTimeout, policy[0].data.minutes); + let timeout = Math.min(vaultTimeout, policies[0].data.minutes); if (vaultTimeout == null || timeout < 0) { - timeout = policy[0].data.minutes; + timeout = policies[0].data.minutes; } // TODO @jlf0dev: Can we move this somwhere else? Maybe add it to the initialization process? @@ -111,23 +111,23 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return vaultTimeout; } - vaultTimeoutAction$(userId?: string) { + vaultTimeoutAction$(userId?: UserId) { return defer(() => this.getVaultTimeoutAction(userId)); } - async getVaultTimeoutAction(userId?: string): Promise { + async getVaultTimeoutAction(userId?: UserId): Promise { const availableActions = await this.getAvailableVaultTimeoutActions(); if (availableActions.length === 1) { return availableActions[0]; } const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction({ userId: userId }); + const policies = await firstValueFrom( + this.policyService.getAll$(PolicyType.MaximumVaultTimeout, userId), + ); - if ( - await this.policyService.policyAppliesToUser(PolicyType.MaximumVaultTimeout, null, userId) - ) { - const policy = await this.policyService.getAll(PolicyType.MaximumVaultTimeout, userId); - const action = policy[0].data.action; + if (policies?.length) { + const action = policies[0].data.action; // We really shouldn't need to set the value here, but multiple services relies on this value being correct. if (action && vaultTimeoutAction !== action) { await this.stateService.setVaultTimeoutAction(action, { userId: userId }); diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 3b21713539..db29ddf26f 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -25,6 +25,7 @@ import { BadgeSettingsMigrator } from "./migrations/27-move-badge-settings-to-st import { MoveBiometricUnlockToStateProviders } from "./migrations/28-move-biometric-unlock-to-state-providers"; import { UserNotificationSettingsKeyMigrator } from "./migrations/29-move-user-notification-settings-to-state-provider"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; +import { PolicyMigrator } from "./migrations/30-move-policy-state-to-state-provider"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; @@ -34,7 +35,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 29; +export const CURRENT_VERSION = 30; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -66,7 +67,8 @@ export function createMigrationBuilder() { .with(RevertLastSyncMigrator, 25, 26) .with(BadgeSettingsMigrator, 26, 27) .with(MoveBiometricUnlockToStateProviders, 27, 28) - .with(UserNotificationSettingsKeyMigrator, 28, CURRENT_VERSION); + .with(UserNotificationSettingsKeyMigrator, 28, 29) + .with(PolicyMigrator, 29, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts new file mode 100644 index 0000000000..23ed3778a7 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts @@ -0,0 +1,192 @@ +import { MockProxy, any } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { PolicyMigrator } from "./30-move-policy-state-to-state-provider"; + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2"], + "user-1": { + data: { + policies: { + encrypted: { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, // max vault timeout + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, // single org + enabled: true, + }, + }, + }, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + data: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +function rollbackJSON() { + return { + "user_user-1_policies_policies": { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, + enabled: true, + }, + }, + "user_user-2_policies_policies": null as any, + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2"], + "user-1": { + data: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + data: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +describe("PoliciesMigrator", () => { + let helper: MockProxy; + let sut: PolicyMigrator; + const keyDefinitionLike = { + key: "policies", + stateDefinition: { + name: "policies", + }, + }; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 22); + sut = new PolicyMigrator(29, 30); + }); + + it("should remove policies from all old accounts", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledWith("user-1", { + data: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should set policies value in StateProvider framework for each account", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("user-1", keyDefinitionLike, { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, + enabled: true, + }, + }); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 23); + sut = new PolicyMigrator(29, 30); + }); + + it.each(["user-1", "user-2"])("should null out new values", async (userId) => { + await sut.rollback(helper); + expect(helper.setToUser).toHaveBeenCalledWith(userId, keyDefinitionLike, null); + }); + + it("should add policy values back to accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalled(); + expect(helper.set).toHaveBeenCalledWith("user-1", { + data: { + policies: { + encrypted: { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, + enabled: true, + }, + }, + }, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should not try to restore values to missing accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).not.toHaveBeenCalledWith("user-3", any()); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts new file mode 100644 index 0000000000..5f05442c4e --- /dev/null +++ b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts @@ -0,0 +1,76 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +enum PolicyType { + TwoFactorAuthentication = 0, // Requires users to have 2fa enabled + MasterPassword = 1, // Sets minimum requirements for master password complexity + PasswordGenerator = 2, // Sets minimum requirements/default type for generated passwords/passphrases + SingleOrg = 3, // Allows users to only be apart of one organization + RequireSso = 4, // Requires users to authenticate with SSO + PersonalOwnership = 5, // Disables personal vault ownership for adding/cloning items + DisableSend = 6, // Disables the ability to create and edit Bitwarden Sends + SendOptions = 7, // Sets restrictions or defaults for Bitwarden Sends + ResetPassword = 8, // Allows orgs to use reset password : also can enable auto-enrollment during invite flow + MaximumVaultTimeout = 9, // Sets the maximum allowed vault timeout + DisablePersonalVaultExport = 10, // Disable personal vault export + ActivateAutofill = 11, // Activates autofill with page load on the browser extension +} + +type PolicyDataType = { + id: string; + organizationId: string; + type: PolicyType; + data: Record; + enabled: boolean; +}; + +type ExpectedAccountType = { + data?: { + policies?: { + encrypted?: Record; + }; + }; +}; + +const POLICIES_KEY: KeyDefinitionLike = { + key: "policies", + stateDefinition: { + name: "policies", + }, +}; + +export class PolicyMigrator extends Migrator<29, 30> { + async migrate(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + async function migrateAccount(userId: string, account: ExpectedAccountType): Promise { + const value = account?.data?.policies?.encrypted; + if (value != null) { + await helper.setToUser(userId, POLICIES_KEY, value); + delete account.data.policies; + await helper.set(userId, account); + } + } + + await Promise.all(accounts.map(({ userId, account }) => migrateAccount(userId, account))); + } + + async rollback(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + async function rollbackAccount(userId: string, account: ExpectedAccountType): Promise { + const value = await helper.getFromUser(userId, POLICIES_KEY); + if (account) { + account.data = Object.assign(account.data ?? {}, { + policies: { + encrypted: value, + }, + }); + + await helper.set(userId, account); + } + await helper.setToUser(userId, POLICIES_KEY, null); + } + await Promise.all(accounts.map(({ userId, account }) => rollbackAccount(userId, account))); + } +}