1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-22 11:45:59 +01:00

[AC-2008] [AC-2122] [Pt 1] Transition PolicyService to use StateProvider (#7959)

* Delete unnecessary StateDefinition

* Add StateProvider to PolicyService

* Add new getters using StateProvider (not exposed or used yet)
This commit is contained in:
Thomas Rittson 2024-02-27 08:51:10 +10:00 committed by GitHub
parent 996823169a
commit 0a5c9d3525
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 363 additions and 22 deletions

View File

@ -5,6 +5,10 @@ import {
factory,
FactoryOptions,
} from "../../../platform/background/service-factories/factory-options";
import {
stateProviderFactory,
StateProviderInitOptions,
} from "../../../platform/background/service-factories/state-provider.factory";
import {
stateServiceFactory as stateServiceFactory,
StateServiceInitOptions,
@ -20,6 +24,7 @@ type PolicyServiceFactoryOptions = FactoryOptions;
export type PolicyServiceInitOptions = PolicyServiceFactoryOptions &
StateServiceInitOptions &
StateProviderInitOptions &
OrganizationServiceInitOptions;
export function policyServiceFactory(
@ -33,6 +38,7 @@ export function policyServiceFactory(
async () =>
new BrowserPolicyService(
await stateServiceFactory(cache, opts),
await stateProviderFactory(cache, opts),
await organizationServiceFactory(cache, opts),
),
);

View File

@ -473,7 +473,11 @@ export default class MainBackground {
this.stateService,
this.stateProvider,
);
this.policyService = new BrowserPolicyService(this.stateService, this.organizationService);
this.policyService = new BrowserPolicyService(
this.stateService,
this.stateProvider,
this.organizationService,
);
this.autofillSettingsService = new AutofillSettingsService(
this.stateProvider,
this.policyService,

View File

@ -326,11 +326,12 @@ function getBgService<T>(service: keyof MainBackground) {
provide: PolicyService,
useFactory: (
stateService: StateServiceAbstraction,
stateProvider: StateProvider,
organizationService: OrganizationService,
) => {
return new BrowserPolicyService(stateService, organizationService);
return new BrowserPolicyService(stateService, stateProvider, organizationService);
},
deps: [StateServiceAbstraction, OrganizationService],
deps: [StateServiceAbstraction, StateProvider, OrganizationService],
},
{
provide: PolicyApiServiceAbstraction,

View File

@ -375,7 +375,11 @@ export class Main {
this.organizationUserService = new OrganizationUserServiceImplementation(this.apiService);
this.policyService = new PolicyService(this.stateService, this.organizationService);
this.policyService = new PolicyService(
this.stateService,
this.stateProvider,
this.organizationService,
);
this.policyApiService = new PolicyApiService(
this.policyService,

View File

@ -670,7 +670,7 @@ import { ModalService } from "./modal.service";
{
provide: PolicyServiceAbstraction,
useClass: PolicyService,
deps: [StateServiceAbstraction, OrganizationServiceAbstraction],
deps: [StateServiceAbstraction, StateProvider, OrganizationServiceAbstraction],
},
{
provide: InternalPolicyService,

View File

@ -5,10 +5,14 @@ export class PolicyData {
id: string;
organizationId: string;
type: PolicyType;
data: any;
data: Record<string, string | number | boolean>;
enabled: boolean;
constructor(response: PolicyResponse) {
constructor(response?: PolicyResponse) {
if (response == null) {
return;
}
this.id = response.id;
this.organizationId = response.organizationId;
this.type = response.type;

View File

@ -1,8 +1,14 @@
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom } from "rxjs";
import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec";
import { FakeActiveUserState } from "../../../../spec/fake-state";
import { OrganizationService } from "../../../admin-console/abstractions/organization/organization.service.abstraction";
import { OrganizationUserStatusType, PolicyType } from "../../../admin-console/enums";
import {
OrganizationUserStatusType,
OrganizationUserType,
PolicyType,
} from "../../../admin-console/enums";
import { PermissionsApi } from "../../../admin-console/models/api/permissions.api";
import { OrganizationData } from "../../../admin-console/models/data/organization.data";
import { PolicyData } from "../../../admin-console/models/data/policy.data";
@ -11,18 +17,20 @@ import { Organization } from "../../../admin-console/models/domain/organization"
import { Policy } from "../../../admin-console/models/domain/policy";
import { ResetPasswordPolicyOptions } from "../../../admin-console/models/domain/reset-password-policy-options";
import { PolicyResponse } from "../../../admin-console/models/response/policy.response";
import { PolicyService } from "../../../admin-console/services/policy/policy.service";
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<CryptoService>;
let stateService: MockProxy<StateService>;
let stateProvider: FakeStateProvider;
let organizationService: MockProxy<OrganizationService>;
let encryptService: MockProxy<EncryptService>;
let activeAccount: BehaviorSubject<string>;
@ -30,6 +38,9 @@ describe("PolicyService", () => {
beforeEach(() => {
stateService = mock<StateService>();
const accountService = mockAccountServiceWith("userId" as UserId);
stateProvider = new FakeStateProvider(accountService);
organizationService = mock<OrganizationService>();
organizationService.getAll
.calledWith("user")
@ -64,7 +75,7 @@ describe("PolicyService", () => {
stateService.getUserId.mockResolvedValue("user");
(window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService);
policyService = new PolicyService(stateService, organizationService);
policyService = new PolicyService(stateService, stateProvider, organizationService);
});
afterEach(() => {
@ -378,6 +389,227 @@ describe("PolicyService", () => {
});
});
// TODO: remove this nesting once fully migrated to StateProvider
describe("stateProvider methods", () => {
let policyState$: FakeActiveUserState<Record<PolicyId, PolicyData>>;
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),
]);
});
describe("get_vNext$", () => {
it("returns the specified PolicyType", async () => {
policyState$.nextState(
arrayToRecord([
policyData("policy1", "org1", PolicyType.ActivateAutofill, true),
policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, true),
]),
);
const result = await firstValueFrom(
policyService.get_vNext$(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,
},
]);
});
});
});
function policyData(
id: string,
organizationId: string,
@ -401,6 +633,7 @@ describe("PolicyService", () => {
usePolicies: boolean,
status: OrganizationUserStatusType,
managePolicies: boolean,
type: OrganizationUserType = OrganizationUserType.User,
) {
const organizationData = new OrganizationData({} as any, {} as any);
organizationData.id = id;
@ -408,6 +641,24 @@ describe("PolicyService", () => {
organizationData.usePolicies = usePolicies;
organizationData.status = status;
organizationData.permissions = new PermissionsApi({ managePolicies: managePolicies } as any);
organizationData.type = type;
return organizationData;
}
function organization(
id: string,
enabled: boolean,
usePolicies: boolean,
status: OrganizationUserStatusType,
managePolicies: boolean,
type: OrganizationUserType = OrganizationUserType.User,
) {
return new Organization(
organizationData(id, enabled, usePolicies, status, managePolicies, type),
);
}
function arrayToRecord(input: PolicyData[]): Record<PolicyId, PolicyData> {
return Object.fromEntries(input.map((i) => [i.id, i]));
}
});

View File

@ -1,11 +1,13 @@
import { BehaviorSubject, concatMap, map, Observable, of } from "rxjs";
import { BehaviorSubject, combineLatest, concatMap, 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";
import { InternalPolicyService as InternalPolicyServiceAbstraction } from "../../abstractions/policy/policy.service.abstraction";
import { OrganizationUserStatusType, OrganizationUserType, PolicyType } from "../../enums";
import { OrganizationUserStatusType, PolicyType } from "../../enums";
import { PolicyData } from "../../models/data/policy.data";
import { MasterPasswordPolicyOptions } from "../../models/domain/master-password-policy-options";
import { Organization } from "../../models/domain/organization";
@ -13,13 +15,26 @@ import { Policy } from "../../models/domain/policy";
import { ResetPasswordPolicyOptions } from "../../models/domain/reset-password-policy-options";
import { PolicyResponse } from "../../models/response/policy.response";
const policyRecordToArray = (policiesMap: { [id: string]: PolicyData }) =>
Object.values(policiesMap || {}).map((f) => new Policy(f));
export const POLICIES = KeyDefinition.record<PolicyData, PolicyId>(POLICIES_DISK, "policies", {
deserializer: (policyData) => policyData,
});
export class PolicyService implements InternalPolicyServiceAbstraction {
protected _policies: BehaviorSubject<Policy[]> = new BehaviorSubject([]);
policies$ = this._policies.asObservable();
private activeUserPolicyState = this.stateProvider.getActive(POLICIES);
activeUserPolicies$ = this.activeUserPolicyState.state$.pipe(
map((policyData) => policyRecordToArray(policyData)),
);
constructor(
protected stateService: StateService,
private stateProvider: StateProvider,
private organizationService: OrganizationService,
) {
this.stateService.activeAccountUnlocked$
@ -42,6 +57,56 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
.subscribe();
}
// --- StateProvider methods - not yet wired up
get_vNext$(policyType: PolicyType) {
const filteredPolicies$ = this.activeUserPolicies$.pipe(
map((policies) => policies.filter((p) => p.type === policyType)),
);
return combineLatest([filteredPolicies$, this.organizationService.organizations$]).pipe(
map(
([policies, organizations]) =>
this.enforcedPolicyFilter(policies, organizations)?.at(0) ?? null,
),
);
}
getAll_vNext$(policyType: PolicyType, userId?: UserId) {
const filteredPolicies$ = this.stateProvider.getUserState$(POLICIES, userId).pipe(
map((policyData) => policyRecordToArray(policyData)),
map((policies) => policies.filter((p) => p.type === policyType)),
);
return combineLatest([filteredPolicies$, this.organizationService.organizations$]).pipe(
map(([policies, organizations]) => this.enforcedPolicyFilter(policies, organizations)),
);
}
policyAppliesToActiveUser_vNext$(policyType: PolicyType) {
return this.get_vNext$(policyType).pipe(map((policy) => policy != null));
}
private enforcedPolicyFilter(policies: Policy[], organizations: Organization[]) {
const orgDict = Object.fromEntries(organizations.map((o) => [o.id, o]));
return policies.filter((policy) => {
const organization = orgDict[policy.organizationId];
// This shouldn't happen, i.e. the user should only have policies for orgs they are a member of
// But if it does, err on the side of enforcing the policy
if (organization == null) {
return true;
}
return (
policy.enabled &&
organization.status >= OrganizationUserStatusType.Accepted &&
organization.usePolicies &&
!this.isExemptFromPolicy(policy.type, organization)
);
});
}
// --- End StateProvider methods
get$(policyType: PolicyType, policyFilter?: (policy: Policy) => boolean): Observable<Policy> {
return this.policies$.pipe(
concatMap(async (policies) => {
@ -260,14 +325,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
await this.stateService.setEncryptedPolicies(null, { userId: userId });
}
private isExemptFromPolicies(organization: Organization, policyType: PolicyType) {
if (policyType === PolicyType.MaximumVaultTimeout) {
return organization.type === OrganizationUserType.Owner;
}
return organization.isExemptFromPolicies;
}
private async updateObservables(policiesMap: { [id: string]: PolicyData }) {
const policies = Object.values(policiesMap || {}).map((f) => new Policy(f));
@ -291,7 +348,21 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
o.status >= OrganizationUserStatusType.Accepted &&
o.usePolicies &&
policySet.has(o.id) &&
!this.isExemptFromPolicies(o, policyType),
!this.isExemptFromPolicy(policyType, o),
);
}
/**
* 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
*/
private isExemptFromPolicy(policyType: PolicyType, organization: Organization) {
switch (policyType) {
case PolicyType.MaximumVaultTimeout:
// Max Vault Timeout applies to everyone except owners
return organization.isOwner;
default:
return organization.canManagePolicies;
}
}
}

View File

@ -40,7 +40,6 @@ export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings",
// Admin Console
export const ORGANIZATIONS_DISK = new StateDefinition("organizations", "disk");
export const POLICIES_DISK = new StateDefinition("policies", "disk");
export const POLICIES_MEMORY = new StateDefinition("policies", "memory");
export const PROVIDERS_DISK = new StateDefinition("providers", "disk");
export const FOLDER_DISK = new StateDefinition("folder", "disk", { web: "memory" });

View File

@ -6,3 +6,4 @@ export type UserId = Opaque<string, "UserId">;
export type OrganizationId = Opaque<string, "OrganizationId">;
export type CollectionId = Opaque<string, "CollectionId">;
export type ProviderId = Opaque<string, "ProviderId">;
export type PolicyId = Opaque<string, "PolicyId">;