From b455cb5986fd6bb7cc28ddf4203b537d718499a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Mon, 22 Sep 2025 16:06:28 +0100 Subject: [PATCH] [PM-24146] Remove stateProvider.activeUserId from ProviderService (#16258) * Refactor provider service calls to include userId parameter - Updated multiple components and services to pass userId when fetching provider data. - Adjusted the ProviderService interface to require userId for get, get$, and getAll methods. - Ensured consistent handling of userId across various components, enhancing data retrieval based on active user context. * Remove deprecated type safety comments and use the getById utility for fetching providers. * Update ProviderService methods to return undefined for non-existent providers - Modified the return types of get$ and get methods in ProviderService to allow for undefined values, enhancing type safety. - Adjusted the providers$ method to return only defined Provider arrays, ensuring consistent handling of provider data. * Enhance provider permissions guard tests to include userId parameter - Updated test cases in provider-permissions.guard.spec.ts to pass userId when calling ProviderService methods. - Mocked AccountService to provide active account details for improved test coverage. - Ensured consistent handling of userId across all relevant test scenarios. * remove promise based api's from provider service, continue refactor * cleanup observable logic * cleanup --------- Co-authored-by: Brandon --- .../layouts/organization-layout.component.ts | 9 +- .../organizations/manage/events.component.ts | 36 +-- .../guards/organization-is-unmanaged.guard.ts | 2 +- .../navigation-switcher.stories.ts | 6 +- .../product-switcher.stories.ts | 6 +- .../shared/product-switcher.service.spec.ts | 6 +- .../shared/product-switcher.service.ts | 288 +++++++++--------- .../clients/add-organization.component.html | 9 +- .../clients/add-organization.component.ts | 15 +- .../providers/clients/clients.component.ts | 9 +- .../guards/provider-permissions.guard.spec.ts | 31 +- .../guards/provider-permissions.guard.ts | 11 +- .../providers/manage/events.component.ts | 10 +- .../providers/manage/members.component.ts | 13 +- .../providers/providers-layout.component.ts | 10 +- .../providers/providers.component.html | 1 + .../providers/providers.component.ts | 22 +- .../clients/manage-clients.component.ts | 15 +- .../guards/has-consolidated-billing.guard.ts | 6 +- .../provider-payment-details.component.ts | 8 +- .../add-account-credit-dialog.component.ts | 4 +- .../abstractions/provider.service.ts | 7 +- .../services/provider.service.spec.ts | 56 ++-- .../services/provider.service.ts | 43 +-- 24 files changed, 342 insertions(+), 281 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts index 6b2f6261454..a9b61debf89 100644 --- a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts +++ b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts @@ -121,8 +121,13 @@ export class OrganizationLayoutComponent implements OnInit { switchMap((userId) => this.policyService.policyAppliesToUser$(PolicyType.SingleOrg, userId)), ); - const provider$ = this.organization$.pipe( - switchMap((organization) => this.providerService.get$(organization.providerId)), + const provider$ = combineLatest([ + this.organization$, + this.accountService.activeAccount$.pipe(getUserId), + ]).pipe( + switchMap(([organization, userId]) => + this.providerService.get$(organization.providerId, userId), + ), ); this.organizationIsUnmanaged$ = combineLatest([this.organization$, provider$]).pipe( diff --git a/apps/web/src/app/admin-console/organizations/manage/events.component.ts b/apps/web/src/app/admin-console/organizations/manage/events.component.ts index 58f7fc2563d..966499c0bee 100644 --- a/apps/web/src/app/admin-console/organizations/manage/events.component.ts +++ b/apps/web/src/app/admin-console/organizations/manage/events.component.ts @@ -2,7 +2,7 @@ // @ts-strict-ignore import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { concatMap, firstValueFrom, lastValueFrom, takeUntil } from "rxjs"; +import { concatMap, filter, firstValueFrom, lastValueFrom, map, switchMap, takeUntil } from "rxjs"; import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; @@ -136,22 +136,24 @@ export class EventsComponent extends BaseEventsComponent implements OnInit, OnDe if (this.organization.providerId != null) { try { - const provider = await this.providerService.get(this.organization.providerId); - if ( - provider != null && - (await this.providerService.get(this.organization.providerId)).canManageUsers - ) { - const providerUsersResponse = await this.apiService.getProviderUsers( - this.organization.providerId, - ); - providerUsersResponse.data.forEach((u) => { - const name = this.userNamePipe.transform(u); - this.orgUsersUserIdMap.set(u.userId, { - name: `${name} (${this.organization.providerName})`, - email: u.email, - }); - }); - } + await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.providerService.get$(this.organization.providerId, userId)), + map((provider) => provider != null && provider.canManageUsers), + filter((result) => result), + switchMap(() => this.apiService.getProviderUsers(this.organization.id)), + map((providerUsersResponse) => + providerUsersResponse.data.forEach((u) => { + const name = this.userNamePipe.transform(u); + this.orgUsersUserIdMap.set(u.userId, { + name: `${name} (${this.organization.providerName})`, + email: u.email, + }); + }), + ), + ), + ); } catch (e) { this.logService.warning(e); } diff --git a/apps/web/src/app/billing/guards/organization-is-unmanaged.guard.ts b/apps/web/src/app/billing/guards/organization-is-unmanaged.guard.ts index 75054d78db1..8f7eeaafb70 100644 --- a/apps/web/src/app/billing/guards/organization-is-unmanaged.guard.ts +++ b/apps/web/src/app/billing/guards/organization-is-unmanaged.guard.ts @@ -35,7 +35,7 @@ export const organizationIsUnmanaged: CanActivateFn = async (route: ActivatedRou return true; } - const provider = await providerService.get(organization.providerId); + const provider = await firstValueFrom(providerService.get$(organization.providerId, userId)); if (!provider) { return true; diff --git a/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts b/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts index f0660f7d655..b5f10f9158e 100644 --- a/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts +++ b/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts @@ -1,7 +1,7 @@ import { Component, Directive, importProvidersFrom, Input } from "@angular/core"; import { RouterModule } from "@angular/router"; import { applicationConfig, Meta, moduleMetadata, StoryObj } from "@storybook/angular"; -import { BehaviorSubject, firstValueFrom, Observable, of } from "rxjs"; +import { BehaviorSubject, Observable, of } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -47,8 +47,8 @@ class MockOrganizationService implements Partial { class MockProviderService implements Partial { private static _providers = new BehaviorSubject([]); - async getAll() { - return await firstValueFrom(MockProviderService._providers); + providers$() { + return MockProviderService._providers.asObservable(); } @Input() diff --git a/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts b/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts index cc919a929a9..1a44df4dd00 100644 --- a/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts +++ b/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts @@ -1,7 +1,7 @@ import { Component, Directive, importProvidersFrom, Input } from "@angular/core"; import { RouterModule } from "@angular/router"; import { applicationConfig, Meta, moduleMetadata, StoryObj } from "@storybook/angular"; -import { BehaviorSubject, firstValueFrom, Observable, of } from "rxjs"; +import { BehaviorSubject, Observable, of } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -47,8 +47,8 @@ class MockOrganizationService implements Partial { class MockProviderService implements Partial { private static _providers = new BehaviorSubject([]); - async getAll() { - return await firstValueFrom(MockProviderService._providers); + providers$() { + return MockProviderService._providers.asObservable(); } @Input() diff --git a/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts b/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts index f72557ac57f..efbeb786f77 100644 --- a/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts +++ b/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts @@ -52,7 +52,7 @@ describe("ProductSwitcherService", () => { router.url = "/"; router.events = of({}); organizationService.organizations$.mockReturnValue(of([{}] as Organization[])); - providerService.getAll.mockResolvedValue([] as Provider[]); + providerService.providers$.mockReturnValue(of([]) as Observable); platformUtilsService.isSelfHost.mockReturnValue(false); TestBed.configureTestingModule({ @@ -212,7 +212,7 @@ describe("ProductSwitcherService", () => { }); it("is included when there are providers", async () => { - providerService.getAll.mockResolvedValue([{ id: "67899" }] as Provider[]); + providerService.providers$.mockReturnValue(of([{ id: "67899" }]) as Observable); initiateService(); @@ -263,7 +263,7 @@ describe("ProductSwitcherService", () => { }); it("marks Provider Portal as active", async () => { - providerService.getAll.mockResolvedValue([{ id: "67899" }] as Provider[]); + providerService.providers$.mockReturnValue(of([{ id: "67899" }]) as Observable); router.url = "/providers/"; initiateService(); diff --git a/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.ts b/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.ts index 2d296ac7d62..ba86e9fed28 100644 --- a/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.ts +++ b/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.ts @@ -2,17 +2,7 @@ // @ts-strict-ignore import { Injectable } from "@angular/core"; import { ActivatedRoute, NavigationEnd, NavigationStart, ParamMap, Router } from "@angular/router"; -import { - combineLatest, - concatMap, - filter, - firstValueFrom, - map, - Observable, - ReplaySubject, - startWith, - switchMap, -} from "rxjs"; +import { combineLatest, filter, map, Observable, ReplaySubject, startWith, switchMap } from "rxjs"; import { canAccessOrgAdmin, @@ -22,6 +12,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { PolicyType, ProviderType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -117,148 +108,159 @@ export class ProductSwitcherService { switchMap((id) => this.organizationService.organizations$(id)), ); + providers$ = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((id) => this.providerService.providers$(id)), + ); + + userHasSingleOrgPolicy$ = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.policyService.policyAppliesToUser$(PolicyType.SingleOrg, userId)), + ); + products$: Observable<{ bento: ProductSwitcherItem[]; other: ProductSwitcherItem[]; - }> = combineLatest([this.organizations$, this.route.paramMap, this.triggerProductUpdate$]).pipe( - map(([orgs, ...rest]): [Organization[], ParamMap, void] => { - return [ + }> = combineLatest([ + this.organizations$, + this.providers$, + this.userHasSingleOrgPolicy$, + this.route.paramMap, + this.triggerProductUpdate$, + ]).pipe( + map( + ([orgs, providers, userHasSingleOrgPolicy, paramMap]: [ + Organization[], + Provider[], + boolean, + ParamMap, + void, + ]) => { // Sort orgs by name to match the order within the sidebar - orgs.sort((a, b) => a.name.localeCompare(b.name)), - ...rest, - ]; - }), - concatMap(async ([orgs, paramMap]) => { - let routeOrg = orgs.find((o) => o.id === paramMap.get("organizationId")); + orgs.sort((a, b) => a.name.localeCompare(b.name)); - let organizationIdViaPath: string | null = null; + let routeOrg = orgs.find((o) => o.id === paramMap.get("organizationId")); - if (["/sm/", "/organizations/"].some((path) => this.router.url.includes(path))) { - // Grab the organization ID from the URL - organizationIdViaPath = this.router.url.split("/")[2] ?? null; - } + let organizationIdViaPath: string | null = null; - // When the user is already viewing an organization within an application use it as the active route org - if (organizationIdViaPath && !routeOrg) { - routeOrg = orgs.find((o) => o.id === organizationIdViaPath); - } - - // If the active route org doesn't have access to SM, find the first org that does. - const smOrg = - routeOrg?.canAccessSecretsManager && routeOrg?.enabled == true - ? routeOrg - : orgs.find((o) => o.canAccessSecretsManager && o.enabled == true); - - // If the active route org doesn't have access to AC, find the first org that does. - const acOrg = - routeOrg != null && canAccessOrgAdmin(routeOrg) - ? routeOrg - : orgs.find((o) => canAccessOrgAdmin(o)); - - // TODO: This should be migrated to an Observable provided by the provider service and moved to the combineLatest above. See AC-2092. - const providers = await this.providerService.getAll(); - - const providerPortalName = - providers[0]?.providerType === ProviderType.BusinessUnit - ? "Business Unit Portal" - : "Provider Portal"; - - const orgsMarketingRoute = this.platformUtilsService.isSelfHost() - ? { - route: "https://bitwarden.com/products/business/", - external: true, - } - : { - route: "/create-organization", - external: false, - }; - - const products = { - pm: { - name: "Password Manager", - icon: "bwi-lock", - appRoute: "/vault", - marketingRoute: { - route: "https://bitwarden.com/products/personal/", - external: true, - }, - isActive: - !this.router.url.includes("/sm/") && - !this.router.url.includes("/organizations/") && - !this.router.url.includes("/providers/"), - }, - sm: { - name: "Secrets Manager", - icon: "bwi-cli", - appRoute: ["/sm", smOrg?.id], - marketingRoute: { - route: "/sm-landing", - external: false, - }, - isActive: this.router.url.includes("/sm/"), - otherProductOverrides: { - supportingText: this.i18nService.t("secureYourInfrastructure"), - }, - }, - ac: { - name: "Admin Console", - icon: "bwi-business", - appRoute: ["/organizations", acOrg?.id], - marketingRoute: { - route: "https://bitwarden.com/products/business/", - external: true, - }, - isActive: this.router.url.includes("/organizations/"), - }, - provider: { - name: providerPortalName, - icon: "bwi-provider", - appRoute: ["/providers", providers[0]?.id], - isActive: this.router.url.includes("/providers/"), - }, - orgs: { - name: "Organizations", - icon: "bwi-business", - marketingRoute: orgsMarketingRoute, - otherProductOverrides: { - name: "Share your passwords", - supportingText: this.i18nService.t("protectYourFamilyOrBusiness"), - }, - }, - } satisfies Record; - - const bento: ProductSwitcherItem[] = [products.pm]; - const other: ProductSwitcherItem[] = []; - - if (smOrg) { - bento.push(products.sm); - } else { - other.push(products.sm); - } - - if (acOrg) { - bento.push(products.ac); - } else { - const activeUserId = await firstValueFrom( - this.accountService.activeAccount$.pipe(getUserId), - ); - const userHasSingleOrgPolicy = await firstValueFrom( - this.policyService.policyAppliesToUser$(PolicyType.SingleOrg, activeUserId), - ); - if (!userHasSingleOrgPolicy) { - other.push(products.orgs); + if (["/sm/", "/organizations/"].some((path) => this.router.url.includes(path))) { + // Grab the organization ID from the URL + organizationIdViaPath = this.router.url.split("/")[2] ?? null; } - } - if (providers.length > 0) { - bento.push(products.provider); - } + // When the user is already viewing an organization within an application use it as the active route org + if (organizationIdViaPath && !routeOrg) { + routeOrg = orgs.find((o) => o.id === organizationIdViaPath); + } - return { - bento, - other, - }; - }), + // If the active route org doesn't have access to SM, find the first org that does. + const smOrg = + routeOrg?.canAccessSecretsManager && routeOrg?.enabled == true + ? routeOrg + : orgs.find((o) => o.canAccessSecretsManager && o.enabled == true); + + // If the active route org doesn't have access to AC, find the first org that does. + const acOrg = + routeOrg != null && canAccessOrgAdmin(routeOrg) + ? routeOrg + : orgs.find((o) => canAccessOrgAdmin(o)); + + const providerPortalName = + providers[0]?.providerType === ProviderType.BusinessUnit + ? "Business Unit Portal" + : "Provider Portal"; + + const orgsMarketingRoute = this.platformUtilsService.isSelfHost() + ? { + route: "https://bitwarden.com/products/business/", + external: true, + } + : { + route: "/create-organization", + external: false, + }; + + const products = { + pm: { + name: "Password Manager", + icon: "bwi-lock", + appRoute: "/vault", + marketingRoute: { + route: "https://bitwarden.com/products/personal/", + external: true, + }, + isActive: + !this.router.url.includes("/sm/") && + !this.router.url.includes("/organizations/") && + !this.router.url.includes("/providers/"), + }, + sm: { + name: "Secrets Manager", + icon: "bwi-cli", + appRoute: ["/sm", smOrg?.id], + marketingRoute: { + route: "/sm-landing", + external: false, + }, + isActive: this.router.url.includes("/sm/"), + otherProductOverrides: { + supportingText: this.i18nService.t("secureYourInfrastructure"), + }, + }, + ac: { + name: "Admin Console", + icon: "bwi-business", + appRoute: ["/organizations", acOrg?.id], + marketingRoute: { + route: "https://bitwarden.com/products/business/", + external: true, + }, + isActive: this.router.url.includes("/organizations/"), + }, + provider: { + name: providerPortalName, + icon: "bwi-provider", + appRoute: ["/providers", providers[0]?.id], + isActive: this.router.url.includes("/providers/"), + }, + orgs: { + name: "Organizations", + icon: "bwi-business", + marketingRoute: orgsMarketingRoute, + otherProductOverrides: { + name: "Share your passwords", + supportingText: this.i18nService.t("protectYourFamilyOrBusiness"), + }, + }, + } satisfies Record; + + const bento: ProductSwitcherItem[] = [products.pm]; + const other: ProductSwitcherItem[] = []; + + if (smOrg) { + bento.push(products.sm); + } else { + other.push(products.sm); + } + + if (acOrg) { + bento.push(products.ac); + } else { + if (!userHasSingleOrgPolicy) { + other.push(products.orgs); + } + } + + if (providers.length > 0) { + bento.push(products.provider); + } + + return { + bento, + other, + }; + }, + ), ); /** Poll the `syncService` until a sync is completed */ diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.html index 0fa39f2f292..82e412c2ef6 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.html @@ -11,7 +11,14 @@ {{ o.name }} - + diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.ts index 51296c52281..4f4b85e544b 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/add-organization.component.ts @@ -1,10 +1,13 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { Component, Inject, OnInit } from "@angular/core"; +import { Observable, switchMap } from "rxjs"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -22,7 +25,7 @@ interface AddOrganizationDialogData { standalone: false, }) export class AddOrganizationComponent implements OnInit { - protected provider: Provider; + protected provider$: Observable; protected loading = true; constructor( @@ -35,6 +38,7 @@ export class AddOrganizationComponent implements OnInit { private validationService: ValidationService, private dialogService: DialogService, private toastService: ToastService, + private accountService: AccountService, ) {} async ngOnInit() { @@ -46,18 +50,21 @@ export class AddOrganizationComponent implements OnInit { return; } - this.provider = await this.providerService.get(this.data.providerId); + this.provider$ = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.providerService.get$(this.data.providerId, userId)), + ); this.loading = false; } - add(organization: Organization) { + add(organization: Organization, provider: Provider) { return async () => { const confirmed = await this.dialogService.openSimpleDialog({ title: organization.name, content: { key: "addOrganizationConfirmation", - placeholders: [organization.name, this.provider.name], + placeholders: [organization.name, provider.name], }, type: "warning", }); diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts index 325301f6001..6d0b52b05d6 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts @@ -3,7 +3,7 @@ import { Component } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormControl } from "@angular/forms"; import { ActivatedRoute, Router, RouterModule } from "@angular/router"; -import { firstValueFrom, from, map, Observable, switchMap } from "rxjs"; +import { combineLatest, firstValueFrom, from, map, Observable, switchMap } from "rxjs"; import { debounceTime, first } from "rxjs/operators"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -65,9 +65,10 @@ export class ClientsComponent { this.activatedRoute.parent?.params.pipe(map((params) => params.providerId as string)) ?? new Observable(); - protected provider$ = this.providerId$.pipe( - switchMap((providerId) => this.providerService.get$(providerId)), - ); + protected provider$ = combineLatest([ + this.providerId$, + this.accountService.activeAccount$.pipe(getUserId), + ]).pipe(switchMap(([providerId, userId]) => this.providerService.get$(providerId, userId))); protected isAdminOrServiceUser$ = this.provider$.pipe( map( diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts index 086be39b754..a0a881dbad7 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts @@ -3,12 +3,16 @@ import { TestBed } from "@angular/core/testing"; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from "@angular/router"; import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { ProviderUserType } from "@bitwarden/common/admin-console/enums"; import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { ToastService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; import { providerPermissionsGuard } from "./provider-permissions.guard"; @@ -25,11 +29,23 @@ const providerFactory = (props: Partial = {}) => describe("Provider Permissions Guard", () => { let providerService: MockProxy; + let accountService: MockProxy; let route: MockProxy; let state: MockProxy; + const mockUserId = newGuid() as UserId; + beforeEach(() => { providerService = mock(); + accountService = mock(); + + accountService.activeAccount$ = of({ + id: mockUserId, + email: "test@example.com", + emailVerified: true, + name: "Test User", + }); + route = mock({ params: { providerId: providerFactory().id, @@ -44,12 +60,15 @@ describe("Provider Permissions Guard", () => { { provide: ToastService, useValue: mock() }, { provide: I18nService, useValue: mock() }, { provide: Router, useValue: mock() }, + { provide: AccountService, useValue: accountService }, ], }); }); it("blocks navigation if provider does not exist", async () => { - providerService.get.mockResolvedValue(null); + providerService.get$ + .calledWith(providerFactory().id, mockUserId) + .mockReturnValue(of(undefined)); const actual = await TestBed.runInInjectionContext( async () => await providerPermissionsGuard()(route, state), @@ -60,7 +79,7 @@ describe("Provider Permissions Guard", () => { it("permits navigation if no permissions are specified", async () => { const provider = providerFactory(); - providerService.get.calledWith(provider.id).mockResolvedValue(provider); + providerService.get$.calledWith(provider.id, mockUserId).mockReturnValue(of(provider)); const actual = await TestBed.runInInjectionContext( async () => await providerPermissionsGuard()(route, state), @@ -74,7 +93,7 @@ describe("Provider Permissions Guard", () => { permissionsCallback.mockImplementation((_provider) => true); const provider = providerFactory(); - providerService.get.calledWith(provider.id).mockResolvedValue(provider); + providerService.get$.calledWith(provider.id, mockUserId).mockReturnValue(of(provider)); const actual = await TestBed.runInInjectionContext( async () => await providerPermissionsGuard(permissionsCallback)(route, state), @@ -88,7 +107,7 @@ describe("Provider Permissions Guard", () => { const permissionsCallback = jest.fn(); permissionsCallback.mockImplementation((_org) => false); const provider = providerFactory(); - providerService.get.calledWith(provider.id).mockResolvedValue(provider); + providerService.get$.calledWith(provider.id, mockUserId).mockReturnValue(of(provider)); const actual = await TestBed.runInInjectionContext( async () => await providerPermissionsGuard(permissionsCallback)(route, state), @@ -104,7 +123,7 @@ describe("Provider Permissions Guard", () => { type: ProviderUserType.ServiceUser, enabled: false, }); - providerService.get.calledWith(org.id).mockResolvedValue(org); + providerService.get$.calledWith(org.id, mockUserId).mockReturnValue(of(org)); const actual = await TestBed.runInInjectionContext( async () => await providerPermissionsGuard()(route, state), @@ -118,7 +137,7 @@ describe("Provider Permissions Guard", () => { type: ProviderUserType.ProviderAdmin, enabled: false, }); - providerService.get.calledWith(org.id).mockResolvedValue(org); + providerService.get$.calledWith(org.id, mockUserId).mockReturnValue(of(org)); const actual = await TestBed.runInInjectionContext( async () => await providerPermissionsGuard()(route, state), diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts index 6146e990972..7971ab9a99e 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts @@ -7,9 +7,12 @@ import { Router, RouterStateSnapshot, } from "@angular/router"; +import { firstValueFrom, switchMap } from "rxjs"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ToastService } from "@bitwarden/components"; @@ -40,8 +43,14 @@ export function providerPermissionsGuard( const router = inject(Router); const i18nService = inject(I18nService); const toastService = inject(ToastService); + const accountService = inject(AccountService); - const provider = await providerService.get(route.params.providerId); + const provider = await firstValueFrom( + accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => providerService.get$(route.params.providerId, userId)), + ), + ); if (provider == null) { return router.createUrlTree(["/"]); } diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/events.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/events.component.ts index d8ad2e01343..43fc958585a 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/events.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/events.component.ts @@ -2,12 +2,14 @@ // @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; +import { firstValueFrom, switchMap } from "rxjs"; import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { EventResponse } from "@bitwarden/common/models/response/event.response"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -64,7 +66,13 @@ export class EventsComponent extends BaseEventsComponent implements OnInit { // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe this.route.parent.parent.params.subscribe(async (params) => { this.providerId = params.providerId; - const provider = await this.providerService.get(this.providerId); + const provider = await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.providerService.get$(this.providerId, userId)), + ), + ); + if (provider == null || !provider.useEvents) { // 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 diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts index b4433f84fe5..affcfce9c17 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts @@ -3,7 +3,7 @@ import { Component } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute, Router } from "@angular/router"; -import { combineLatest, lastValueFrom, switchMap } from "rxjs"; +import { combineLatest, firstValueFrom, lastValueFrom, switchMap } from "rxjs"; import { first } from "rxjs/operators"; import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; @@ -14,6 +14,8 @@ import { ProviderUserStatusType, ProviderUserType } from "@bitwarden/common/admi import { ProviderUserBulkRequest } from "@bitwarden/common/admin-console/models/request/provider/provider-user-bulk.request"; import { ProviderUserConfirmRequest } from "@bitwarden/common/admin-console/models/request/provider/provider-user-confirm.request"; import { ProviderUserUserDetailsResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-user.response"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -73,6 +75,7 @@ export class MembersComponent extends BaseMembersComponent { private activatedRoute: ActivatedRoute, private providerService: ProviderService, private router: Router, + private accountService: AccountService, ) { super( apiService, @@ -96,7 +99,13 @@ export class MembersComponent extends BaseMembersComponent { this.dataSource.filter = peopleFilter(queryParams.search, null); this.providerId = urlParams.providerId; - const provider = await this.providerService.get(this.providerId); + const provider = await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.providerService.get$(this.providerId, userId)), + ), + ); + if (!provider || !provider.canManageUsers) { return await this.router.navigate(["../"], { relativeTo: this.activatedRoute }); } diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts index 159a0105df9..f3e7a9f442e 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts @@ -11,6 +11,8 @@ import { BusinessUnitPortalLogo, Icon, ProviderPortalLogo } from "@bitwarden/ass import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { ProviderStatusType, ProviderType } from "@bitwarden/common/admin-console/enums"; import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { IconModule } from "@bitwarden/components"; @@ -56,6 +58,7 @@ export class ProvidersLayoutComponent implements OnInit, OnDestroy { private providerService: ProviderService, private configService: ConfigService, private providerWarningsService: ProviderWarningsService, + private accountService: AccountService, ) {} ngOnInit() { @@ -65,8 +68,11 @@ export class ProvidersLayoutComponent implements OnInit, OnDestroy { map((params) => params.providerId), ); - this.provider$ = providerId$.pipe( - switchMap((providerId) => this.providerService.get$(providerId)), + this.provider$ = combineLatest([ + providerId$, + this.accountService.activeAccount$.pipe(getUserId), + ]).pipe( + switchMap(([providerId, userId]) => this.providerService.get$(providerId, userId)), takeUntil(this.destroy$), ); diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.html index 1c8c12a1fe1..eec6f713903 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.html @@ -1,3 +1,4 @@ +@let providers = providers$ | async; diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.ts index f3d67abd4e1..d13ac863437 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.component.ts @@ -1,9 +1,12 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { Component, OnInit } from "@angular/core"; +import { map, Observable, switchMap, tap } from "rxjs"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -13,24 +16,27 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; standalone: false, }) export class ProvidersComponent implements OnInit { - providers: Provider[]; + providers$: Observable; loaded = false; actionPromise: Promise; constructor( private providerService: ProviderService, private i18nService: I18nService, + private accountService: AccountService, ) {} - async ngOnInit() { + ngOnInit() { document.body.classList.remove("layout_frontend"); - await this.load(); + this.load(); } - async load() { - const providers = await this.providerService.getAll(); - providers.sort(Utils.getSortFunction(this.i18nService, "name")); - this.providers = providers; - this.loaded = true; + load() { + this.providers$ = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.providerService.providers$(userId)), + map((p) => p.sort(Utils.getSortFunction(this.i18nService, "name"))), + tap(() => (this.loaded = true)), + ); } } diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts index 27f45cb250e..9933c316869 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.ts @@ -21,6 +21,8 @@ import { } from "@bitwarden/common/admin-console/enums"; import { Provider } from "@bitwarden/common/admin-console/models/domain/provider"; import { ProviderOrganizationOrganizationDetailsResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-organization.response"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions"; import { PlanResponse } from "@bitwarden/common/billing/models/response/plan.response"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -87,9 +89,10 @@ export class ManageClientsComponent { this.activatedRoute.parent?.params.pipe(map((params) => params.providerId as string)) ?? new Observable(); - protected provider$ = this.providerId$.pipe( - switchMap((providerId) => this.providerService.get$(providerId)), - ); + protected provider$ = combineLatest([ + this.providerId$, + this.accountService.activeAccount$.pipe(getUserId), + ]).pipe(switchMap(([providerId, userId]) => this.providerService.get$(providerId, userId))); protected isAdminOrServiceUser$ = this.provider$.pipe( map( @@ -126,6 +129,7 @@ export class ManageClientsComponent { private webProviderService: WebProviderService, private billingNotificationService: BillingNotificationService, private configService: ConfigService, + private accountService: AccountService, ) { this.activatedRoute.queryParams.pipe(first(), takeUntilDestroyed()).subscribe((queryParams) => { this.searchControl.setValue(queryParams.search); @@ -133,7 +137,7 @@ export class ManageClientsComponent { this.provider$ .pipe( - map((provider: Provider) => { + map((provider: Provider | undefined) => { if (provider?.providerStatus !== ProviderStatusType.Billable) { return from( this.router.navigate(["../clients"], { @@ -158,7 +162,8 @@ export class ManageClientsComponent { async load() { try { const providerId = await firstValueFrom(this.providerId$); - const provider = await firstValueFrom(this.providerService.get$(providerId)); + const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); + const provider = await firstValueFrom(this.providerService.get$(providerId, userId)); if (provider?.providerType === ProviderType.BusinessUnit) { this.pageTitle = this.i18nService.t("businessUnits"); this.clientColumnHeader = this.i18nService.t("businessUnit"); diff --git a/bitwarden_license/bit-web/src/app/billing/providers/guards/has-consolidated-billing.guard.ts b/bitwarden_license/bit-web/src/app/billing/providers/guards/has-consolidated-billing.guard.ts index 60dbf4b3b82..de13cca3ad4 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/guards/has-consolidated-billing.guard.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/guards/has-consolidated-billing.guard.ts @@ -4,11 +4,15 @@ import { firstValueFrom } from "rxjs"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { ProviderStatusType } from "@bitwarden/common/admin-console/enums"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; export const hasConsolidatedBilling: CanActivateFn = async (route: ActivatedRouteSnapshot) => { const providerService = inject(ProviderService); + const accountService = inject(AccountService); - const provider = await firstValueFrom(providerService.get$(route.params.providerId)); + const userId = await firstValueFrom(getUserId(accountService.activeAccount$)); + const provider = await firstValueFrom(providerService.get$(route.params.providerId, userId)); if (!provider || provider.providerStatus !== ProviderStatusType.Billable) { return createUrlTreeFromSnapshot(route, ["/providers", route.params.providerId]); diff --git a/bitwarden_license/bit-web/src/app/billing/providers/payment-details/provider-payment-details.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/payment-details/provider-payment-details.component.ts index 05cf8f55858..d2ac2cede2f 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/payment-details/provider-payment-details.component.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/payment-details/provider-payment-details.component.ts @@ -83,8 +83,12 @@ const BANK_ACCOUNT_VERIFIED_COMMAND = new CommandDefinition<{ export class ProviderPaymentDetailsComponent implements OnInit, OnDestroy { private viewState$ = new BehaviorSubject(null); - private provider$ = this.activatedRoute.params.pipe( - switchMap(({ providerId }) => this.providerService.get$(providerId)), + private provider$ = combineLatest([ + this.activatedRoute.params, + this.accountService.activeAccount$.pipe(getUserId), + ]).pipe( + switchMap(([{ providerId }, userId]) => this.providerService.get$(providerId, userId)), + filter((provider) => provider != null), ); private load$: Observable = this.provider$.pipe( diff --git a/libs/angular/src/billing/components/add-account-credit-dialog/add-account-credit-dialog.component.ts b/libs/angular/src/billing/components/add-account-credit-dialog/add-account-credit-dialog.component.ts index a6f1db54241..3dc56c55b0c 100644 --- a/libs/angular/src/billing/components/add-account-credit-dialog/add-account-credit-dialog.component.ts +++ b/libs/angular/src/billing/components/add-account-credit-dialog/add-account-credit-dialog.component.ts @@ -123,7 +123,9 @@ export class AddAccountCreditDialogComponent implements OnInit { this.formGroup.patchValue({ creditAmount: 20.0, }); - this.provider = await this.providerService.get(this.dialogParams.providerId); + this.provider = await firstValueFrom( + this.providerService.get$(this.dialogParams.providerId, this.user.id), + ); payPalCustomField = "provider_id:" + this.provider.id; this.payPalConfig.subject = this.provider.name; } else { diff --git a/libs/common/src/admin-console/abstractions/provider.service.ts b/libs/common/src/admin-console/abstractions/provider.service.ts index 340156020ff..3f2c7f68a9a 100644 --- a/libs/common/src/admin-console/abstractions/provider.service.ts +++ b/libs/common/src/admin-console/abstractions/provider.service.ts @@ -5,8 +5,7 @@ import { ProviderData } from "../models/data/provider.data"; import { Provider } from "../models/domain/provider"; export abstract class ProviderService { - abstract get$(id: string): Observable; - abstract get(id: string): Promise; - abstract getAll(): Promise; - abstract save(providers: { [id: string]: ProviderData }, userId?: UserId): Promise; + abstract providers$(userId: UserId): Observable; + abstract get$(id: string, userId: UserId): Observable; + abstract save(providers: { [id: string]: ProviderData }, userId: UserId): Promise; } diff --git a/libs/common/src/admin-console/services/provider.service.spec.ts b/libs/common/src/admin-console/services/provider.service.spec.ts index 92d5ae2e801..e7e8a6ba293 100644 --- a/libs/common/src/admin-console/services/provider.service.spec.ts +++ b/libs/common/src/admin-console/services/provider.service.spec.ts @@ -1,7 +1,7 @@ import { firstValueFrom } from "rxjs"; import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../spec"; -import { FakeActiveUserState, FakeSingleUserState } from "../../../spec/fake-state"; +import { FakeSingleUserState } from "../../../spec/fake-state"; import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; import { @@ -20,11 +20,11 @@ import { PROVIDERS, ProviderService } from "./provider.service"; * in state. This helper methods lets us build provider arrays in tests * and easily map them to records before storing them in state. */ -function arrayToRecord(input: ProviderData[]): Record { - if (input == null) { - return undefined; +function arrayToRecord(input: ProviderData[] | undefined): Record | null { + if (input == null || input.length < 1) { + return null; } - return Object.fromEntries(input?.map((i) => [i.id, i])); + return Object.fromEntries(input.map((i) => [i.id, i])); } /** @@ -39,7 +39,7 @@ function arrayToRecord(input: ProviderData[]): Record { */ function buildMockProviders(count = 1, suffix?: string): ProviderData[] { if (count < 1) { - return undefined; + return []; } function buildMockProvider(id: string, name: string): ProviderData { @@ -87,30 +87,28 @@ describe("ProviderService", () => { let fakeAccountService: FakeAccountService; let fakeStateProvider: FakeStateProvider; let fakeUserState: FakeSingleUserState>; - let fakeActiveUserState: FakeActiveUserState>; beforeEach(async () => { fakeAccountService = mockAccountServiceWith(fakeUserId); fakeStateProvider = new FakeStateProvider(fakeAccountService); fakeUserState = fakeStateProvider.singleUser.getFake(fakeUserId, PROVIDERS); - fakeActiveUserState = fakeStateProvider.activeUser.getFake(PROVIDERS); providerService = new ProviderService(fakeStateProvider); }); - describe("getAll()", () => { + describe("providers$()", () => { it("Returns an array of all providers stored in state", async () => { - const mockData: ProviderData[] = buildMockProviders(5); + const mockData = buildMockProviders(5); fakeUserState.nextState(arrayToRecord(mockData)); - const providers = await providerService.getAll(); + const providers = await firstValueFrom(providerService.providers$(fakeUserId)); expect(providers).toHaveLength(5); expect(providers).toEqual(mockData.map((x) => new Provider(x))); }); it("Returns an empty array if no providers are found in state", async () => { - const mockData: ProviderData[] = undefined; + let mockData; fakeUserState.nextState(arrayToRecord(mockData)); - const result = await providerService.getAll(); + const result = await firstValueFrom(providerService.providers$(fakeUserId)); expect(result).toEqual([]); }); }); @@ -119,50 +117,38 @@ describe("ProviderService", () => { it("Returns an observable of a single provider from state that matches the specified id", async () => { const mockData = buildMockProviders(5); fakeUserState.nextState(arrayToRecord(mockData)); - const result = providerService.get$(mockData[3].id); + const result = providerService.get$(mockData[3].id, fakeUserId); const provider = await firstValueFrom(result); expect(provider).toEqual(new Provider(mockData[3])); }); it("Returns an observable of undefined if the specified provider is not found", async () => { - const result = providerService.get$("this-provider-does-not-exist"); + const result = providerService.get$("this-provider-does-not-exist", fakeUserId); const provider = await firstValueFrom(result); expect(provider).toBe(undefined); }); }); - describe("get()", () => { - it("Returns a single provider from state that matches the specified id", async () => { - const mockData = buildMockProviders(5); - fakeUserState.nextState(arrayToRecord(mockData)); - const result = await providerService.get(mockData[3].id); - expect(result).toEqual(new Provider(mockData[3])); - }); - - it("Returns undefined if the specified provider id is not found", async () => { - const result = await providerService.get("this-provider-does-not-exist"); - expect(result).toBe(undefined); - }); - }); - describe("save()", () => { - it("replaces the entire provider list in state for the active user", async () => { + it("replaces the entire provider list in state for the specified user", async () => { const originalData = buildMockProviders(10); fakeUserState.nextState(arrayToRecord(originalData)); const newData = arrayToRecord(buildMockProviders(10, "newData")); - await providerService.save(newData); + if (newData) { + await providerService.save(newData, fakeUserId); + } - expect(fakeActiveUserState.nextMock).toHaveBeenCalledWith([fakeUserId, newData]); + expect(fakeUserState.nextMock).toHaveBeenCalledWith(newData); }); // This is more or less a test for logouts it("can replace state with null", async () => { const originalData = buildMockProviders(2); - fakeActiveUserState.nextState(arrayToRecord(originalData)); - await providerService.save(null); + fakeUserState.nextState(arrayToRecord(originalData)); + await providerService.save(null, fakeUserId); - expect(fakeActiveUserState.nextMock).toHaveBeenCalledWith([fakeUserId, null]); + expect(fakeUserState.nextMock).toHaveBeenCalledWith(null); }); }); }); diff --git a/libs/common/src/admin-console/services/provider.service.ts b/libs/common/src/admin-console/services/provider.service.ts index 05909f9d8a1..7385055c3b4 100644 --- a/libs/common/src/admin-console/services/provider.service.ts +++ b/libs/common/src/admin-console/services/provider.service.ts @@ -1,7 +1,6 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { firstValueFrom, map, Observable, of, switchMap, take } from "rxjs"; +import { map, Observable } from "rxjs"; +import { getById } from "../../platform/misc"; import { PROVIDERS_DISK, StateProvider, UserKeyDefinition } from "../../platform/state"; import { UserId } from "../../types/guid"; import { ProviderService as ProviderServiceAbstraction } from "../abstractions/provider.service"; @@ -13,46 +12,26 @@ export const PROVIDERS = UserKeyDefinition.record(PROVIDERS_DISK, clearOn: ["logout"], }); -function mapToSingleProvider(providerId: string) { - return map((providers) => providers?.find((p) => p.id === providerId)); -} - export class ProviderService implements ProviderServiceAbstraction { constructor(private stateProvider: StateProvider) {} - private providers$(userId?: UserId): Observable { - // FIXME: Can be replaced with `getUserStateOrDefault$` if we weren't trying to pick this. - return ( - userId != null - ? this.stateProvider.getUser(userId, PROVIDERS).state$ - : this.stateProvider.activeUserId$.pipe( - take(1), - switchMap((userId) => - userId != null ? this.stateProvider.getUser(userId, PROVIDERS).state$ : of(null), - ), - ) - ).pipe(this.mapProviderRecordToArray()); + providers$(userId: UserId): Observable { + return this.stateProvider + .getUser(userId, PROVIDERS) + .state$.pipe(this.mapProviderRecordToArray()); } private mapProviderRecordToArray() { - return map, Provider[]>((providers) => - Object.values(providers ?? {})?.map((o) => new Provider(o)), + return map | null, Provider[]>((providers) => + Object.values(providers ?? {}).map((o) => new Provider(o)), ); } - get$(id: string): Observable { - return this.providers$().pipe(mapToSingleProvider(id)); + get$(id: string, userId: UserId): Observable { + return this.providers$(userId).pipe(getById(id)); } - async get(id: string): Promise { - return await firstValueFrom(this.providers$().pipe(mapToSingleProvider(id))); - } - - async getAll(): Promise { - return await firstValueFrom(this.providers$()); - } - - async save(providers: { [id: string]: ProviderData }, userId?: UserId) { + async save(providers: { [id: string]: ProviderData }, userId: UserId) { await this.stateProvider.setUserState(PROVIDERS, providers, userId); } }