From 0b51d67530c12cc9df9cf2c99a0639a4685cb8ce Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Tue, 27 Feb 2024 14:53:42 -0600 Subject: [PATCH 01/17] Migrate OrganizationService to StateProvider --- .../organization-service.factory.ts | 13 +- .../services/browser-organization.service.ts | 12 - .../browser/src/background/main.background.ts | 8 +- .../src/popup/services/services.module.ts | 8 - apps/cli/src/bw.ts | 3 +- apps/desktop/src/app/app.component.ts | 3 + apps/web/src/app/app.component.ts | 3 + .../src/services/jslib-services.module.ts | 2 +- .../organization.service.abstraction.ts | 121 ++++- .../models/domain/organization.ts | 4 + .../organization/organization.service.spec.ts | 414 ++++++++++++------ .../organization/organization.service.ts | 163 ++++--- libs/common/src/state-migrations/migrate.ts | 6 +- ...ganization-state-to-state-provider.spec.ts | 183 ++++++++ ...ve-organization-state-to-state-provider.ts | 148 +++++++ 15 files changed, 829 insertions(+), 262 deletions(-) delete mode 100644 apps/browser/src/admin-console/services/browser-organization.service.ts create mode 100644 libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.spec.ts create mode 100644 libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.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 c77508b0f888..b7f6f98ea23d 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 @@ -1,4 +1,5 @@ import { OrganizationService as AbstractOrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/services/organization/organization.service"; import { FactoryOptions, @@ -6,11 +7,7 @@ import { factory, } from "../../../platform/background/service-factories/factory-options"; import { stateProviderFactory } from "../../../platform/background/service-factories/state-provider.factory"; -import { - stateServiceFactory, - StateServiceInitOptions, -} from "../../../platform/background/service-factories/state-service.factory"; -import { BrowserOrganizationService } from "../../services/browser-organization.service"; +import { StateServiceInitOptions } from "../../../platform/background/service-factories/state-service.factory"; type OrganizationServiceFactoryOptions = FactoryOptions; @@ -25,10 +22,6 @@ export function organizationServiceFactory( cache, "organizationService", opts, - async () => - new BrowserOrganizationService( - await stateServiceFactory(cache, opts), - await stateProviderFactory(cache, opts), - ), + async () => new OrganizationService(await stateProviderFactory(cache, opts)), ); } diff --git a/apps/browser/src/admin-console/services/browser-organization.service.ts b/apps/browser/src/admin-console/services/browser-organization.service.ts deleted file mode 100644 index 6294756cdf7f..000000000000 --- a/apps/browser/src/admin-console/services/browser-organization.service.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { BehaviorSubject } from "rxjs"; - -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { OrganizationService } from "@bitwarden/common/admin-console/services/organization/organization.service"; - -import { browserSession, sessionSync } from "../../platform/decorators/session-sync-observable"; - -@browserSession -export class BrowserOrganizationService extends OrganizationService { - @sessionSync({ initializer: Organization.fromJSON, initializeAs: "array" }) - protected _organizations: BehaviorSubject; -} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 724c2ee9949f..2967372193b8 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -21,6 +21,7 @@ import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin- import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; 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 { OrganizationService } from "@bitwarden/common/admin-console/services/organization/organization.service"; import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; @@ -165,7 +166,6 @@ 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 ContextMenusBackground from "../autofill/background/context-menus.background"; import NotificationBackground from "../autofill/background/notification.background"; @@ -474,10 +474,7 @@ export default class MainBackground { this.stateProvider, ); this.syncNotifierService = new SyncNotifierService(); - this.organizationService = new BrowserOrganizationService( - this.stateService, - this.stateProvider, - ); + this.organizationService = new OrganizationService(this.stateProvider); this.policyService = new BrowserPolicyService( this.stateService, this.stateProvider, @@ -1085,6 +1082,7 @@ export default class MainBackground { this.keyConnectorService.clear(), this.vaultFilterService.clear(), this.biometricStateService.logout(userId), + this.organizationService.replace(null, userId), /* We intentionally do not clear: * - autofillSettingsService * - badgeSettingsService diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index d6b2954503cf..4d15c6ece266 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -103,7 +103,6 @@ import { DialogService } from "@bitwarden/components"; 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"; @@ -451,13 +450,6 @@ function getBgService(service: keyof MainBackground) { useFactory: getBgService("logService"), deps: [], }, - { - provide: OrganizationService, - useFactory: (stateService: StateServiceAbstraction, stateProvider: StateProvider) => { - return new BrowserOrganizationService(stateService, stateProvider); - }, - deps: [StateServiceAbstraction, StateProvider], - }, { provide: VaultFilterService, useClass: VaultFilterService, diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 4713b947f022..9faa75db6c35 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -369,7 +369,7 @@ export class Main { this.providerService = new ProviderService(this.stateService); - this.organizationService = new OrganizationService(this.stateService, this.stateProvider); + this.organizationService = new OrganizationService(this.stateProvider); this.organizationUserService = new OrganizationUserServiceImplementation(this.apiService); @@ -635,6 +635,7 @@ export class Main { this.collectionService.clear(userId as UserId), this.policyService.clear(userId), this.passwordGenerationService.clear(), + this.organizationService.replace(null, userId as UserId), ]); await this.stateService.clean(); process.env.BW_SESSION = null; diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index afc37005df44..53381bbdb46d 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -22,6 +22,7 @@ import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; +import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; @@ -149,6 +150,7 @@ export class AppComponent implements OnInit, OnDestroy { private configService: ConfigServiceAbstraction, private dialogService: DialogService, private biometricStateService: BiometricStateService, + private organizationService: InternalOrganizationServiceAbstraction, ) {} ngOnInit() { @@ -582,6 +584,7 @@ export class AppComponent implements OnInit, OnDestroy { await this.policyService.clear(userBeingLoggedOut); await this.keyConnectorService.clear(); await this.biometricStateService.logout(userBeingLoggedOut as UserId); + await this.organizationService.replace(null, userBeingLoggedOut as UserId); preLogoutActiveUserId = this.activeUserId; await this.stateService.clean({ userId: userBeingLoggedOut }); diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index f9479a92ce30..72fc38272142 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -11,6 +11,7 @@ import { NotificationsService } from "@bitwarden/common/abstractions/notificatio import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; +import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; @@ -85,6 +86,7 @@ export class AppComponent implements OnDestroy, OnInit { private configService: ConfigServiceAbstraction, private dialogService: DialogService, private biometricStateService: BiometricStateService, + private organizationService: InternalOrganizationServiceAbstraction, ) {} ngOnInit() { @@ -260,6 +262,7 @@ export class AppComponent implements OnDestroy, OnInit { this.passwordGenerationService.clear(), this.keyConnectorService.clear(), this.biometricStateService.logout(userId as UserId), + this.organizationService.replace(null, userId as UserId), ]); this.searchService.clearIndex(); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index a08b75ff57cc..92a8c7b29b48 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -716,7 +716,7 @@ import { ModalService } from "./modal.service"; { provide: OrganizationServiceAbstraction, useClass: OrganizationService, - deps: [StateServiceAbstraction, StateProvider], + deps: [StateProvider], }, { provide: InternalOrganizationServiceAbstraction, diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 06fe8efc87ee..1e8540d2f42a 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -2,6 +2,7 @@ import { map, Observable } from "rxjs"; import { I18nService } from "../../../platform/abstractions/i18n.service"; import { Utils } from "../../../platform/misc/utils"; +import { UserId } from "../../../types/guid"; import { OrganizationData } from "../../models/data/organization.data"; import { Organization } from "../../models/domain/organization"; @@ -47,6 +48,9 @@ export function canAccessOrgAdmin(org: Organization): boolean { ); } +/** + * @deprecated Use mapToSingleOrganization() + */ export function getOrganizationById(id: string) { return map((orgs) => orgs.find((o) => o.id === id)); } @@ -82,34 +86,135 @@ export function canAccessImport(i18nService: I18nService) { /** * Returns `true` if a user is a member of an organization (rather than only being a ProviderUser) - * @deprecated Use organizationService.memberOrganizations$ instead + * @deprecated Use organizationService.organizations$() with a + * mapToExcludeSpecialOrganizations() pipe instead */ export function isMember(org: Organization): boolean { return org.isMember; } +/** + * Filter out organizations from an observable that __do not__ offer a + * families-for-enterprise sponsorship to members. + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +export function mapToExcludeOrganizationsWithoutFamilySponsorshipSupport() { + return map((orgs) => orgs.filter((o) => o.canManageSponsorships)); +} + +/** + * Filter out organizations from an observable that the organization user + * __is not__ a direct member of. This will exclude organizations only + * accessible as a provider, for example. + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +export function mapToExcludeSpecialOrganizations() { + return map((orgs) => orgs.filter((o) => o.isMember)); +} + +/** + * Map an observable stream of organizations down to a boolean indicating + * if any organizations exist (`orgs.length > 0`). + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +export function mapToBooleanHasAnyOrganizations() { + return map((orgs) => orgs.length > 0); +} + +/** + * Map an observable stream of organizations down to a single organization. + * @param `organizationId` The ID of the organization you'd like to subscribe to + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +export function mapToSingleOrganization(organizationId: string) { + return map((orgs) => orgs?.find((o) => o.id === organizationId)); +} + +/** + * Publishes an observable stream of organizations. This service is meant to + * be used widely across Bitwarden as the primary way of fetching organizations. + * Risky operations like updates are isolated to the + * internal extension `InternalOrganizationServiceAbstraction`. + */ export abstract class OrganizationService { + /** + * Publishes state for all organizations under the active user. + * + * There are helper functions available for use in pipes that will + * filter subscriptions for common tasks like subscribing to only one + * organization, but they must be imported directly. See + * `organization.service.abstraction` for details. + * @returns An observable list of organizations that meet the search criteria + * provided. + */ organizations$: Observable; /** + * @deprecated Use organizations$() with `mapToExcludeSpecialOrganizations` instead. * Organizations that the user is a member of (excludes organizations that they only have access to via a provider) */ memberOrganizations$: Observable; - get$: (id: string) => Observable; - get: (id: string) => Organization; - getByIdentifier: (identifier: string) => Organization; - getAll: (userId?: string) => Promise; /** - * @deprecated For the CLI only + * @deprecated Use organizations$ with a pipe to `mapToSingleOrganizaiton()` instead. * @param id id of the organization */ getFromState: (id: string) => Promise; + + /** + * @deprecated Use organizations$ with a pipe to `mapToExcludeOrganizationsWithoutFamilySponsorshipSupport()` instead. + */ canManageSponsorships: () => Promise; + + /** + * @deprecated Use organizations$ with a pipe to `mapToBooleanHasAnyOrganizations()` instead. + */ hasOrganizations: () => boolean; + + /** + * @deprecated Use organizations$ with a pipe to `mapToSingleOrganization()` instead. + */ + get$: (id: string) => Observable; + + /** + * @deprecated Use organizations$ with a pipe to `mapToSingleOrganization()` instead. + */ + get: (id: string) => Organization; + + /** + * @deprecated Use organizations$ instead. + */ + getAll: (userId?: string) => Promise; } +/** + * Big scary buttons that **update** organization state. These should only be + * called from within admin-console scoped code. Extends the base + * `OrganizationService` for easy access to `get` calls. + * @internal + */ export abstract class InternalOrganizationServiceAbstraction extends OrganizationService { - replace: (organizations: { [id: string]: OrganizationData }) => Promise; - upsert: (OrganizationData: OrganizationData | OrganizationData[]) => Promise; + /** + * Replaces state for the provided organization, or creates it if not found. + * @param organization The organization state being saved. + * @param userId The userId to replace state for. Defaults to the active + * user. + */ + upsert: (OrganizationData: OrganizationData) => Promise; + + /** + * Replaces state for the entire registered organization list for the active user. + * You probably don't want this unless you're calling from a full sync + * operation or a logout. See `upsert` for creating & updating a single + * organization in the state. + * @param organizations A complete list of all organization state for the active + * user. + * @param userId The userId to replace state for. Defaults to the active + * user. + */ + replace: (organizations: { [id: string]: OrganizationData }, userId?: UserId) => Promise; } diff --git a/libs/common/src/admin-console/models/domain/organization.ts b/libs/common/src/admin-console/models/domain/organization.ts index 8eba83ba3efe..18b762207a18 100644 --- a/libs/common/src/admin-console/models/domain/organization.ts +++ b/libs/common/src/admin-console/models/domain/organization.ts @@ -320,6 +320,10 @@ export class Organization { return !this.useTotp; } + get canManageSponsorships() { + return this.familySponsorshipAvailable || this.familySponsorshipFriendlyName !== null; + } + static fromJSON(json: Jsonify) { if (json == null) { return null; 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 a4b7e9588868..a5f23f41f975 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,114 +1,165 @@ -import { MockProxy, mock, any, mockClear } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom } from "rxjs"; +import { firstValueFrom } from "rxjs"; import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { FakeActiveUserState } from "../../../../spec/fake-state"; -import { StateService } from "../../../platform/abstractions/state.service"; +import { AccountInfo } from "../../../auth/abstractions/account.service"; +import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { Utils } from "../../../platform/misc/utils"; -import { UserId } from "../../../types/guid"; +import { OrganizationId, UserId } from "../../../types/guid"; import { OrganizationData } from "../../models/data/organization.data"; +import { Organization } from "../../models/domain/organization"; import { OrganizationService, ORGANIZATIONS } from "./organization.service"; -describe("Organization Service", () => { +describe("OrganizationService", () => { let organizationService: OrganizationService; - let stateService: MockProxy; - 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, - ) => { - mockClear(stateService); - stateService = mock(); - stateService.activeAccount$ = activeAccount; - stateService.activeAccountUnlocked$ = activeAccountUnlocked; - customizeStateService(stateService); - organizationService = new OrganizationService(stateService, stateProvider); - await new Promise((r) => setTimeout(r, 50)); - }; - - function prepareStateProvider(): void { - accountService = mockAccountServiceWith(mockUserId); - stateProvider = new FakeStateProvider(accountService); + const fakeUserId = Utils.newGuid() as UserId; + let fakeAccountService: FakeAccountService; + let fakeStateProvider: FakeStateProvider; + let fakeActiveUserState: FakeActiveUserState>; + + /** + * It is easier to read arrays than records in code, but we store a record + * in state. This helper methods lets us build organization arrays in tests + * and easily map them to records before storing them in state. + */ + function arrayToRecord(input: OrganizationData[]): Record { + if (input == null) { + return undefined; + } + return Object.fromEntries(input?.map((i) => [i.id, i])); } - function seedTestData(): void { - activeUserOrganizationsState = stateProvider.activeUser.getFake(ORGANIZATIONS); - activeUserOrganizationsState.nextState({ "1": organizationData("1", "Test Org") }); + /** + * There are a few assertions in this spec that check for array equality + * but want to ignore a specific index that _should_ be different. This + * function takes two arrays, and an index. It checks for equality of the + * arrays, but splices out the specified index from both arrays first. + */ + function isEqualExceptForIndex(x: any[], y: any[], indexToExclude: number) { + return ( + JSON.stringify([ + ...x.slice(0, indexToExclude - 1), + ...x.slice(indexToExclude + 1, x.length), + ]) === + JSON.stringify([...y.slice(0, indexToExclude - 1), ...y.slice(indexToExclude + 1, y.length)]) + ); } - beforeEach(() => { - activeAccount = new BehaviorSubject("123"); - activeAccountUnlocked = new BehaviorSubject(true); - - stateService = mock(); - stateService.activeAccount$ = activeAccount; - stateService.activeAccountUnlocked$ = activeAccountUnlocked; - - stateService.getOrganizations.calledWith(any()).mockResolvedValue({ - "1": organizationData("1", "Test Org"), - }); - - prepareStateProvider(); - - organizationService = new OrganizationService(stateService, stateProvider); + /** + * Builds a simple mock `OrganizationData[]` array that can be used in tests + * to populate state. + * @param count The number of organizations to populate the list with. The + * function returns undefined if this is less than 1. The default value is 1. + * @param suffix A string to append to data fields on each organization. + * This defaults to the index of the organization in the list. + * @returns an `OrganizationData[]` array that can be used to populate + * stateProvider. + */ + function buildMockOrganizations(count = 1, suffix?: string): OrganizationData[] { + if (count < 1) { + return undefined; + } + + function buildMockOrganization(id: OrganizationId, name: string, identifier: string) { + const data = new OrganizationData({} as any, {} as any); + data.id = id; + data.name = name; + data.identifier = identifier; + + return data; + } + + const mockOrganizations = []; + for (let i = 0; i < count; i++) { + const s = suffix ? suffix + i.toString() : i.toString(); + mockOrganizations.push( + buildMockOrganization(("org" + s) as OrganizationId, "org" + s, "orgIdentifier" + s), + ); + } + + return mockOrganizations; + } - seedTestData(); - }); + /** + * `OrganizationService` deals with multiple accounts at times. This helper + * function can be used to add a new non-active account to the test data. + * This function is **not** needed to handle creation of the first account, + * as that is handled by the `FakeAccountService` in `mockAccountServiceWith()` + * @param opts.createWithTestOrgs Will add a couple of slim test organizations to + * the state of the user being created. Defaults to false. + * @returns The `UserId` of the newly created state account and the mock data + * created for them as an `Organization[]`. + */ + async function addNonActiveAccountToStateProvider(opts?: { + createWithTestOrgs: boolean; + }): Promise<[UserId, OrganizationData[]]> { + const nonActiveUserId = Utils.newGuid() as UserId; + // This is the same partial object setup that `mockAccountServiceWith()` uses. + // I'm assuming at the time of writing that name, email, and status are + // important to the internal functions of StateProvider and should + // always be mocked when testing. + const fullInfo: AccountInfo = { + name: "nonActiveUserName", + email: "nonActiveUserEmail", + status: AuthenticationStatus.Locked, + }; + // This does **not** change the active user, and instead adds this account + // in an inactive state. + await fakeAccountService.addAccount(nonActiveUserId, fullInfo); + + if (!opts?.createWithTestOrgs) { + return [nonActiveUserId, undefined]; + } + + const mockOrganizations = buildMockOrganizations(10); + const fakeNonActiveUserState = fakeStateProvider.singleUser.getFake( + nonActiveUserId, + ORGANIZATIONS, + ); + fakeNonActiveUserState.nextState(arrayToRecord(mockOrganizations)); + + return [nonActiveUserId, mockOrganizations]; + } - afterEach(() => { - activeAccount.complete(); - activeAccountUnlocked.complete(); + beforeEach(async () => { + fakeAccountService = mockAccountServiceWith(fakeUserId); + fakeStateProvider = new FakeStateProvider(fakeAccountService); + fakeActiveUserState = fakeStateProvider.activeUser.getFake(ORGANIZATIONS); + organizationService = new OrganizationService(fakeStateProvider); }); it("getAll", async () => { + const mockData: OrganizationData[] = buildMockOrganizations(1); + fakeActiveUserState.nextState(arrayToRecord(mockData)); const orgs = await organizationService.getAll(); expect(orgs).toHaveLength(1); const org = orgs[0]; - expect(org).toEqual({ - id: "1", - name: "Test Org", - identifier: "test", - }); + expect(org).toEqual(new Organization(mockData[0])); }); describe("canManageSponsorships", () => { it("can because one is available", async () => { - await resetStateService((stateService) => { - stateService.getOrganizations.mockResolvedValue({ - "1": { ...organizationData("1", "Org"), familySponsorshipAvailable: true }, - }); - }); - + const mockData: OrganizationData[] = buildMockOrganizations(1); + mockData[0].familySponsorshipAvailable = true; + fakeActiveUserState.nextState(arrayToRecord(mockData)); const result = await organizationService.canManageSponsorships(); expect(result).toBe(true); }); it("can because one is used", async () => { - await resetStateService((stateService) => { - stateService.getOrganizations.mockResolvedValue({ - "1": { ...organizationData("1", "Test Org"), familySponsorshipFriendlyName: "Something" }, - }); - }); - + const mockData: OrganizationData[] = buildMockOrganizations(1); + mockData[0].familySponsorshipFriendlyName = "Something"; + fakeActiveUserState.nextState(arrayToRecord(mockData)); const result = await organizationService.canManageSponsorships(); expect(result).toBe(true); }); it("can not because one isn't available or taken", async () => { - await resetStateService((stateService) => { - stateService.getOrganizations.mockResolvedValue({ - "1": { ...organizationData("1", "Org"), familySponsorshipFriendlyName: null }, - }); - }); - + const mockData: OrganizationData[] = buildMockOrganizations(1); + mockData[0].familySponsorshipFriendlyName = null; + fakeActiveUserState.nextState(arrayToRecord(mockData)); const result = await organizationService.canManageSponsorships(); expect(result).toBe(false); }); @@ -116,81 +167,188 @@ describe("Organization Service", () => { describe("get", () => { it("exists", async () => { - const result = organizationService.get("1"); - - expect(result).toEqual({ - id: "1", - name: "Test Org", - identifier: "test", - }); + const mockData = buildMockOrganizations(1); + fakeActiveUserState.nextState(arrayToRecord(mockData)); + const result = organizationService.get(mockData[0].id); + expect(result).toEqual(new Organization(mockData[0])); }); it("does not exist", async () => { - const result = organizationService.get("2"); - + const result = organizationService.get("this-org-does-not-exist"); expect(result).toBe(undefined); }); }); - it("upsert", async () => { - await organizationService.upsert(organizationData("2", "Test 2")); - - expect(await firstValueFrom(organizationService.organizations$)).toEqual([ - { - id: "1", - name: "Test Org", - identifier: "test", - }, - { - id: "2", - name: "Test 2", - identifier: "test", - }, - ]); - }); + describe("organizations$", () => { + describe("null checking behavior", () => { + it("publishes an empty array if organizations in state = undefined", async () => { + const mockData: OrganizationData[] = undefined; + fakeActiveUserState.nextState(arrayToRecord(mockData)); + const result = await firstValueFrom(organizationService.organizations$); + expect(result).toEqual([]); + }); - describe("getByIdentifier", () => { - it("exists", async () => { - const result = organizationService.getByIdentifier("test"); + it("publishes an empty array if organizations in state = null", async () => { + const mockData: OrganizationData[] = null; + fakeActiveUserState.nextState(arrayToRecord(mockData)); + const result = await firstValueFrom(organizationService.organizations$); + expect(result).toEqual([]); + }); - expect(result).toEqual({ - id: "1", - name: "Test Org", - identifier: "test", + it("publishes an empty array if organizations in state = []", async () => { + const mockData: OrganizationData[] = []; + fakeActiveUserState.nextState(arrayToRecord(mockData)); + const result = await firstValueFrom(organizationService.organizations$); + expect(result).toEqual([]); }); }); - it("does not exist", async () => { - const result = organizationService.getByIdentifier("blah"); + describe("parameter handling & returns", () => { + it("publishes all organizations for the active user by default", async () => { + const mockData = buildMockOrganizations(10); + fakeActiveUserState.nextState(arrayToRecord(mockData)); + const result = await firstValueFrom(organizationService.organizations$); + expect(result).toEqual(mockData); + }); + + it("can be used to publish the organizations of a non active user if requested", async () => { + const activeUserMockData = buildMockOrganizations(10, "activeUserState"); + fakeActiveUserState.nextState(arrayToRecord(activeUserMockData)); + + const [nonActiveUserId, nonActiveUserMockOrganizations] = + await addNonActiveAccountToStateProvider({ createWithTestOrgs: true }); + // This can be updated to use + // `firstValueFrom(organizations$(nonActiveUserId)` once all the + // promise based methods are removed from `OrganizationService` and the + // main observable is refactored to accept a userId + const result = await organizationService.getAll(nonActiveUserId); - expect(result).toBeUndefined(); + expect(result).toEqual(nonActiveUserMockOrganizations); + expect(result).not.toEqual(await firstValueFrom(organizationService.organizations$)); + }); }); }); - describe("delete", () => { - it("exists", async () => { - await organizationService.delete("1"); + describe("upsert()", () => { + it("can create the organization list if necassary", async () => { + // Notice that no default state is provided in this test, so the list in + // `stateProvider` will be null when the `upsert` method is called. + const mockData = buildMockOrganizations(); + await organizationService.upsert(mockData[0]); + const result = await firstValueFrom(organizationService.organizations$); + expect(result).not.toEqual(undefined || null || []); + expect(result).toEqual(mockData.map((x) => new Organization(x))); + }); - expect(stateService.getOrganizations).toHaveBeenCalledTimes(2); + it("updates an organization that already exists in state, defaulting to the active user", async () => { + const mockData = buildMockOrganizations(10); + fakeActiveUserState.nextState(arrayToRecord(mockData)); + const indexToUpdate = 5; + const anUpdatedOrganization = { + ...buildMockOrganizations(1, "UPDATED").pop(), + id: mockData[indexToUpdate].id, + }; + await organizationService.upsert(anUpdatedOrganization); + const result = await firstValueFrom(organizationService.organizations$); + expect(result[indexToUpdate]).not.toEqual(new Organization(mockData[indexToUpdate])); + expect(result[indexToUpdate].id).toEqual(new Organization(mockData[indexToUpdate]).id); + expect( + isEqualExceptForIndex( + result, + mockData.map((x) => new Organization(x)), + indexToUpdate, + ), + ).toBeTruthy(); + }); - expect(stateService.setOrganizations).toHaveBeenCalledTimes(1); + it("can also update an organization in state for a non-active user, if requested", async () => { + const activeUserMockData = buildMockOrganizations(10, "activeUserOrganizations"); + fakeActiveUserState.nextState(arrayToRecord(activeUserMockData)); + + const [nonActiveUserId, nonActiveUserMockOrganizations] = + await addNonActiveAccountToStateProvider({ createWithTestOrgs: true }); + const indexToUpdate = 5; + const anUpdatedOrganization = { + ...buildMockOrganizations(1, "UPDATED").pop(), + id: nonActiveUserMockOrganizations[indexToUpdate].id, + }; + + await organizationService.upsert(anUpdatedOrganization, nonActiveUserId); + // This can be updated to use + // `firstValueFrom(organizations$(nonActiveUserId)` once all the + // promise based methods are removed from `OrganizationService` and the + // main observable is refactored to accept a userId + const result = await organizationService.getAll(nonActiveUserId); + + expect(result[indexToUpdate]).not.toEqual( + new Organization(nonActiveUserMockOrganizations[indexToUpdate]), + ); + expect(result[indexToUpdate].id).toEqual( + new Organization(nonActiveUserMockOrganizations[indexToUpdate]).id, + ); + expect( + isEqualExceptForIndex( + result, + nonActiveUserMockOrganizations.map((x) => new Organization(x)), + indexToUpdate, + ), + ).toBeTruthy(); + + // Just to be safe, lets make sure the active user didn't get updated + // at all + const activeUserState = await firstValueFrom(organizationService.organizations$); + expect(activeUserState).toEqual(activeUserMockData.map((x) => new Organization(x))); + expect(activeUserState).not.toEqual(result); }); + }); - it("does not exist", async () => { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - organizationService.delete("1"); + describe("replace()", () => { + it("replaces the entire organization list in state, defaulting to the active user", async () => { + const originalData = buildMockOrganizations(10); + fakeActiveUserState.nextState(arrayToRecord(originalData)); + + const newData = buildMockOrganizations(10, "newData"); + await organizationService.replace(arrayToRecord(newData)); + + const result = await firstValueFrom(organizationService.organizations$); - expect(stateService.getOrganizations).toHaveBeenCalledTimes(2); + expect(result).toEqual(newData); + expect(result).not.toEqual(originalData); + }); + + // This is more or less a test for logouts + it("can replace state with null", async () => { + const originalData = buildMockOrganizations(2); + fakeActiveUserState.nextState(arrayToRecord(originalData)); + await organizationService.replace(null); + const result = await firstValueFrom(organizationService.organizations$); + expect(result).toEqual([]); + expect(result).not.toEqual(originalData); }); - }); - function organizationData(id: string, name: string) { - const data = new OrganizationData({} as any, {} as any); - data.id = id; - data.name = name; - data.identifier = "test"; + it("can also replace state for a non-active user, if requested", async () => { + const activeUserMockData = buildMockOrganizations(10, "activeUserOrganizations"); + fakeActiveUserState.nextState(arrayToRecord(activeUserMockData)); - return data; - } + const [nonActiveUserId, originalOrganizations] = await addNonActiveAccountToStateProvider({ + createWithTestOrgs: true, + }); + const newData = buildMockOrganizations(10, "newData"); + + await organizationService.replace(arrayToRecord(newData), nonActiveUserId); + // This can be updated to use + // `firstValueFrom(organizations$(nonActiveUserId)` once all the + // promise based methods are removed from `OrganizationService` and the + // main observable is refactored to accept a userId + const result = await organizationService.getAll(nonActiveUserId); + expect(result).toEqual(newData); + expect(result).not.toEqual(originalOrganizations); + + // Just to be safe, lets make sure the active user didn't get updated + // at all + const activeUserState = await firstValueFrom(organizationService.organizations$); + expect(activeUserState).toEqual(activeUserMockData.map((x) => new Organization(x))); + expect(activeUserState).not.toEqual(result); + }); + }); }); 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 2c2c8b4e36a6..ae5729698bd9 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -1,15 +1,24 @@ -import { BehaviorSubject, concatMap, map, Observable } from "rxjs"; +import { map, Observable, firstValueFrom, first } from "rxjs"; import { Jsonify } from "type-fest"; -import { StateService } from "../../../platform/abstractions/state.service"; import { KeyDefinition, ORGANIZATIONS_DISK, StateProvider } from "../../../platform/state"; +import { UserId } from "../../../types/guid"; import { InternalOrganizationServiceAbstraction, - isMember, + mapToBooleanHasAnyOrganizations, + mapToExcludeOrganizationsWithoutFamilySponsorshipSupport, + mapToExcludeSpecialOrganizations, + mapToSingleOrganization, } from "../../abstractions/organization/organization.service.abstraction"; import { OrganizationData } from "../../models/data/organization.data"; import { Organization } from "../../models/domain/organization"; +/** + * The `KeyDefinition` for accessing organization lists in application state. + * @todo Ideally this wouldn't require a `fromJSON()` call, but `OrganizationData` + * has some properties that contain functions. This should probably get + * cleaned up. + */ export const ORGANIZATIONS = KeyDefinition.record( ORGANIZATIONS_DISK, "organizations", @@ -19,93 +28,53 @@ export const ORGANIZATIONS = KeyDefinition.record( ); 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))); + organizations$ = this.getOrganizationsFromState$(); + memberOrganizations$ = this.organizations$.pipe(mapToExcludeSpecialOrganizations()); - 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) => { - if (!unlocked) { - this._organizations.next([]); - return; - } - - const data = await this.stateService.getOrganizations(); - this.updateObservables(data); - }), - ) - .subscribe(); - } + constructor(private stateProvider: StateProvider) {} get$(id: string): Observable { - return this.organizations$.pipe(map((orgs) => orgs.find((o) => o.id === id))); + return this.organizations$.pipe(mapToSingleOrganization(id)); } async getAll(userId?: string): Promise { - const organizationsMap = await this.stateService.getOrganizations({ userId: userId }); + const organizationsMap = await firstValueFrom( + this.getOrganizationsFromState$(userId as UserId), + ); return Object.values(organizationsMap || {}).map((o) => new Organization(o)); } async canManageSponsorships(): Promise { - const organizations = this._organizations.getValue(); - return organizations.some( - (o) => o.familySponsorshipAvailable || o.familySponsorshipFriendlyName !== null, + return await firstValueFrom( + this.organizations$.pipe( + mapToExcludeOrganizationsWithoutFamilySponsorshipSupport(), + mapToBooleanHasAnyOrganizations(), + ), ); } hasOrganizations(): boolean { - const organizations = this._organizations.getValue(); - return organizations.length > 0; + let value = false; + this.organizations$.pipe(mapToBooleanHasAnyOrganizations(), first()).subscribe((x) => { + value = x; + }); + return value; } - async upsert(organization: OrganizationData): Promise { - let organizations = await this.stateService.getOrganizations(); - if (organizations == null) { - organizations = {}; - } - - organizations[organization.id] = organization; - - await this.replace(organizations); - } - - async delete(id: string): Promise { - const organizations = await this.stateService.getOrganizations(); - if (organizations == null) { - return; - } - - if (organizations[id] == null) { - return; - } - - delete organizations[id]; - await this.replace(organizations); + async upsert(organization: OrganizationData, userId?: UserId): Promise { + await this.stateFor(userId).update((existingOrganizations) => { + const organizations = existingOrganizations ?? {}; + organizations[organization.id] = organization; + return organizations; + }); } get(id: string): Organization { - const organizations = this._organizations.getValue(); - - return organizations.find((organization) => organization.id === id); + let value: Organization = undefined; + this.organizations$.pipe(mapToSingleOrganization(id), first()).subscribe((x) => { + value = x; + }); + return value; } /** @@ -113,28 +82,48 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti * @param id id of the organization */ async getFromState(id: string): Promise { - const organizationsMap = await this.stateService.getOrganizations(); - const organization = organizationsMap[id]; - if (organization == null) { - return null; - } - - return new Organization(organization); + return await firstValueFrom(this.organizations$.pipe(mapToSingleOrganization(id))); } - getByIdentifier(identifier: string): Organization { - const organizations = this._organizations.getValue(); + async replace(organizations: { [id: string]: OrganizationData }, userId?: UserId): Promise { + await this.stateFor(userId).update(() => { + return organizations; + }); + } - return organizations.find((organization) => organization.identifier === identifier); + // Ideally this method would be renamed to organizations$() and the + // $organizations observable as it stands would be removed. This will + // require updates to callers, and so this method exists as a temporary + // workaround until we have time & a plan to update callers. + // + // It can be thought of as "organizations$ but with a userId option". + private getOrganizationsFromState$(userId?: UserId): Observable { + return this.stateFor(userId).state$.pipe(this.mapOrganizationRecordToArray()); } - async replace(organizations: { [id: string]: OrganizationData }) { - await this.stateService.setOrganizations(organizations); - this.updateObservables(organizations); + /** + * Accepts a record of `OrganizationData`, which is how we store the + * organization list as a JSON object on disk, to an array of + * `Organization`, which is how the data is published to callers of the + * service. + * @returns a function that can be used to pipe organization data from + * stored state to an exposed object easily consumable by others. + */ + private mapOrganizationRecordToArray() { + return map, Organization[]>((orgs) => + Object.values(orgs ?? {})?.map((o) => new Organization(o)), + ); } - private updateObservables(organizationsMap: { [id: string]: OrganizationData }) { - const organizations = Object.values(organizationsMap || {}).map((o) => new Organization(o)); - this._organizations.next(organizations); + /** + * Fetches the organization list from on disk state for the specified user. + * @param userId the user ID to fetch the organization list for. Defaults to + * the currently active user. + * @returns an observable of organization state as it is stored on disk. + */ + private stateFor(userId?: UserId) { + return userId + ? this.stateProvider.getUser(userId, ORGANIZATIONS) + : this.stateProvider.getActive(ORGANIZATIONS); } } diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index bec8e0241f61..4d1bd47c2140 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -21,6 +21,7 @@ import { MoveBiometricPromptsToStateProviders } from "./migrations/23-move-biome import { SmOnboardingTasksMigrator } from "./migrations/24-move-sm-onboarding-key-to-state-providers"; import { ClearClipboardDelayMigrator } from "./migrations/25-move-clear-clipboard-to-autofill-settings-state-provider"; import { BadgeSettingsMigrator } from "./migrations/26-move-badge-settings-to-state-providers"; +import { OrganizationMigrator } from "./migrations/27-move-organization-state-to-state-provider"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; @@ -31,7 +32,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 26; +export const CURRENT_VERSION = 27; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -60,7 +61,8 @@ export function createMigrationBuilder() { .with(MoveBiometricPromptsToStateProviders, 22, 23) .with(SmOnboardingTasksMigrator, 23, 24) .with(ClearClipboardDelayMigrator, 24, 25) - .with(BadgeSettingsMigrator, 25, CURRENT_VERSION); + .with(BadgeSettingsMigrator, 25, 26) + .with(OrganizationMigrator, 26, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.spec.ts new file mode 100644 index 000000000000..633118b9245f --- /dev/null +++ b/libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.spec.ts @@ -0,0 +1,183 @@ +import { any, MockProxy } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { OrganizationMigrator } from "./27-move-organization-state-to-state-provider"; + +const testDate = new Date(); +function exampleOrganization1() { + return JSON.stringify({ + id: "id", + name: "name", + status: 0, + type: 0, + 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", + resetPasswordEnrolled: false, + userId: "userId", + hasPublicAndPrivateKeys: false, + providerId: "providerId", + providerName: "providerName", + isProviderUser: false, + isMember: false, + familySponsorshipFriendlyName: "fsfn", + familySponsorshipAvailable: false, + planProductType: 0, + keyConnectorEnabled: false, + keyConnectorUrl: "kcu", + accessSecretsManager: false, + limitCollectionCreationDeletion: false, + allowAdminAccessToAllCollectionItems: false, + flexibleCollections: false, + familySponsorshipLastSyncDate: testDate, + }); +} + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2"], + "user-1": { + data: { + organizations: { + "organization-id-1": exampleOrganization1(), + "organization-id-2": { + // ... + }, + }, + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + data: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +function rollbackJSON() { + return { + "user_user-1_organizations_organizations": { + "organization-id-1": exampleOrganization1(), + "organization-id-2": { + // ... + }, + }, + "user_user-2_organizations_organizations": null as any, + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2"], + "user-1": { + data: { + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + data: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +describe("OrganizationMigrator", () => { + let helper: MockProxy; + let sut: OrganizationMigrator; + const keyDefinitionLike = { + key: "organizations", + stateDefinition: { + name: "organizations", + }, + }; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 27); + sut = new OrganizationMigrator(26, 27); + }); + + it("should remove organizations from all accounts", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledWith("user-1", { + data: { + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should set organizations value for each account", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("user-1", keyDefinitionLike, { + "organization-id-1": exampleOrganization1(), + "organization-id-2": { + // ... + }, + }); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 27); + sut = new OrganizationMigrator(26, 27); + }); + + 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 explicit value back to accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledWith("user-1", { + data: { + organizations: { + "organization-id-1": exampleOrganization1(), + "organization-id-2": { + // ... + }, + }, + otherStuff: "overStuff2", + }, + 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/27-move-organization-state-to-state-provider.ts b/libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.ts new file mode 100644 index 000000000000..0e7b811da0bf --- /dev/null +++ b/libs/common/src/state-migrations/migrations/27-move-organization-state-to-state-provider.ts @@ -0,0 +1,148 @@ +import { Jsonify } from "type-fest"; + +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +// Local declarations of `OrganizationData` and the types of it's properties. +// Duplicated to remain frozen in time when migration occurs. +enum OrganizationUserStatusType { + Invited = 0, + Accepted = 1, + Confirmed = 2, + Revoked = -1, +} + +enum OrganizationUserType { + Owner = 0, + Admin = 1, + User = 2, + Manager = 3, + Custom = 4, +} + +type PermissionsApi = { + accessEventLogs: boolean; + accessImportExport: boolean; + accessReports: boolean; + createNewCollections: boolean; + editAnyCollection: boolean; + deleteAnyCollection: boolean; + editAssignedCollections: boolean; + deleteAssignedCollections: boolean; + manageCiphers: boolean; + manageGroups: boolean; + manageSso: boolean; + managePolicies: boolean; + manageUsers: boolean; + manageResetPassword: boolean; + manageScim: boolean; +}; + +enum ProviderType { + Msp = 0, + Reseller = 1, +} + +enum ProductType { + Free = 0, + Families = 1, + Teams = 2, + Enterprise = 3, + TeamsStarter = 4, +} + +type OrganizationData = { + id: string; + name: string; + status: OrganizationUserStatusType; + type: OrganizationUserType; + enabled: boolean; + usePolicies: boolean; + useGroups: boolean; + useDirectory: boolean; + useEvents: boolean; + useTotp: boolean; + use2fa: boolean; + useApi: boolean; + useSso: boolean; + useKeyConnector: boolean; + useScim: boolean; + useCustomPermissions: boolean; + useResetPassword: boolean; + useSecretsManager: boolean; + usePasswordManager: boolean; + useActivateAutofillPolicy: boolean; + selfHost: boolean; + usersGetPremium: boolean; + seats: number; + maxCollections: number; + maxStorageGb?: number; + ssoBound: boolean; + identifier: string; + permissions: PermissionsApi; + resetPasswordEnrolled: boolean; + userId: string; + hasPublicAndPrivateKeys: boolean; + providerId: string; + providerName: string; + providerType?: ProviderType; + isProviderUser: boolean; + isMember: boolean; + familySponsorshipFriendlyName: string; + familySponsorshipAvailable: boolean; + planProductType: ProductType; + keyConnectorEnabled: boolean; + keyConnectorUrl: string; + familySponsorshipLastSyncDate?: Date; + familySponsorshipValidUntil?: Date; + familySponsorshipToDelete?: boolean; + accessSecretsManager: boolean; + limitCollectionCreationDeletion: boolean; + allowAdminAccessToAllCollectionItems: boolean; + flexibleCollections: boolean; +}; + +type ExpectedAccountType = { + data?: { + organizations?: Record>; + }; +}; + +const USER_ORGANIZATIONS: KeyDefinitionLike = { + key: "organizations", + stateDefinition: { + name: "organizations", + }, +}; + +export class OrganizationMigrator extends Migrator<26, 27> { + async migrate(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + async function migrateAccount(userId: string, account: ExpectedAccountType): Promise { + const value = account?.data?.organizations; + if (value != null) { + await helper.setToUser(userId, USER_ORGANIZATIONS, value); + delete account.data.organizations; + 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, USER_ORGANIZATIONS); + if (account) { + account.data = Object.assign(account.data ?? {}, { + organizations: value, + }); + await helper.set(userId, account); + } + await helper.setToUser(userId, USER_ORGANIZATIONS, null); + } + + await Promise.all(accounts.map(({ userId, account }) => rollbackAccount(userId, account))); + } +} From cea6af0d1bb54d1ac941d60e4d4261db45c5de95 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Feb 2024 11:13:34 -0600 Subject: [PATCH 02/17] Update libs/common/src/admin-console/services/organization/organization.service.ts Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> --- .../services/organization/organization.service.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 ae5729698bd9..2de3889c963f 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -86,9 +86,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti } async replace(organizations: { [id: string]: OrganizationData }, userId?: UserId): Promise { - await this.stateFor(userId).update(() => { - return organizations; - }); + await this.stateFor(userId).update(() => organizations); } // Ideally this method would be renamed to organizations$() and the From 2fdd09a6d4aec7024287af584a72191e8f830a2e Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Feb 2024 11:29:07 -0600 Subject: [PATCH 03/17] Remove deprecation notices --- .../organization.service.abstraction.ts | 38 ++----------------- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 1e8540d2f42a..39545dbaf498 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -48,9 +48,6 @@ export function canAccessOrgAdmin(org: Organization): boolean { ); } -/** - * @deprecated Use mapToSingleOrganization() - */ export function getOrganizationById(id: string) { return map((orgs) => orgs.find((o) => o.id === id)); } @@ -86,8 +83,7 @@ export function canAccessImport(i18nService: I18nService) { /** * Returns `true` if a user is a member of an organization (rather than only being a ProviderUser) - * @deprecated Use organizationService.organizations$() with a - * mapToExcludeSpecialOrganizations() pipe instead + * @deprecated Use organizationService.organizations$ with a filter instead */ export function isMember(org: Organization): boolean { return org.isMember; @@ -153,42 +149,16 @@ export abstract class OrganizationService { */ organizations$: Observable; - /** - * @deprecated Use organizations$() with `mapToExcludeSpecialOrganizations` instead. - * Organizations that the user is a member of (excludes organizations that they only have access to via a provider) - */ + // @todo Clean these up. Continuing to expand them is not recommended. + // @see https://bitwarden.atlassian.net/browse/AC-2252 memberOrganizations$: Observable; - - /** - * @deprecated Use organizations$ with a pipe to `mapToSingleOrganizaiton()` instead. - * @param id id of the organization - */ getFromState: (id: string) => Promise; - - /** - * @deprecated Use organizations$ with a pipe to `mapToExcludeOrganizationsWithoutFamilySponsorshipSupport()` instead. - */ canManageSponsorships: () => Promise; - - /** - * @deprecated Use organizations$ with a pipe to `mapToBooleanHasAnyOrganizations()` instead. - */ hasOrganizations: () => boolean; - - /** - * @deprecated Use organizations$ with a pipe to `mapToSingleOrganization()` instead. - */ get$: (id: string) => Observable; - - /** - * @deprecated Use organizations$ with a pipe to `mapToSingleOrganization()` instead. - */ get: (id: string) => Organization; - - /** - * @deprecated Use organizations$ instead. - */ getAll: (userId?: string) => Promise; + // } /** From b9a05b430638f52b639b18d212ab1ffc7a4d84c6 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Feb 2024 11:31:29 -0600 Subject: [PATCH 04/17] Move functions inline with their one calling class --- .../organization.service.abstraction.ts | 41 ---------------- .../organization/organization.service.ts | 49 ++++++++++++++++--- 2 files changed, 42 insertions(+), 48 deletions(-) diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 39545dbaf498..6d5ed34a08fc 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -89,47 +89,6 @@ export function isMember(org: Organization): boolean { return org.isMember; } -/** - * Filter out organizations from an observable that __do not__ offer a - * families-for-enterprise sponsorship to members. - * @returns a function that can be used in `Observable` pipes, - * like `organizationService.organizations$` - */ -export function mapToExcludeOrganizationsWithoutFamilySponsorshipSupport() { - return map((orgs) => orgs.filter((o) => o.canManageSponsorships)); -} - -/** - * Filter out organizations from an observable that the organization user - * __is not__ a direct member of. This will exclude organizations only - * accessible as a provider, for example. - * @returns a function that can be used in `Observable` pipes, - * like `organizationService.organizations$` - */ -export function mapToExcludeSpecialOrganizations() { - return map((orgs) => orgs.filter((o) => o.isMember)); -} - -/** - * Map an observable stream of organizations down to a boolean indicating - * if any organizations exist (`orgs.length > 0`). - * @returns a function that can be used in `Observable` pipes, - * like `organizationService.organizations$` - */ -export function mapToBooleanHasAnyOrganizations() { - return map((orgs) => orgs.length > 0); -} - -/** - * Map an observable stream of organizations down to a single organization. - * @param `organizationId` The ID of the organization you'd like to subscribe to - * @returns a function that can be used in `Observable` pipes, - * like `organizationService.organizations$` - */ -export function mapToSingleOrganization(organizationId: string) { - return map((orgs) => orgs?.find((o) => o.id === organizationId)); -} - /** * Publishes an observable stream of organizations. This service is meant to * be used widely across Bitwarden as the primary way of fetching organizations. 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 2de3889c963f..5e207eb7dc24 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -3,13 +3,7 @@ import { Jsonify } from "type-fest"; import { KeyDefinition, ORGANIZATIONS_DISK, StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; -import { - InternalOrganizationServiceAbstraction, - mapToBooleanHasAnyOrganizations, - mapToExcludeOrganizationsWithoutFamilySponsorshipSupport, - mapToExcludeSpecialOrganizations, - mapToSingleOrganization, -} from "../../abstractions/organization/organization.service.abstraction"; +import { InternalOrganizationServiceAbstraction } from "../../abstractions/organization/organization.service.abstraction"; import { OrganizationData } from "../../models/data/organization.data"; import { Organization } from "../../models/domain/organization"; @@ -27,6 +21,47 @@ export const ORGANIZATIONS = KeyDefinition.record( }, ); +/** + * Filter out organizations from an observable that __do not__ offer a + * families-for-enterprise sponsorship to members. + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +function mapToExcludeOrganizationsWithoutFamilySponsorshipSupport() { + return map((orgs) => orgs.filter((o) => o.canManageSponsorships)); +} + +/** + * Filter out organizations from an observable that the organization user + * __is not__ a direct member of. This will exclude organizations only + * accessible as a provider, for example. + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +function mapToExcludeSpecialOrganizations() { + return map((orgs) => orgs.filter((o) => o.isMember)); +} + +/** + * Map an observable stream of organizations down to a boolean indicating + * if any organizations exist (`orgs.length > 0`). + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +function mapToBooleanHasAnyOrganizations() { + return map((orgs) => orgs.length > 0); +} + +/** + * Map an observable stream of organizations down to a single organization. + * @param `organizationId` The ID of the organization you'd like to subscribe to + * @returns a function that can be used in `Observable` pipes, + * like `organizationService.organizations$` + */ +function mapToSingleOrganization(organizationId: string) { + return map((orgs) => orgs?.find((o) => o.id === organizationId)); +} + export class OrganizationService implements InternalOrganizationServiceAbstraction { organizations$ = this.getOrganizationsFromState$(); memberOrganizations$ = this.organizations$.pipe(mapToExcludeSpecialOrganizations()); From 9fd1b6d2306f84bf07b749eee1696edde6318e13 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Feb 2024 11:36:22 -0600 Subject: [PATCH 05/17] Rename `mapToExcludeSpecialOrganizations` --- .../services/organization/organization.service.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 5e207eb7dc24..0bbc29ca84d7 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -34,11 +34,11 @@ function mapToExcludeOrganizationsWithoutFamilySponsorshipSupport() { /** * Filter out organizations from an observable that the organization user * __is not__ a direct member of. This will exclude organizations only - * accessible as a provider, for example. + * accessible as a provider. * @returns a function that can be used in `Observable` pipes, * like `organizationService.organizations$` */ -function mapToExcludeSpecialOrganizations() { +function mapToExcludeProviderOrganizations() { return map((orgs) => orgs.filter((o) => o.isMember)); } @@ -64,7 +64,7 @@ function mapToSingleOrganization(organizationId: string) { export class OrganizationService implements InternalOrganizationServiceAbstraction { organizations$ = this.getOrganizationsFromState$(); - memberOrganizations$ = this.organizations$.pipe(mapToExcludeSpecialOrganizations()); + memberOrganizations$ = this.organizations$.pipe(mapToExcludeProviderOrganizations()); constructor(private stateProvider: StateProvider) {} From a5d1e28f2c725d048899da31bfac5c56f682a1dd Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Feb 2024 11:38:53 -0600 Subject: [PATCH 06/17] Remove unecassary object assignment in `getAll()` --- .../services/organization/organization.service.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 0bbc29ca84d7..69ed0468dcb3 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -73,10 +73,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti } async getAll(userId?: string): Promise { - const organizationsMap = await firstValueFrom( - this.getOrganizationsFromState$(userId as UserId), - ); - return Object.values(organizationsMap || {}).map((o) => new Organization(o)); + return await firstValueFrom(this.getOrganizationsFromState$(userId as UserId)); } async canManageSponsorships(): Promise { From 5a114f6aa659b4ece253295ffa024ae205b1856f Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Feb 2024 17:12:51 -0600 Subject: [PATCH 07/17] Rework existing interface methods to be promises --- .../components/vault/current-tab.component.ts | 2 +- .../components/vault/vault-items.component.ts | 2 +- .../organizations/guards/is-paid-org.guard.ts | 2 +- .../guards/org-permissions.guard.spec.ts | 12 ++++++------ .../guards/org-permissions.guard.ts | 2 +- .../guards/org-redirect.guard.ts | 2 +- .../member-dialog/member-dialog.component.ts | 2 +- .../organizations/members/people.component.ts | 2 +- .../reporting/reports-home.component.ts | 4 ++-- .../settings/two-factor-setup.component.ts | 13 +++++++++---- .../organization-plans.component.ts | 2 +- ...ganization-subscription-cloud.component.ts | 2 +- ...ization-subscription-selfhost.component.ts | 2 +- .../collection-dialog.component.ts | 6 +++--- .../vault/individual-vault/vault.component.ts | 4 ++-- .../guards/sm-org-enabled.guard.ts | 2 +- .../layout/navigation.component.ts | 2 +- .../overview/overview.component.ts | 3 ++- .../project/project-secrets.component.ts | 4 +++- .../projects/project/project.component.ts | 19 ++++++++++++++----- .../projects/projects/projects.component.ts | 4 +++- .../secrets/dialog/secret-dialog.component.ts | 2 +- .../secrets/secrets.component.ts | 4 +++- .../service-accounts.component.ts | 4 +++- .../access-policy-selector.service.ts | 2 +- .../shared/new-menu.component.ts | 19 ++++++++++++++----- .../shared/org-suspended.component.ts | 5 +++-- .../export-scope-callout.component.ts | 4 ++-- .../organization.service.abstraction.ts | 4 ++-- .../organization/organization.service.spec.ts | 4 ++-- .../organization/organization.service.ts | 18 +++++------------- .../src/components/import.component.ts | 8 ++++---- 32 files changed, 96 insertions(+), 71 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts index 5bf770a2187c..6a7e56a556bd 100644 --- a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts +++ b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts @@ -256,7 +256,7 @@ export class CurrentTabComponent implements OnInit, OnDestroy { const otherTypes: CipherType[] = []; const dontShowCards = await this.stateService.getDontShowCardsCurrentTab(); const dontShowIdentities = await this.stateService.getDontShowIdentitiesCurrentTab(); - this.showOrganizations = this.organizationService.hasOrganizations(); + this.showOrganizations = await this.organizationService.hasOrganizations(); if (!dontShowCards) { otherTypes.push(CipherType.Card); } diff --git a/apps/browser/src/vault/popup/components/vault/vault-items.component.ts b/apps/browser/src/vault/popup/components/vault/vault-items.component.ts index 60a8f4b78e7b..96d5fe170b02 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-items.component.ts +++ b/apps/browser/src/vault/popup/components/vault/vault-items.component.ts @@ -74,7 +74,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnIn async ngOnInit() { this.searchTypeSearch = !this.platformUtilsService.isSafari(); - this.showOrganizations = this.organizationService.hasOrganizations(); + this.showOrganizations = await this.organizationService.hasOrganizations(); this.vaultFilter = this.vaultFilterService.getVaultFilter(); // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe this.route.queryParams.pipe(first()).subscribe(async (params) => { diff --git a/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts b/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts index 16050a1555f4..f6968daca9f2 100644 --- a/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts +++ b/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts @@ -17,7 +17,7 @@ export class IsPaidOrgGuard implements CanActivate { ) {} async canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { - const org = this.organizationService.get(route.params.organizationId); + const org = await this.organizationService.get(route.params.organizationId); if (org == null) { return this.router.createUrlTree(["/"]); diff --git a/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.spec.ts b/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.spec.ts index 4a03f6a8c1e3..570c35d5f8f7 100644 --- a/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.spec.ts +++ b/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.spec.ts @@ -66,7 +66,7 @@ describe("Organization Permissions Guard", () => { it("permits navigation if no permissions are specified", async () => { const org = orgFactory(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const actual = await organizationPermissionsGuard.canActivate(route, state); @@ -81,7 +81,7 @@ describe("Organization Permissions Guard", () => { }; const org = orgFactory(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const actual = await organizationPermissionsGuard.canActivate(route, state); @@ -104,7 +104,7 @@ describe("Organization Permissions Guard", () => { }); const org = orgFactory(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const actual = await organizationPermissionsGuard.canActivate(route, state); @@ -124,7 +124,7 @@ describe("Organization Permissions Guard", () => { }), }); const org = orgFactory(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const actual = await organizationPermissionsGuard.canActivate(route, state); @@ -141,7 +141,7 @@ describe("Organization Permissions Guard", () => { type: OrganizationUserType.Admin, enabled: false, }); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const actual = await organizationPermissionsGuard.canActivate(route, state); @@ -153,7 +153,7 @@ describe("Organization Permissions Guard", () => { type: OrganizationUserType.Owner, enabled: false, }); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const actual = await organizationPermissionsGuard.canActivate(route, state); diff --git a/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.ts b/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.ts index 76df518cf7ad..63b89284ec60 100644 --- a/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.ts +++ b/apps/web/src/app/admin-console/organizations/guards/org-permissions.guard.ts @@ -28,7 +28,7 @@ export class OrganizationPermissionsGuard implements CanActivate { await this.syncService.fullSync(false); } - const org = this.organizationService.get(route.params.organizationId); + const org = await this.organizationService.get(route.params.organizationId); if (org == null) { return this.router.createUrlTree(["/"]); } diff --git a/apps/web/src/app/admin-console/organizations/guards/org-redirect.guard.ts b/apps/web/src/app/admin-console/organizations/guards/org-redirect.guard.ts index 61d79f8bd5d0..bbfb51ed9492 100644 --- a/apps/web/src/app/admin-console/organizations/guards/org-redirect.guard.ts +++ b/apps/web/src/app/admin-console/organizations/guards/org-redirect.guard.ts @@ -16,7 +16,7 @@ export class OrganizationRedirectGuard implements CanActivate { ) {} async canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { - const org = this.organizationService.get(route.params.organizationId); + const org = await this.organizationService.get(route.params.organizationId); const customRedirect = route.data?.autoRedirectCallback; if (customRedirect) { diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts index 5262b3f64abe..84a5318c0ede 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts @@ -146,7 +146,7 @@ export class MemberDialogComponent implements OnInit, OnDestroy { this.tabIndex = this.params.initialTab ?? MemberDialogTab.Role; this.title = this.i18nService.t(this.editMode ? "editMember" : "inviteMember"); - const organization$ = of(this.organizationService.get(this.params.organizationId)).pipe( + const organization$ = of(await this.organizationService.get(this.params.organizationId)).pipe( shareReplay({ refCount: true, bufferSize: 1 }), ); const groups$ = organization$.pipe( diff --git a/apps/web/src/app/admin-console/organizations/members/people.component.ts b/apps/web/src/app/admin-console/organizations/members/people.component.ts index afe07aaad1b0..4e45f70b6ddc 100644 --- a/apps/web/src/app/admin-console/organizations/members/people.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/people.component.ts @@ -143,7 +143,7 @@ export class PeopleComponent async ngOnInit() { const organization$ = this.route.params.pipe( - map((params) => this.organizationService.get(params.organizationId)), + concatMap((params) => this.organizationService.get$(params.organizationId)), shareReplay({ refCount: true, bufferSize: 1 }), ); diff --git a/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts b/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts index 51f34481d36c..c66cd9f494d4 100644 --- a/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts +++ b/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts @@ -1,6 +1,6 @@ import { Component, OnInit } from "@angular/core"; import { ActivatedRoute, NavigationEnd, Router } from "@angular/router"; -import { filter, map, Observable, startWith } from "rxjs"; +import { filter, map, Observable, startWith, concatMap } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -30,7 +30,7 @@ export class ReportsHomeComponent implements OnInit { ); this.reports$ = this.route.params.pipe( - map((params) => this.organizationService.get(params.organizationId)), + concatMap((params) => this.organizationService.get$(params.organizationId)), map((org) => this.buildReports(org.isFreeOrg)), ); } diff --git a/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts b/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts index 58953cf0a21f..5677b1f5b132 100644 --- a/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts +++ b/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts @@ -1,6 +1,6 @@ import { Component } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { concatMap, takeUntil } from "rxjs"; +import { concatMap, takeUntil, map } from "rxjs"; import { tap } from "rxjs/operators"; import { ModalService } from "@bitwarden/angular/services/modal.service"; @@ -36,9 +36,14 @@ export class TwoFactorSetupComponent extends BaseTwoFactorSetupComponent { async ngOnInit() { this.route.params .pipe( - tap((params) => { - this.organizationId = params.organizationId; - this.organization = this.organizationService.get(this.organizationId); + concatMap((params) => + this.organizationService + .get$(params.organizationId) + .pipe(map((organization) => ({ params, organization }))), + ), + tap(async (mapResponse) => { + this.organizationId = mapResponse.params.organizationId; + this.organization = mapResponse.organization; }), concatMap(async () => await super.ngOnInit()), takeUntil(this.destroy$), diff --git a/apps/web/src/app/billing/organizations/organization-plans.component.ts b/apps/web/src/app/billing/organizations/organization-plans.component.ts index b7070be1cfb2..1242018673f7 100644 --- a/apps/web/src/app/billing/organizations/organization-plans.component.ts +++ b/apps/web/src/app/billing/organizations/organization-plans.component.ts @@ -150,7 +150,7 @@ export class OrganizationPlansComponent implements OnInit, OnDestroy { async ngOnInit() { if (this.organizationId) { - this.organization = this.organizationService.get(this.organizationId); + this.organization = await this.organizationService.get(this.organizationId); this.billing = await this.organizationApiService.getBilling(this.organizationId); this.sub = await this.organizationApiService.getSubscription(this.organizationId); } diff --git a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts index 45c7ea1a2108..24374ee8969b 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts +++ b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts @@ -94,7 +94,7 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy return; } this.loading = true; - this.userOrg = this.organizationService.get(this.organizationId); + this.userOrg = await this.organizationService.get(this.organizationId); if (this.userOrg.canViewSubscription) { this.sub = await this.organizationApiService.getSubscription(this.organizationId); this.lineItems = this.sub?.subscription?.items; diff --git a/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.ts b/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.ts index 5a9a8eb08125..1b66f5400afe 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.ts +++ b/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.ts @@ -110,7 +110,7 @@ export class OrganizationSubscriptionSelfhostComponent implements OnInit, OnDest return; } this.loading = true; - this.userOrg = this.organizationService.get(this.organizationId); + this.userOrg = await this.organizationService.get(this.organizationId); if (this.userOrg.canViewSubscription) { const subscriptionResponse = await this.organizationApiService.getSubscription( this.organizationId, diff --git a/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts index b10d6b1e28eb..357d2217e472 100644 --- a/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts @@ -139,9 +139,9 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { } async loadOrg(orgId: string, collectionIds: string[]) { - const organization$ = of(this.organizationService.get(orgId)).pipe( - shareReplay({ refCount: true, bufferSize: 1 }), - ); + const organization$ = this.organizationService + .get$(orgId) + .pipe(shareReplay({ refCount: true, bufferSize: 1 })); const groups$ = organization$.pipe( switchMap((organization) => { if (!organization.useGroups) { diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index 7894677b69c0..46921a6d7be1 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -520,7 +520,7 @@ export class VaultComponent implements OnInit, OnDestroy { this.messagingService.send("premiumRequired"); return; } else if (cipher.organizationId != null) { - const org = this.organizationService.get(cipher.organizationId); + const org = await this.organizationService.get(cipher.organizationId); if (org != null && (org.maxStorageGb == null || org.maxStorageGb === 0)) { this.messagingService.send("upgradeOrganization", { organizationId: cipher.organizationId, @@ -699,7 +699,7 @@ export class VaultComponent implements OnInit, OnDestroy { } async deleteCollection(collection: CollectionView): Promise { - const organization = this.organizationService.get(collection.organizationId); + const organization = await this.organizationService.get(collection.organizationId); if (!collection.canDelete(organization)) { this.platformUtilsService.showToast( "error", diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/guards/sm-org-enabled.guard.ts b/bitwarden_license/bit-web/src/app/secrets-manager/guards/sm-org-enabled.guard.ts index 3ff4d998a3de..a1f7564156c6 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/guards/sm-org-enabled.guard.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/guards/sm-org-enabled.guard.ts @@ -16,7 +16,7 @@ export const organizationEnabledGuard: CanActivateFn = async (route: ActivatedRo await syncService.fullSync(false); } - const org = orgService.get(route.params.organizationId); + const org = await orgService.get(route.params.organizationId); if (org == null || !org.canAccessSecretsManager) { return createUrlTreeFromSnapshot(route, ["/"]); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/layout/navigation.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/layout/navigation.component.ts index 90faf165381c..cd117819a9fe 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/layout/navigation.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/layout/navigation.component.ts @@ -14,7 +14,7 @@ export class NavigationComponent { protected readonly logo = SecretsManagerLogo; protected orgFilter = (org: Organization) => org.canAccessSecretsManager; protected isAdmin$ = this.route.params.pipe( - map((params) => this.organizationService.get(params.organizationId)?.isAdmin), + map(async (params) => (await this.organizationService.get(params.organizationId))?.isAdmin), ); constructor( diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts index 8dff4cf92f8d..95c176425382 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts @@ -12,6 +12,7 @@ import { take, share, firstValueFrom, + concatMap, } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -105,7 +106,7 @@ export class OverviewComponent implements OnInit, OnDestroy { orgId$ .pipe( - map((orgId) => this.organizationService.get(orgId)), + concatMap(async (orgId) => await this.organizationService.get(orgId)), takeUntil(this.destroy$), ) .subscribe((org) => { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts index ebfb006c0ad8..07d50b28ee18 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts @@ -63,7 +63,9 @@ export class ProjectSecretsComponent { switchMap(async ([_, params]) => { this.organizationId = params.organizationId; this.projectId = params.projectId; - this.organizationEnabled = this.organizationService.get(params.organizationId)?.enabled; + this.organizationEnabled = ( + await this.organizationService.get(params.organizationId) + )?.enabled; return await this.getSecretsByProject(); }), ); diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts index d24ff7a3b092..742c2bea1d8f 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts @@ -10,6 +10,8 @@ import { Subject, switchMap, takeUntil, + map, + concatMap, } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -70,11 +72,18 @@ export class ProjectComponent implements OnInit, OnDestroy { }), ); - this.route.params.pipe(takeUntil(this.destroy$)).subscribe((params) => { - this.organizationId = params.organizationId; - this.projectId = params.projectId; - this.organizationEnabled = this.organizationService.get(params.organizationId)?.enabled; - }); + const projectId$ = this.route.params.pipe(map((p) => p.projectId)); + const organization$ = this.route.params.pipe( + concatMap((params) => this.organizationService.get$(params.organizationId)), + ); + + combineLatest([projectId$, organization$]) + .pipe(takeUntil(this.destroy$)) + .subscribe(([projectId, organization]) => { + this.organizationId = organization.id; + this.projectId = projectId; + this.organizationEnabled = organization.enabled; + }); } ngOnDestroy(): void { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects/projects.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects/projects.component.ts index 83541a37697c..831ee4df9bc9 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects/projects.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects/projects.component.ts @@ -51,7 +51,9 @@ export class ProjectsComponent implements OnInit { ]).pipe( switchMap(async ([params]) => { this.organizationId = params.organizationId; - this.organizationEnabled = this.organizationService.get(params.organizationId)?.enabled; + this.organizationEnabled = ( + await this.organizationService.get(params.organizationId) + )?.enabled; return await this.getProjects(); }), diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts index e944fbce0c70..b1bd91a04fbe 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts @@ -87,7 +87,7 @@ export class SecretDialogComponent implements OnInit { this.formGroup.get("project").setValue(this.data.projectId); } - if (this.organizationService.get(this.data.organizationId)?.isAdmin) { + if ((await this.organizationService.get(this.data.organizationId))?.isAdmin) { this.formGroup.get("project").removeValidators(Validators.required); this.formGroup.get("project").updateValueAndValidity(); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts index 64bbf479d882..a7413c9b59fd 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts @@ -47,7 +47,9 @@ export class SecretsComponent implements OnInit { combineLatestWith(this.route.params), switchMap(async ([_, params]) => { this.organizationId = params.organizationId; - this.organizationEnabled = this.organizationService.get(params.organizationId)?.enabled; + this.organizationEnabled = ( + await this.organizationService.get(params.organizationId) + )?.enabled; return await this.getSecrets(); }), diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts.component.ts index a5e3cd29d247..d015cccd99d2 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts.component.ts @@ -46,7 +46,9 @@ export class ServiceAccountsComponent implements OnInit { ]).pipe( switchMap(async ([params]) => { this.organizationId = params.organizationId; - this.organizationEnabled = this.organizationService.get(params.organizationId)?.enabled; + this.organizationEnabled = ( + await this.organizationService.get(params.organizationId) + )?.enabled; return await this.getServiceAccounts(); }), diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts index 4a90172d45bf..b219bfd33d2e 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts @@ -17,7 +17,7 @@ export class AccessPolicySelectorService { organizationId: string, selectedPoliciesValues: ApItemValueType[], ): Promise { - const organization = this.organizationService.get(organizationId); + const organization = await this.organizationService.get(organizationId); if (organization.isOwner || organization.isAdmin) { return false; } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/new-menu.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/new-menu.component.ts index 6cb5722be048..d2533d8dbf74 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/new-menu.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/new-menu.component.ts @@ -1,6 +1,6 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { Subject, takeUntil } from "rxjs"; +import { Subject, takeUntil, concatMap, map } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { DialogService } from "@bitwarden/components"; @@ -34,10 +34,19 @@ export class NewMenuComponent implements OnInit, OnDestroy { ) {} ngOnInit() { - this.route.params.pipe(takeUntil(this.destroy$)).subscribe((params: any) => { - this.organizationId = params.organizationId; - this.organizationEnabled = this.organizationService.get(params.organizationId)?.enabled; - }); + this.route.params + .pipe( + concatMap((params) => + this.organizationService + .get$(params.organizationId) + .pipe(map((organization) => ({ params, organization }))), + ), + takeUntil(this.destroy$), + ) + .subscribe((mapResult) => { + this.organizationId = mapResult?.params?.organizationId; + this.organizationEnabled = mapResult?.organization?.enabled; + }); } ngOnDestroy(): void { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/org-suspended.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/org-suspended.component.ts index ee94a78bb83d..1683d4479061 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/org-suspended.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/org-suspended.component.ts @@ -1,6 +1,6 @@ import { Component } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { map } from "rxjs"; +import { map, concatMap } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Icon, Icons } from "@bitwarden/components"; @@ -16,6 +16,7 @@ export class OrgSuspendedComponent { protected NoAccess: Icon = Icons.NoAccess; protected organizationName$ = this.route.params.pipe( - map((params) => this.organizationService.get(params.organizationId)?.name), + concatMap((params) => this.organizationService.get$(params.organizationId)), + map((org) => org?.name), ); } diff --git a/libs/angular/src/tools/export/components/export-scope-callout.component.ts b/libs/angular/src/tools/export/components/export-scope-callout.component.ts index 48651d822abf..545dfe4560aa 100644 --- a/libs/angular/src/tools/export/components/export-scope-callout.component.ts +++ b/libs/angular/src/tools/export/components/export-scope-callout.component.ts @@ -34,7 +34,7 @@ export class ExportScopeCalloutComponent implements OnInit { ) {} async ngOnInit(): Promise { - if (!this.organizationService.hasOrganizations()) { + if (!(await this.organizationService.hasOrganizations())) { return; } @@ -48,7 +48,7 @@ export class ExportScopeCalloutComponent implements OnInit { ? { title: "exportingOrganizationVaultTitle", description: "exportingOrganizationVaultDesc", - scopeIdentifier: this.organizationService.get(organizationId).name, + scopeIdentifier: (await this.organizationService.get(organizationId)).name, } : { title: "exportingPersonalVaultTitle", diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 6d5ed34a08fc..7c93cb015f15 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -113,9 +113,9 @@ export abstract class OrganizationService { memberOrganizations$: Observable; getFromState: (id: string) => Promise; canManageSponsorships: () => Promise; - hasOrganizations: () => boolean; + hasOrganizations: () => Promise; get$: (id: string) => Observable; - get: (id: string) => Organization; + get: (id: string) => Promise; getAll: (userId?: string) => Promise; // } 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 a5f23f41f975..65d784a898d4 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 @@ -169,12 +169,12 @@ describe("OrganizationService", () => { it("exists", async () => { const mockData = buildMockOrganizations(1); fakeActiveUserState.nextState(arrayToRecord(mockData)); - const result = organizationService.get(mockData[0].id); + const result = await organizationService.get(mockData[0].id); expect(result).toEqual(new Organization(mockData[0])); }); it("does not exist", async () => { - const result = organizationService.get("this-org-does-not-exist"); + const result = await organizationService.get("this-org-does-not-exist"); expect(result).toBe(undefined); }); }); 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 69ed0468dcb3..351ce0881c5d 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -1,4 +1,4 @@ -import { map, Observable, firstValueFrom, first } from "rxjs"; +import { map, Observable, firstValueFrom } from "rxjs"; import { Jsonify } from "type-fest"; import { KeyDefinition, ORGANIZATIONS_DISK, StateProvider } from "../../../platform/state"; @@ -85,12 +85,8 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti ); } - hasOrganizations(): boolean { - let value = false; - this.organizations$.pipe(mapToBooleanHasAnyOrganizations(), first()).subscribe((x) => { - value = x; - }); - return value; + async hasOrganizations(): Promise { + return await firstValueFrom(this.organizations$.pipe(mapToBooleanHasAnyOrganizations())); } async upsert(organization: OrganizationData, userId?: UserId): Promise { @@ -101,12 +97,8 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti }); } - get(id: string): Organization { - let value: Organization = undefined; - this.organizations$.pipe(mapToSingleOrganization(id), first()).subscribe((x) => { - value = x; - }); - return value; + async get(id: string): Promise { + return await firstValueFrom(this.organizations$.pipe(mapToSingleOrganization(id))); } /** diff --git a/libs/importer/src/components/import.component.ts b/libs/importer/src/components/import.component.ts index 796f02c77614..dfbe8d8c5e98 100644 --- a/libs/importer/src/components/import.component.ts +++ b/libs/importer/src/components/import.component.ts @@ -207,7 +207,7 @@ export class ImportComponent implements OnInit, OnDestroy { this.setImportOptions(); await this.initializeOrganizations(); - if (this.organizationId && this.canAccessImportExport(this.organizationId)) { + if (this.organizationId && (await this.canAccessImportExport(this.organizationId))) { this.handleOrganizationImportInit(); } else { this.handleImportInit(); @@ -359,7 +359,7 @@ export class ImportComponent implements OnInit, OnDestroy { importContents, this.organizationId, this.formGroup.controls.targetSelector.value, - this.canAccessImportExport(this.organizationId) && this._isFromAC, + (await this.canAccessImportExport(this.organizationId)) && this._isFromAC, ); //No errors, display success message @@ -379,11 +379,11 @@ export class ImportComponent implements OnInit, OnDestroy { } } - private canAccessImportExport(organizationId?: string): boolean { + private async canAccessImportExport(organizationId?: string): Promise { if (!organizationId) { return false; } - return this.organizationService.get(this.organizationId)?.canAccessImportExport; + return (await this.organizationService.get(this.organizationId))?.canAccessImportExport; } getFormatInstructionTitle() { From b6a163262d9ee94c2df294b371d7050f2713c701 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Fri, 1 Mar 2024 15:50:58 -0600 Subject: [PATCH 08/17] Update a comment --- .../organization/organization.service.abstraction.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 7c93cb015f15..cd0bafe1eff9 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -98,13 +98,7 @@ export function isMember(org: Organization): boolean { export abstract class OrganizationService { /** * Publishes state for all organizations under the active user. - * - * There are helper functions available for use in pipes that will - * filter subscriptions for common tasks like subscribing to only one - * organization, but they must be imported directly. See - * `organization.service.abstraction` for details. - * @returns An observable list of organizations that meet the search criteria - * provided. + * @returns An observable list of organizations */ organizations$: Observable; From 0429340e4e42bb4156f578b43f292601a62e2618 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Tue, 5 Mar 2024 15:05:48 -0600 Subject: [PATCH 09/17] Remove complex and unecassary assertion --- .../services/organization/organization.service.spec.ts | 1 - 1 file changed, 1 deletion(-) 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 65d784a898d4..b60bb08aba26 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 @@ -236,7 +236,6 @@ describe("OrganizationService", () => { const mockData = buildMockOrganizations(); await organizationService.upsert(mockData[0]); const result = await firstValueFrom(organizationService.organizations$); - expect(result).not.toEqual(undefined || null || []); expect(result).toEqual(mockData.map((x) => new Organization(x))); }); From d91d84248683858ab61e2a1451ac0096d8a1b9ee Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Tue, 5 Mar 2024 16:44:41 -0600 Subject: [PATCH 10/17] Remove unecassary test logic --- .../organization/organization.service.spec.ts | 32 +++---------------- 1 file changed, 4 insertions(+), 28 deletions(-) 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 b60bb08aba26..3e4091e5985a 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 @@ -2,8 +2,6 @@ import { firstValueFrom } from "rxjs"; import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { FakeActiveUserState } from "../../../../spec/fake-state"; -import { AccountInfo } from "../../../auth/abstractions/account.service"; -import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { Utils } from "../../../platform/misc/utils"; import { OrganizationId, UserId } from "../../../types/guid"; import { OrganizationData } from "../../models/data/organization.data"; @@ -87,31 +85,11 @@ describe("OrganizationService", () => { * function can be used to add a new non-active account to the test data. * This function is **not** needed to handle creation of the first account, * as that is handled by the `FakeAccountService` in `mockAccountServiceWith()` - * @param opts.createWithTestOrgs Will add a couple of slim test organizations to - * the state of the user being created. Defaults to false. * @returns The `UserId` of the newly created state account and the mock data * created for them as an `Organization[]`. */ - async function addNonActiveAccountToStateProvider(opts?: { - createWithTestOrgs: boolean; - }): Promise<[UserId, OrganizationData[]]> { + async function addNonActiveAccountToStateProvider(): Promise<[UserId, OrganizationData[]]> { const nonActiveUserId = Utils.newGuid() as UserId; - // This is the same partial object setup that `mockAccountServiceWith()` uses. - // I'm assuming at the time of writing that name, email, and status are - // important to the internal functions of StateProvider and should - // always be mocked when testing. - const fullInfo: AccountInfo = { - name: "nonActiveUserName", - email: "nonActiveUserEmail", - status: AuthenticationStatus.Locked, - }; - // This does **not** change the active user, and instead adds this account - // in an inactive state. - await fakeAccountService.addAccount(nonActiveUserId, fullInfo); - - if (!opts?.createWithTestOrgs) { - return [nonActiveUserId, undefined]; - } const mockOrganizations = buildMockOrganizations(10); const fakeNonActiveUserState = fakeStateProvider.singleUser.getFake( @@ -216,7 +194,7 @@ describe("OrganizationService", () => { fakeActiveUserState.nextState(arrayToRecord(activeUserMockData)); const [nonActiveUserId, nonActiveUserMockOrganizations] = - await addNonActiveAccountToStateProvider({ createWithTestOrgs: true }); + await addNonActiveAccountToStateProvider(); // This can be updated to use // `firstValueFrom(organizations$(nonActiveUserId)` once all the // promise based methods are removed from `OrganizationService` and the @@ -265,7 +243,7 @@ describe("OrganizationService", () => { fakeActiveUserState.nextState(arrayToRecord(activeUserMockData)); const [nonActiveUserId, nonActiveUserMockOrganizations] = - await addNonActiveAccountToStateProvider({ createWithTestOrgs: true }); + await addNonActiveAccountToStateProvider(); const indexToUpdate = 5; const anUpdatedOrganization = { ...buildMockOrganizations(1, "UPDATED").pop(), @@ -329,9 +307,7 @@ describe("OrganizationService", () => { const activeUserMockData = buildMockOrganizations(10, "activeUserOrganizations"); fakeActiveUserState.nextState(arrayToRecord(activeUserMockData)); - const [nonActiveUserId, originalOrganizations] = await addNonActiveAccountToStateProvider({ - createWithTestOrgs: true, - }); + const [nonActiveUserId, originalOrganizations] = await addNonActiveAccountToStateProvider(); const newData = buildMockOrganizations(10, "newData"); await organizationService.replace(arrayToRecord(newData), nonActiveUserId); From 36434d17497b86d16cc9e48914d41da4b0d2ea65 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Tue, 5 Mar 2024 16:48:15 -0600 Subject: [PATCH 11/17] Add back a deprecation notice --- .../organization/organization.service.abstraction.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index cd0bafe1eff9..cb05c9519766 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -105,6 +105,12 @@ export abstract class OrganizationService { // @todo Clean these up. Continuing to expand them is not recommended. // @see https://bitwarden.atlassian.net/browse/AC-2252 memberOrganizations$: Observable; + /** + * @deprecated This is currently only used in the CLI, and should not be + * used in any new calls. Use get$ instead for the time being, and we'll be + * removing this method soon. See Jira for details: + * https://bitwarden.atlassian.net/browse/AC-2252. + */ getFromState: (id: string) => Promise; canManageSponsorships: () => Promise; hasOrganizations: () => Promise; From eda7f94a3f917b1db73bd0d1f239f215bfabc75b Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Mon, 11 Mar 2024 07:17:09 -0500 Subject: [PATCH 12/17] Make test assertion less code-y --- .../organization/organization.service.spec.ts | 42 ++++++------------- 1 file changed, 12 insertions(+), 30 deletions(-) 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 3e4091e5985a..661d0b6521ac 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 @@ -29,22 +29,6 @@ describe("OrganizationService", () => { return Object.fromEntries(input?.map((i) => [i.id, i])); } - /** - * There are a few assertions in this spec that check for array equality - * but want to ignore a specific index that _should_ be different. This - * function takes two arrays, and an index. It checks for equality of the - * arrays, but splices out the specified index from both arrays first. - */ - function isEqualExceptForIndex(x: any[], y: any[], indexToExclude: number) { - return ( - JSON.stringify([ - ...x.slice(0, indexToExclude - 1), - ...x.slice(indexToExclude + 1, x.length), - ]) === - JSON.stringify([...y.slice(0, indexToExclude - 1), ...y.slice(indexToExclude + 1, y.length)]) - ); - } - /** * Builds a simple mock `OrganizationData[]` array that can be used in tests * to populate state. @@ -229,13 +213,12 @@ describe("OrganizationService", () => { const result = await firstValueFrom(organizationService.organizations$); expect(result[indexToUpdate]).not.toEqual(new Organization(mockData[indexToUpdate])); expect(result[indexToUpdate].id).toEqual(new Organization(mockData[indexToUpdate]).id); - expect( - isEqualExceptForIndex( - result, - mockData.map((x) => new Organization(x)), - indexToUpdate, - ), - ).toBeTruthy(); + + // As an extra precaution lets ensure no other element of the array was + // modified. + delete result[indexToUpdate]; + delete mockData[indexToUpdate]; + expect(result).toEqual(mockData); }); it("can also update an organization in state for a non-active user, if requested", async () => { @@ -263,13 +246,12 @@ describe("OrganizationService", () => { expect(result[indexToUpdate].id).toEqual( new Organization(nonActiveUserMockOrganizations[indexToUpdate]).id, ); - expect( - isEqualExceptForIndex( - result, - nonActiveUserMockOrganizations.map((x) => new Organization(x)), - indexToUpdate, - ), - ).toBeTruthy(); + + // As an extra precaution lets ensure no other element of the array was + // modified. + delete result[indexToUpdate]; + delete nonActiveUserMockOrganizations[indexToUpdate]; + expect(result).toEqual(nonActiveUserMockOrganizations.map((x) => new Organization(x))); // Just to be safe, lets make sure the active user didn't get updated // at all From a354c7513af8bad286f23e119b897b1c3a53c97e Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Mon, 11 Mar 2024 16:25:50 -0500 Subject: [PATCH 13/17] Implement UserKeyDefinition for Organizations --- apps/browser/src/background/main.background.ts | 1 - apps/cli/src/bw.ts | 1 - apps/desktop/src/app/app.component.ts | 1 - apps/web/src/app/app.component.ts | 1 - .../services/organization/organization.service.ts | 5 +++-- 5 files changed, 3 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 7ccb0fb5a645..374215d77523 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1105,7 +1105,6 @@ export default class MainBackground { this.vaultFilterService.clear(), this.biometricStateService.logout(userId), this.providerService.save(null, userId), - this.organizationService.replace(null, userId), /* We intentionally do not clear: * - autofillSettingsService * - badgeSettingsService diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 07e929a04bc2..773a80508499 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -653,7 +653,6 @@ export class Main { this.policyService.clear(userId as UserId), this.passwordGenerationService.clear(), this.providerService.save(null, userId as UserId), - this.organizationService.replace(null, userId as UserId), ]); await this.stateEventRunnerService.handleEvent("logout", userId as UserId); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 5658b90965ac..77c29522bf25 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -589,7 +589,6 @@ export class AppComponent implements OnInit, OnDestroy { await this.keyConnectorService.clear(); await this.biometricStateService.logout(userBeingLoggedOut as UserId); await this.providerService.save(null, userBeingLoggedOut as UserId); - await this.organizationService.replace(null, userBeingLoggedOut as UserId); await this.stateEventRunnerService.handleEvent("logout", userBeingLoggedOut as UserId); diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 4b3ff573f214..eb36b58c9764 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -284,7 +284,6 @@ export class AppComponent implements OnDestroy, OnInit { this.keyConnectorService.clear(), this.biometricStateService.logout(userId as UserId), this.paymentMethodWarningService.clear(), - this.organizationService.replace(null, userId as UserId), ]); await this.stateEventRunnerService.handleEvent("logout", userId as UserId); 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 351ce0881c5d..3c651f4660e5 100644 --- a/libs/common/src/admin-console/services/organization/organization.service.ts +++ b/libs/common/src/admin-console/services/organization/organization.service.ts @@ -1,7 +1,7 @@ import { map, Observable, firstValueFrom } from "rxjs"; import { Jsonify } from "type-fest"; -import { KeyDefinition, ORGANIZATIONS_DISK, StateProvider } from "../../../platform/state"; +import { ORGANIZATIONS_DISK, StateProvider, UserKeyDefinition } from "../../../platform/state"; import { UserId } from "../../../types/guid"; import { InternalOrganizationServiceAbstraction } from "../../abstractions/organization/organization.service.abstraction"; import { OrganizationData } from "../../models/data/organization.data"; @@ -13,11 +13,12 @@ import { Organization } from "../../models/domain/organization"; * has some properties that contain functions. This should probably get * cleaned up. */ -export const ORGANIZATIONS = KeyDefinition.record( +export const ORGANIZATIONS = UserKeyDefinition.record( ORGANIZATIONS_DISK, "organizations", { deserializer: (obj: Jsonify) => OrganizationData.fromJSON(obj), + clearOn: ["logout"], }, ); From 5fec46550689c359026ac1fe5c50310be1361840 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Mon, 11 Mar 2024 18:33:44 -0500 Subject: [PATCH 14/17] Rework complex assertion one more time --- .../organization/organization.service.spec.ts | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) 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 661d0b6521ac..706f38a5a808 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 @@ -29,6 +29,21 @@ describe("OrganizationService", () => { return Object.fromEntries(input?.map((i) => [i.id, i])); } + /** + * There are a few assertions in this spec that check for array equality + * but want to ignore a specific index that _should_ be different. This + * function takes two arrays, and an index. It checks for equality of the + * arrays, but splices out the specified index from both arrays first. + */ + function isEqualExceptForIndex(x: any[], y: any[], indexToExclude: number) { + // Clone the arrays to avoid modifying the reference values + const a = [...x]; + const b = [...y]; + delete a[indexToExclude]; + delete b[indexToExclude]; + return a === b; + } + /** * Builds a simple mock `OrganizationData[]` array that can be used in tests * to populate state. @@ -213,12 +228,13 @@ describe("OrganizationService", () => { const result = await firstValueFrom(organizationService.organizations$); expect(result[indexToUpdate]).not.toEqual(new Organization(mockData[indexToUpdate])); expect(result[indexToUpdate].id).toEqual(new Organization(mockData[indexToUpdate]).id); - - // As an extra precaution lets ensure no other element of the array was - // modified. - delete result[indexToUpdate]; - delete mockData[indexToUpdate]; - expect(result).toEqual(mockData); + expect( + isEqualExceptForIndex( + result, + mockData.map((x) => new Organization(x)), + indexToUpdate, + ), + ).toBeTruthy(); }); it("can also update an organization in state for a non-active user, if requested", async () => { @@ -246,12 +262,13 @@ describe("OrganizationService", () => { expect(result[indexToUpdate].id).toEqual( new Organization(nonActiveUserMockOrganizations[indexToUpdate]).id, ); - - // As an extra precaution lets ensure no other element of the array was - // modified. - delete result[indexToUpdate]; - delete nonActiveUserMockOrganizations[indexToUpdate]; - expect(result).toEqual(nonActiveUserMockOrganizations.map((x) => new Organization(x))); + expect( + isEqualExceptForIndex( + result, + nonActiveUserMockOrganizations.map((x) => new Organization(x)), + indexToUpdate, + ), + ).toBeTruthy(); // Just to be safe, lets make sure the active user didn't get updated // at all From 43419f5d326b59ed213d8df358195b4b1a7fe6e2 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 13 Mar 2024 14:09:03 -0500 Subject: [PATCH 15/17] Rework complex assertion one-more-one-more-time --- .../organization/organization.service.spec.ts | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) 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 706f38a5a808..908f4b8e28bb 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 @@ -35,13 +35,13 @@ describe("OrganizationService", () => { * function takes two arrays, and an index. It checks for equality of the * arrays, but splices out the specified index from both arrays first. */ - function isEqualExceptForIndex(x: any[], y: any[], indexToExclude: number) { + function expectIsEqualExceptForIndex(x: any[], y: any[], indexToExclude: number) { // Clone the arrays to avoid modifying the reference values const a = [...x]; const b = [...y]; delete a[indexToExclude]; delete b[indexToExclude]; - return a === b; + expect(a).toEqual(b); } /** @@ -228,13 +228,11 @@ describe("OrganizationService", () => { const result = await firstValueFrom(organizationService.organizations$); expect(result[indexToUpdate]).not.toEqual(new Organization(mockData[indexToUpdate])); expect(result[indexToUpdate].id).toEqual(new Organization(mockData[indexToUpdate]).id); - expect( - isEqualExceptForIndex( - result, - mockData.map((x) => new Organization(x)), - indexToUpdate, - ), - ).toBeTruthy(); + expectIsEqualExceptForIndex( + result, + mockData.map((x) => new Organization(x)), + indexToUpdate, + ); }); it("can also update an organization in state for a non-active user, if requested", async () => { @@ -262,13 +260,11 @@ describe("OrganizationService", () => { expect(result[indexToUpdate].id).toEqual( new Organization(nonActiveUserMockOrganizations[indexToUpdate]).id, ); - expect( - isEqualExceptForIndex( - result, - nonActiveUserMockOrganizations.map((x) => new Organization(x)), - indexToUpdate, - ), - ).toBeTruthy(); + expectIsEqualExceptForIndex( + result, + nonActiveUserMockOrganizations.map((x) => new Organization(x)), + indexToUpdate, + ); // Just to be safe, lets make sure the active user didn't get updated // at all From 1e995cae88c8bc431d48ac201dd0de5ecb6228fa Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 13 Mar 2024 16:03:08 -0500 Subject: [PATCH 16/17] Remove stateService Organizations logic --- .../platform/abstractions/state.service.ts | 12 ------ .../src/platform/models/domain/account.ts | 2 - .../src/platform/services/state.service.ts | 41 ------------------- 3 files changed, 55 deletions(-) diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index d9b18e509b81..8a6eaa1621e8 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -1,6 +1,5 @@ import { Observable } from "rxjs"; -import { OrganizationData } from "../../admin-console/models/data/organization.data"; 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"; @@ -305,17 +304,6 @@ export abstract class StateService { setOpenAtLogin: (value: boolean, options?: StorageOptions) => Promise; getOrganizationInvitation: (options?: StorageOptions) => Promise; setOrganizationInvitation: (value: any, options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this directly, use OrganizationService - */ - getOrganizations: (options?: StorageOptions) => Promise<{ [id: string]: OrganizationData }>; - /** - * @deprecated Do not call this directly, use OrganizationService - */ - setOrganizations: ( - value: { [id: string]: OrganizationData }, - options?: StorageOptions, - ) => Promise; getPasswordGenerationOptions: (options?: StorageOptions) => Promise; setPasswordGenerationOptions: ( value: PasswordGeneratorOptions, diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 2c3c2eab672b..de074248187d 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -1,6 +1,5 @@ import { Jsonify } from "type-fest"; -import { OrganizationData } from "../../../admin-console/models/data/organization.data"; 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"; @@ -91,7 +90,6 @@ export class AccountData { > = new EncryptionPair(); addEditCipherInfo?: AddEditCipherInfo; eventCollection?: EventData[]; - organizations?: { [id: string]: OrganizationData }; static fromJSON(obj: DeepJsonify): AccountData { if (obj == null) { diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index d7d302db4c69..fde7bfd8d2bf 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,7 +1,6 @@ import { BehaviorSubject, Observable, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; -import { OrganizationData } from "../../admin-console/models/data/organization.data"; import { AccountService } from "../../auth/abstractions/account.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; @@ -411,20 +410,6 @@ export class StateService< return true; } - // TODO: older server versions won't send the hasPremiumFromOrganization flag, so we're keeping the old logic - // for backwards compatibility. It can be removed after everyone has upgraded. - const organizations = await this.getOrganizations(options); - if (organizations == null) { - return false; - } - - for (const id of Object.keys(organizations)) { - const o = organizations[id]; - if (o.enabled && o.usersGetPremium && !o.isProviderUser) { - return true; - } - } - return false; } @@ -1608,32 +1593,6 @@ export class StateService< ); } - /** - * @deprecated Do not call this directly, use OrganizationService - */ - async getOrganizations(options?: StorageOptions): Promise<{ [id: string]: OrganizationData }> { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.data?.organizations; - } - - /** - * @deprecated Do not call this directly, use OrganizationService - */ - async setOrganizations( - value: { [id: string]: OrganizationData }, - options?: StorageOptions, - ): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.data.organizations = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getPasswordGenerationOptions(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions())) From ce86d04043924b8bd7107e1395a14a6da441117b Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 13 Mar 2024 16:57:47 -0500 Subject: [PATCH 17/17] Update tests to reflect new async methods --- .../access-policy-selector.service.spec.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts index 482d2bb06b46..8c0f9c373114 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts @@ -26,7 +26,7 @@ describe("AccessPolicySelectorService", () => { describe("showAccessRemovalWarning", () => { it("returns false when current user is admin", async () => { const org = orgFactory(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = []; @@ -38,7 +38,7 @@ describe("AccessPolicySelectorService", () => { it("returns false when current user is owner", async () => { const org = orgFactory(); org.type = OrganizationUserType.Owner; - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = []; @@ -49,7 +49,7 @@ describe("AccessPolicySelectorService", () => { it("returns true when current user isn't owner/admin and all policies are removed", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = []; @@ -60,7 +60,7 @@ describe("AccessPolicySelectorService", () => { it("returns true when current user isn't owner/admin and user policy is set to canRead", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = []; selectedPolicyValues.push( @@ -77,7 +77,7 @@ describe("AccessPolicySelectorService", () => { it("returns false when current user isn't owner/admin and user policy is set to canReadWrite", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({ @@ -93,7 +93,7 @@ describe("AccessPolicySelectorService", () => { it("returns true when current user isn't owner/admin and a group Read policy is submitted that the user is a member of", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({ @@ -111,7 +111,7 @@ describe("AccessPolicySelectorService", () => { it("returns false when current user isn't owner/admin and a group ReadWrite policy is submitted that the user is a member of", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({ @@ -129,7 +129,7 @@ describe("AccessPolicySelectorService", () => { it("returns true when current user isn't owner/admin and a group ReadWrite policy is submitted that the user is not a member of", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({ @@ -147,7 +147,7 @@ describe("AccessPolicySelectorService", () => { it("returns false when current user isn't owner/admin, user policy is set to CanRead, and user is in read write group", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({ @@ -169,7 +169,7 @@ describe("AccessPolicySelectorService", () => { it("returns true when current user isn't owner/admin, user policy is set to CanRead, and user is not in ReadWrite group", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({ @@ -191,7 +191,7 @@ describe("AccessPolicySelectorService", () => { it("returns true when current user isn't owner/admin, user policy is set to CanRead, and user is in Read group", async () => { const org = setupUserOrg(); - organizationService.get.calledWith(org.id).mockReturnValue(org); + organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ createApItemValueType({