From b3135403e8ba4f0c05eb8eeef300ab872ad1752a Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Tue, 6 Feb 2024 18:48:34 -0600 Subject: [PATCH] Wire up key definitions for OrganizationService (#7781) * Wire up key definitions for OrganizationService [`AC-2009`: Transition OrganizationService to use StateProvider]( https://bitwarden.atlassian.net/browse/AC-2009) In order to support the new `StateProvider` APIs for managing application state this commit modifies `OrganizationService` in the following ways: 1. Adding a `KeyDefinition` object to `OrganizationService` to store the `organization` record in `StateProvider`. 1. Injecting `StateProvider` and wiring up `OrganizationService` to read from the `organizations` key definition for the active user account. 1. Expanding the capabilities of `OrganizationData` to be able to read itself from a JSON string. Previously this was handled directly by `StateService`. 1. Updating tests to include requirements for testing against `StateProvider`. 1. Marking the existing `StateService`-backed `organizations` `Observable` and `BehaviorSubject` as deprecated. This is largely unimplemented code with no intended visible effects to the system. Implementing getting & updating the `organizations` value from `StateProvider` will the next step in this work. * Rework null check on OrganizationData * Remove deprecation signals for the time being * Move key definition inline with its service * Create date objects when deserialzing json from state --- .../organization-service.factory.ts | 7 ++- .../browser/src/background/main.background.ts | 5 +- .../src/popup/services/services.module.ts | 6 +- apps/cli/src/bw.ts | 2 +- .../models/data/organization.data.spec.ts | 62 +++++++++++++++++++ .../models/data/organization.data.ts | 23 ++++++- .../organization/organization.service.spec.ts | 29 ++++++++- .../organization/organization.service.ts | 32 +++++++++- 8 files changed, 152 insertions(+), 14 deletions(-) create mode 100644 libs/common/src/admin-console/models/data/organization.data.spec.ts diff --git a/apps/browser/src/admin-console/background/service-factories/organization-service.factory.ts b/apps/browser/src/admin-console/background/service-factories/organization-service.factory.ts index 0b9dc08ad2c..c77508b0f88 100644 --- a/apps/browser/src/admin-console/background/service-factories/organization-service.factory.ts +++ b/apps/browser/src/admin-console/background/service-factories/organization-service.factory.ts @@ -5,6 +5,7 @@ import { CachedServices, factory, } from "../../../platform/background/service-factories/factory-options"; +import { stateProviderFactory } from "../../../platform/background/service-factories/state-provider.factory"; import { stateServiceFactory, StateServiceInitOptions, @@ -24,6 +25,10 @@ export function organizationServiceFactory( cache, "organizationService", opts, - async () => new BrowserOrganizationService(await stateServiceFactory(cache, opts)), + async () => + new BrowserOrganizationService( + await stateServiceFactory(cache, opts), + await stateProviderFactory(cache, opts), + ), ); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 2f47c4d71fb..3e169e28fb2 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -442,7 +442,10 @@ export default class MainBackground { this.stateService, ); this.syncNotifierService = new SyncNotifierService(); - this.organizationService = new BrowserOrganizationService(this.stateService); + this.organizationService = new BrowserOrganizationService( + this.stateService, + this.stateProvider, + ); this.policyService = new BrowserPolicyService(this.stateService, this.organizationService); this.policyApiService = new PolicyApiService( this.policyService, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index d58be12b6b1..be0508f5803 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -68,7 +68,7 @@ import { GlobalState } from "@bitwarden/common/platform/models/domain/global-sta import { ConfigService } from "@bitwarden/common/platform/services/config/config.service"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; -import { DerivedStateProvider } from "@bitwarden/common/platform/state"; +import { DerivedStateProvider, StateProvider } from "@bitwarden/common/platform/state"; import { SearchService } from "@bitwarden/common/services/search.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { UsernameGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/username"; @@ -415,8 +415,8 @@ function getBgService(service: keyof MainBackground) { }, { provide: OrganizationService, - useFactory: (stateService: StateServiceAbstraction) => { - return new BrowserOrganizationService(stateService); + useFactory: (stateService: StateServiceAbstraction, stateProvider: StateProvider) => { + return new BrowserOrganizationService(stateService, stateProvider); }, deps: [StateServiceAbstraction], }, diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 7becee70cca..161b33a0080 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -352,7 +352,7 @@ export class Main { this.providerService = new ProviderService(this.stateService); - this.organizationService = new OrganizationService(this.stateService); + this.organizationService = new OrganizationService(this.stateService, this.stateProvider); this.organizationUserService = new OrganizationUserServiceImplementation(this.apiService); diff --git a/libs/common/src/admin-console/models/data/organization.data.spec.ts b/libs/common/src/admin-console/models/data/organization.data.spec.ts new file mode 100644 index 00000000000..eb65303bce9 --- /dev/null +++ b/libs/common/src/admin-console/models/data/organization.data.spec.ts @@ -0,0 +1,62 @@ +import { ProductType } from "../../../enums/product-type.enum"; +import { OrganizationUserStatusType, OrganizationUserType } from "../../enums"; +import { ORGANIZATIONS } from "../../services/organization/organization.service"; + +import { OrganizationData } from "./organization.data"; + +describe("ORGANIZATIONS state", () => { + const sut = ORGANIZATIONS; + + it("should deserialize JSON string to proper object", async () => { + const expectedResult: Record = { + "1": { + id: "id", + name: "name", + status: OrganizationUserStatusType.Invited, + type: OrganizationUserType.Owner, + enabled: false, + usePolicies: false, + useGroups: false, + useDirectory: false, + useEvents: false, + useTotp: false, + use2fa: false, + useApi: false, + useSso: false, + useKeyConnector: false, + useScim: false, + useCustomPermissions: false, + useResetPassword: false, + useSecretsManager: false, + usePasswordManager: false, + useActivateAutofillPolicy: false, + selfHost: false, + usersGetPremium: false, + seats: 0, + maxCollections: 0, + ssoBound: false, + identifier: "identifier", + permissions: undefined, + resetPasswordEnrolled: false, + userId: "userId", + hasPublicAndPrivateKeys: false, + providerId: "providerId", + providerName: "providerName", + isProviderUser: false, + isMember: false, + familySponsorshipFriendlyName: "fsfn", + familySponsorshipAvailable: false, + planProductType: ProductType.Free, + keyConnectorEnabled: false, + keyConnectorUrl: "kcu", + accessSecretsManager: false, + limitCollectionCreationDeletion: false, + allowAdminAccessToAllCollectionItems: false, + flexibleCollections: false, + familySponsorshipLastSyncDate: new Date(), + }, + }; + const result = sut.deserializer(JSON.parse(JSON.stringify(expectedResult))); + expect(result).toEqual(expectedResult); + }); +}); diff --git a/libs/common/src/admin-console/models/data/organization.data.ts b/libs/common/src/admin-console/models/data/organization.data.ts index 512b25178b1..02fe0d6bf26 100644 --- a/libs/common/src/admin-console/models/data/organization.data.ts +++ b/libs/common/src/admin-console/models/data/organization.data.ts @@ -1,3 +1,5 @@ +import { Jsonify } from "type-fest"; + import { ProductType } from "../../../enums"; import { OrganizationUserStatusType, OrganizationUserType, ProviderType } from "../../enums"; import { PermissionsApi } from "../api/permissions.api"; @@ -54,12 +56,16 @@ export class OrganizationData { flexibleCollections: boolean; constructor( - response: ProfileOrganizationResponse, - options: { + response?: ProfileOrganizationResponse, + options?: { isMember: boolean; isProviderUser: boolean; }, ) { + if (response == null) { + return; + } + this.id = response.id; this.name = response.name; this.status = response.status; @@ -110,4 +116,17 @@ export class OrganizationData { this.isMember = options.isMember; this.isProviderUser = options.isProviderUser; } + + static fromJSON(obj: Jsonify) { + return Object.assign(new OrganizationData(), obj, { + familySponsorshipLastSyncDate: + obj.familySponsorshipLastSyncDate != null + ? new Date(obj.familySponsorshipLastSyncDate) + : obj.familySponsorshipLastSyncDate, + familySponsorshipValidUntil: + obj.familySponsorshipValidUntil != null + ? new Date(obj.familySponsorshipValidUntil) + : obj.familySponsorshipValidUntil, + }); + } } diff --git a/libs/common/src/admin-console/services/organization/organization.service.spec.ts b/libs/common/src/admin-console/services/organization/organization.service.spec.ts index 7e9a9b5988e..a4b7e958886 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.spec.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.spec.ts @@ -1,10 +1,14 @@ import { MockProxy, mock, any, mockClear } from "jest-mock-extended"; import { BehaviorSubject, firstValueFrom } from "rxjs"; +import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; +import { FakeActiveUserState } from "../../../../spec/fake-state"; import { StateService } from "../../../platform/abstractions/state.service"; +import { Utils } from "../../../platform/misc/utils"; +import { UserId } from "../../../types/guid"; import { OrganizationData } from "../../models/data/organization.data"; -import { OrganizationService } from "./organization.service"; +import { OrganizationService, ORGANIZATIONS } from "./organization.service"; describe("Organization Service", () => { let organizationService: OrganizationService; @@ -13,6 +17,11 @@ describe("Organization Service", () => { let activeAccount: BehaviorSubject; let activeAccountUnlocked: BehaviorSubject; + const mockUserId = Utils.newGuid() as UserId; + let accountService: FakeAccountService; + let stateProvider: FakeStateProvider; + let activeUserOrganizationsState: FakeActiveUserState>; + const resetStateService = async ( customizeStateService: (stateService: MockProxy) => void, ) => { @@ -21,10 +30,20 @@ describe("Organization Service", () => { stateService.activeAccount$ = activeAccount; stateService.activeAccountUnlocked$ = activeAccountUnlocked; customizeStateService(stateService); - organizationService = new OrganizationService(stateService); + organizationService = new OrganizationService(stateService, stateProvider); await new Promise((r) => setTimeout(r, 50)); }; + function prepareStateProvider(): void { + accountService = mockAccountServiceWith(mockUserId); + stateProvider = new FakeStateProvider(accountService); + } + + function seedTestData(): void { + activeUserOrganizationsState = stateProvider.activeUser.getFake(ORGANIZATIONS); + activeUserOrganizationsState.nextState({ "1": organizationData("1", "Test Org") }); + } + beforeEach(() => { activeAccount = new BehaviorSubject("123"); activeAccountUnlocked = new BehaviorSubject(true); @@ -37,7 +56,11 @@ describe("Organization Service", () => { "1": organizationData("1", "Test Org"), }); - organizationService = new OrganizationService(stateService); + prepareStateProvider(); + + organizationService = new OrganizationService(stateService, stateProvider); + + seedTestData(); }); afterEach(() => { diff --git a/libs/common/src/admin-console/services/organization/organization.service.ts b/libs/common/src/admin-console/services/organization/organization.service.ts index b10f24ca851..2c2c8b4e36a 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -1,6 +1,8 @@ import { BehaviorSubject, concatMap, map, Observable } from "rxjs"; +import { Jsonify } from "type-fest"; import { StateService } from "../../../platform/abstractions/state.service"; +import { KeyDefinition, ORGANIZATIONS_DISK, StateProvider } from "../../../platform/state"; import { InternalOrganizationServiceAbstraction, isMember, @@ -8,13 +10,37 @@ import { import { OrganizationData } from "../../models/data/organization.data"; import { Organization } from "../../models/domain/organization"; -export class OrganizationService implements InternalOrganizationServiceAbstraction { - protected _organizations = new BehaviorSubject([]); +export const ORGANIZATIONS = KeyDefinition.record( + ORGANIZATIONS_DISK, + "organizations", + { + deserializer: (obj: Jsonify) => OrganizationData.fromJSON(obj), + }, +); +export class OrganizationService implements InternalOrganizationServiceAbstraction { + // marked for removal during AC-2009 + protected _organizations = new BehaviorSubject([]); + // marked for removal during AC-2009 organizations$ = this._organizations.asObservable(); + // marked for removal during AC-2009 memberOrganizations$ = this.organizations$.pipe(map((orgs) => orgs.filter(isMember))); - constructor(private stateService: StateService) { + activeUserOrganizations$: Observable; + activeUserMemberOrganizations$: Observable; + + constructor( + private stateService: StateService, + private stateProvider: StateProvider, + ) { + this.activeUserOrganizations$ = this.stateProvider + .getActive(ORGANIZATIONS) + .state$.pipe(map((data) => Object.values(data).map((o) => new Organization(o)))); + + this.activeUserMemberOrganizations$ = this.activeUserOrganizations$.pipe( + map((orgs) => orgs.filter(isMember)), + ); + this.stateService.activeAccountUnlocked$ .pipe( concatMap(async (unlocked) => {