From 2b02356c6839c54b6aeee7e7306b823fb54e37e6 Mon Sep 17 00:00:00 2001 From: Brandon Date: Fri, 26 Sep 2025 11:54:33 -0400 Subject: [PATCH 01/10] WIP: added new services, refactor members to use billing service and member action service --- .../browser-system-notification.service.ts | 2 +- .../common/base-members.component.ts | 2 + .../members/members.component.html | 2 +- .../members/members.component.ts | 309 +++------------ .../organizations/members/members.module.ts | 14 + .../billing-constraint.service.ts | 206 ++++++++++ .../organizations/members/services/index.ts | 7 + .../member-actions/member-actions.service.ts | 238 ++++++++++++ .../member-dialog-manager.service.ts | 355 ++++++++++++++++++ .../member-permissions.service.ts | 145 +++++++ .../organization-members-facade.service.ts | 163 ++++++++ 11 files changed, 1192 insertions(+), 251 deletions(-) create mode 100644 apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/index.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts diff --git a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts index b835c711853d..33e60894a543 100644 --- a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts +++ b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts @@ -70,7 +70,7 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ } async clear(clearInfo: SystemNotificationClearInfo): Promise { - // eslint-disable-next-line @typescript-eslint/no-floating-promises + chrome.notifications.clear(clearInfo.id); } diff --git a/apps/web/src/app/admin-console/common/base-members.component.ts b/apps/web/src/app/admin-console/common/base-members.component.ts index 21c529492541..936d69729be8 100644 --- a/apps/web/src/app/admin-console/common/base-members.component.ts +++ b/apps/web/src/app/admin-console/common/base-members.component.ts @@ -24,6 +24,7 @@ import { KeyService } from "@bitwarden/key-management"; import { OrganizationUserView } from "../organizations/core/views/organization-user.view"; import { UserConfirmComponent } from "../organizations/manage/user-confirm.component"; +import { MemberActionResult } from "../organizations/members/services/member-actions/member-actions.service"; import { PeopleTableDataSource, peopleFilter } from "./people-table-data-source"; @@ -76,6 +77,7 @@ export abstract class BaseMembersComponent { * The currently executing promise - used to avoid multiple user actions executing at once. */ actionPromise?: Promise; + actionPromise2?: Promise; protected searchControl = new FormControl("", { nonNullable: true }); protected statusToggle = new BehaviorSubject(undefined); diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.html b/apps/web/src/app/admin-console/organizations/members/members.component.html index 282291eb60ec..021157a1977d 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.html +++ b/apps/web/src/app/admin-console/organizations/members/members.component.html @@ -2,7 +2,7 @@ @if (organization) { diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 5a1ae6cd98b0..dbbd2af43cd0 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -1,6 +1,6 @@ import { Component, computed, Signal } from "@angular/core"; import { takeUntilDestroyed, toSignal } from "@angular/core/rxjs-interop"; -import { ActivatedRoute, Router } from "@angular/router"; +import { ActivatedRoute } from "@angular/router"; import { BehaviorSubject, combineLatest, @@ -19,7 +19,6 @@ import { import { OrganizationUserApiService, - OrganizationUserConfirmRequest, OrganizationUserUserDetailsResponse, CollectionService, CollectionData, @@ -28,7 +27,6 @@ import { } from "@bitwarden/admin-console/common"; import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { getOrganizationById, OrganizationService, @@ -43,28 +41,18 @@ import { } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request"; 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/billing-api.service.abstraction"; -import { isNotSelfUpgradable, ProductTierType } from "@bitwarden/common/billing/enums"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; -import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; -import { DialogService, SimpleDialogOptions, ToastService } from "@bitwarden/components"; +import { DialogService, ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; -import { - ChangePlanDialogResultType, - openChangePlanDialog, -} from "../../../billing/organizations/change-plan-dialog.component"; import { BaseMembersComponent } from "../../common/base-members.component"; import { PeopleTableDataSource } from "../../common/people-table-data-source"; import { GroupApiService } from "../core"; @@ -86,8 +74,9 @@ import { MemberDialogTab, openUserAddEditDialog, } from "./components/member-dialog"; -import { isFixedSeatPlan } from "./components/member-dialog/validators/org-seat-limit-reached.validator"; +import { BillingConstraintService } from "./services"; import { DeleteManagedMemberWarningService } from "./services/delete-managed-member/delete-managed-member-warning.service"; +import { MemberActionsService } from "./services/member-actions/member-actions.service"; import { OrganizationUserService } from "./services/organization-user/organization-user.service"; class MembersTableDataSource extends PeopleTableDataSource { @@ -128,7 +117,6 @@ export class MembersComponent extends BaseMembersComponent i18nService: I18nService, organizationManagementPreferencesService: OrganizationManagementPreferencesService, keyService: KeyService, - private encryptService: EncryptService, validationService: ValidationService, logService: LogService, userNamePipe: UserNamePipe, @@ -137,19 +125,16 @@ export class MembersComponent extends BaseMembersComponent private policyService: PolicyService, private policyApiService: PolicyApiService, private route: ActivatedRoute, - private syncService: SyncService, private organizationService: OrganizationService, private accountService: AccountService, - private organizationApiService: OrganizationApiServiceAbstraction, private organizationUserApiService: OrganizationUserApiService, - private router: Router, private groupService: GroupApiService, private collectionService: CollectionService, - private billingApiService: BillingApiServiceAbstraction, protected deleteManagedMemberWarningService: DeleteManagedMemberWarningService, - private configService: ConfigService, private organizationUserService: OrganizationUserService, private organizationWarningsService: OrganizationWarningsService, + private memberActionsService: MemberActionsService, + protected billingConstraint: BillingConstraintService, ) { super( apiService, @@ -193,34 +178,10 @@ export class MembersComponent extends BaseMembersComponent .pipe( concatMap(async ([qParams, policies, organization]) => { // Backfill pub/priv key if necessary - if (organization.canManageUsersPassword && !organization.hasPublicAndPrivateKeys) { - const orgShareKey = await firstValueFrom( - this.userId$.pipe( - switchMap((userId) => this.keyService.orgKeys$(userId)), - map((orgKeys) => { - if (orgKeys == null || orgKeys[organization.id] == null) { - throw new Error("Organization keys not found for provided User."); - } - return orgKeys[organization.id]; - }), - ), - ); - - const [orgPublicKey, encryptedOrgPrivateKey] = - await this.keyService.makeKeyPair(orgShareKey); - if (encryptedOrgPrivateKey.encryptedString == null) { - throw new Error("Encrypted private key is null."); - } - const request = new OrganizationKeysRequest( - orgPublicKey, - encryptedOrgPrivateKey.encryptedString, - ); - const response = await this.organizationApiService.updateKeys(organization.id, request); - if (response != null) { - await this.syncService.fullSync(true); // Replace organizations with new data - } else { - throw new Error(this.i18nService.t("resetPasswordOrgKeysError")); - } + try { + await this.memberActionsService.ensureOrganizationKeys(organization); + } catch { + throw new Error(this.i18nService.t("resetPasswordOrgKeysError")); } const resetPasswordPolicy = policies @@ -255,11 +216,8 @@ export class MembersComponent extends BaseMembersComponent ) .subscribe(); - this.billingMetadata$ = combineLatest([this.refreshBillingMetadata$, organization$]).pipe( - switchMap(([_, organization]) => - this.billingApiService.getOrganizationBillingMetadata(organization.id), - ), - takeUntilDestroyed(), + this.billingMetadata$ = organization$.pipe( + switchMap((organization) => this.billingConstraint.getBillingMetadata$(organization.id)), shareReplay({ bufferSize: 1, refCount: false }), ); @@ -353,20 +311,32 @@ export class MembersComponent extends BaseMembersComponent return await firstValueFrom(decryptedCollections$); } - removeUser(id: string, organization: Organization): Promise { - return this.organizationUserApiService.removeOrganizationUser(organization.id, id); + async removeUser(id: string, organization: Organization): Promise { + const result = await this.memberActionsService.removeUser(organization, id); + if (!result.success) { + throw new Error(result.error); + } } - revokeUser(id: string, organization: Organization): Promise { - return this.organizationUserApiService.revokeOrganizationUser(organization.id, id); + async revokeUser(id: string, organization: Organization): Promise { + const result = await this.memberActionsService.revokeUser(organization, id); + if (!result.success) { + throw new Error(result.error); + } } - restoreUser(id: string, organization: Organization): Promise { - return this.organizationUserApiService.restoreOrganizationUser(organization.id, id); + async restoreUser(id: string, organization: Organization): Promise { + const result = await this.memberActionsService.restoreUser(organization, id); + if (!result.success) { + throw new Error(result.error); + } } - reinviteUser(id: string, organization: Organization): Promise { - return this.organizationUserApiService.postOrganizationUserReinvite(organization.id, id); + async reinviteUser(id: string, organization: Organization): Promise { + const result = await this.memberActionsService.reinviteUser(organization, id); + if (!result.success) { + throw new Error(result.error); + } } async confirmUser( @@ -374,30 +344,9 @@ export class MembersComponent extends BaseMembersComponent publicKey: Uint8Array, organization: Organization, ): Promise { - if ( - await firstValueFrom(this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation)) - ) { - await firstValueFrom(this.organizationUserService.confirmUser(organization, user, publicKey)); - } else { - const request = await firstValueFrom( - this.userId$.pipe( - switchMap((userId) => this.keyService.orgKeys$(userId)), - filter((orgKeys) => orgKeys != null), - map((orgKeys) => orgKeys[organization.id]), - switchMap((orgKey) => this.encryptService.encapsulateKeyUnsigned(orgKey, publicKey)), - map((encKey) => { - const req = new OrganizationUserConfirmRequest(); - req.key = encKey.encryptedString; - return req; - }), - ), - ); - - await this.organizationUserApiService.postOrganizationUserConfirm( - organization.id, - user.id, - request, - ); + const result = await this.memberActionsService.confirmUser(user, publicKey, organization); + if (!result.success) { + throw new Error(result.error); } } @@ -438,30 +387,10 @@ export class MembersComponent extends BaseMembersComponent } allowResetPassword(orgUser: OrganizationUserView, organization: Organization): boolean { - let callingUserHasPermission = false; - - switch (organization.type) { - case OrganizationUserType.Owner: - callingUserHasPermission = true; - break; - case OrganizationUserType.Admin: - callingUserHasPermission = orgUser.type !== OrganizationUserType.Owner; - break; - case OrganizationUserType.Custom: - callingUserHasPermission = - orgUser.type !== OrganizationUserType.Owner && - orgUser.type !== OrganizationUserType.Admin; - break; - } - - return ( - organization.canManageUsersPassword && - callingUserHasPermission && - organization.useResetPassword && - organization.hasPublicAndPrivateKeys && - orgUser.resetPasswordEnrolled && - this.orgResetPasswordPolicyEnabled && - orgUser.status === OrganizationUserStatusType.Confirmed + return this.memberActionsService.allowResetPassword( + orgUser, + organization, + this.orgResetPasswordPolicyEnabled, ); } @@ -476,83 +405,6 @@ export class MembersComponent extends BaseMembersComponent ); } - private getManageBillingText(organization: Organization): string { - return organization.canEditSubscription ? "ManageBilling" : "NoManageBilling"; - } - - private getProductKey(organization: Organization): string { - let product = ""; - switch (organization.productTierType) { - case ProductTierType.Free: - product = "freeOrg"; - break; - case ProductTierType.TeamsStarter: - product = "teamsStarterPlan"; - break; - case ProductTierType.Families: - product = "familiesPlan"; - break; - default: - throw new Error(`Unsupported product type: ${organization.productTierType}`); - } - return `${product}InvLimitReached${this.getManageBillingText(organization)}`; - } - - private getDialogContent(organization: Organization): string { - return this.i18nService.t(this.getProductKey(organization), organization.seats); - } - - private getAcceptButtonText(organization: Organization): string { - if (!organization.canEditSubscription) { - return this.i18nService.t("ok"); - } - - const productType = organization.productTierType; - - if (isNotSelfUpgradable(productType)) { - throw new Error(`Unsupported product type: ${productType}`); - } - - return this.i18nService.t("upgrade"); - } - - private async handleDialogClose( - result: boolean | undefined, - organization: Organization, - ): Promise { - if (!result || !organization.canEditSubscription) { - return; - } - - const productType = organization.productTierType; - - if (isNotSelfUpgradable(productType)) { - throw new Error(`Unsupported product type: ${organization.productTierType}`); - } - - await this.router.navigate(["/organizations", organization.id, "billing", "subscription"], { - queryParams: { upgrade: true }, - }); - } - - private async showSeatLimitReachedDialog(organization: Organization): Promise { - const orgUpgradeSimpleDialogOpts: SimpleDialogOptions = { - title: this.i18nService.t("upgradeOrganization"), - content: this.getDialogContent(organization), - type: "primary", - acceptButtonText: this.getAcceptButtonText(organization), - }; - - if (!organization.canEditSubscription) { - orgUpgradeSimpleDialogOpts.cancelButtonText = null; - } - - const simpleDialog = this.dialogService.openSimpleDialogRef(orgUpgradeSimpleDialogOpts); - await lastValueFrom( - simpleDialog.closed.pipe(map((closed) => this.handleDialogClose(closed, organization))), - ); - } - private async handleInviteDialog(organization: Organization) { const billingMetadata = await firstValueFrom(this.billingMetadata$); const dialog = openUserAddEditDialog(this.dialogService, { @@ -572,51 +424,13 @@ export class MembersComponent extends BaseMembersComponent } } - private async handleSeatLimitForFixedTiers(organization: Organization) { - if (!organization.canEditSubscription) { - await this.showSeatLimitReachedDialog(organization); - return; - } - - const reference = openChangePlanDialog(this.dialogService, { - data: { - organizationId: organization.id, - productTierType: organization.productTierType, - }, - }); - - const result = await lastValueFrom(reference.closed); - - if (result === ChangePlanDialogResultType.Submitted) { - await this.load(organization); - } - } - async invite(organization: Organization) { const billingMetadata = await firstValueFrom(this.billingMetadata$); - if ( - organization.hasReseller && - organization.seats === billingMetadata?.organizationOccupiedSeats - ) { - this.toastService.showToast({ - variant: "error", - title: this.i18nService.t("seatLimitReached"), - message: this.i18nService.t("contactYourProvider"), - }); - - return; - } - - if ( - billingMetadata?.organizationOccupiedSeats === organization.seats && - isFixedSeatPlan(organization.productTierType) - ) { - await this.handleSeatLimitForFixedTiers(organization); - - return; + const seatLimitResult = this.billingConstraint.checkSeatLimit(organization, billingMetadata); + if (!(await this.billingConstraint.seatLimitReached(seatLimitResult, organization))) { + await this.handleInviteDialog(organization); + this.billingConstraint.refreshBillingMetadata(); } - - await this.handleInviteDialog(organization); } async edit( @@ -624,7 +438,9 @@ export class MembersComponent extends BaseMembersComponent organization: Organization, initialTab: MemberDialogTab = MemberDialogTab.Role, ) { - const billingMetadata = await firstValueFrom(this.billingMetadata$); + const billingMetadata = await firstValueFrom( + this.billingConstraint.getBillingMetadata$(organization.id), + ); const dialog = openUserAddEditDialog(this.dialogService, { data: { kind: "Edit", @@ -737,16 +553,21 @@ export class MembersComponent extends BaseMembersComponent } try { - const response = this.organizationUserApiService.postManyOrganizationUserReinvite( - organization.id, + const result = await this.memberActionsService.bulkReinvite( + organization, filteredUsers.map((user) => user.id), ); + + if (!result.successful) { + throw new Error(); + } + // Bulk Status component open const dialogRef = BulkStatusComponent.open(this.dialogService, { data: { users: users, filteredUsers: filteredUsers, - request: response, + request: Promise.resolve(result.successful), successfulMessage: this.i18nService.t("bulkReinviteMessage"), }, }); @@ -916,12 +737,12 @@ export class MembersComponent extends BaseMembersComponent await this.deleteManagedMemberWarningService.acknowledgeWarning(organization.id); - this.actionPromise = this.organizationUserApiService.deleteOrganizationUser( - organization.id, - user.id, - ); + this.actionPromise2 = this.memberActionsService.deleteUser(organization, user.id); try { - await this.actionPromise; + const result = await this.actionPromise2; + if (!result.success) { + throw new Error(result.error); + } this.toastService.showToast({ variant: "success", message: this.i18nService.t("organizationUserDeleted", this.userNamePipe.transform(user)), @@ -973,14 +794,4 @@ export class MembersComponent extends BaseMembersComponent .getCheckedUsers() .every((member) => member.managedByOrganization && validStatuses.includes(member.status)); } - - async navigateToPaymentMethod(organization: Organization) { - const managePaymentDetailsOutsideCheckout = await this.configService.getFeatureFlag( - FeatureFlag.PM21881_ManagePaymentDetailsOutsideCheckout, - ); - const route = managePaymentDetailsOutsideCheckout ? "payment-details" : "payment-method"; - await this.router.navigate(["organizations", `${organization.id}`, "billing", route], { - state: { launchPaymentModalAutomatically: true }, - }); - } } diff --git a/apps/web/src/app/admin-console/organizations/members/members.module.ts b/apps/web/src/app/admin-console/organizations/members/members.module.ts index e5bc5f29a3b6..014dd5ba0a41 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.module.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.module.ts @@ -18,6 +18,13 @@ import { BulkStatusComponent } from "./components/bulk/bulk-status.component"; import { UserDialogModule } from "./components/member-dialog"; import { MembersRoutingModule } from "./members-routing.module"; import { MembersComponent } from "./members.component"; +import { + OrganizationMembersFacadeService, + MemberActionsService, + BillingConstraintService, + MemberPermissionsService, + MemberDialogManagerService, +} from "./services"; @NgModule({ imports: [ @@ -40,5 +47,12 @@ import { MembersComponent } from "./members.component"; MembersComponent, BulkDeleteDialogComponent, ], + providers: [ + OrganizationMembersFacadeService, + MemberActionsService, + BillingConstraintService, + MemberPermissionsService, + MemberDialogManagerService, + ], }) export class MembersModule {} diff --git a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts new file mode 100644 index 000000000000..59b3fc24a48a --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts @@ -0,0 +1,206 @@ +import { Injectable } from "@angular/core"; +import { Router } from "@angular/router"; +import { + lastValueFrom, + Observable, + BehaviorSubject, + combineLatest, + switchMap, + shareReplay, +} from "rxjs"; + +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction"; +import { isNotSelfUpgradable, ProductTierType } from "@bitwarden/common/billing/enums"; +import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { OrganizationId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; + +import { + ChangePlanDialogResultType, + openChangePlanDialog, +} from "../../../../../billing/organizations/change-plan-dialog.component"; +import { isFixedSeatPlan } from "../../components/member-dialog/validators/org-seat-limit-reached.validator"; + +export interface SeatLimitResult { + canAddUsers: boolean; + reason?: "reseller-limit" | "fixed-seat-limit" | "no-billing-permission"; + shouldShowUpgradeDialog?: boolean; +} + +@Injectable() +export class BillingConstraintService { + private refreshTrigger$ = new BehaviorSubject(undefined); + + constructor( + private i18nService: I18nService, + private dialogService: DialogService, + private toastService: ToastService, + private router: Router, + private billingApiService: BillingApiServiceAbstraction, + ) {} + + getBillingMetadata$( + organizationId: OrganizationId, + ): Observable { + return combineLatest([this.refreshTrigger$]).pipe( + switchMap(([_]) => this.billingApiService.getOrganizationBillingMetadata(organizationId)), + shareReplay({ bufferSize: 1, refCount: false }), + ); + } + + refreshBillingMetadata(): void { + this.refreshTrigger$.next(); + } + + checkSeatLimit( + organization: Organization, + billingMetadata: OrganizationBillingMetadataResponse, + ): SeatLimitResult { + const occupiedSeats = billingMetadata?.organizationOccupiedSeats ?? 0; + const totalSeats = organization.seats; + + if (occupiedSeats < totalSeats) { + return { canAddUsers: true }; + } + + if (organization.hasReseller) { + return { + canAddUsers: false, + reason: "reseller-limit", + }; + } + + if (isFixedSeatPlan(organization.productTierType)) { + return { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: organization.canEditSubscription, + }; + } + + return { canAddUsers: true }; + } + + async seatLimitReached(result: SeatLimitResult, organization: Organization): Promise { + if (result.canAddUsers) { + return false; + } + + switch (result.reason) { + case "reseller-limit": + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("seatLimitReached"), + message: this.i18nService.t("contactYourProvider"), + }); + return true; + + case "fixed-seat-limit": + if (result.shouldShowUpgradeDialog) { + return await this.handleFixedSeatLimitUpgrade(organization); + } else { + await this.showSeatLimitReachedDialog(organization); + return true; + } + + default: + return true; + } + } + + private async handleFixedSeatLimitUpgrade(organization: Organization): Promise { + const reference = openChangePlanDialog(this.dialogService, { + data: { + organizationId: organization.id, + productTierType: organization.productTierType, + }, + }); + + const result = await lastValueFrom(reference.closed); + return result === ChangePlanDialogResultType.Submitted; + } + + private async showSeatLimitReachedDialog(organization: Organization): Promise { + const dialogContent = this.getDialogContent(organization); + const acceptButtonText = this.getAcceptButtonText(organization); + + const orgUpgradeSimpleDialogOpts = { + title: this.i18nService.t("upgradeOrganization"), + content: dialogContent, + type: "primary" as const, + acceptButtonText, + cancelButtonText: organization.canEditSubscription ? undefined : (null as string | null), + }; + + const simpleDialog = this.dialogService.openSimpleDialogRef(orgUpgradeSimpleDialogOpts); + const result = await lastValueFrom(simpleDialog.closed); + + if (result && organization.canEditSubscription) { + await this.handleUpgradeNavigation(organization); + } + } + + private async handleUpgradeNavigation(organization: Organization): Promise { + const productType = organization.productTierType; + + if (isNotSelfUpgradable(productType)) { + throw new Error(`Unsupported product type: ${organization.productTierType}`); + } + + await this.router.navigate(["/organizations", organization.id, "billing", "subscription"], { + queryParams: { upgrade: true }, + }); + } + + private getDialogContent(organization: Organization): string { + const productKey = this.getProductKey(organization); + return this.i18nService.t(productKey, organization.seats); + } + + private getAcceptButtonText(organization: Organization): string { + if (!organization.canEditSubscription) { + return this.i18nService.t("ok"); + } + + const productType = organization.productTierType; + + if (isNotSelfUpgradable(productType)) { + throw new Error(`Unsupported product type: ${productType}`); + } + + return this.i18nService.t("upgrade"); + } + + private getProductKey(organization: Organization): string { + const manageBillingText = organization.canEditSubscription + ? "ManageBilling" + : "NoManageBilling"; + + let product = ""; + switch (organization.productTierType) { + case ProductTierType.Free: + product = "freeOrg"; + break; + case ProductTierType.TeamsStarter: + product = "teamsStarterPlan"; + break; + case ProductTierType.Families: + product = "familiesPlan"; + break; + default: + throw new Error(`Unsupported product type: ${organization.productTierType}`); + } + return `${product}InvLimitReached${manageBillingText}`; + } + + async navigateToPaymentMethod(organization: Organization): Promise { + await this.router.navigate( + ["organizations", `${organization.id}`, "billing", "payment-method"], + { + state: { launchPaymentModalAutomatically: true }, + }, + ); + } +} diff --git a/apps/web/src/app/admin-console/organizations/members/services/index.ts b/apps/web/src/app/admin-console/organizations/members/services/index.ts new file mode 100644 index 000000000000..ef294463b51f --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/index.ts @@ -0,0 +1,7 @@ +export { OrganizationMembersFacadeService } from "./organization-members-facade/organization-members-facade.service"; +export { MemberActionsService } from "./member-actions/member-actions.service"; +export { BillingConstraintService } from "./billing-constraint/billing-constraint.service"; +export { MemberPermissionsService } from "./member-permissions/member-permissions.service"; +export { MemberDialogManagerService } from "./member-dialog-manager/member-dialog-manager.service"; +export { DeleteManagedMemberWarningService } from "./delete-managed-member/delete-managed-member-warning.service"; +export { OrganizationUserService } from "./organization-user/organization-user.service"; diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts new file mode 100644 index 000000000000..9902f83c0900 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -0,0 +1,238 @@ +import { Injectable } from "@angular/core"; +import { firstValueFrom, switchMap, map } from "rxjs"; + +import { + OrganizationUserApiService, + OrganizationUserBulkResponse, + OrganizationUserConfirmRequest, +} from "@bitwarden/admin-console/common"; +import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; +import { + OrganizationUserType, + OrganizationUserStatusType, +} from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request"; +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 { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { ListResponse } from "@bitwarden/common/models/response/list.response"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; +import { KeyService } from "@bitwarden/key-management"; + +import { OrganizationUserView } from "../../../core/views/organization-user.view"; +import { OrganizationUserService } from "../organization-user/organization-user.service"; + +export interface MemberActionResult { + success: boolean; + error?: string; +} + +export interface BulkActionResult { + successful?: ListResponse; + failed: { id: string; error: string }[]; +} + +@Injectable() +export class MemberActionsService { + private userId$ = this.accountService.activeAccount$.pipe(getUserId); + + constructor( + private organizationUserApiService: OrganizationUserApiService, + private organizationApiService: OrganizationApiServiceAbstraction, + private organizationUserService: OrganizationUserService, + private keyService: KeyService, + private encryptService: EncryptService, + private syncService: SyncService, + private configService: ConfigService, + private accountService: AccountService, + ) {} + + async inviteUser( + organization: Organization, + email: string, + type: OrganizationUserType, + permissions?: any, + collections?: any[], + groups?: string[], + ): Promise { + try { + await this.organizationUserApiService.postOrganizationUserInvite(organization.id, { + emails: [email], + type, + accessSecretsManager: false, + collections: collections ?? [], + groups: groups ?? [], + permissions, + }); + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async removeUser(organization: Organization, userId: string): Promise { + try { + await this.organizationUserApiService.removeOrganizationUser(organization.id, userId); + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async revokeUser(organization: Organization, userId: string): Promise { + try { + await this.organizationUserApiService.revokeOrganizationUser(organization.id, userId); + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async restoreUser(organization: Organization, userId: string): Promise { + try { + await this.organizationUserApiService.restoreOrganizationUser(organization.id, userId); + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async deleteUser(organization: Organization, userId: string): Promise { + try { + await this.organizationUserApiService.deleteOrganizationUser(organization.id, userId); + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async reinviteUser(organization: Organization, userId: string): Promise { + try { + await this.organizationUserApiService.postOrganizationUserReinvite(organization.id, userId); + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async confirmUser( + user: OrganizationUserView, + publicKey: Uint8Array, + organization: Organization, + ): Promise { + try { + if ( + await firstValueFrom(this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation)) + ) { + await firstValueFrom( + this.organizationUserService.confirmUser(organization, user, publicKey), + ); + } else { + const request = await firstValueFrom( + this.userId$.pipe( + switchMap((userId) => this.keyService.orgKeys$(userId)), + map((orgKeys) => { + if (orgKeys == null || orgKeys[organization.id] == null) { + throw new Error("Organization keys not found for provided User."); + } + return orgKeys[organization.id]; + }), + switchMap((orgKey) => this.encryptService.encapsulateKeyUnsigned(orgKey, publicKey)), + map((encKey) => { + const req = new OrganizationUserConfirmRequest(); + req.key = encKey.encryptedString; + return req; + }), + ), + ); + + await this.organizationUserApiService.postOrganizationUserConfirm( + organization.id, + user.id, + request, + ); + } + return { success: true }; + } catch (error) { + return { success: false, error: (error as Error).message ?? String(error) }; + } + } + + async ensureOrganizationKeys(organization: Organization): Promise { + if (organization.canManageUsersPassword && !organization.hasPublicAndPrivateKeys) { + const orgShareKey = await firstValueFrom( + this.userId$.pipe( + switchMap((userId) => this.keyService.orgKeys$(userId)), + map((orgKeys) => { + if (orgKeys == null || orgKeys[organization.id] == null) { + throw new Error("Organization keys not found for provided User."); + } + return orgKeys[organization.id]; + }), + ), + ); + + const [orgPublicKey, encryptedOrgPrivateKey] = await this.keyService.makeKeyPair(orgShareKey); + if (encryptedOrgPrivateKey.encryptedString == null) { + throw new Error("Encrypted private key is null."); + } + const request = new OrganizationKeysRequest( + orgPublicKey, + encryptedOrgPrivateKey.encryptedString, + ); + const response = await this.organizationApiService.updateKeys(organization.id, request); + if (response != null) { + await this.syncService.fullSync(true); + } + } + } + + async bulkReinvite(organization: Organization, userIds: string[]): Promise { + try { + const result = await this.organizationUserApiService.postManyOrganizationUserReinvite( + organization.id, + userIds, + ); + return { successful: result, failed: [] }; + } catch (error) { + return { + failed: userIds.map((id) => ({ id, error: (error as Error).message ?? String(error) })), + }; + } + } + + allowResetPassword( + orgUser: OrganizationUserView, + organization: Organization, + resetPasswordEnabled: boolean, + ): boolean { + let callingUserHasPermission = false; + + switch (organization.type) { + case OrganizationUserType.Owner: + callingUserHasPermission = true; + break; + case OrganizationUserType.Admin: + callingUserHasPermission = orgUser.type !== OrganizationUserType.Owner; + break; + case OrganizationUserType.Custom: + callingUserHasPermission = + orgUser.type !== OrganizationUserType.Owner && + orgUser.type !== OrganizationUserType.Admin; + break; + } + + return ( + organization.canManageUsersPassword && + callingUserHasPermission && + organization.useResetPassword && + organization.hasPublicAndPrivateKeys && + orgUser.resetPasswordEnrolled && + resetPasswordEnabled && + orgUser.status === OrganizationUserStatusType.Confirmed + ); + } +} diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts new file mode 100644 index 000000000000..b109bfc4b760 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts @@ -0,0 +1,355 @@ +import { Injectable } from "@angular/core"; +import { firstValueFrom, lastValueFrom } from "rxjs"; + +import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; +import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { OrganizationId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; + +import { OrganizationUserView } from "../../../core/views/organization-user.view"; +import { openEntityEventsDialog } from "../../../manage/entity-events.component"; +import { + AccountRecoveryDialogComponent, + AccountRecoveryDialogResultType, +} from "../../components/account-recovery/account-recovery-dialog.component"; +import { BulkConfirmDialogComponent } from "../../components/bulk/bulk-confirm-dialog.component"; +import { BulkDeleteDialogComponent } from "../../components/bulk/bulk-delete-dialog.component"; +import { BulkEnableSecretsManagerDialogComponent } from "../../components/bulk/bulk-enable-sm-dialog.component"; +import { BulkRemoveDialogComponent } from "../../components/bulk/bulk-remove-dialog.component"; +import { BulkRestoreRevokeComponent } from "../../components/bulk/bulk-restore-revoke.component"; +import { BulkStatusComponent } from "../../components/bulk/bulk-status.component"; +import { + MemberDialogResult, + MemberDialogTab, + openUserAddEditDialog, +} from "../../components/member-dialog"; +import { DeleteManagedMemberWarningService } from "../delete-managed-member/delete-managed-member-warning.service"; + +export interface DialogResult { + result: T; + action?: "saved" | "deleted" | "revoked" | "restored" | "confirmed"; +} + +@Injectable() +export class MemberDialogManagerService { + constructor( + private dialogService: DialogService, + private i18nService: I18nService, + private toastService: ToastService, + private userNamePipe: UserNamePipe, + private deleteManagedMemberWarningService: DeleteManagedMemberWarningService, + ) {} + + async openInviteDialog( + organization: Organization, + billingMetadata: OrganizationBillingMetadataResponse, + allUserEmails: string[], + ): Promise> { + const dialog = openUserAddEditDialog(this.dialogService, { + data: { + kind: "Add", + organizationId: organization.id, + allOrganizationUserEmails: allUserEmails, + occupiedSeatCount: billingMetadata?.organizationOccupiedSeats ?? 0, + isOnSecretsManagerStandalone: billingMetadata?.isOnSecretsManagerStandalone ?? false, + }, + }); + + const result = await lastValueFrom(dialog.closed); + return { + result: result ?? MemberDialogResult.Canceled, + action: result === MemberDialogResult.Saved ? "saved" : undefined, + }; + } + + async openEditDialog( + user: OrganizationUserView, + organization: Organization, + billingMetadata: OrganizationBillingMetadataResponse, + initialTab: MemberDialogTab = MemberDialogTab.Role, + ): Promise> { + const dialog = openUserAddEditDialog(this.dialogService, { + data: { + kind: "Edit", + name: this.userNamePipe.transform(user), + organizationId: organization.id, + organizationUserId: user.id, + usesKeyConnector: user.usesKeyConnector, + isOnSecretsManagerStandalone: billingMetadata?.isOnSecretsManagerStandalone ?? false, + initialTab: initialTab, + managedByOrganization: user.managedByOrganization, + }, + }); + + const result = await lastValueFrom(dialog.closed); + const dialogResult = result ?? MemberDialogResult.Canceled; + let action: DialogResult["action"]; + switch (dialogResult) { + case MemberDialogResult.Saved: + action = "saved"; + break; + case MemberDialogResult.Deleted: + action = "deleted"; + break; + case MemberDialogResult.Revoked: + action = "revoked"; + break; + case MemberDialogResult.Restored: + action = "restored"; + break; + default: + action = undefined; + } + + return { result: dialogResult, action }; + } + + async openAccountRecoveryDialog( + user: OrganizationUserView, + organization: Organization, + ): Promise> { + const dialogRef = AccountRecoveryDialogComponent.open(this.dialogService, { + data: { + name: this.userNamePipe.transform(user), + email: user.email, + organizationId: organization.id as OrganizationId, + organizationUserId: user.id, + }, + }); + + const result = await lastValueFrom(dialogRef.closed); + return { result: result ?? AccountRecoveryDialogResultType.Ok }; + } + + async openBulkConfirmDialog( + organization: Organization, + users: OrganizationUserView[], + ): Promise> { + const dialogRef = BulkConfirmDialogComponent.open(this.dialogService, { + data: { + organization: organization, + users: users, + }, + }); + + await lastValueFrom(dialogRef.closed); + return { result: undefined, action: "confirmed" }; + } + + async openBulkRemoveDialog( + organization: Organization, + users: OrganizationUserView[], + ): Promise> { + const dialogRef = BulkRemoveDialogComponent.open(this.dialogService, { + data: { + organizationId: organization.id, + users: users, + }, + }); + + await lastValueFrom(dialogRef.closed); + return { result: undefined }; + } + + async openBulkDeleteDialog( + organization: Organization, + users: OrganizationUserView[], + ): Promise> { + const warningAcknowledged = await firstValueFrom( + this.deleteManagedMemberWarningService.warningAcknowledged(organization.id), + ); + + if ( + !warningAcknowledged && + organization.canManageUsers && + organization.productTierType === ProductTierType.Enterprise + ) { + const acknowledged = await this.deleteManagedMemberWarningService.showWarning(); + if (!acknowledged) { + return { result: undefined }; + } + } + + const dialogRef = BulkDeleteDialogComponent.open(this.dialogService, { + data: { + organizationId: organization.id, + users: users, + }, + }); + + await lastValueFrom(dialogRef.closed); + return { result: undefined, action: "deleted" }; + } + + async openBulkRestoreRevokeDialog( + organization: Organization, + users: OrganizationUserView[], + isRevoking: boolean, + ): Promise> { + const ref = BulkRestoreRevokeComponent.open(this.dialogService, { + organizationId: organization.id, + users: users, + isRevoking: isRevoking, + }); + + await firstValueFrom(ref.closed); + return { result: undefined, action: isRevoking ? "revoked" : "restored" }; + } + + async openBulkEnableSecretsManagerDialog( + organization: Organization, + users: OrganizationUserView[], + ): Promise> { + const eligibleUsers = users.filter((ou) => !ou.accessSecretsManager); + + if (eligibleUsers.length === 0) { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: this.i18nService.t("noSelectedUsersApplicable"), + }); + return { result: undefined }; + } + + const dialogRef = BulkEnableSecretsManagerDialogComponent.open(this.dialogService, { + orgId: organization.id, + users: eligibleUsers, + }); + + await lastValueFrom(dialogRef.closed); + return { result: undefined }; + } + + async openBulkStatusDialog( + users: OrganizationUserView[], + filteredUsers: OrganizationUserView[], + request: Promise, + successMessage: string, + ): Promise> { + const dialogRef = BulkStatusComponent.open(this.dialogService, { + data: { + users: users, + filteredUsers: filteredUsers, + request: request, + successfulMessage: successMessage, + }, + }); + + await lastValueFrom(dialogRef.closed); + return { result: undefined }; + } + + openEventsDialog(user: OrganizationUserView, organization: Organization): void { + openEntityEventsDialog(this.dialogService, { + data: { + name: this.userNamePipe.transform(user), + organizationId: organization.id, + entityId: user.id, + showUser: false, + entity: "user", + }, + }); + } + + async openRemoveUserConfirmationDialog(user: OrganizationUserView): Promise { + const content = user.usesKeyConnector + ? "removeUserConfirmationKeyConnector" + : "removeOrgUserConfirmation"; + + const confirmed = await this.dialogService.openSimpleDialog({ + title: { + key: "removeUserIdAccess", + placeholders: [this.userNamePipe.transform(user)], + }, + content: { key: content }, + type: "warning", + }); + + if (!confirmed) { + return false; + } + + if (user.status > 0 && user.hasMasterPassword === false) { + return await this.openNoMasterPasswordConfirmationDialog(user); + } + + return true; + } + + async openRevokeUserConfirmationDialog(user: OrganizationUserView): Promise { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "revokeAccess", placeholders: [this.userNamePipe.transform(user)] }, + content: this.i18nService.t("revokeUserConfirmation"), + acceptButtonText: { key: "revokeAccess" }, + type: "warning", + }); + + if (!confirmed) { + return false; + } + + if (user.status > 0 && user.hasMasterPassword === false) { + return await this.openNoMasterPasswordConfirmationDialog(user); + } + + return true; + } + + async openDeleteUserConfirmationDialog( + user: OrganizationUserView, + organization: Organization, + ): Promise { + const warningAcknowledged = await firstValueFrom( + this.deleteManagedMemberWarningService.warningAcknowledged(organization.id), + ); + + if ( + !warningAcknowledged && + organization.canManageUsers && + organization.productTierType === ProductTierType.Enterprise + ) { + const acknowledged = await this.deleteManagedMemberWarningService.showWarning(); + if (!acknowledged) { + return false; + } + } + + const confirmed = await this.dialogService.openSimpleDialog({ + title: { + key: "deleteOrganizationUser", + placeholders: [this.userNamePipe.transform(user)], + }, + content: { + key: "deleteOrganizationUserWarningDesc", + placeholders: [this.userNamePipe.transform(user)], + }, + type: "warning", + acceptButtonText: { key: "delete" }, + cancelButtonText: { key: "cancel" }, + }); + + if (confirmed) { + await this.deleteManagedMemberWarningService.acknowledgeWarning(organization.id); + } + + return confirmed; + } + + private async openNoMasterPasswordConfirmationDialog( + user: OrganizationUserView, + ): Promise { + return this.dialogService.openSimpleDialog({ + title: { + key: "removeOrgUserNoMasterPasswordTitle", + }, + content: { + key: "removeOrgUserNoMasterPasswordDesc", + placeholders: [this.userNamePipe.transform(user)], + }, + type: "warning", + }); + } +} diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts new file mode 100644 index 000000000000..450dbc398ba5 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts @@ -0,0 +1,145 @@ +import { Injectable } from "@angular/core"; + +import { OrganizationUserUserDetailsResponse } from "@bitwarden/admin-console/common"; +import { + OrganizationUserType, + OrganizationUserStatusType, +} from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; + +import { OrganizationUserView } from "../../../core/views/organization-user.view"; + +@Injectable() +export class MemberPermissionsService { + canManageUsers(organization: Organization): boolean { + return organization?.canManageUsers ?? false; + } + + canUseSecretsManager(organization: Organization): boolean { + return organization?.useSecretsManager ?? false; + } + + showUserManagementControls(organization: Organization): boolean { + return organization?.canManageUsers ?? false; + } + + allowResetPassword( + orgUser: OrganizationUserView, + organization: Organization, + resetPasswordPolicyEnabled: boolean, + ): boolean { + let callingUserHasPermission = false; + + switch (organization.type) { + case OrganizationUserType.Owner: + callingUserHasPermission = true; + break; + case OrganizationUserType.Admin: + callingUserHasPermission = orgUser.type !== OrganizationUserType.Owner; + break; + case OrganizationUserType.Custom: + callingUserHasPermission = + orgUser.type !== OrganizationUserType.Owner && + orgUser.type !== OrganizationUserType.Admin; + break; + } + + return ( + organization.canManageUsersPassword && + callingUserHasPermission && + organization.useResetPassword && + organization.hasPublicAndPrivateKeys && + orgUser.resetPasswordEnrolled && + resetPasswordPolicyEnabled && + orgUser.status === OrganizationUserStatusType.Confirmed + ); + } + + showEnrolledStatus( + orgUser: OrganizationUserUserDetailsResponse, + organization: Organization, + resetPasswordPolicyEnabled: boolean, + ): boolean { + return ( + organization.useResetPassword && orgUser.resetPasswordEnrolled && resetPasswordPolicyEnabled + ); + } + + canDeleteUser(user: OrganizationUserView): boolean { + const validStatuses = [ + OrganizationUserStatusType.Accepted, + OrganizationUserStatusType.Confirmed, + OrganizationUserStatusType.Revoked, + ]; + + return user.managedByOrganization && validStatuses.includes(user.status); + } + + canRemoveUser(user: OrganizationUserView): boolean { + return !user.managedByOrganization; + } + + canRevokeUser(user: OrganizationUserView): boolean { + return user.status !== OrganizationUserStatusType.Revoked; + } + + canRestoreUser(user: OrganizationUserView): boolean { + return user.status === OrganizationUserStatusType.Revoked; + } + + canReinviteUser(user: OrganizationUserView): boolean { + return user.status === OrganizationUserStatusType.Invited; + } + + canConfirmUser(user: OrganizationUserView): boolean { + return user.status === OrganizationUserStatusType.Accepted; + } + + requiresManagedUserWarning(organization: Organization): boolean { + return ( + organization.canManageUsers && organization.productTierType === ProductTierType.Enterprise + ); + } + + canEditUser(user: OrganizationUserView, organization: Organization): boolean { + if (!organization.canManageUsers) { + return false; + } + + // Add additional permission checks as needed + return true; + } + + canViewEvents(user: OrganizationUserView, organization: Organization): boolean { + return organization.useEvents && user.status === OrganizationUserStatusType.Confirmed; + } + + canEnableSecretsManager(user: OrganizationUserView, organization: Organization): boolean { + return organization.useSecretsManager && !user.accessSecretsManager; + } + + getBulkActionPermissions(users: OrganizationUserView[]) { + return { + showBulkRemove: users.every((user) => this.canRemoveUser(user)), + showBulkDelete: users.every((user) => this.canDeleteUser(user)), + showBulkRevoke: users.every((user) => this.canRevokeUser(user)), + showBulkRestore: users.every((user) => this.canRestoreUser(user)), + showBulkReinvite: users.every((user) => this.canReinviteUser(user)), + showBulkConfirm: users.every((user) => this.canConfirmUser(user)), + }; + } + + shouldShowConfirmBanner( + activeUserCount: number, + confirmedUserCount: number, + acceptedUserCount: number, + ): boolean { + return ( + activeUserCount > 1 && + confirmedUserCount > 0 && + confirmedUserCount < 3 && + acceptedUserCount > 0 + ); + } +} diff --git a/apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts b/apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts new file mode 100644 index 000000000000..37b496255f0e --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts @@ -0,0 +1,163 @@ +import { Injectable } from "@angular/core"; +import { + Observable, + combineLatest, + switchMap, + map, + filter, + shareReplay, + BehaviorSubject, + from, +} from "rxjs"; + +import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { + getOrganizationById, + OrganizationService, +} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyApiServiceAbstraction as PolicyApiService } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { OrganizationId } from "@bitwarden/common/types/guid"; + +import { GroupApiService } from "../../../core"; +import { OrganizationUserView } from "../../../core/views/organization-user.view"; +import { BillingConstraintService } from "../billing-constraint/billing-constraint.service"; + +export interface OrganizationMemberState { + organization: Organization | null; + users: OrganizationUserView[]; + policies: Policy[]; + resetPasswordPolicyEnabled: boolean; + loading: boolean; + error: string | null; +} + +@Injectable() +export class OrganizationMembersFacadeService { + private refreshTrigger$ = new BehaviorSubject(undefined); + + constructor( + private organizationService: OrganizationService, + private organizationUserApiService: OrganizationUserApiService, + private policyService: PolicyService, + private policyApiService: PolicyApiService, + private billingConstraintService: BillingConstraintService, + private groupService: GroupApiService, + private apiService: ApiService, + private accountService: AccountService, + ) {} + + getMemberState(organizationId: OrganizationId): Observable { + const userId$ = this.accountService.activeAccount$.pipe(getUserId); + + const organization$ = userId$.pipe( + switchMap((userId) => + this.organizationService.organizations$(userId).pipe(getOrganizationById(organizationId)), + ), + filter((organization): organization is Organization => organization != null), + shareReplay({ refCount: true, bufferSize: 1 }), + ); + + const policies$ = combineLatest([userId$, organization$]).pipe( + switchMap(([userId, organization]) => + organization.isProviderUser + ? from(this.policyApiService.getPolicies(organization.id)).pipe( + map((response) => Policy.fromListResponse(response)), + ) + : this.policyService.policies$(userId), + ), + ); + + const users$ = combineLatest([this.refreshTrigger$, organization$]).pipe( + switchMap(([_, organization]) => this.loadUsers(organization)), + ); + + return combineLatest([organization$, users$, policies$]).pipe( + map(([organization, users, policies]) => { + const resetPasswordPolicy = policies + .filter((policy) => policy.type === PolicyType.ResetPassword) + .find((p) => p.organizationId === organization.id); + + return { + organization, + users, + policies, + resetPasswordPolicyEnabled: resetPasswordPolicy?.enabled ?? false, + loading: false, // Data is loaded when we reach this point + error: null, + }; + }), + shareReplay({ refCount: true, bufferSize: 1 }), + ); + } + + refreshData(): void { + this.refreshTrigger$.next(); + this.billingConstraintService.refreshBillingMetadata(); + } + + private async loadUsers(organization: Organization): Promise { + let groupsPromise: Promise> | undefined; + let collectionsPromise: Promise> | undefined; + + const userPromise = this.organizationUserApiService.getAllUsers(organization.id, { + includeGroups: organization.useGroups, + includeCollections: !organization.useGroups, + }); + + if (organization.useGroups) { + groupsPromise = this.getGroupNameMap(organization); + } else { + collectionsPromise = this.getCollectionNameMap(organization); + } + + const [usersResponse, groupNamesMap, collectionNamesMap] = await Promise.all([ + userPromise, + groupsPromise, + collectionsPromise, + ]); + + return ( + usersResponse.data?.map((r) => { + const userView = OrganizationUserView.fromResponse(r); + + userView.groupNames = userView.groups + .map((g: string) => groupNamesMap?.get(g)) + .filter((name): name is string => name != null) + .sort(); + userView.collectionNames = userView.collections + .map((c: { id: string }) => collectionNamesMap?.get(c.id)) + .filter((name): name is string => name != null) + .sort(); + + return userView; + }) ?? [] + ); + } + + private async getGroupNameMap(organization: Organization): Promise> { + const groups = await this.groupService.getAll(organization.id); + const groupNameMap = new Map(); + groups.forEach((g: { id: string; name: string }) => groupNameMap.set(g.id, g.name)); + return groupNameMap; + } + + private async getCollectionNameMap(organization: Organization): Promise> { + const response = this.apiService + .getCollections(organization.id) + .then((res) => + res.data.map((r: { id: string; name: string }) => ({ id: r.id, name: r.name })), + ); + + const collections = await response; + const collectionMap = new Map(); + collections.forEach((c: { id: string; name: string }) => collectionMap.set(c.id, c.name)); + return collectionMap; + } +} From 7a67efeddbb0f037dcea59024469187e59c5212a Mon Sep 17 00:00:00 2001 From: Brandon Date: Fri, 26 Sep 2025 14:09:02 -0400 Subject: [PATCH 02/10] replace dialog logic and user logic with service implementations --- .../members/members.component.ts | 421 +++--------------- .../organizations/members/members.module.ts | 6 +- .../organizations/members/services/index.ts | 3 +- .../member-dialog-manager.service.ts | 61 +-- .../member-permissions.service.ts | 145 ------ .../organization-members.service.ts} | 30 +- 6 files changed, 91 insertions(+), 575 deletions(-) delete mode 100644 apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts rename apps/web/src/app/admin-console/organizations/members/services/{organization-members-facade/organization-members-facade.service.ts => organization-members-service/organization-members.service.ts} (86%) diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index dbbd2af43cd0..f9a22322009f 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -2,13 +2,10 @@ import { Component, computed, Signal } from "@angular/core"; import { takeUntilDestroyed, toSignal } from "@angular/core/rxjs-interop"; import { ActivatedRoute } from "@angular/router"; import { - BehaviorSubject, combineLatest, concatMap, filter, firstValueFrom, - from, - lastValueFrom, map, merge, Observable, @@ -17,67 +14,36 @@ import { take, } from "rxjs"; -import { - OrganizationUserApiService, - OrganizationUserUserDetailsResponse, - CollectionService, - CollectionData, - Collection, - CollectionDetailsResponse, -} from "@bitwarden/admin-console/common"; +import { OrganizationUserUserDetailsResponse } from "@bitwarden/admin-console/common"; import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationManagementPreferencesService } from "@bitwarden/common/admin-console/abstractions/organization-management-preferences/organization-management-preferences.service"; -import { PolicyApiServiceAbstraction as PolicyApiService } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; -import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { OrganizationUserStatusType, OrganizationUserType, - PolicyType, } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { ProductTierType } from "@bitwarden/common/billing/enums"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; -import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { DialogService, ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; import { BaseMembersComponent } from "../../common/base-members.component"; import { PeopleTableDataSource } from "../../common/people-table-data-source"; -import { GroupApiService } from "../core"; import { OrganizationUserView } from "../core/views/organization-user.view"; -import { openEntityEventsDialog } from "../manage/entity-events.component"; +import { AccountRecoveryDialogResultType } from "./components/account-recovery/account-recovery-dialog.component"; +import { MemberDialogResult, MemberDialogTab } from "./components/member-dialog"; import { - AccountRecoveryDialogComponent, - AccountRecoveryDialogResultType, -} from "./components/account-recovery/account-recovery-dialog.component"; -import { BulkConfirmDialogComponent } from "./components/bulk/bulk-confirm-dialog.component"; -import { BulkDeleteDialogComponent } from "./components/bulk/bulk-delete-dialog.component"; -import { BulkEnableSecretsManagerDialogComponent } from "./components/bulk/bulk-enable-sm-dialog.component"; -import { BulkRemoveDialogComponent } from "./components/bulk/bulk-remove-dialog.component"; -import { BulkRestoreRevokeComponent } from "./components/bulk/bulk-restore-revoke.component"; -import { BulkStatusComponent } from "./components/bulk/bulk-status.component"; -import { - MemberDialogResult, - MemberDialogTab, - openUserAddEditDialog, -} from "./components/member-dialog"; -import { BillingConstraintService } from "./services"; + BillingConstraintService, + MemberDialogManagerService, + OrganizationMembersService, +} from "./services"; import { DeleteManagedMemberWarningService } from "./services/delete-managed-member/delete-managed-member-warning.service"; import { MemberActionsService } from "./services/member-actions/member-actions.service"; -import { OrganizationUserService } from "./services/organization-user/organization-user.service"; class MembersTableDataSource extends PeopleTableDataSource { protected statusType = OrganizationUserStatusType; @@ -103,15 +69,12 @@ export class MembersComponent extends BaseMembersComponent protected showUserManagementControls: Signal = computed( () => this.organization()?.canManageUsers ?? false, ); - private refreshBillingMetadata$: BehaviorSubject = new BehaviorSubject(null); protected billingMetadata$: Observable; // Fixed sizes used for cdkVirtualScroll protected rowHeight = 66; protected rowHeightClass = `tw-h-[66px]`; - private userId$: Observable = this.accountService.activeAccount$.pipe(getUserId); - constructor( apiService: ApiService, i18nService: I18nService, @@ -122,19 +85,13 @@ export class MembersComponent extends BaseMembersComponent userNamePipe: UserNamePipe, dialogService: DialogService, toastService: ToastService, - private policyService: PolicyService, - private policyApiService: PolicyApiService, private route: ActivatedRoute, - private organizationService: OrganizationService, - private accountService: AccountService, - private organizationUserApiService: OrganizationUserApiService, - private groupService: GroupApiService, - private collectionService: CollectionService, protected deleteManagedMemberWarningService: DeleteManagedMemberWarningService, - private organizationUserService: OrganizationUserService, private organizationWarningsService: OrganizationWarningsService, private memberActionsService: MemberActionsService, + private memberDialogManager: MemberDialogManagerService, protected billingConstraint: BillingConstraintService, + protected memberService: OrganizationMembersService, ) { super( apiService, @@ -148,55 +105,39 @@ export class MembersComponent extends BaseMembersComponent toastService, ); - const organization$ = this.route.params.pipe( - concatMap((params) => - this.userId$.pipe( - switchMap((userId) => - this.organizationService - .organizations$(userId) - .pipe(getOrganizationById(params.organizationId)), - ), - ), - ), + const memberState$ = this.route.params.pipe( + switchMap((params) => this.memberService.getMemberState(params.organizationId)), + shareReplay({ refCount: true, bufferSize: 1 }), + ); + + const organization$ = memberState$.pipe( + map((state) => state.organization), filter((organization): organization is Organization => organization != null), shareReplay({ refCount: true, bufferSize: 1 }), ); this.organization = toSignal(organization$); - const policies$ = combineLatest([this.userId$, organization$]).pipe( - switchMap(([userId, organization]) => - organization.isProviderUser - ? from(this.policyApiService.getPolicies(organization.id)).pipe( - map((response) => Policy.fromListResponse(response)), - ) - : this.policyService.policies$(userId), - ), - ); - - combineLatest([this.route.queryParams, policies$, organization$]) + combineLatest([this.route.queryParams, memberState$]) .pipe( - concatMap(async ([qParams, policies, organization]) => { + concatMap(async ([qParams, memberState]) => { // Backfill pub/priv key if necessary try { - await this.memberActionsService.ensureOrganizationKeys(organization); + await this.memberActionsService.ensureOrganizationKeys(memberState.organization!); } catch { throw new Error(this.i18nService.t("resetPasswordOrgKeysError")); } - const resetPasswordPolicy = policies - .filter((policy) => policy.type === PolicyType.ResetPassword) - .find((p) => p.organizationId === organization.id); - this.orgResetPasswordPolicyEnabled = resetPasswordPolicy?.enabled ?? false; + this.orgResetPasswordPolicyEnabled = memberState.resetPasswordPolicyEnabled; - await this.load(organization); + await this.load(memberState.organization!); this.searchControl.setValue(qParams.search); if (qParams.viewEvents != null) { const user = this.dataSource.data.filter((u) => u.id === qParams.viewEvents); if (user.length > 0 && user[0].status === OrganizationUserStatusType.Confirmed) { - this.openEventsDialog(user[0], organization); + this.openEventsDialog(user[0], memberState.organization!); } } }), @@ -227,88 +168,11 @@ export class MembersComponent extends BaseMembersComponent } override async load(organization: Organization) { - this.refreshBillingMetadata$.next(null); await super.load(organization); } async getUsers(organization: Organization): Promise { - let groupsPromise: Promise> | undefined; - let collectionsPromise: Promise> | undefined; - - // We don't need both groups and collections for the table, so only load one - const userPromise = this.organizationUserApiService.getAllUsers(organization.id, { - includeGroups: organization.useGroups, - includeCollections: !organization.useGroups, - }); - - // Depending on which column is displayed, we need to load the group/collection names - if (organization.useGroups) { - groupsPromise = this.getGroupNameMap(organization); - } else { - collectionsPromise = this.getCollectionNameMap(organization); - } - - const [usersResponse, groupNamesMap, collectionNamesMap] = await Promise.all([ - userPromise, - groupsPromise, - collectionsPromise, - ]); - - return ( - usersResponse.data?.map((r) => { - const userView = OrganizationUserView.fromResponse(r); - - userView.groupNames = userView.groups - .map((g) => groupNamesMap?.get(g)) - .filter((name): name is string => name != null) - .sort(this.i18nService.collator?.compare); - userView.collectionNames = userView.collections - .map((c) => collectionNamesMap?.get(c.id)) - .filter((name): name is string => name != null) - .sort(this.i18nService.collator?.compare); - - return userView; - }) ?? [] - ); - } - - async getGroupNameMap(organization: Organization): Promise> { - const groups = await this.groupService.getAll(organization.id); - const groupNameMap = new Map(); - groups.forEach((g) => groupNameMap.set(g.id, g.name)); - return groupNameMap; - } - - /** - * Retrieve a map of all collection IDs <-> names for the organization. - */ - async getCollectionNameMap(organization: Organization) { - const response = from(this.apiService.getCollections(organization.id)).pipe( - map((res) => - res.data.map((r) => - Collection.fromCollectionData(new CollectionData(r as CollectionDetailsResponse)), - ), - ), - ); - - const decryptedCollections$ = combineLatest([ - this.userId$.pipe( - switchMap((userId) => this.keyService.orgKeys$(userId)), - filter((orgKeys) => orgKeys != null), - ), - response, - ]).pipe( - switchMap(([orgKeys, collections]) => - this.collectionService.decryptMany$(collections, orgKeys), - ), - map((collections) => { - const collectionMap = new Map(); - collections.forEach((c) => collectionMap.set(c.id, c.name)); - return collectionMap; - }), - ); - - return await firstValueFrom(decryptedCollections$); + return await this.memberService.loadUsers(organization); } async removeUser(id: string, organization: Organization): Promise { @@ -407,17 +271,13 @@ export class MembersComponent extends BaseMembersComponent private async handleInviteDialog(organization: Organization) { const billingMetadata = await firstValueFrom(this.billingMetadata$); - const dialog = openUserAddEditDialog(this.dialogService, { - data: { - kind: "Add", - organizationId: organization.id, - allOrganizationUserEmails: this.dataSource.data?.map((user) => user.email) ?? [], - occupiedSeatCount: billingMetadata?.organizationOccupiedSeats ?? 0, - isOnSecretsManagerStandalone: billingMetadata?.isOnSecretsManagerStandalone ?? false, - }, - }); - - const result = await lastValueFrom(dialog.closed); + const allUserEmails = this.dataSource.data?.map((user) => user.email) ?? []; + + const result = await this.memberDialogManager.openInviteDialog( + organization, + billingMetadata, + allUserEmails, + ); if (result === MemberDialogResult.Saved) { await this.load(organization); @@ -441,20 +301,14 @@ export class MembersComponent extends BaseMembersComponent const billingMetadata = await firstValueFrom( this.billingConstraint.getBillingMetadata$(organization.id), ); - const dialog = openUserAddEditDialog(this.dialogService, { - data: { - kind: "Edit", - name: this.userNamePipe.transform(user), - organizationId: organization.id, - organizationUserId: user.id, - usesKeyConnector: user.usesKeyConnector, - isOnSecretsManagerStandalone: billingMetadata?.isOnSecretsManagerStandalone ?? false, - initialTab: initialTab, - managedByOrganization: user.managedByOrganization, - }, - }); - - const result = await lastValueFrom(dialog.closed); + + const result = await this.memberDialogManager.openEditDialog( + user, + organization, + billingMetadata, + initialTab, + ); + switch (result) { case MemberDialogResult.Deleted: this.dataSource.removeUser(user); @@ -472,43 +326,22 @@ export class MembersComponent extends BaseMembersComponent return; } - const dialogRef = BulkRemoveDialogComponent.open(this.dialogService, { - data: { - organizationId: organization.id, - users: this.dataSource.getCheckedUsers(), - }, - }); - await lastValueFrom(dialogRef.closed); + await this.memberDialogManager.openBulkRemoveDialog( + organization, + this.dataSource.getCheckedUsers(), + ); await this.load(organization); } async bulkDelete(organization: Organization) { - const warningAcknowledged = await firstValueFrom( - this.deleteManagedMemberWarningService.warningAcknowledged(organization.id), - ); - - if ( - !warningAcknowledged && - organization.canManageUsers && - organization.productTierType === ProductTierType.Enterprise - ) { - const acknowledged = await this.deleteManagedMemberWarningService.showWarning(); - if (!acknowledged) { - return; - } - } - if (this.actionPromise != null) { return; } - const dialogRef = BulkDeleteDialogComponent.open(this.dialogService, { - data: { - organizationId: organization.id, - users: this.dataSource.getCheckedUsers(), - }, - }); - await lastValueFrom(dialogRef.closed); + await this.memberDialogManager.openBulkDeleteDialog( + organization, + this.dataSource.getCheckedUsers(), + ); await this.load(organization); } @@ -525,13 +358,11 @@ export class MembersComponent extends BaseMembersComponent return; } - const ref = BulkRestoreRevokeComponent.open(this.dialogService, { - organizationId: organization.id, - users: this.dataSource.getCheckedUsers(), - isRevoking: isRevoking, - }); - - await firstValueFrom(ref.closed); + await this.memberDialogManager.openBulkRestoreRevokeDialog( + organization, + this.dataSource.getCheckedUsers(), + isRevoking, + ); await this.load(organization); } @@ -563,15 +394,12 @@ export class MembersComponent extends BaseMembersComponent } // Bulk Status component open - const dialogRef = BulkStatusComponent.open(this.dialogService, { - data: { - users: users, - filteredUsers: filteredUsers, - request: Promise.resolve(result.successful), - successfulMessage: this.i18nService.t("bulkReinviteMessage"), - }, - }); - await lastValueFrom(dialogRef.closed); + await this.memberDialogManager.openBulkStatusDialog( + users, + filteredUsers, + Promise.resolve(result.successful), + this.i18nService.t("bulkReinviteMessage"), + ); } catch (e) { this.validationService.showError(e); } @@ -583,49 +411,24 @@ export class MembersComponent extends BaseMembersComponent return; } - const dialogRef = BulkConfirmDialogComponent.open(this.dialogService, { - data: { - organization: organization, - users: this.dataSource.getCheckedUsers(), - }, - }); - - await lastValueFrom(dialogRef.closed); + await this.memberDialogManager.openBulkConfirmDialog( + organization, + this.dataSource.getCheckedUsers(), + ); await this.load(organization); } async bulkEnableSM(organization: Organization) { - const users = this.dataSource.getCheckedUsers().filter((ou) => !ou.accessSecretsManager); - - if (users.length === 0) { - this.toastService.showToast({ - variant: "error", - title: this.i18nService.t("errorOccurred"), - message: this.i18nService.t("noSelectedUsersApplicable"), - }); - return; - } + const users = this.dataSource.getCheckedUsers(); - const dialogRef = BulkEnableSecretsManagerDialogComponent.open(this.dialogService, { - orgId: organization.id, - users, - }); + await this.memberDialogManager.openBulkEnableSecretsManagerDialog(organization, users); - await lastValueFrom(dialogRef.closed); this.dataSource.uncheckAllUsers(); await this.load(organization); } openEventsDialog(user: OrganizationUserView, organization: Organization) { - openEntityEventsDialog(this.dialogService, { - data: { - name: this.userNamePipe.transform(user), - organizationId: organization.id, - entityId: user.id, - showUser: false, - entity: "user", - }, - }); + this.memberDialogManager.openEventsDialog(user, organization); } async resetPassword(user: OrganizationUserView, organization: Organization) { @@ -640,16 +443,7 @@ export class MembersComponent extends BaseMembersComponent return; } - const dialogRef = AccountRecoveryDialogComponent.open(this.dialogService, { - data: { - name: this.userNamePipe.transform(user), - email: user.email, - organizationId: organization.id as OrganizationId, - organizationUserId: user.id, - }, - }); - - const result = await lastValueFrom(dialogRef.closed); + const result = await this.memberDialogManager.openAccountRecoveryDialog(user, organization); if (result === AccountRecoveryDialogResultType.Ok) { await this.load(organization); } @@ -658,85 +452,23 @@ export class MembersComponent extends BaseMembersComponent } protected async removeUserConfirmationDialog(user: OrganizationUserView) { - const content = user.usesKeyConnector - ? "removeUserConfirmationKeyConnector" - : "removeOrgUserConfirmation"; - - const confirmed = await this.dialogService.openSimpleDialog({ - title: { - key: "removeUserIdAccess", - placeholders: [this.userNamePipe.transform(user)], - }, - content: { key: content }, - type: "warning", - }); - - if (!confirmed) { - return false; - } - - if (user.status > OrganizationUserStatusType.Invited && user.hasMasterPassword === false) { - return await this.noMasterPasswordConfirmationDialog(user); - } - - return true; + return await this.memberDialogManager.openRemoveUserConfirmationDialog(user); } protected async revokeUserConfirmationDialog(user: OrganizationUserView) { - const confirmed = await this.dialogService.openSimpleDialog({ - title: { key: "revokeAccess", placeholders: [this.userNamePipe.transform(user)] }, - content: this.i18nService.t("revokeUserConfirmation"), - acceptButtonText: { key: "revokeAccess" }, - type: "warning", - }); - - if (!confirmed) { - return false; - } - - if (user.status > OrganizationUserStatusType.Invited && user.hasMasterPassword === false) { - return await this.noMasterPasswordConfirmationDialog(user); - } - - return true; + return await this.memberDialogManager.openRevokeUserConfirmationDialog(user); } async deleteUser(user: OrganizationUserView, organization: Organization) { - const warningAcknowledged = await firstValueFrom( - this.deleteManagedMemberWarningService.warningAcknowledged(organization.id), + const confirmed = await this.memberDialogManager.openDeleteUserConfirmationDialog( + user, + organization, ); - if ( - !warningAcknowledged && - organization.canManageUsers && - organization.productTierType === ProductTierType.Enterprise - ) { - const acknowledged = await this.deleteManagedMemberWarningService.showWarning(); - if (!acknowledged) { - return false; - } - } - - const confirmed = await this.dialogService.openSimpleDialog({ - title: { - key: "deleteOrganizationUser", - placeholders: [this.userNamePipe.transform(user)], - }, - content: { - key: "deleteOrganizationUserWarningDesc", - placeholders: [this.userNamePipe.transform(user)], - }, - type: "warning", - acceptButtonText: { key: "delete" }, - cancelButtonText: { key: "cancel" }, - }); - if (!confirmed) { return false; } - await this.deleteManagedMemberWarningService.acknowledgeWarning(organization.id); - this.actionPromise2 = this.memberActionsService.deleteUser(organization, user.id); try { const result = await this.actionPromise2; @@ -754,19 +486,6 @@ export class MembersComponent extends BaseMembersComponent this.actionPromise = undefined; } - private async noMasterPasswordConfirmationDialog(user: OrganizationUserView) { - return this.dialogService.openSimpleDialog({ - title: { - key: "removeOrgUserNoMasterPasswordTitle", - }, - content: { - key: "removeOrgUserNoMasterPasswordDesc", - placeholders: [this.userNamePipe.transform(user)], - }, - type: "warning", - }); - } - get showBulkRestoreUsers(): boolean { return this.dataSource .getCheckedUsers() diff --git a/apps/web/src/app/admin-console/organizations/members/members.module.ts b/apps/web/src/app/admin-console/organizations/members/members.module.ts index 014dd5ba0a41..526558450d5b 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.module.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.module.ts @@ -19,10 +19,9 @@ import { UserDialogModule } from "./components/member-dialog"; import { MembersRoutingModule } from "./members-routing.module"; import { MembersComponent } from "./members.component"; import { - OrganizationMembersFacadeService, + OrganizationMembersService, MemberActionsService, BillingConstraintService, - MemberPermissionsService, MemberDialogManagerService, } from "./services"; @@ -48,10 +47,9 @@ import { BulkDeleteDialogComponent, ], providers: [ - OrganizationMembersFacadeService, + OrganizationMembersService, MemberActionsService, BillingConstraintService, - MemberPermissionsService, MemberDialogManagerService, ], }) diff --git a/apps/web/src/app/admin-console/organizations/members/services/index.ts b/apps/web/src/app/admin-console/organizations/members/services/index.ts index ef294463b51f..96b9ab5d6d6f 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/index.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/index.ts @@ -1,7 +1,6 @@ -export { OrganizationMembersFacadeService } from "./organization-members-facade/organization-members-facade.service"; +export { OrganizationMembersService } from "./organization-members-service/organization-members.service"; export { MemberActionsService } from "./member-actions/member-actions.service"; export { BillingConstraintService } from "./billing-constraint/billing-constraint.service"; -export { MemberPermissionsService } from "./member-permissions/member-permissions.service"; export { MemberDialogManagerService } from "./member-dialog-manager/member-dialog-manager.service"; export { DeleteManagedMemberWarningService } from "./delete-managed-member/delete-managed-member-warning.service"; export { OrganizationUserService } from "./organization-user/organization-user.service"; diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts index b109bfc4b760..c6ef536af2b8 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.ts @@ -28,11 +28,6 @@ import { } from "../../components/member-dialog"; import { DeleteManagedMemberWarningService } from "../delete-managed-member/delete-managed-member-warning.service"; -export interface DialogResult { - result: T; - action?: "saved" | "deleted" | "revoked" | "restored" | "confirmed"; -} - @Injectable() export class MemberDialogManagerService { constructor( @@ -47,7 +42,7 @@ export class MemberDialogManagerService { organization: Organization, billingMetadata: OrganizationBillingMetadataResponse, allUserEmails: string[], - ): Promise> { + ): Promise { const dialog = openUserAddEditDialog(this.dialogService, { data: { kind: "Add", @@ -59,10 +54,7 @@ export class MemberDialogManagerService { }); const result = await lastValueFrom(dialog.closed); - return { - result: result ?? MemberDialogResult.Canceled, - action: result === MemberDialogResult.Saved ? "saved" : undefined, - }; + return result ?? MemberDialogResult.Canceled; } async openEditDialog( @@ -70,7 +62,7 @@ export class MemberDialogManagerService { organization: Organization, billingMetadata: OrganizationBillingMetadataResponse, initialTab: MemberDialogTab = MemberDialogTab.Role, - ): Promise> { + ): Promise { const dialog = openUserAddEditDialog(this.dialogService, { data: { kind: "Edit", @@ -85,32 +77,13 @@ export class MemberDialogManagerService { }); const result = await lastValueFrom(dialog.closed); - const dialogResult = result ?? MemberDialogResult.Canceled; - let action: DialogResult["action"]; - switch (dialogResult) { - case MemberDialogResult.Saved: - action = "saved"; - break; - case MemberDialogResult.Deleted: - action = "deleted"; - break; - case MemberDialogResult.Revoked: - action = "revoked"; - break; - case MemberDialogResult.Restored: - action = "restored"; - break; - default: - action = undefined; - } - - return { result: dialogResult, action }; + return result ?? MemberDialogResult.Canceled; } async openAccountRecoveryDialog( user: OrganizationUserView, organization: Organization, - ): Promise> { + ): Promise { const dialogRef = AccountRecoveryDialogComponent.open(this.dialogService, { data: { name: this.userNamePipe.transform(user), @@ -121,13 +94,13 @@ export class MemberDialogManagerService { }); const result = await lastValueFrom(dialogRef.closed); - return { result: result ?? AccountRecoveryDialogResultType.Ok }; + return result ?? AccountRecoveryDialogResultType.Ok; } async openBulkConfirmDialog( organization: Organization, users: OrganizationUserView[], - ): Promise> { + ): Promise { const dialogRef = BulkConfirmDialogComponent.open(this.dialogService, { data: { organization: organization, @@ -136,13 +109,12 @@ export class MemberDialogManagerService { }); await lastValueFrom(dialogRef.closed); - return { result: undefined, action: "confirmed" }; } async openBulkRemoveDialog( organization: Organization, users: OrganizationUserView[], - ): Promise> { + ): Promise { const dialogRef = BulkRemoveDialogComponent.open(this.dialogService, { data: { organizationId: organization.id, @@ -151,13 +123,12 @@ export class MemberDialogManagerService { }); await lastValueFrom(dialogRef.closed); - return { result: undefined }; } async openBulkDeleteDialog( organization: Organization, users: OrganizationUserView[], - ): Promise> { + ): Promise { const warningAcknowledged = await firstValueFrom( this.deleteManagedMemberWarningService.warningAcknowledged(organization.id), ); @@ -169,7 +140,7 @@ export class MemberDialogManagerService { ) { const acknowledged = await this.deleteManagedMemberWarningService.showWarning(); if (!acknowledged) { - return { result: undefined }; + return; } } @@ -181,14 +152,13 @@ export class MemberDialogManagerService { }); await lastValueFrom(dialogRef.closed); - return { result: undefined, action: "deleted" }; } async openBulkRestoreRevokeDialog( organization: Organization, users: OrganizationUserView[], isRevoking: boolean, - ): Promise> { + ): Promise { const ref = BulkRestoreRevokeComponent.open(this.dialogService, { organizationId: organization.id, users: users, @@ -196,13 +166,12 @@ export class MemberDialogManagerService { }); await firstValueFrom(ref.closed); - return { result: undefined, action: isRevoking ? "revoked" : "restored" }; } async openBulkEnableSecretsManagerDialog( organization: Organization, users: OrganizationUserView[], - ): Promise> { + ): Promise { const eligibleUsers = users.filter((ou) => !ou.accessSecretsManager); if (eligibleUsers.length === 0) { @@ -211,7 +180,7 @@ export class MemberDialogManagerService { title: this.i18nService.t("errorOccurred"), message: this.i18nService.t("noSelectedUsersApplicable"), }); - return { result: undefined }; + return; } const dialogRef = BulkEnableSecretsManagerDialogComponent.open(this.dialogService, { @@ -220,7 +189,6 @@ export class MemberDialogManagerService { }); await lastValueFrom(dialogRef.closed); - return { result: undefined }; } async openBulkStatusDialog( @@ -228,7 +196,7 @@ export class MemberDialogManagerService { filteredUsers: OrganizationUserView[], request: Promise, successMessage: string, - ): Promise> { + ): Promise { const dialogRef = BulkStatusComponent.open(this.dialogService, { data: { users: users, @@ -239,7 +207,6 @@ export class MemberDialogManagerService { }); await lastValueFrom(dialogRef.closed); - return { result: undefined }; } openEventsDialog(user: OrganizationUserView, organization: Organization): void { diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts deleted file mode 100644 index 450dbc398ba5..000000000000 --- a/apps/web/src/app/admin-console/organizations/members/services/member-permissions/member-permissions.service.ts +++ /dev/null @@ -1,145 +0,0 @@ -import { Injectable } from "@angular/core"; - -import { OrganizationUserUserDetailsResponse } from "@bitwarden/admin-console/common"; -import { - OrganizationUserType, - OrganizationUserStatusType, -} from "@bitwarden/common/admin-console/enums"; -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { ProductTierType } from "@bitwarden/common/billing/enums"; - -import { OrganizationUserView } from "../../../core/views/organization-user.view"; - -@Injectable() -export class MemberPermissionsService { - canManageUsers(organization: Organization): boolean { - return organization?.canManageUsers ?? false; - } - - canUseSecretsManager(organization: Organization): boolean { - return organization?.useSecretsManager ?? false; - } - - showUserManagementControls(organization: Organization): boolean { - return organization?.canManageUsers ?? false; - } - - allowResetPassword( - orgUser: OrganizationUserView, - organization: Organization, - resetPasswordPolicyEnabled: boolean, - ): boolean { - let callingUserHasPermission = false; - - switch (organization.type) { - case OrganizationUserType.Owner: - callingUserHasPermission = true; - break; - case OrganizationUserType.Admin: - callingUserHasPermission = orgUser.type !== OrganizationUserType.Owner; - break; - case OrganizationUserType.Custom: - callingUserHasPermission = - orgUser.type !== OrganizationUserType.Owner && - orgUser.type !== OrganizationUserType.Admin; - break; - } - - return ( - organization.canManageUsersPassword && - callingUserHasPermission && - organization.useResetPassword && - organization.hasPublicAndPrivateKeys && - orgUser.resetPasswordEnrolled && - resetPasswordPolicyEnabled && - orgUser.status === OrganizationUserStatusType.Confirmed - ); - } - - showEnrolledStatus( - orgUser: OrganizationUserUserDetailsResponse, - organization: Organization, - resetPasswordPolicyEnabled: boolean, - ): boolean { - return ( - organization.useResetPassword && orgUser.resetPasswordEnrolled && resetPasswordPolicyEnabled - ); - } - - canDeleteUser(user: OrganizationUserView): boolean { - const validStatuses = [ - OrganizationUserStatusType.Accepted, - OrganizationUserStatusType.Confirmed, - OrganizationUserStatusType.Revoked, - ]; - - return user.managedByOrganization && validStatuses.includes(user.status); - } - - canRemoveUser(user: OrganizationUserView): boolean { - return !user.managedByOrganization; - } - - canRevokeUser(user: OrganizationUserView): boolean { - return user.status !== OrganizationUserStatusType.Revoked; - } - - canRestoreUser(user: OrganizationUserView): boolean { - return user.status === OrganizationUserStatusType.Revoked; - } - - canReinviteUser(user: OrganizationUserView): boolean { - return user.status === OrganizationUserStatusType.Invited; - } - - canConfirmUser(user: OrganizationUserView): boolean { - return user.status === OrganizationUserStatusType.Accepted; - } - - requiresManagedUserWarning(organization: Organization): boolean { - return ( - organization.canManageUsers && organization.productTierType === ProductTierType.Enterprise - ); - } - - canEditUser(user: OrganizationUserView, organization: Organization): boolean { - if (!organization.canManageUsers) { - return false; - } - - // Add additional permission checks as needed - return true; - } - - canViewEvents(user: OrganizationUserView, organization: Organization): boolean { - return organization.useEvents && user.status === OrganizationUserStatusType.Confirmed; - } - - canEnableSecretsManager(user: OrganizationUserView, organization: Organization): boolean { - return organization.useSecretsManager && !user.accessSecretsManager; - } - - getBulkActionPermissions(users: OrganizationUserView[]) { - return { - showBulkRemove: users.every((user) => this.canRemoveUser(user)), - showBulkDelete: users.every((user) => this.canDeleteUser(user)), - showBulkRevoke: users.every((user) => this.canRevokeUser(user)), - showBulkRestore: users.every((user) => this.canRestoreUser(user)), - showBulkReinvite: users.every((user) => this.canReinviteUser(user)), - showBulkConfirm: users.every((user) => this.canConfirmUser(user)), - }; - } - - shouldShowConfirmBanner( - activeUserCount: number, - confirmedUserCount: number, - acceptedUserCount: number, - ): boolean { - return ( - activeUserCount > 1 && - confirmedUserCount > 0 && - confirmedUserCount < 3 && - acceptedUserCount > 0 - ); - } -} diff --git a/apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts b/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts similarity index 86% rename from apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts rename to apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts index 37b496255f0e..bd470f89a200 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/organization-members-facade/organization-members-facade.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts @@ -1,14 +1,5 @@ import { Injectable } from "@angular/core"; -import { - Observable, - combineLatest, - switchMap, - map, - filter, - shareReplay, - BehaviorSubject, - from, -} from "rxjs"; +import { Observable, combineLatest, switchMap, map, filter, shareReplay, from } from "rxjs"; import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -27,7 +18,6 @@ import { OrganizationId } from "@bitwarden/common/types/guid"; import { GroupApiService } from "../../../core"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; -import { BillingConstraintService } from "../billing-constraint/billing-constraint.service"; export interface OrganizationMemberState { organization: Organization | null; @@ -35,19 +25,15 @@ export interface OrganizationMemberState { policies: Policy[]; resetPasswordPolicyEnabled: boolean; loading: boolean; - error: string | null; } @Injectable() -export class OrganizationMembersFacadeService { - private refreshTrigger$ = new BehaviorSubject(undefined); - +export class OrganizationMembersService { constructor( private organizationService: OrganizationService, private organizationUserApiService: OrganizationUserApiService, private policyService: PolicyService, private policyApiService: PolicyApiService, - private billingConstraintService: BillingConstraintService, private groupService: GroupApiService, private apiService: ApiService, private accountService: AccountService, @@ -74,9 +60,7 @@ export class OrganizationMembersFacadeService { ), ); - const users$ = combineLatest([this.refreshTrigger$, organization$]).pipe( - switchMap(([_, organization]) => this.loadUsers(organization)), - ); + const users$ = organization$.pipe(switchMap((organization) => this.loadUsers(organization))); return combineLatest([organization$, users$, policies$]).pipe( map(([organization, users, policies]) => { @@ -90,19 +74,13 @@ export class OrganizationMembersFacadeService { policies, resetPasswordPolicyEnabled: resetPasswordPolicy?.enabled ?? false, loading: false, // Data is loaded when we reach this point - error: null, }; }), shareReplay({ refCount: true, bufferSize: 1 }), ); } - refreshData(): void { - this.refreshTrigger$.next(); - this.billingConstraintService.refreshBillingMetadata(); - } - - private async loadUsers(organization: Organization): Promise { + async loadUsers(organization: Organization): Promise { let groupsPromise: Promise> | undefined; let collectionsPromise: Promise> | undefined; From 79635b46e8773802fc9b829ee97d54d79fdb4919 Mon Sep 17 00:00:00 2001 From: Brandon Date: Fri, 26 Sep 2025 15:38:40 -0400 Subject: [PATCH 03/10] WIP --- .../common/base-members.component.ts | 59 ++++++++------ .../members/members.component.ts | 76 +++++++++---------- .../providers/manage/members.component.ts | 38 +++++++--- 3 files changed, 99 insertions(+), 74 deletions(-) diff --git a/apps/web/src/app/admin-console/common/base-members.component.ts b/apps/web/src/app/admin-console/common/base-members.component.ts index 936d69729be8..5ecf4269a1aa 100644 --- a/apps/web/src/app/admin-console/common/base-members.component.ts +++ b/apps/web/src/app/admin-console/common/base-members.component.ts @@ -76,8 +76,7 @@ export abstract class BaseMembersComponent { /** * The currently executing promise - used to avoid multiple user actions executing at once. */ - actionPromise?: Promise; - actionPromise2?: Promise; + actionPromise?: Promise; protected searchControl = new FormControl("", { nonNullable: true }); protected statusToggle = new BehaviorSubject(undefined); @@ -103,13 +102,13 @@ export abstract class BaseMembersComponent { abstract edit(user: UserView, organization?: Organization): void; abstract getUsers(organization?: Organization): Promise | UserView[]>; - abstract removeUser(id: string, organization?: Organization): Promise; - abstract reinviteUser(id: string, organization?: Organization): Promise; + abstract removeUser(id: string, organization?: Organization): Promise; + abstract reinviteUser(id: string, organization?: Organization): Promise; abstract confirmUser( user: UserView, publicKey: Uint8Array, organization?: Organization, - ): Promise; + ): Promise; abstract invite(organization?: Organization): void; async load(organization?: Organization) { @@ -142,12 +141,16 @@ export abstract class BaseMembersComponent { this.actionPromise = this.removeUser(user.id, organization); try { - await this.actionPromise; - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("removedUserId", this.userNamePipe.transform(user)), - }); - this.dataSource.removeUser(user); + const result = await this.actionPromise; + if (result.success) { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("removedUserId", this.userNamePipe.transform(user)), + }); + this.dataSource.removeUser(user); + } else { + throw new Error(result.error); + } } catch (e) { this.validationService.showError(e); } @@ -161,11 +164,15 @@ export abstract class BaseMembersComponent { this.actionPromise = this.reinviteUser(user.id, organization); try { - await this.actionPromise; - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("hasBeenReinvited", this.userNamePipe.transform(user)), - }); + const result = await this.actionPromise; + if (result.success) { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("hasBeenReinvited", this.userNamePipe.transform(user)), + }); + } else { + throw new Error(result.error); + } } catch (e) { this.validationService.showError(e); } @@ -176,14 +183,18 @@ export abstract class BaseMembersComponent { const confirmUser = async (publicKey: Uint8Array) => { try { this.actionPromise = this.confirmUser(user, publicKey, organization); - await this.actionPromise; - user.status = this.userStatusType.Confirmed; - this.dataSource.replaceUser(user); - - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("hasBeenConfirmed", this.userNamePipe.transform(user)), - }); + const result = await this.actionPromise; + if (result.success) { + user.status = this.userStatusType.Confirmed; + this.dataSource.replaceUser(user); + + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("hasBeenConfirmed", this.userNamePipe.transform(user)), + }); + } else { + throw new Error(result.error); + } } catch (e) { this.validationService.showError(e); throw e; diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index f9a22322009f..df6eacca24a2 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -43,7 +43,10 @@ import { OrganizationMembersService, } from "./services"; import { DeleteManagedMemberWarningService } from "./services/delete-managed-member/delete-managed-member-warning.service"; -import { MemberActionsService } from "./services/member-actions/member-actions.service"; +import { + MemberActionsService, + MemberActionResult, +} from "./services/member-actions/member-actions.service"; class MembersTableDataSource extends PeopleTableDataSource { protected statusType = OrganizationUserStatusType; @@ -175,43 +178,28 @@ export class MembersComponent extends BaseMembersComponent return await this.memberService.loadUsers(organization); } - async removeUser(id: string, organization: Organization): Promise { - const result = await this.memberActionsService.removeUser(organization, id); - if (!result.success) { - throw new Error(result.error); - } + async removeUser(id: string, organization: Organization): Promise { + return await this.memberActionsService.removeUser(organization, id); } - async revokeUser(id: string, organization: Organization): Promise { - const result = await this.memberActionsService.revokeUser(organization, id); - if (!result.success) { - throw new Error(result.error); - } + async revokeUser(id: string, organization: Organization): Promise { + return await this.memberActionsService.revokeUser(organization, id); } - async restoreUser(id: string, organization: Organization): Promise { - const result = await this.memberActionsService.restoreUser(organization, id); - if (!result.success) { - throw new Error(result.error); - } + async restoreUser(id: string, organization: Organization): Promise { + return await this.memberActionsService.restoreUser(organization, id); } - async reinviteUser(id: string, organization: Organization): Promise { - const result = await this.memberActionsService.reinviteUser(organization, id); - if (!result.success) { - throw new Error(result.error); - } + async reinviteUser(id: string, organization: Organization): Promise { + return await this.memberActionsService.reinviteUser(organization, id); } async confirmUser( user: OrganizationUserView, publicKey: Uint8Array, organization: Organization, - ): Promise { - const result = await this.memberActionsService.confirmUser(user, publicKey, organization); - if (!result.success) { - throw new Error(result.error); - } + ): Promise { + return await this.memberActionsService.confirmUser(user, publicKey, organization); } async revoke(user: OrganizationUserView, organization: Organization) { @@ -223,12 +211,16 @@ export class MembersComponent extends BaseMembersComponent this.actionPromise = this.revokeUser(user.id, organization); try { - await this.actionPromise; - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("revokedUserId", this.userNamePipe.transform(user)), - }); - await this.load(organization); + const result = await this.actionPromise; + if (result.success) { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("revokedUserId", this.userNamePipe.transform(user)), + }); + await this.load(organization); + } else { + throw new Error(result.error); + } } catch (e) { this.validationService.showError(e); } @@ -238,12 +230,16 @@ export class MembersComponent extends BaseMembersComponent async restore(user: OrganizationUserView, organization: Organization) { this.actionPromise = this.restoreUser(user.id, organization); try { - await this.actionPromise; - this.toastService.showToast({ - variant: "success", - message: this.i18nService.t("restoredUserId", this.userNamePipe.transform(user)), - }); - await this.load(organization); + const result = await this.actionPromise; + if (result.success) { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("restoredUserId", this.userNamePipe.transform(user)), + }); + await this.load(organization); + } else { + throw new Error(result.error); + } } catch (e) { this.validationService.showError(e); } @@ -469,9 +465,9 @@ export class MembersComponent extends BaseMembersComponent return false; } - this.actionPromise2 = this.memberActionsService.deleteUser(organization, user.id); + this.actionPromise = this.memberActionsService.deleteUser(organization, user.id); try { - const result = await this.actionPromise2; + const result = await this.actionPromise; if (!result.success) { throw new Error(result.error); } 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 affcfce9c174..e86956dec93e 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 @@ -30,6 +30,7 @@ import { } from "@bitwarden/web-vault/app/admin-console/common/people-table-data-source"; import { openEntityEventsDialog } from "@bitwarden/web-vault/app/admin-console/organizations/manage/entity-events.component"; import { BulkStatusComponent } from "@bitwarden/web-vault/app/admin-console/organizations/members/components/bulk/bulk-status.component"; +import { MemberActionResult } from "@bitwarden/web-vault/app/admin-console/organizations/members/services/member-actions/member-actions.service"; import { AddEditMemberDialogComponent, @@ -199,16 +200,27 @@ export class MembersComponent extends BaseMembersComponent { await this.load(); } - async confirmUser(user: ProviderUser, publicKey: Uint8Array): Promise { - const providerKey = await this.keyService.getProviderKey(this.providerId); - const key = await this.encryptService.encapsulateKeyUnsigned(providerKey, publicKey); - const request = new ProviderUserConfirmRequest(); - request.key = key.encryptedString; - await this.apiService.postProviderUserConfirm(this.providerId, user.id, request); + async confirmUser(user: ProviderUser, publicKey: Uint8Array): Promise { + try { + const providerKey = await this.keyService.getProviderKey(this.providerId); + const key = await this.encryptService.encapsulateKeyUnsigned(providerKey, publicKey); + const request = new ProviderUserConfirmRequest(); + request.key = key.encryptedString; + await this.apiService.postProviderUserConfirm(this.providerId, user.id, request); + return { success: true }; + } catch (error) { + return { success: false, error: error.message }; + } } - removeUser = (id: string): Promise => - this.apiService.deleteProviderUser(this.providerId, id); + removeUser = async (id: string): Promise => { + try { + await this.apiService.deleteProviderUser(this.providerId, id); + return { success: true }; + } catch (error) { + return { success: false, error: error.message }; + } + }; edit = async (user: ProviderUser | null): Promise => { const data: AddEditMemberDialogParams = { @@ -251,6 +263,12 @@ export class MembersComponent extends BaseMembersComponent { getUsers = (): Promise> => this.apiService.getProviderUsers(this.providerId); - reinviteUser = (id: string): Promise => - this.apiService.postProviderUserReinvite(this.providerId, id); + reinviteUser = async (id: string): Promise => { + try { + await this.apiService.postProviderUserReinvite(this.providerId, id); + return { success: true }; + } catch (error) { + return { success: false, error: error.message }; + } + }; } From f717d9397c3a00dbe3035131aef26be0ca4fd869 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 29 Sep 2025 09:47:21 -0400 Subject: [PATCH 04/10] wip add tests --- .../billing-constraint.service.spec.ts | 506 ++++++++++++++++++ 1 file changed, 506 insertions(+) create mode 100644 apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts diff --git a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts new file mode 100644 index 000000000000..a120f54126c3 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts @@ -0,0 +1,506 @@ +import { TestBed } from "@angular/core/testing"; +import { Router } from "@angular/router"; +import { BehaviorSubject, of } from "rxjs"; + +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; +import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { OrganizationId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; + +import { + ChangePlanDialogResultType, + openChangePlanDialog, +} from "../../../../../billing/organizations/change-plan-dialog.component"; + +import { BillingConstraintService, SeatLimitResult } from "./billing-constraint.service"; + +jest.mock("../../../../../billing/organizations/change-plan-dialog.component"); + +describe("BillingConstraintService", () => { + let service: BillingConstraintService; + let i18nService: jest.Mocked; + let dialogService: jest.Mocked; + let toastService: jest.Mocked; + let router: jest.Mocked; + let billingApiService: jest.Mocked; + + const mockOrganizationId = "org-123" as OrganizationId; + + const createMockOrganization = (overrides: Partial = {}): Organization => { + const org = new Organization(); + org.id = mockOrganizationId; + org.seats = 10; + org.productTierType = ProductTierType.Teams; + + Object.defineProperty(org, "hasReseller", { + value: false, + writable: true, + configurable: true, + }); + + Object.defineProperty(org, "canEditSubscription", { + value: true, + writable: true, + configurable: true, + }); + + return Object.assign(org, overrides); + }; + + const createMockBillingMetadata = ( + overrides: Partial = {}, + ): OrganizationBillingMetadataResponse => { + return { + organizationOccupiedSeats: 5, + ...overrides, + } as OrganizationBillingMetadataResponse; + }; + + beforeEach(() => { + const mockDialogRef = { + closed: of(true), + }; + + const mockSimpleDialogRef = { + closed: of(true), + }; + + i18nService = { + t: jest.fn().mockReturnValue("translated-text"), + } as any; + + dialogService = { + openSimpleDialogRef: jest.fn().mockReturnValue(mockSimpleDialogRef), + } as any; + + toastService = { + showToast: jest.fn(), + } as any; + + router = { + navigate: jest.fn().mockResolvedValue(true), + } as any; + + billingApiService = { + getOrganizationBillingMetadata: jest.fn(), + } as any; + + (openChangePlanDialog as jest.Mock).mockReturnValue(mockDialogRef); + + TestBed.configureTestingModule({ + providers: [ + BillingConstraintService, + { provide: I18nService, useValue: i18nService }, + { provide: DialogService, useValue: dialogService }, + { provide: ToastService, useValue: toastService }, + { provide: Router, useValue: router }, + { provide: BillingApiServiceAbstraction, useValue: billingApiService }, + ], + }); + + service = TestBed.inject(BillingConstraintService); + }); + + describe("getBillingMetadata$", () => { + it("should return billing metadata observable", (done) => { + const mockMetadata = createMockBillingMetadata(); + billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockMetadata); + + service.getBillingMetadata$(mockOrganizationId).subscribe((result) => { + expect(result).toBe(mockMetadata); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith( + mockOrganizationId, + ); + done(); + }); + }); + + it("should refresh when refreshTrigger$ emits", (done) => { + const mockMetadata = createMockBillingMetadata(); + billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockMetadata); + + let callCount = 0; + + service.getBillingMetadata$(mockOrganizationId).subscribe(() => { + callCount++; + + if (callCount === 1) { + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1); + // Trigger refresh for second emission + service.refreshBillingMetadata(); + } else if (callCount === 2) { + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2); + done(); + } + }); + }); + }); + + describe("refreshBillingMetadata", () => { + it("should trigger refresh by emitting next value", () => { + const refreshTrigger = (service as any).refreshTrigger$ as BehaviorSubject; + const nextSpy = jest.spyOn(refreshTrigger, "next"); + + service.refreshBillingMetadata(); + + expect(nextSpy).toHaveBeenCalled(); + }); + }); + + describe("checkSeatLimit", () => { + it("should allow users when occupied seats are less than total seats", () => { + const organization = createMockOrganization({ seats: 10 }); + const billingMetadata = createMockBillingMetadata({ organizationOccupiedSeats: 5 }); + + const result = service.checkSeatLimit(organization, billingMetadata); + + expect(result).toEqual({ canAddUsers: true }); + }); + + it("should allow users when occupied seats equal total seats for non-fixed seat plans", () => { + const organization = createMockOrganization({ + seats: 10, + productTierType: ProductTierType.Teams, + }); + const billingMetadata = createMockBillingMetadata({ organizationOccupiedSeats: 10 }); + + const result = service.checkSeatLimit(organization, billingMetadata); + + expect(result).toEqual({ canAddUsers: true }); + }); + + it("should block users with reseller-limit reason when organization has reseller", () => { + const organization = createMockOrganization({ + seats: 10, + hasReseller: true, + }); + const billingMetadata = createMockBillingMetadata({ organizationOccupiedSeats: 10 }); + + const result = service.checkSeatLimit(organization, billingMetadata); + + expect(result).toEqual({ + canAddUsers: false, + reason: "reseller-limit", + }); + }); + + it("should block users with fixed-seat-limit reason for fixed seat plans", () => { + const organization = createMockOrganization({ + seats: 10, + productTierType: ProductTierType.Free, + canEditSubscription: true, + }); + const billingMetadata = createMockBillingMetadata({ organizationOccupiedSeats: 10 }); + + const result = service.checkSeatLimit(organization, billingMetadata); + + expect(result).toEqual({ + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: true, + }); + }); + + it("should not show upgrade dialog when organization cannot edit subscription", () => { + const organization = createMockOrganization({ + seats: 10, + productTierType: ProductTierType.TeamsStarter, + canEditSubscription: false, + }); + const billingMetadata = createMockBillingMetadata({ organizationOccupiedSeats: 10 }); + + const result = service.checkSeatLimit(organization, billingMetadata); + + expect(result).toEqual({ + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }); + }); + + it("should handle missing billing metadata gracefully", () => { + const organization = createMockOrganization({ seats: 10 }); + const billingMetadata = createMockBillingMetadata({ + organizationOccupiedSeats: undefined as any, + }); + + const result = service.checkSeatLimit(organization, billingMetadata); + + expect(result).toEqual({ canAddUsers: true }); + }); + }); + + describe("seatLimitReached", () => { + it("should return false when canAddUsers is true", async () => { + const result: SeatLimitResult = { canAddUsers: true }; + const organization = createMockOrganization(); + + const seatLimitReached = await service.seatLimitReached(result, organization); + + expect(seatLimitReached).toBe(false); + }); + + it("should show toast and return true for reseller-limit", async () => { + const result: SeatLimitResult = { canAddUsers: false, reason: "reseller-limit" }; + const organization = createMockOrganization(); + + const seatLimitReached = await service.seatLimitReached(result, organization); + + expect(toastService.showToast).toHaveBeenCalledWith({ + variant: "error", + title: "translated-text", + message: "translated-text", + }); + expect(i18nService.t).toHaveBeenCalledWith("seatLimitReached"); + expect(i18nService.t).toHaveBeenCalledWith("contactYourProvider"); + expect(seatLimitReached).toBe(true); + }); + + it("should show upgrade dialog when shouldShowUpgradeDialog is true", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: true, + }; + const organization = createMockOrganization(); + const mockDialogRef = { closed: of(ChangePlanDialogResultType.Submitted) }; + (openChangePlanDialog as jest.Mock).mockReturnValue(mockDialogRef); + + const seatLimitReached = await service.seatLimitReached(result, organization); + + expect(openChangePlanDialog).toHaveBeenCalledWith(dialogService, { + data: { + organizationId: organization.id, + productTierType: organization.productTierType, + }, + }); + expect(seatLimitReached).toBe(true); + }); + + it("should return false when upgrade dialog is submitted", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: true, + }; + const organization = createMockOrganization(); + const mockDialogRef = { closed: of(ChangePlanDialogResultType.Submitted) }; + (openChangePlanDialog as jest.Mock).mockReturnValue(mockDialogRef); + + const seatLimitReached = await service.seatLimitReached(result, organization); + + expect(seatLimitReached).toBe(true); + }); + + it("should show seat limit dialog when shouldShowUpgradeDialog is false", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + canEditSubscription: false, + productTierType: ProductTierType.Free, + }); + + const seatLimitReached = await service.seatLimitReached(result, organization); + + expect(dialogService.openSimpleDialogRef).toHaveBeenCalled(); + expect(seatLimitReached).toBe(true); + }); + + it("should return true for unknown reasons", async () => { + const result: SeatLimitResult = { canAddUsers: false }; + const organization = createMockOrganization(); + + const seatLimitReached = await service.seatLimitReached(result, organization); + + expect(seatLimitReached).toBe(true); + }); + }); + + describe("navigateToPaymentMethod", () => { + it("should navigate to payment method with correct parameters", async () => { + const organization = createMockOrganization(); + + await service.navigateToPaymentMethod(organization); + + expect(router.navigate).toHaveBeenCalledWith( + ["organizations", organization.id, "billing", "payment-method"], + { + state: { launchPaymentModalAutomatically: true }, + }, + ); + }); + }); + + describe("private methods through public method coverage", () => { + describe("getDialogContent via showSeatLimitReachedDialog", () => { + it("should get correct dialog content for Free organization", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + productTierType: ProductTierType.Free, + canEditSubscription: false, + seats: 5, + }); + + await service.seatLimitReached(result, organization); + + expect(i18nService.t).toHaveBeenCalledWith("freeOrgInvLimitReachedNoManageBilling", 5); + }); + + it("should get correct dialog content for TeamsStarter organization", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + productTierType: ProductTierType.TeamsStarter, + canEditSubscription: false, + seats: 3, + }); + + await service.seatLimitReached(result, organization); + + expect(i18nService.t).toHaveBeenCalledWith( + "teamsStarterPlanInvLimitReachedNoManageBilling", + 3, + ); + }); + + it("should get correct dialog content for Families organization", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + productTierType: ProductTierType.Families, + canEditSubscription: false, + seats: 6, + }); + + await service.seatLimitReached(result, organization); + + expect(i18nService.t).toHaveBeenCalledWith("familiesPlanInvLimitReachedNoManageBilling", 6); + }); + + it("should throw error for unsupported product type in getProductKey", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + productTierType: ProductTierType.Enterprise, + canEditSubscription: false, + }); + + await expect(service.seatLimitReached(result, organization)).rejects.toThrow( + `Unsupported product type: ${ProductTierType.Enterprise}`, + ); + }); + }); + + describe("getAcceptButtonText via showSeatLimitReachedDialog", () => { + it("should return 'ok' when organization cannot edit subscription", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + canEditSubscription: false, + productTierType: ProductTierType.Free, + }); + + await service.seatLimitReached(result, organization); + + expect(i18nService.t).toHaveBeenCalledWith("ok"); + }); + + it("should return 'upgrade' when organization can edit subscription", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + canEditSubscription: true, + productTierType: ProductTierType.Free, + }); + const mockSimpleDialogRef = { closed: of(false) }; + dialogService.openSimpleDialogRef.mockReturnValue(mockSimpleDialogRef); + + await service.seatLimitReached(result, organization); + + expect(i18nService.t).toHaveBeenCalledWith("upgrade"); + }); + + it("should throw error for unsupported product type in getAcceptButtonText", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + canEditSubscription: true, + productTierType: ProductTierType.Enterprise, + }); + + await expect(service.seatLimitReached(result, organization)).rejects.toThrow( + `Unsupported product type: ${ProductTierType.Enterprise}`, + ); + }); + }); + + describe("handleUpgradeNavigation", () => { + it("should navigate to billing subscription with upgrade query param", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + canEditSubscription: true, + productTierType: ProductTierType.Free, + }); + const mockSimpleDialogRef = { closed: of(true) }; + dialogService.openSimpleDialogRef.mockReturnValue(mockSimpleDialogRef); + + await service.seatLimitReached(result, organization); + + expect(router.navigate).toHaveBeenCalledWith( + ["/organizations", organization.id, "billing", "subscription"], + { queryParams: { upgrade: true } }, + ); + }); + + it("should throw error for non-self-upgradable product type", async () => { + const result: SeatLimitResult = { + canAddUsers: false, + reason: "fixed-seat-limit", + shouldShowUpgradeDialog: false, + }; + const organization = createMockOrganization({ + canEditSubscription: true, + productTierType: ProductTierType.Enterprise, + }); + const mockSimpleDialogRef = { closed: of(true) }; + dialogService.openSimpleDialogRef.mockReturnValue(mockSimpleDialogRef); + + await expect(service.seatLimitReached(result, organization)).rejects.toThrow( + `Unsupported product type: ${ProductTierType.Enterprise}`, + ); + }); + }); + }); +}); From cb343832646c2642b92eadc030eeb29e5ba8d5a7 Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 2 Oct 2025 11:03:49 -0400 Subject: [PATCH 05/10] add tests, continue refactoring --- .../browser-system-notification.service.ts | 2 +- .../members/members.component.html | 7 +- .../members/members.component.ts | 83 ++- .../billing-constraint.service.spec.ts | 6 +- .../billing-constraint.service.ts | 5 +- .../member-actions.service.spec.ts | 459 +++++++++++++ .../member-actions/member-actions.service.ts | 34 - .../member-dialog-manager.service.spec.ts | 640 ++++++++++++++++++ .../organization-members.service.spec.ts | 362 ++++++++++ .../organization-members.service.ts | 65 -- 10 files changed, 1532 insertions(+), 131 deletions(-) create mode 100644 apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.spec.ts create mode 100644 apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.spec.ts diff --git a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts index 33e60894a543..b835c711853d 100644 --- a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts +++ b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts @@ -70,7 +70,7 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ } async clear(clearInfo: SystemNotificationClearInfo): Promise { - + // eslint-disable-next-line @typescript-eslint/no-floating-promises chrome.notifications.clear(clearInfo.id); } diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.html b/apps/web/src/app/admin-console/organizations/members/members.component.html index 021157a1977d..9401a88ab766 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.html +++ b/apps/web/src/app/admin-console/organizations/members/members.component.html @@ -339,7 +339,10 @@ > {{ "userUsingTwoStep" | i18n }} - + @let resetPasswordPolicyEnabled = resetPasswordPolicyEnabled$ | async; + {{ "recoverAccount" | i18n }} diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index df6eacca24a2..da27b318fcce 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -6,6 +6,7 @@ import { concatMap, filter, firstValueFrom, + from, map, merge, Observable, @@ -17,18 +18,27 @@ import { import { OrganizationUserUserDetailsResponse } from "@bitwarden/admin-console/common"; 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 { OrganizationManagementPreferencesService } from "@bitwarden/common/admin-console/abstractions/organization-management-preferences/organization-management-preferences.service"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { OrganizationUserStatusType, OrganizationUserType, + PolicyType, } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { DialogService, ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; +import { UserId } from "@bitwarden/user-core"; import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; import { BaseMembersComponent } from "../../common/base-members.component"; @@ -64,7 +74,10 @@ export class MembersComponent extends BaseMembersComponent organization: Signal; status: OrganizationUserStatusType | undefined; - orgResetPasswordPolicyEnabled = false; + + private userId$: Observable = this.accountService.activeAccount$.pipe(getUserId); + + resetPasswordPolicyEnabled$: Observable; protected canUseSecretsManager: Signal = computed( () => this.organization()?.useSecretsManager ?? false, @@ -95,6 +108,10 @@ export class MembersComponent extends BaseMembersComponent private memberDialogManager: MemberDialogManagerService, protected billingConstraint: BillingConstraintService, protected memberService: OrganizationMembersService, + private organizationService: OrganizationService, + private accountService: AccountService, + private policyService: PolicyService, + private policyApiService: PolicyApiServiceAbstraction, ) { super( apiService, @@ -108,39 +125,48 @@ export class MembersComponent extends BaseMembersComponent toastService, ); - const memberState$ = this.route.params.pipe( - switchMap((params) => this.memberService.getMemberState(params.organizationId)), - shareReplay({ refCount: true, bufferSize: 1 }), - ); - - const organization$ = memberState$.pipe( - map((state) => state.organization), - filter((organization): organization is Organization => organization != null), - shareReplay({ refCount: true, bufferSize: 1 }), + const organization$ = this.route.params.pipe( + concatMap((params) => + this.userId$.pipe( + switchMap((userId) => + this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), + ), + ), + ), ); this.organization = toSignal(organization$); - combineLatest([this.route.queryParams, memberState$]) - .pipe( - concatMap(async ([qParams, memberState]) => { - // Backfill pub/priv key if necessary - try { - await this.memberActionsService.ensureOrganizationKeys(memberState.organization!); - } catch { - throw new Error(this.i18nService.t("resetPasswordOrgKeysError")); - } + const policies$ = combineLatest([this.userId$, organization$]).pipe( + switchMap(([userId, organization]) => + organization?.isProviderUser + ? from(this.policyApiService.getPolicies(organization.id)).pipe( + map((response) => Policy.fromListResponse(response)), + ) + : this.policyService.policies$(userId), + ), + ); - this.orgResetPasswordPolicyEnabled = memberState.resetPasswordPolicyEnabled; + this.resetPasswordPolicyEnabled$ = combineLatest([organization$, policies$]).pipe( + map( + ([organization, policies]) => + policies + .filter((policy) => policy.type === PolicyType.ResetPassword) + .find((p) => p.organizationId === organization?.id)?.enabled ?? false, + ), + ); - await this.load(memberState.organization!); + combineLatest([this.route.queryParams, organization$]) + .pipe( + concatMap(async ([qParams, organization]) => { + await this.load(organization!); this.searchControl.setValue(qParams.search); if (qParams.viewEvents != null) { const user = this.dataSource.data.filter((u) => u.id === qParams.viewEvents); if (user.length > 0 && user[0].status === OrganizationUserStatusType.Confirmed) { - this.openEventsDialog(user[0], memberState.organization!); + this.openEventsDialog(user[0], organization!); } } }), @@ -150,6 +176,7 @@ export class MembersComponent extends BaseMembersComponent organization$ .pipe( + filter((organization) => organization != null), switchMap((organization) => merge( this.organizationWarningsService.showInactiveSubscriptionDialog$(organization), @@ -161,6 +188,7 @@ export class MembersComponent extends BaseMembersComponent .subscribe(); this.billingMetadata$ = organization$.pipe( + filter((organization) => organization != null), switchMap((organization) => this.billingConstraint.getBillingMetadata$(organization.id)), shareReplay({ bufferSize: 1, refCount: false }), ); @@ -246,22 +274,27 @@ export class MembersComponent extends BaseMembersComponent this.actionPromise = undefined; } - allowResetPassword(orgUser: OrganizationUserView, organization: Organization): boolean { + allowResetPassword( + orgUser: OrganizationUserView, + organization: Organization, + orgResetPasswordPolicyEnabled: boolean, + ): boolean { return this.memberActionsService.allowResetPassword( orgUser, organization, - this.orgResetPasswordPolicyEnabled, + orgResetPasswordPolicyEnabled, ); } showEnrolledStatus( orgUser: OrganizationUserUserDetailsResponse, organization: Organization, + orgResetPasswordPolicyEnabled: boolean, ): boolean { return ( organization.useResetPassword && orgUser.resetPasswordEnrolled && - this.orgResetPasswordPolicyEnabled + orgResetPasswordPolicyEnabled ); } diff --git a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts index a120f54126c3..53e0978e602a 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts @@ -221,15 +221,15 @@ describe("BillingConstraintService", () => { }); }); - it("should handle missing billing metadata gracefully", () => { + it("shoud throw if missing billingMetadata", () => { const organization = createMockOrganization({ seats: 10 }); const billingMetadata = createMockBillingMetadata({ organizationOccupiedSeats: undefined as any, }); - const result = service.checkSeatLimit(organization, billingMetadata); + const err = () => service.checkSeatLimit(organization, billingMetadata); - expect(result).toEqual({ canAddUsers: true }); + expect(err).toThrow("Cannot check seat limit: billingMetadata is null or undefined."); }); }); diff --git a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts index 59b3fc24a48a..c01bd0e39d4c 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts @@ -58,7 +58,10 @@ export class BillingConstraintService { organization: Organization, billingMetadata: OrganizationBillingMetadataResponse, ): SeatLimitResult { - const occupiedSeats = billingMetadata?.organizationOccupiedSeats ?? 0; + const occupiedSeats = billingMetadata?.organizationOccupiedSeats; + if (occupiedSeats == null) { + throw new Error("Cannot check seat limit: billingMetadata is null or undefined."); + } const totalSeats = organization.seats; if (occupiedSeats < totalSeats) { diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts new file mode 100644 index 000000000000..af6cddaeacd4 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts @@ -0,0 +1,459 @@ +import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { + OrganizationUserApiService, + OrganizationUserBulkResponse, +} from "@bitwarden/admin-console/common"; +import { + OrganizationUserType, + OrganizationUserStatusType, +} from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { ListResponse } from "@bitwarden/common/models/response/list.response"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { OrgKey } from "@bitwarden/common/types/key"; +import { newGuid } from "@bitwarden/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { OrganizationUserView } from "../../../core/views/organization-user.view"; +import { OrganizationUserService } from "../organization-user/organization-user.service"; + +import { MemberActionsService } from "./member-actions.service"; + +describe("MemberActionsService", () => { + let service: MemberActionsService; + let organizationUserApiService: MockProxy; + let organizationUserService: MockProxy; + let keyService: MockProxy; + let encryptService: MockProxy; + let configService: MockProxy; + let accountService: FakeAccountService; + + const userId = newGuid() as UserId; + const organizationId = newGuid() as OrganizationId; + const userIdToManage = newGuid(); + + let mockOrganization: Organization; + let mockOrgUser: OrganizationUserView; + + beforeEach(() => { + organizationUserApiService = mock(); + organizationUserService = mock(); + keyService = mock(); + encryptService = mock(); + configService = mock(); + accountService = mockAccountServiceWith(userId); + + mockOrganization = { + id: organizationId, + type: OrganizationUserType.Owner, + canManageUsersPassword: true, + hasPublicAndPrivateKeys: true, + useResetPassword: true, + } as Organization; + + mockOrgUser = { + id: userIdToManage, + userId: userIdToManage, + type: OrganizationUserType.User, + status: OrganizationUserStatusType.Confirmed, + resetPasswordEnrolled: true, + } as OrganizationUserView; + + service = new MemberActionsService( + organizationUserApiService, + organizationUserService, + keyService, + encryptService, + configService, + accountService, + ); + }); + + describe("inviteUser", () => { + it("should successfully invite a user", async () => { + organizationUserApiService.postOrganizationUserInvite.mockResolvedValue(undefined); + + const result = await service.inviteUser( + mockOrganization, + "test@example.com", + OrganizationUserType.User, + {}, + [], + [], + ); + + expect(result).toEqual({ success: true }); + expect(organizationUserApiService.postOrganizationUserInvite).toHaveBeenCalledWith( + organizationId, + { + emails: ["test@example.com"], + type: OrganizationUserType.User, + accessSecretsManager: false, + collections: [], + groups: [], + permissions: {}, + }, + ); + }); + + it("should handle invite errors", async () => { + const errorMessage = "Invitation failed"; + organizationUserApiService.postOrganizationUserInvite.mockRejectedValue( + new Error(errorMessage), + ); + + const result = await service.inviteUser( + mockOrganization, + "test@example.com", + OrganizationUserType.User, + ); + + expect(result).toEqual({ success: false, error: errorMessage }); + }); + }); + + describe("removeUser", () => { + it("should successfully remove a user", async () => { + organizationUserApiService.removeOrganizationUser.mockResolvedValue(undefined); + + const result = await service.removeUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: true }); + expect(organizationUserApiService.removeOrganizationUser).toHaveBeenCalledWith( + organizationId, + userIdToManage, + ); + }); + + it("should handle remove errors", async () => { + const errorMessage = "Remove failed"; + organizationUserApiService.removeOrganizationUser.mockRejectedValue(new Error(errorMessage)); + + const result = await service.removeUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: false, error: errorMessage }); + }); + }); + + describe("revokeUser", () => { + it("should successfully revoke a user", async () => { + organizationUserApiService.revokeOrganizationUser.mockResolvedValue(undefined); + + const result = await service.revokeUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: true }); + expect(organizationUserApiService.revokeOrganizationUser).toHaveBeenCalledWith( + organizationId, + userIdToManage, + ); + }); + + it("should handle revoke errors", async () => { + const errorMessage = "Revoke failed"; + organizationUserApiService.revokeOrganizationUser.mockRejectedValue(new Error(errorMessage)); + + const result = await service.revokeUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: false, error: errorMessage }); + }); + }); + + describe("restoreUser", () => { + it("should successfully restore a user", async () => { + organizationUserApiService.restoreOrganizationUser.mockResolvedValue(undefined); + + const result = await service.restoreUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: true }); + expect(organizationUserApiService.restoreOrganizationUser).toHaveBeenCalledWith( + organizationId, + userIdToManage, + ); + }); + + it("should handle restore errors", async () => { + const errorMessage = "Restore failed"; + organizationUserApiService.restoreOrganizationUser.mockRejectedValue(new Error(errorMessage)); + + const result = await service.restoreUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: false, error: errorMessage }); + }); + }); + + describe("deleteUser", () => { + it("should successfully delete a user", async () => { + organizationUserApiService.deleteOrganizationUser.mockResolvedValue(undefined); + + const result = await service.deleteUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: true }); + expect(organizationUserApiService.deleteOrganizationUser).toHaveBeenCalledWith( + organizationId, + userIdToManage, + ); + }); + + it("should handle delete errors", async () => { + const errorMessage = "Delete failed"; + organizationUserApiService.deleteOrganizationUser.mockRejectedValue(new Error(errorMessage)); + + const result = await service.deleteUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: false, error: errorMessage }); + }); + }); + + describe("reinviteUser", () => { + it("should successfully reinvite a user", async () => { + organizationUserApiService.postOrganizationUserReinvite.mockResolvedValue(undefined); + + const result = await service.reinviteUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: true }); + expect(organizationUserApiService.postOrganizationUserReinvite).toHaveBeenCalledWith( + organizationId, + userIdToManage, + ); + }); + + it("should handle reinvite errors", async () => { + const errorMessage = "Reinvite failed"; + organizationUserApiService.postOrganizationUserReinvite.mockRejectedValue( + new Error(errorMessage), + ); + + const result = await service.reinviteUser(mockOrganization, userIdToManage); + + expect(result).toEqual({ success: false, error: errorMessage }); + }); + }); + + describe("confirmUser", () => { + const publicKey = new Uint8Array([1, 2, 3, 4, 5]); + + it("should confirm user using new flow when feature flag is enabled", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + organizationUserService.confirmUser.mockReturnValue(of(undefined)); + + const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); + + expect(result).toEqual({ success: true }); + expect(organizationUserService.confirmUser).toHaveBeenCalledWith( + mockOrganization, + mockOrgUser, + publicKey, + ); + expect(organizationUserApiService.postOrganizationUserConfirm).not.toHaveBeenCalled(); + }); + + it("should confirm user using exising flow when feature flag is disabled", async () => { + configService.getFeatureFlag$.mockReturnValue(of(false)); + + const mockOrgKey = mock(); + const mockOrgKeys = { [organizationId]: mockOrgKey }; + keyService.orgKeys$.mockReturnValue(of(mockOrgKeys)); + + const mockEncryptedKey = new EncString("encrypted-key-data"); + encryptService.encapsulateKeyUnsigned.mockResolvedValue(mockEncryptedKey); + + organizationUserApiService.postOrganizationUserConfirm.mockResolvedValue(undefined); + + const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); + + expect(result).toEqual({ success: true }); + expect(keyService.orgKeys$).toHaveBeenCalledWith(userId); + expect(encryptService.encapsulateKeyUnsigned).toHaveBeenCalledWith(mockOrgKey, publicKey); + expect(organizationUserApiService.postOrganizationUserConfirm).toHaveBeenCalledWith( + organizationId, + userIdToManage, + expect.objectContaining({ + key: "encrypted-key-data", + }), + ); + }); + + it("should handle missing organization keys", async () => { + configService.getFeatureFlag$.mockReturnValue(of(false)); + keyService.orgKeys$.mockReturnValue(of({})); + + const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); + + expect(result.success).toBe(false); + expect(result.error).toContain("Organization keys not found"); + }); + + it("should handle confirm errors", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + const errorMessage = "Confirm failed"; + organizationUserService.confirmUser.mockImplementation(() => { + throw new Error(errorMessage); + }); + + const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); + + expect(result.success).toBe(false); + expect(result.error).toContain(errorMessage); + }); + }); + + describe("bulkReinvite", () => { + const userIds = [newGuid(), newGuid(), newGuid()]; + + it("should successfully reinvite multiple users", async () => { + const mockResponse = { + data: userIds.map((id) => ({ + id, + error: null, + })), + continuationToken: null, + } as ListResponse; + organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); + + const result = await service.bulkReinvite(mockOrganization, userIds); + + expect(result).toEqual({ + successful: mockResponse, + failed: [], + }); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( + organizationId, + userIds, + ); + }); + + it("should handle bulk reinvite errors", async () => { + const errorMessage = "Bulk reinvite failed"; + organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( + new Error(errorMessage), + ); + + const result = await service.bulkReinvite(mockOrganization, userIds); + + expect(result.successful).toBeUndefined(); + expect(result.failed).toHaveLength(3); + expect(result.failed[0]).toEqual({ id: userIds[0], error: errorMessage }); + }); + }); + + describe("allowResetPassword", () => { + const resetPasswordEnabled = true; + + it("should allow reset password for Owner over User", () => { + const result = service.allowResetPassword( + mockOrgUser, + mockOrganization, + resetPasswordEnabled, + ); + + expect(result).toBe(true); + }); + + it("should allow reset password for Admin over User", () => { + const adminOrg = { ...mockOrganization, type: OrganizationUserType.Admin } as Organization; + + const result = service.allowResetPassword(mockOrgUser, adminOrg, resetPasswordEnabled); + + expect(result).toBe(true); + }); + + it("should not allow reset password for Admin over Owner", () => { + const adminOrg = { ...mockOrganization, type: OrganizationUserType.Admin } as Organization; + const ownerUser = { + ...mockOrgUser, + type: OrganizationUserType.Owner, + } as OrganizationUserView; + + const result = service.allowResetPassword(ownerUser, adminOrg, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should allow reset password for Custom over User", () => { + const customOrg = { ...mockOrganization, type: OrganizationUserType.Custom } as Organization; + + const result = service.allowResetPassword(mockOrgUser, customOrg, resetPasswordEnabled); + + expect(result).toBe(true); + }); + + it("should not allow reset password for Custom over Admin", () => { + const customOrg = { ...mockOrganization, type: OrganizationUserType.Custom } as Organization; + const adminUser = { + ...mockOrgUser, + type: OrganizationUserType.Admin, + } as OrganizationUserView; + + const result = service.allowResetPassword(adminUser, customOrg, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should not allow reset password for Custom over Owner", () => { + const customOrg = { ...mockOrganization, type: OrganizationUserType.Custom } as Organization; + const ownerUser = { + ...mockOrgUser, + type: OrganizationUserType.Owner, + } as OrganizationUserView; + + const result = service.allowResetPassword(ownerUser, customOrg, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should not allow reset password when organization cannot manage users password", () => { + const org = { ...mockOrganization, canManageUsersPassword: false } as Organization; + + const result = service.allowResetPassword(mockOrgUser, org, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should not allow reset password when organization does not use reset password", () => { + const org = { ...mockOrganization, useResetPassword: false } as Organization; + + const result = service.allowResetPassword(mockOrgUser, org, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should not allow reset password when organization lacks public and private keys", () => { + const org = { ...mockOrganization, hasPublicAndPrivateKeys: false } as Organization; + + const result = service.allowResetPassword(mockOrgUser, org, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should not allow reset password when user is not enrolled in reset password", () => { + const user = { ...mockOrgUser, resetPasswordEnrolled: false } as OrganizationUserView; + + const result = service.allowResetPassword(user, mockOrganization, resetPasswordEnabled); + + expect(result).toBe(false); + }); + + it("should not allow reset password when reset password is disabled", () => { + const result = service.allowResetPassword(mockOrgUser, mockOrganization, false); + + expect(result).toBe(false); + }); + + it("should not allow reset password when user status is not confirmed", () => { + const user = { + ...mockOrgUser, + status: OrganizationUserStatusType.Invited, + } as OrganizationUserView; + + const result = service.allowResetPassword(user, mockOrganization, resetPasswordEnabled); + + expect(result).toBe(false); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts index 9902f83c0900..f5bfb49f9a13 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -6,20 +6,17 @@ import { OrganizationUserBulkResponse, OrganizationUserConfirmRequest, } from "@bitwarden/admin-console/common"; -import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { OrganizationUserType, OrganizationUserStatusType, } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request"; 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 { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { KeyService } from "@bitwarden/key-management"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; @@ -41,11 +38,9 @@ export class MemberActionsService { constructor( private organizationUserApiService: OrganizationUserApiService, - private organizationApiService: OrganizationApiServiceAbstraction, private organizationUserService: OrganizationUserService, private keyService: KeyService, private encryptService: EncryptService, - private syncService: SyncService, private configService: ConfigService, private accountService: AccountService, ) {} @@ -161,35 +156,6 @@ export class MemberActionsService { } } - async ensureOrganizationKeys(organization: Organization): Promise { - if (organization.canManageUsersPassword && !organization.hasPublicAndPrivateKeys) { - const orgShareKey = await firstValueFrom( - this.userId$.pipe( - switchMap((userId) => this.keyService.orgKeys$(userId)), - map((orgKeys) => { - if (orgKeys == null || orgKeys[organization.id] == null) { - throw new Error("Organization keys not found for provided User."); - } - return orgKeys[organization.id]; - }), - ), - ); - - const [orgPublicKey, encryptedOrgPrivateKey] = await this.keyService.makeKeyPair(orgShareKey); - if (encryptedOrgPrivateKey.encryptedString == null) { - throw new Error("Encrypted private key is null."); - } - const request = new OrganizationKeysRequest( - orgPublicKey, - encryptedOrgPrivateKey.encryptedString, - ); - const response = await this.organizationApiService.updateKeys(organization.id, request); - if (response != null) { - await this.syncService.fullSync(true); - } - } - } - async bulkReinvite(organization: Organization, userIds: string[]): Promise { try { const result = await this.organizationUserApiService.postManyOrganizationUserReinvite( diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.spec.ts new file mode 100644 index 000000000000..e478f8bbb41e --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-dialog-manager/member-dialog-manager.service.spec.ts @@ -0,0 +1,640 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; +import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; +import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { DialogService, ToastService } from "@bitwarden/components"; + +import { OrganizationUserView } from "../../../core/views/organization-user.view"; +import { EntityEventsComponent } from "../../../manage/entity-events.component"; +import { AccountRecoveryDialogComponent } from "../../components/account-recovery/account-recovery-dialog.component"; +import { BulkConfirmDialogComponent } from "../../components/bulk/bulk-confirm-dialog.component"; +import { BulkDeleteDialogComponent } from "../../components/bulk/bulk-delete-dialog.component"; +import { BulkEnableSecretsManagerDialogComponent } from "../../components/bulk/bulk-enable-sm-dialog.component"; +import { BulkRemoveDialogComponent } from "../../components/bulk/bulk-remove-dialog.component"; +import { BulkRestoreRevokeComponent } from "../../components/bulk/bulk-restore-revoke.component"; +import { BulkStatusComponent } from "../../components/bulk/bulk-status.component"; +import { + MemberDialogComponent, + MemberDialogResult, + MemberDialogTab, +} from "../../components/member-dialog"; +import { DeleteManagedMemberWarningService } from "../delete-managed-member/delete-managed-member-warning.service"; + +import { MemberDialogManagerService } from "./member-dialog-manager.service"; + +describe("MemberDialogManagerService", () => { + let service: MemberDialogManagerService; + let dialogService: MockProxy; + let i18nService: MockProxy; + let toastService: MockProxy; + let userNamePipe: MockProxy; + let deleteManagedMemberWarningService: MockProxy; + + let mockOrganization: Organization; + let mockUser: OrganizationUserView; + let mockBillingMetadata: OrganizationBillingMetadataResponse; + + beforeEach(() => { + dialogService = mock(); + i18nService = mock(); + toastService = mock(); + userNamePipe = mock(); + deleteManagedMemberWarningService = mock(); + + service = new MemberDialogManagerService( + dialogService, + i18nService, + toastService, + userNamePipe, + deleteManagedMemberWarningService, + ); + + // Setup mock data + mockOrganization = { + id: "org-id", + canManageUsers: true, + productTierType: ProductTierType.Enterprise, + } as Organization; + + mockUser = { + id: "user-id", + email: "test@example.com", + name: "Test User", + usesKeyConnector: false, + status: OrganizationUserStatusType.Confirmed, + hasMasterPassword: true, + accessSecretsManager: false, + managedByOrganization: false, + } as OrganizationUserView; + + mockBillingMetadata = { + organizationOccupiedSeats: 10, + isOnSecretsManagerStandalone: false, + } as OrganizationBillingMetadataResponse; + + userNamePipe.transform.mockReturnValue("Test User"); + }); + + describe("openInviteDialog", () => { + it("should open the invite dialog with correct parameters", async () => { + const mockDialogRef = { closed: of(MemberDialogResult.Saved) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const allUserEmails = ["user1@example.com", "user2@example.com"]; + + const result = await service.openInviteDialog( + mockOrganization, + mockBillingMetadata, + allUserEmails, + ); + + expect(dialogService.open).toHaveBeenCalledWith( + MemberDialogComponent, + expect.objectContaining({ + data: { + kind: "Add", + organizationId: mockOrganization.id, + allOrganizationUserEmails: allUserEmails, + occupiedSeatCount: 10, + isOnSecretsManagerStandalone: false, + }, + }), + ); + expect(result).toBe(MemberDialogResult.Saved); + }); + + it("should return Canceled when dialog is closed without result", async () => { + const mockDialogRef = { closed: of(null) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const result = await service.openInviteDialog(mockOrganization, mockBillingMetadata, []); + + expect(result).toBe(MemberDialogResult.Canceled); + }); + + it("should handle null billing metadata", async () => { + const mockDialogRef = { closed: of(MemberDialogResult.Saved) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + await service.openInviteDialog(mockOrganization, null, []); + + expect(dialogService.open).toHaveBeenCalledWith( + MemberDialogComponent, + expect.objectContaining({ + data: expect.objectContaining({ + occupiedSeatCount: 0, + isOnSecretsManagerStandalone: false, + }), + }), + ); + }); + }); + + describe("openEditDialog", () => { + it("should open the edit dialog with correct parameters", async () => { + const mockDialogRef = { closed: of(MemberDialogResult.Saved) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const result = await service.openEditDialog(mockUser, mockOrganization, mockBillingMetadata); + + expect(dialogService.open).toHaveBeenCalledWith( + MemberDialogComponent, + expect.objectContaining({ + data: { + kind: "Edit", + name: "Test User", + organizationId: mockOrganization.id, + organizationUserId: mockUser.id, + usesKeyConnector: false, + isOnSecretsManagerStandalone: false, + initialTab: MemberDialogTab.Role, + managedByOrganization: false, + }, + }), + ); + expect(result).toBe(MemberDialogResult.Saved); + }); + + it("should use custom initial tab when provided", async () => { + const mockDialogRef = { closed: of(MemberDialogResult.Saved) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + await service.openEditDialog( + mockUser, + mockOrganization, + mockBillingMetadata, + MemberDialogTab.AccountRecovery, + ); + + expect(dialogService.open).toHaveBeenCalledWith( + MemberDialogComponent, + expect.objectContaining({ + data: expect.objectContaining({ + initialTab: 0, // MemberDialogTab.AccountRecovery is 0 + }), + }), + ); + }); + + it("should return Canceled when dialog is closed without result", async () => { + const mockDialogRef = { closed: of(null) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const result = await service.openEditDialog(mockUser, mockOrganization, mockBillingMetadata); + + expect(result).toBe(MemberDialogResult.Canceled); + }); + }); + + describe("openAccountRecoveryDialog", () => { + it("should open account recovery dialog with correct parameters", async () => { + const mockDialogRef = { closed: of("recovered") }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const result = await service.openAccountRecoveryDialog(mockUser, mockOrganization); + + expect(dialogService.open).toHaveBeenCalledWith( + AccountRecoveryDialogComponent, + expect.objectContaining({ + data: { + name: "Test User", + email: mockUser.email, + organizationId: mockOrganization.id, + organizationUserId: mockUser.id, + }, + }), + ); + expect(result).toBe("recovered"); + }); + + it("should return Ok when dialog is closed without result", async () => { + const mockDialogRef = { closed: of(null) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const result = await service.openAccountRecoveryDialog(mockUser, mockOrganization); + + expect(result).toBe("ok"); + }); + }); + + describe("openBulkConfirmDialog", () => { + it("should open bulk confirm dialog with correct parameters", async () => { + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkConfirmDialog(mockOrganization, users); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkConfirmDialogComponent, + expect.objectContaining({ + data: { + organization: mockOrganization, + users: users, + }, + }), + ); + }); + }); + + describe("openBulkRemoveDialog", () => { + it("should open bulk remove dialog with correct parameters", async () => { + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkRemoveDialog(mockOrganization, users); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkRemoveDialogComponent, + expect.objectContaining({ + data: { + organizationId: mockOrganization.id, + users: users, + }, + }), + ); + }); + }); + + describe("openBulkDeleteDialog", () => { + it("should open bulk delete dialog when warning already acknowledged", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(true)); + + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkDeleteDialog(mockOrganization, users); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkDeleteDialogComponent, + expect.objectContaining({ + data: { + organizationId: mockOrganization.id, + users: users, + }, + }), + ); + expect(deleteManagedMemberWarningService.showWarning).not.toHaveBeenCalled(); + }); + + it("should show warning before opening dialog for enterprise organizations", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(false)); + deleteManagedMemberWarningService.showWarning.mockResolvedValue(true); + + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkDeleteDialog(mockOrganization, users); + + expect(deleteManagedMemberWarningService.showWarning).toHaveBeenCalled(); + expect(dialogService.open).toHaveBeenCalled(); + }); + + it("should not open dialog if warning is not acknowledged", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(false)); + deleteManagedMemberWarningService.showWarning.mockResolvedValue(false); + + const users = [mockUser]; + await service.openBulkDeleteDialog(mockOrganization, users); + + expect(deleteManagedMemberWarningService.showWarning).toHaveBeenCalled(); + expect(dialogService.open).not.toHaveBeenCalled(); + }); + + it("should skip warning for non-enterprise organizations", async () => { + const nonEnterpriseOrg = { + ...mockOrganization, + productTierType: ProductTierType.Free, + } as Organization; + + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(false)); + + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkDeleteDialog(nonEnterpriseOrg, users); + + expect(deleteManagedMemberWarningService.showWarning).not.toHaveBeenCalled(); + expect(dialogService.open).toHaveBeenCalled(); + }); + }); + + describe("openBulkRestoreRevokeDialog", () => { + it("should open bulk restore revoke dialog with correct parameters for revoking", async () => { + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkRestoreRevokeDialog(mockOrganization, users, true); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkRestoreRevokeComponent, + expect.objectContaining({ + data: expect.objectContaining({ + organizationId: mockOrganization.id, + users: users, + isRevoking: true, + }), + }), + ); + }); + + it("should open bulk restore revoke dialog with correct parameters for restoring", async () => { + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + await service.openBulkRestoreRevokeDialog(mockOrganization, users, false); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkRestoreRevokeComponent, + expect.objectContaining({ + data: expect.objectContaining({ + organizationId: mockOrganization.id, + users: users, + isRevoking: false, + }), + }), + ); + }); + }); + + describe("openBulkEnableSecretsManagerDialog", () => { + it("should open dialog with eligible users only", async () => { + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const user1 = { ...mockUser, accessSecretsManager: false } as OrganizationUserView; + const user2 = { + ...mockUser, + id: "user-2", + accessSecretsManager: true, + } as OrganizationUserView; + const users = [user1, user2]; + + await service.openBulkEnableSecretsManagerDialog(mockOrganization, users); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkEnableSecretsManagerDialogComponent, + expect.objectContaining({ + data: expect.objectContaining({ + orgId: mockOrganization.id, + users: [user1], + }), + }), + ); + }); + + it("should show error toast when no eligible users", async () => { + i18nService.t.mockImplementation((key) => key); + + const user1 = { ...mockUser, accessSecretsManager: true } as OrganizationUserView; + const users = [user1]; + + await service.openBulkEnableSecretsManagerDialog(mockOrganization, users); + + expect(toastService.showToast).toHaveBeenCalledWith({ + variant: "error", + title: "errorOccurred", + message: "noSelectedUsersApplicable", + }); + expect(dialogService.open).not.toHaveBeenCalled(); + }); + }); + + describe("openBulkStatusDialog", () => { + it("should open bulk status dialog with correct parameters", async () => { + const mockDialogRef = { closed: of(undefined) }; + dialogService.open.mockReturnValue(mockDialogRef as any); + + const users = [mockUser]; + const filteredUsers = [mockUser]; + const request = Promise.resolve(); + const successMessage = "Success!"; + + await service.openBulkStatusDialog(users, filteredUsers, request, successMessage); + + expect(dialogService.open).toHaveBeenCalledWith( + BulkStatusComponent, + expect.objectContaining({ + data: { + users: users, + filteredUsers: filteredUsers, + request: request, + successfulMessage: successMessage, + }, + }), + ); + }); + }); + + describe("openEventsDialog", () => { + it("should open events dialog with correct parameters", () => { + service.openEventsDialog(mockUser, mockOrganization); + + expect(dialogService.open).toHaveBeenCalledWith( + EntityEventsComponent, + expect.objectContaining({ + data: { + name: "Test User", + organizationId: mockOrganization.id, + entityId: mockUser.id, + showUser: false, + entity: "user", + }, + }), + ); + }); + }); + + describe("openRemoveUserConfirmationDialog", () => { + it("should return true when user confirms removal", async () => { + dialogService.openSimpleDialog.mockResolvedValue(true); + + const result = await service.openRemoveUserConfirmationDialog(mockUser); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith({ + title: { + key: "removeUserIdAccess", + placeholders: ["Test User"], + }, + content: { key: "removeOrgUserConfirmation" }, + type: "warning", + }); + expect(result).toBe(true); + }); + + it("should show key connector warning when user uses key connector", async () => { + const keyConnectorUser = { ...mockUser, usesKeyConnector: true } as OrganizationUserView; + dialogService.openSimpleDialog.mockResolvedValue(true); + + await service.openRemoveUserConfirmationDialog(keyConnectorUser); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith( + expect.objectContaining({ + content: { key: "removeUserConfirmationKeyConnector" }, + }), + ); + }); + + it("should return false when user cancels confirmation", async () => { + dialogService.openSimpleDialog.mockResolvedValue(false); + + const result = await service.openRemoveUserConfirmationDialog(mockUser); + + expect(result).toBe(false); + }); + + it("should show no master password warning for confirmed users without master password", async () => { + const noMpUser = { + ...mockUser, + status: OrganizationUserStatusType.Confirmed, + hasMasterPassword: false, + } as OrganizationUserView; + + dialogService.openSimpleDialog.mockResolvedValueOnce(true).mockResolvedValueOnce(true); + + const result = await service.openRemoveUserConfirmationDialog(noMpUser); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledTimes(2); + expect(dialogService.openSimpleDialog).toHaveBeenLastCalledWith({ + title: { + key: "removeOrgUserNoMasterPasswordTitle", + }, + content: { + key: "removeOrgUserNoMasterPasswordDesc", + placeholders: ["Test User"], + }, + type: "warning", + }); + expect(result).toBe(true); + }); + }); + + describe("openRevokeUserConfirmationDialog", () => { + it("should return true when user confirms revocation", async () => { + i18nService.t.mockReturnValue("Revoke user confirmation"); + dialogService.openSimpleDialog.mockResolvedValue(true); + + const result = await service.openRevokeUserConfirmationDialog(mockUser); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith({ + title: { key: "revokeAccess", placeholders: ["Test User"] }, + content: "Revoke user confirmation", + acceptButtonText: { key: "revokeAccess" }, + type: "warning", + }); + expect(result).toBe(true); + }); + + it("should return false when user cancels confirmation", async () => { + i18nService.t.mockReturnValue("Revoke user confirmation"); + dialogService.openSimpleDialog.mockResolvedValue(false); + + const result = await service.openRevokeUserConfirmationDialog(mockUser); + + expect(result).toBe(false); + }); + + it("should show no master password warning for confirmed users without master password", async () => { + const noMpUser = { + ...mockUser, + status: OrganizationUserStatusType.Confirmed, + hasMasterPassword: false, + } as OrganizationUserView; + + i18nService.t.mockReturnValue("Revoke user confirmation"); + dialogService.openSimpleDialog.mockResolvedValueOnce(true).mockResolvedValueOnce(true); + + const result = await service.openRevokeUserConfirmationDialog(noMpUser); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledTimes(2); + expect(result).toBe(true); + }); + }); + + describe("openDeleteUserConfirmationDialog", () => { + it("should return true when user confirms deletion", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(true)); + dialogService.openSimpleDialog.mockResolvedValue(true); + + const result = await service.openDeleteUserConfirmationDialog(mockUser, mockOrganization); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith({ + title: { + key: "deleteOrganizationUser", + placeholders: ["Test User"], + }, + content: { + key: "deleteOrganizationUserWarningDesc", + placeholders: ["Test User"], + }, + type: "warning", + acceptButtonText: { key: "delete" }, + cancelButtonText: { key: "cancel" }, + }); + expect(deleteManagedMemberWarningService.acknowledgeWarning).toHaveBeenCalledWith( + mockOrganization.id, + ); + expect(result).toBe(true); + }); + + it("should show warning before deletion for enterprise organizations", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(false)); + deleteManagedMemberWarningService.showWarning.mockResolvedValue(true); + dialogService.openSimpleDialog.mockResolvedValue(true); + + const result = await service.openDeleteUserConfirmationDialog(mockUser, mockOrganization); + + expect(deleteManagedMemberWarningService.showWarning).toHaveBeenCalled(); + expect(dialogService.openSimpleDialog).toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("should return false if warning is not acknowledged", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(false)); + deleteManagedMemberWarningService.showWarning.mockResolvedValue(false); + + const result = await service.openDeleteUserConfirmationDialog(mockUser, mockOrganization); + + expect(deleteManagedMemberWarningService.showWarning).toHaveBeenCalled(); + expect(dialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(result).toBe(false); + }); + + it("should skip warning for non-enterprise organizations", async () => { + const nonEnterpriseOrg = { + ...mockOrganization, + productTierType: ProductTierType.Free, + } as Organization; + + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(false)); + dialogService.openSimpleDialog.mockResolvedValue(true); + + const result = await service.openDeleteUserConfirmationDialog(mockUser, nonEnterpriseOrg); + + expect(deleteManagedMemberWarningService.showWarning).not.toHaveBeenCalled(); + expect(dialogService.openSimpleDialog).toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("should return false when user cancels confirmation", async () => { + deleteManagedMemberWarningService.warningAcknowledged.mockReturnValue(of(true)); + dialogService.openSimpleDialog.mockResolvedValue(false); + + const result = await service.openDeleteUserConfirmationDialog(mockUser, mockOrganization); + + expect(result).toBe(false); + expect(deleteManagedMemberWarningService.acknowledgeWarning).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.spec.ts new file mode 100644 index 000000000000..615d2ece463d --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.spec.ts @@ -0,0 +1,362 @@ +import { TestBed } from "@angular/core/testing"; + +import { + OrganizationUserApiService, + OrganizationUserUserDetailsResponse, +} from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ListResponse } from "@bitwarden/common/models/response/list.response"; +import { OrganizationId } from "@bitwarden/common/types/guid"; + +import { GroupApiService } from "../../../core"; + +import { OrganizationMembersService } from "./organization-members.service"; + +describe("OrganizationMembersService", () => { + let service: OrganizationMembersService; + let organizationUserApiService: jest.Mocked; + let groupService: jest.Mocked; + let apiService: jest.Mocked; + + const mockOrganizationId = "org-123" as OrganizationId; + + const createMockOrganization = (overrides: Partial = {}): Organization => { + const org = new Organization(); + org.id = mockOrganizationId; + org.useGroups = false; + + return Object.assign(org, overrides); + }; + + const createMockUserResponse = ( + overrides: Partial = {}, + ): OrganizationUserUserDetailsResponse => { + return { + id: "user-1", + userId: "user-id-1", + email: "test@example.com", + name: "Test User", + collections: [], + groups: [], + ...overrides, + } as OrganizationUserUserDetailsResponse; + }; + + const createMockGroup = (id: string, name: string) => ({ + id, + name, + }); + + const createMockCollection = (id: string, name: string) => ({ + id, + name, + }); + + beforeEach(() => { + organizationUserApiService = { + getAllUsers: jest.fn(), + } as any; + + groupService = { + getAll: jest.fn(), + } as any; + + apiService = { + getCollections: jest.fn(), + } as any; + + TestBed.configureTestingModule({ + providers: [ + OrganizationMembersService, + { provide: OrganizationUserApiService, useValue: organizationUserApiService }, + { provide: GroupApiService, useValue: groupService }, + { provide: ApiService, useValue: apiService }, + ], + }); + + service = TestBed.inject(OrganizationMembersService); + }); + + describe("loadUsers", () => { + it("should load users with collections when organization does not use groups", async () => { + const organization = createMockOrganization({ useGroups: false }); + const mockUser = createMockUserResponse({ + collections: [{ id: "col-1" } as any], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + const mockCollections = [createMockCollection("col-1", "Collection 1")]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + apiService.getCollections.mockResolvedValue({ + data: mockCollections, + } as any); + + const result = await service.loadUsers(organization); + + expect(organizationUserApiService.getAllUsers).toHaveBeenCalledWith(mockOrganizationId, { + includeGroups: false, + includeCollections: true, + }); + expect(apiService.getCollections).toHaveBeenCalledWith(mockOrganizationId); + expect(groupService.getAll).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + expect(result[0].collectionNames).toEqual(["Collection 1"]); + expect(result[0].groupNames).toEqual([]); + }); + + it("should load users with groups when organization uses groups", async () => { + const organization = createMockOrganization({ useGroups: true }); + const mockUser = createMockUserResponse({ + groups: ["group-1", "group-2"], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + const mockGroups = [ + createMockGroup("group-1", "Group 1"), + createMockGroup("group-2", "Group 2"), + ]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + groupService.getAll.mockResolvedValue(mockGroups as any); + + const result = await service.loadUsers(organization); + + expect(organizationUserApiService.getAllUsers).toHaveBeenCalledWith(mockOrganizationId, { + includeGroups: true, + includeCollections: false, + }); + expect(groupService.getAll).toHaveBeenCalledWith(mockOrganizationId); + expect(apiService.getCollections).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + expect(result[0].groupNames).toEqual(["Group 1", "Group 2"]); + expect(result[0].collectionNames).toEqual([]); + }); + + it("should sort group names alphabetically", async () => { + const organization = createMockOrganization({ useGroups: true }); + const mockUser = createMockUserResponse({ + groups: ["group-1", "group-2", "group-3"], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + const mockGroups = [ + createMockGroup("group-1", "Zebra Group"), + createMockGroup("group-2", "Alpha Group"), + createMockGroup("group-3", "Beta Group"), + ]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + groupService.getAll.mockResolvedValue(mockGroups as any); + + const result = await service.loadUsers(organization); + + expect(result[0].groupNames).toEqual(["Alpha Group", "Beta Group", "Zebra Group"]); + }); + + it("should sort collection names alphabetically", async () => { + const organization = createMockOrganization({ useGroups: false }); + const mockUser = createMockUserResponse({ + collections: [{ id: "col-1" } as any, { id: "col-2" } as any, { id: "col-3" } as any], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + const mockCollections = [ + createMockCollection("col-1", "Zebra Collection"), + createMockCollection("col-2", "Alpha Collection"), + createMockCollection("col-3", "Beta Collection"), + ]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + apiService.getCollections.mockResolvedValue({ + data: mockCollections, + } as any); + + const result = await service.loadUsers(organization); + + expect(result[0].collectionNames).toEqual([ + "Alpha Collection", + "Beta Collection", + "Zebra Collection", + ]); + }); + + it("should filter out null or undefined group names", async () => { + const organization = createMockOrganization({ useGroups: true }); + const mockUser = createMockUserResponse({ + groups: ["group-1", "group-2", "group-3"], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + const mockGroups = [ + createMockGroup("group-1", "Group 1"), + // group-2 is missing - should be filtered out + createMockGroup("group-3", "Group 3"), + ]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + groupService.getAll.mockResolvedValue(mockGroups as any); + + const result = await service.loadUsers(organization); + + expect(result[0].groupNames).toEqual(["Group 1", "Group 3"]); + expect(result[0].groupNames).not.toContain(undefined); + expect(result[0].groupNames).not.toContain(null); + }); + + it("should filter out null or undefined collection names", async () => { + const organization = createMockOrganization({ useGroups: false }); + const mockUser = createMockUserResponse({ + collections: [{ id: "col-1" } as any, { id: "col-2" } as any, { id: "col-3" } as any], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + const mockCollections = [ + createMockCollection("col-1", "Collection 1"), + // col-2 is missing - should be filtered out + createMockCollection("col-3", "Collection 3"), + ]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + apiService.getCollections.mockResolvedValue({ + data: mockCollections, + } as any); + + const result = await service.loadUsers(organization); + + expect(result[0].collectionNames).toEqual(["Collection 1", "Collection 3"]); + expect(result[0].collectionNames).not.toContain(undefined); + expect(result[0].collectionNames).not.toContain(null); + }); + + it("should handle multiple users", async () => { + const organization = createMockOrganization({ useGroups: true }); + const mockUser1 = createMockUserResponse({ + id: "user-1", + groups: ["group-1"], + }); + const mockUser2 = createMockUserResponse({ + id: "user-2", + groups: ["group-2"], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser1, mockUser2], + } as any; + const mockGroups = [ + createMockGroup("group-1", "Group 1"), + createMockGroup("group-2", "Group 2"), + ]; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + groupService.getAll.mockResolvedValue(mockGroups as any); + + const result = await service.loadUsers(organization); + + expect(result).toHaveLength(2); + expect(result[0].groupNames).toEqual(["Group 1"]); + expect(result[1].groupNames).toEqual(["Group 2"]); + }); + + it("should return empty array when usersResponse.data is null", async () => { + const organization = createMockOrganization({ useGroups: false }); + const mockUsersResponse: ListResponse = { + data: null as any, + } as any; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + apiService.getCollections.mockResolvedValue({ + data: [], + } as any); + + const result = await service.loadUsers(organization); + + expect(result).toEqual([]); + }); + + it("should return empty array when usersResponse.data is undefined", async () => { + const organization = createMockOrganization({ useGroups: false }); + const mockUsersResponse: ListResponse = { + data: undefined as any, + } as any; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + apiService.getCollections.mockResolvedValue({ + data: [], + } as any); + + const result = await service.loadUsers(organization); + + expect(result).toEqual([]); + }); + + it("should handle empty groups array", async () => { + const organization = createMockOrganization({ useGroups: true }); + const mockUser = createMockUserResponse({ + groups: [], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + groupService.getAll.mockResolvedValue([]); + + const result = await service.loadUsers(organization); + + expect(result).toHaveLength(1); + expect(result[0].groupNames).toEqual([]); + }); + + it("should handle empty collections array", async () => { + const organization = createMockOrganization({ useGroups: false }); + const mockUser = createMockUserResponse({ + collections: [], + }); + const mockUsersResponse: ListResponse = { + data: [mockUser], + } as any; + + organizationUserApiService.getAllUsers.mockResolvedValue(mockUsersResponse); + apiService.getCollections.mockResolvedValue({ + data: [], + } as any); + + const result = await service.loadUsers(organization); + + expect(result).toHaveLength(1); + expect(result[0].collectionNames).toEqual([]); + }); + + it("should fetch data in parallel using Promise.all", async () => { + const organization = createMockOrganization({ useGroups: true }); + const mockUsersResponse: ListResponse = { + data: [], + } as any; + + let getUsersCallTime: number; + let getGroupsCallTime: number; + + organizationUserApiService.getAllUsers.mockImplementation(async () => { + getUsersCallTime = Date.now(); + return mockUsersResponse; + }); + + groupService.getAll.mockImplementation(async () => { + getGroupsCallTime = Date.now(); + return []; + }); + + await service.loadUsers(organization); + + // Both calls should have been initiated at roughly the same time (within 50ms) + expect(Math.abs(getUsersCallTime - getGroupsCallTime)).toBeLessThan(50); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts b/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts index bd470f89a200..613c7c1b9c08 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/organization-members-service/organization-members.service.ts @@ -1,85 +1,20 @@ import { Injectable } from "@angular/core"; -import { Observable, combineLatest, switchMap, map, filter, shareReplay, from } from "rxjs"; import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { PolicyApiServiceAbstraction as PolicyApiService } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; -import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; -import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { OrganizationId } from "@bitwarden/common/types/guid"; import { GroupApiService } from "../../../core"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; -export interface OrganizationMemberState { - organization: Organization | null; - users: OrganizationUserView[]; - policies: Policy[]; - resetPasswordPolicyEnabled: boolean; - loading: boolean; -} - @Injectable() export class OrganizationMembersService { constructor( - private organizationService: OrganizationService, private organizationUserApiService: OrganizationUserApiService, - private policyService: PolicyService, - private policyApiService: PolicyApiService, private groupService: GroupApiService, private apiService: ApiService, - private accountService: AccountService, ) {} - getMemberState(organizationId: OrganizationId): Observable { - const userId$ = this.accountService.activeAccount$.pipe(getUserId); - - const organization$ = userId$.pipe( - switchMap((userId) => - this.organizationService.organizations$(userId).pipe(getOrganizationById(organizationId)), - ), - filter((organization): organization is Organization => organization != null), - shareReplay({ refCount: true, bufferSize: 1 }), - ); - - const policies$ = combineLatest([userId$, organization$]).pipe( - switchMap(([userId, organization]) => - organization.isProviderUser - ? from(this.policyApiService.getPolicies(organization.id)).pipe( - map((response) => Policy.fromListResponse(response)), - ) - : this.policyService.policies$(userId), - ), - ); - - const users$ = organization$.pipe(switchMap((organization) => this.loadUsers(organization))); - - return combineLatest([organization$, users$, policies$]).pipe( - map(([organization, users, policies]) => { - const resetPasswordPolicy = policies - .filter((policy) => policy.type === PolicyType.ResetPassword) - .find((p) => p.organizationId === organization.id); - - return { - organization, - users, - policies, - resetPasswordPolicyEnabled: resetPasswordPolicy?.enabled ?? false, - loading: false, // Data is loaded when we reach this point - }; - }), - shareReplay({ refCount: true, bufferSize: 1 }), - ); - } - async loadUsers(organization: Organization): Promise { let groupsPromise: Promise> | undefined; let collectionsPromise: Promise> | undefined; From b1b214931f324d1f7011c11ec4cec8f767d0d2be Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 2 Oct 2025 11:16:05 -0400 Subject: [PATCH 06/10] clean up --- .../organizations/members/members.component.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index da27b318fcce..0d420e2bf8c0 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -131,6 +131,8 @@ export class MembersComponent extends BaseMembersComponent switchMap((userId) => this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), ), + filter((organization): organization is Organization => organization != null), + shareReplay({ refCount: true, bufferSize: 1 }), ), ), ); @@ -139,7 +141,7 @@ export class MembersComponent extends BaseMembersComponent const policies$ = combineLatest([this.userId$, organization$]).pipe( switchMap(([userId, organization]) => - organization?.isProviderUser + organization.isProviderUser ? from(this.policyApiService.getPolicies(organization.id)).pipe( map((response) => Policy.fromListResponse(response)), ) @@ -152,7 +154,7 @@ export class MembersComponent extends BaseMembersComponent ([organization, policies]) => policies .filter((policy) => policy.type === PolicyType.ResetPassword) - .find((p) => p.organizationId === organization?.id)?.enabled ?? false, + .find((p) => p.organizationId === organization.id)?.enabled ?? false, ), ); @@ -176,7 +178,6 @@ export class MembersComponent extends BaseMembersComponent organization$ .pipe( - filter((organization) => organization != null), switchMap((organization) => merge( this.organizationWarningsService.showInactiveSubscriptionDialog$(organization), @@ -188,7 +189,6 @@ export class MembersComponent extends BaseMembersComponent .subscribe(); this.billingMetadata$ = organization$.pipe( - filter((organization) => organization != null), switchMap((organization) => this.billingConstraint.getBillingMetadata$(organization.id)), shareReplay({ bufferSize: 1, refCount: false }), ); From d6b469dc727a13d869396b8179c2c334902db09f Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 16 Oct 2025 15:20:56 -0400 Subject: [PATCH 07/10] move BillingConstraintService to billing ownership --- .../organizations/members/members.component.ts | 7 ++----- .../admin-console/organizations/members/services/index.ts | 1 - .../billing-constraint/billing-constraint.service.spec.ts | 4 ++-- .../billing-constraint/billing-constraint.service.ts | 4 ++-- 4 files changed, 6 insertions(+), 10 deletions(-) rename apps/web/src/app/{admin-console/organizations/members/services => billing/members}/billing-constraint/billing-constraint.service.spec.ts (99%) rename apps/web/src/app/{admin-console/organizations/members/services => billing/members}/billing-constraint/billing-constraint.service.ts (96%) diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 0d420e2bf8c0..ea1033661ae5 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -39,6 +39,7 @@ import { getById } from "@bitwarden/common/platform/misc"; import { DialogService, ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; import { UserId } from "@bitwarden/user-core"; +import { BillingConstraintService } from "@bitwarden/web-vault/app/billing/members/billing-constraint/billing-constraint.service"; import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; import { BaseMembersComponent } from "../../common/base-members.component"; @@ -47,11 +48,7 @@ import { OrganizationUserView } from "../core/views/organization-user.view"; import { AccountRecoveryDialogResultType } from "./components/account-recovery/account-recovery-dialog.component"; import { MemberDialogResult, MemberDialogTab } from "./components/member-dialog"; -import { - BillingConstraintService, - MemberDialogManagerService, - OrganizationMembersService, -} from "./services"; +import { MemberDialogManagerService, OrganizationMembersService } from "./services"; import { DeleteManagedMemberWarningService } from "./services/delete-managed-member/delete-managed-member-warning.service"; import { MemberActionsService, diff --git a/apps/web/src/app/admin-console/organizations/members/services/index.ts b/apps/web/src/app/admin-console/organizations/members/services/index.ts index 96b9ab5d6d6f..2ac2d31cd691 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/index.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/index.ts @@ -1,6 +1,5 @@ export { OrganizationMembersService } from "./organization-members-service/organization-members.service"; export { MemberActionsService } from "./member-actions/member-actions.service"; -export { BillingConstraintService } from "./billing-constraint/billing-constraint.service"; export { MemberDialogManagerService } from "./member-dialog-manager/member-dialog-manager.service"; export { DeleteManagedMemberWarningService } from "./delete-managed-member/delete-managed-member-warning.service"; export { OrganizationUserService } from "./organization-user/organization-user.service"; diff --git a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts similarity index 99% rename from apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts rename to apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts index 314190de76a9..fc5b00a407c8 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.spec.ts +++ b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts @@ -13,11 +13,11 @@ import { DialogService, ToastService } from "@bitwarden/components"; import { ChangePlanDialogResultType, openChangePlanDialog, -} from "../../../../../billing/organizations/change-plan-dialog.component"; +} from "../../organizations/change-plan-dialog.component"; import { BillingConstraintService, SeatLimitResult } from "./billing-constraint.service"; -jest.mock("../../../../../billing/organizations/change-plan-dialog.component"); +jest.mock("../../organizations/change-plan-dialog.component"); describe("BillingConstraintService", () => { let service: BillingConstraintService; diff --git a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts similarity index 96% rename from apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts rename to apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts index d9f324ccd504..38548e7e2274 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/billing-constraint/billing-constraint.service.ts +++ b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts @@ -10,11 +10,11 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { OrganizationId } from "@bitwarden/common/types/guid"; import { DialogService, ToastService } from "@bitwarden/components"; +import { isFixedSeatPlan } from "../../../admin-console/organizations/members/components/member-dialog/validators/org-seat-limit-reached.validator"; import { ChangePlanDialogResultType, openChangePlanDialog, -} from "../../../../../billing/organizations/change-plan-dialog.component"; -import { isFixedSeatPlan } from "../../components/member-dialog/validators/org-seat-limit-reached.validator"; +} from "../../organizations/change-plan-dialog.component"; export interface SeatLimitResult { canAddUsers: boolean; From cac9dc49a1dbf5bdacda554ab5104e8d119cd4e5 Mon Sep 17 00:00:00 2001 From: Brandon Date: Thu, 16 Oct 2025 15:25:11 -0400 Subject: [PATCH 08/10] fix import --- .../app/admin-console/organizations/members/members.module.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/app/admin-console/organizations/members/members.module.ts b/apps/web/src/app/admin-console/organizations/members/members.module.ts index 526558450d5b..3b233932ed30 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.module.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.module.ts @@ -4,6 +4,7 @@ import { NgModule } from "@angular/core"; import { PasswordStrengthV2Component } from "@bitwarden/angular/tools/password-strength/password-strength-v2.component"; import { PasswordCalloutComponent } from "@bitwarden/auth/angular"; import { ScrollLayoutDirective } from "@bitwarden/components"; +import { BillingConstraintService } from "@bitwarden/web-vault/app/billing/members/billing-constraint/billing-constraint.service"; import { OrganizationFreeTrialWarningComponent } from "@bitwarden/web-vault/app/billing/organizations/warnings/components"; import { HeaderModule } from "../../../layouts/header/header.module"; @@ -21,7 +22,6 @@ import { MembersComponent } from "./members.component"; import { OrganizationMembersService, MemberActionsService, - BillingConstraintService, MemberDialogManagerService, } from "./services"; From 37988a7f9873f887580814f6fdf001de37854c62 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 20 Oct 2025 12:08:31 -0400 Subject: [PATCH 09/10] fix seat count not updating if feature flag is disabled --- .../members/members.component.ts | 1 + .../member-actions.service.spec.ts | 4 + .../member-actions/member-actions.service.ts | 4 + .../organization-metadata.service.spec.ts | 6 +- .../organization-metadata.service.ts | 73 +++++++++---------- 5 files changed, 48 insertions(+), 40 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index ea1033661ae5..d29702cad80a 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -356,6 +356,7 @@ export class MembersComponent extends BaseMembersComponent organization, this.dataSource.getCheckedUsers(), ); + this.billingConstraint.refreshBillingMetadata(); await this.load(organization); } diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts index af6cddaeacd4..6fd7de7b2927 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts @@ -20,6 +20,7 @@ import { OrgKey } from "@bitwarden/common/types/key"; import { newGuid } from "@bitwarden/guid"; import { KeyService } from "@bitwarden/key-management"; +import { BillingConstraintService } from "../../../../../billing/members/billing-constraint/billing-constraint.service"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; import { OrganizationUserService } from "../organization-user/organization-user.service"; @@ -33,6 +34,7 @@ describe("MemberActionsService", () => { let encryptService: MockProxy; let configService: MockProxy; let accountService: FakeAccountService; + let billingConstraintService: MockProxy; const userId = newGuid() as UserId; const organizationId = newGuid() as OrganizationId; @@ -48,6 +50,7 @@ describe("MemberActionsService", () => { encryptService = mock(); configService = mock(); accountService = mockAccountServiceWith(userId); + billingConstraintService = mock(); mockOrganization = { id: organizationId, @@ -72,6 +75,7 @@ describe("MemberActionsService", () => { encryptService, configService, accountService, + billingConstraintService, ); }); diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts index f5bfb49f9a13..bb951cb4c723 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -18,6 +18,7 @@ import { EncryptService } from "@bitwarden/common/key-management/crypto/abstract import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { KeyService } from "@bitwarden/key-management"; +import { BillingConstraintService } from "@bitwarden/web-vault/app/billing/members/billing-constraint/billing-constraint.service"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; import { OrganizationUserService } from "../organization-user/organization-user.service"; @@ -43,6 +44,7 @@ export class MemberActionsService { private encryptService: EncryptService, private configService: ConfigService, private accountService: AccountService, + private billingConstraint: BillingConstraintService, ) {} async inviteUser( @@ -71,6 +73,7 @@ export class MemberActionsService { async removeUser(organization: Organization, userId: string): Promise { try { await this.organizationUserApiService.removeOrganizationUser(organization.id, userId); + this.billingConstraint.refreshBillingMetadata(); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; @@ -98,6 +101,7 @@ export class MemberActionsService { async deleteUser(organization: Organization, userId: string): Promise { try { await this.organizationUserApiService.deleteOrganizationUser(organization.id, userId); + this.billingConstraint.refreshBillingMetadata(); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; diff --git a/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts b/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts index 0ed60bef6052..8cf17ccf4a6e 100644 --- a/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts +++ b/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts @@ -208,7 +208,7 @@ describe("DefaultOrganizationMetadataService", () => { }, 10); }); - it("does not trigger refresh when feature flag is disabled", async () => { + it("does trigger refresh when feature flag is disabled", async () => { featureFlagSubject.next(false); const mockResponse1 = createMockMetadataResponse(false, 10); @@ -235,8 +235,8 @@ describe("DefaultOrganizationMetadataService", () => { // wait to ensure no additional invocations await new Promise((resolve) => setTimeout(resolve, 10)); - expect(invocationCount).toBe(1); - expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1); + expect(invocationCount).toBe(2); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2); subscription.unsubscribe(); }); diff --git a/libs/common/src/billing/services/organization/organization-metadata.service.ts b/libs/common/src/billing/services/organization/organization-metadata.service.ts index 09aaa202112a..fe96f0d984c2 100644 --- a/libs/common/src/billing/services/organization/organization-metadata.service.ts +++ b/libs/common/src/billing/services/organization/organization-metadata.service.ts @@ -1,4 +1,4 @@ -import { filter, from, merge, Observable, shareReplay, Subject, switchMap } from "rxjs"; +import { BehaviorSubject, combineLatest, from, Observable, shareReplay, switchMap } from "rxjs"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions"; @@ -18,57 +18,56 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS private billingApiService: BillingApiServiceAbstraction, private configService: ConfigService, ) {} - private refreshMetadataTrigger = new Subject(); + private refreshMetadataTrigger = new BehaviorSubject(undefined); - refreshMetadataCache = () => this.refreshMetadataTrigger.next(); + refreshMetadataCache = () => { + this.metadataCache.clear(); + this.refreshMetadataTrigger.next(); + }; - getOrganizationMetadata$ = ( - organizationId: OrganizationId, - ): Observable => - this.configService - .getFeatureFlag$(FeatureFlag.PM25379_UseNewOrganizationMetadataStructure) - .pipe( - switchMap((featureFlagEnabled) => { - return merge( - this.getOrganizationMetadataInternal$(organizationId, featureFlagEnabled), - this.refreshMetadataTrigger.pipe( - filter(() => featureFlagEnabled), - switchMap(() => - this.getOrganizationMetadataInternal$(organizationId, featureFlagEnabled, true), - ), - ), - ); - }), - ); + getOrganizationMetadata$(orgId: OrganizationId): Observable { + return combineLatest([ + this.refreshMetadataTrigger, + this.configService.getFeatureFlag$(FeatureFlag.PM25379_UseNewOrganizationMetadataStructure), + ]).pipe( + switchMap(([_, featureFlagEnabled]) => + featureFlagEnabled + ? this.vNextGetOrganizationMetadataInternal$(orgId) + : this.getOrganizationMetadataInternal$(orgId), + ), + ); + } - private getOrganizationMetadataInternal$( - organizationId: OrganizationId, - featureFlagEnabled: boolean, - bypassCache: boolean = false, + private vNextGetOrganizationMetadataInternal$( + orgId: OrganizationId, ): Observable { - if (!bypassCache && featureFlagEnabled && this.metadataCache.has(organizationId)) { - return this.metadataCache.get(organizationId)!; + const cacheHit = this.metadataCache.get(orgId); + if (cacheHit) { + return cacheHit; } - const metadata$ = from(this.fetchMetadata(organizationId, featureFlagEnabled)).pipe( + const result = from(this.fetchMetadata(orgId, true)).pipe( shareReplay({ bufferSize: 1, refCount: false }), ); - if (featureFlagEnabled) { - this.metadataCache.set(organizationId, metadata$); - } + this.metadataCache.set(orgId, result); + return result; + } - return metadata$; + private getOrganizationMetadataInternal$( + organizationId: OrganizationId, + ): Observable { + return from(this.fetchMetadata(organizationId, false)).pipe( + shareReplay({ bufferSize: 1, refCount: false }), + ); } private async fetchMetadata( organizationId: OrganizationId, featureFlagEnabled: boolean, ): Promise { - if (featureFlagEnabled) { - return await this.billingApiService.getOrganizationBillingMetadataVNext(organizationId); - } - - return await this.billingApiService.getOrganizationBillingMetadata(organizationId); + return featureFlagEnabled + ? await this.billingApiService.getOrganizationBillingMetadataVNext(organizationId) + : await this.billingApiService.getOrganizationBillingMetadata(organizationId); } } From 1e208e435719eb10cbb196ed5702e7f8a518af22 Mon Sep 17 00:00:00 2001 From: Brandon Date: Wed, 22 Oct 2025 13:44:39 -0400 Subject: [PATCH 10/10] refactor billingMetadata, clean up --- .../members/members.component.ts | 14 ++--- .../member-actions/member-actions.service.ts | 10 ++-- .../billing-constraint.service.spec.ts | 53 ++----------------- .../billing-constraint.service.ts | 39 ++++++-------- .../organization-metadata.service.spec.ts | 1 - 5 files changed, 33 insertions(+), 84 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index d29702cad80a..69686652c25e 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -31,6 +31,7 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -109,6 +110,7 @@ export class MembersComponent extends BaseMembersComponent private accountService: AccountService, private policyService: PolicyService, private policyApiService: PolicyApiServiceAbstraction, + private organizationMetadataService: OrganizationMetadataServiceAbstraction, ) { super( apiService, @@ -186,7 +188,9 @@ export class MembersComponent extends BaseMembersComponent .subscribe(); this.billingMetadata$ = organization$.pipe( - switchMap((organization) => this.billingConstraint.getBillingMetadata$(organization.id)), + switchMap((organization) => + this.organizationMetadataService.getOrganizationMetadata$(organization.id), + ), shareReplay({ bufferSize: 1, refCount: false }), ); @@ -315,7 +319,7 @@ export class MembersComponent extends BaseMembersComponent const seatLimitResult = this.billingConstraint.checkSeatLimit(organization, billingMetadata); if (!(await this.billingConstraint.seatLimitReached(seatLimitResult, organization))) { await this.handleInviteDialog(organization); - this.billingConstraint.refreshBillingMetadata(); + this.organizationMetadataService.refreshMetadataCache(); } } @@ -324,9 +328,7 @@ export class MembersComponent extends BaseMembersComponent organization: Organization, initialTab: MemberDialogTab = MemberDialogTab.Role, ) { - const billingMetadata = await firstValueFrom( - this.billingConstraint.getBillingMetadata$(organization.id), - ); + const billingMetadata = await firstValueFrom(this.billingMetadata$); const result = await this.memberDialogManager.openEditDialog( user, @@ -356,7 +358,7 @@ export class MembersComponent extends BaseMembersComponent organization, this.dataSource.getCheckedUsers(), ); - this.billingConstraint.refreshBillingMetadata(); + this.organizationMetadataService.refreshMetadataCache(); await this.load(organization); } diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts index bb951cb4c723..3697aba94ffe 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -13,12 +13,12 @@ import { import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { KeyService } from "@bitwarden/key-management"; -import { BillingConstraintService } from "@bitwarden/web-vault/app/billing/members/billing-constraint/billing-constraint.service"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; import { OrganizationUserService } from "../organization-user/organization-user.service"; @@ -44,7 +44,7 @@ export class MemberActionsService { private encryptService: EncryptService, private configService: ConfigService, private accountService: AccountService, - private billingConstraint: BillingConstraintService, + private organizationMetadataService: OrganizationMetadataServiceAbstraction, ) {} async inviteUser( @@ -73,7 +73,7 @@ export class MemberActionsService { async removeUser(organization: Organization, userId: string): Promise { try { await this.organizationUserApiService.removeOrganizationUser(organization.id, userId); - this.billingConstraint.refreshBillingMetadata(); + this.organizationMetadataService.refreshMetadataCache(); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; @@ -83,6 +83,7 @@ export class MemberActionsService { async revokeUser(organization: Organization, userId: string): Promise { try { await this.organizationUserApiService.revokeOrganizationUser(organization.id, userId); + this.organizationMetadataService.refreshMetadataCache(); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; @@ -92,6 +93,7 @@ export class MemberActionsService { async restoreUser(organization: Organization, userId: string): Promise { try { await this.organizationUserApiService.restoreOrganizationUser(organization.id, userId); + this.organizationMetadataService.refreshMetadataCache(); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; @@ -101,7 +103,7 @@ export class MemberActionsService { async deleteUser(organization: Organization, userId: string): Promise { try { await this.organizationUserApiService.deleteOrganizationUser(organization.id, userId); - this.billingConstraint.refreshBillingMetadata(); + this.organizationMetadataService.refreshMetadataCache(); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; diff --git a/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts index fc5b00a407c8..f7bb510f5795 100644 --- a/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts +++ b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.spec.ts @@ -105,53 +105,6 @@ describe("BillingConstraintService", () => { service = TestBed.inject(BillingConstraintService); }); - describe("getBillingMetadata$", () => { - it("should return billing metadata observable", (done) => { - const mockMetadata = createMockBillingMetadata(); - organizationMetadataService.getOrganizationMetadata$.mockReturnValue(of(mockMetadata)); - - service.getBillingMetadata$(mockOrganizationId).subscribe((result) => { - expect(result).toBe(mockMetadata); - expect(organizationMetadataService.getOrganizationMetadata$).toHaveBeenCalledWith( - mockOrganizationId, - ); - done(); - }); - }); - - it("should cache results using shareReplay", (done) => { - const mockMetadata = createMockBillingMetadata(); - let callCount = 0; - organizationMetadataService.getOrganizationMetadata$.mockImplementation(() => { - callCount++; - return of(mockMetadata); - }); - - const metadata$ = service.getBillingMetadata$(mockOrganizationId); - - // First subscription - metadata$.subscribe((result) => { - expect(result).toBe(mockMetadata); - }); - - // Second subscription should use cached value - metadata$.subscribe((result) => { - expect(result).toBe(mockMetadata); - // The organizationMetadataService should only be called once due to shareReplay - expect(callCount).toBe(1); - done(); - }); - }); - }); - - describe("refreshBillingMetadata", () => { - it("should call refreshMetadataCache on organizationMetadataService", () => { - service.refreshBillingMetadata(); - - expect(organizationMetadataService.refreshMetadataCache).toHaveBeenCalled(); - }); - }); - describe("checkSeatLimit", () => { it("should allow users when occupied seats are less than total seats", () => { const organization = createMockOrganization({ seats: 10 }); @@ -261,14 +214,14 @@ describe("BillingConstraintService", () => { expect(seatLimitReached).toBe(true); }); - it("should show upgrade dialog when shouldShowUpgradeDialog is true", async () => { + it("should return true when upgrade dialog is cancelled", async () => { const result: SeatLimitResult = { canAddUsers: false, reason: "fixed-seat-limit", shouldShowUpgradeDialog: true, }; const organization = createMockOrganization(); - const mockDialogRef = { closed: of(ChangePlanDialogResultType.Submitted) }; + const mockDialogRef = { closed: of(ChangePlanDialogResultType.Closed) }; (openChangePlanDialog as jest.Mock).mockReturnValue(mockDialogRef); const seatLimitReached = await service.seatLimitReached(result, organization); @@ -294,7 +247,7 @@ describe("BillingConstraintService", () => { const seatLimitReached = await service.seatLimitReached(result, organization); - expect(seatLimitReached).toBe(true); + expect(seatLimitReached).toBe(false); }); it("should show seat limit dialog when shouldShowUpgradeDialog is false", async () => { diff --git a/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts index 38548e7e2274..d43c2e684978 100644 --- a/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts +++ b/apps/web/src/app/billing/members/billing-constraint/billing-constraint.service.ts @@ -1,13 +1,11 @@ import { Injectable } from "@angular/core"; import { Router } from "@angular/router"; -import { lastValueFrom, Observable, shareReplay } from "rxjs"; +import { lastValueFrom } from "rxjs"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { isNotSelfUpgradable, ProductTierType } from "@bitwarden/common/billing/enums"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { OrganizationId } from "@bitwarden/common/types/guid"; import { DialogService, ToastService } from "@bitwarden/components"; import { isFixedSeatPlan } from "../../../admin-console/organizations/members/components/member-dialog/validators/org-seat-limit-reached.validator"; @@ -28,22 +26,9 @@ export class BillingConstraintService { private i18nService: I18nService, private dialogService: DialogService, private toastService: ToastService, - private organizationMetadataService: OrganizationMetadataServiceAbstraction, private router: Router, ) {} - getBillingMetadata$( - organizationId: OrganizationId, - ): Observable { - return this.organizationMetadataService - .getOrganizationMetadata$(organizationId) - .pipe(shareReplay({ bufferSize: 1, refCount: false })); - } - - refreshBillingMetadata(): void { - this.organizationMetadataService.refreshMetadataCache(); - } - checkSeatLimit( organization: Organization, billingMetadata: OrganizationBillingMetadataResponse, @@ -92,7 +77,9 @@ export class BillingConstraintService { case "fixed-seat-limit": if (result.shouldShowUpgradeDialog) { - return await this.handleFixedSeatLimitUpgrade(organization); + const dialogResult = await this.showChangePlanDialog(organization); + // If the plan was successfully changed, the seat limit is no longer blocking + return dialogResult !== ChangePlanDialogResultType.Submitted; } else { await this.showSeatLimitReachedDialog(organization); return true; @@ -103,7 +90,9 @@ export class BillingConstraintService { } } - private async handleFixedSeatLimitUpgrade(organization: Organization): Promise { + private async showChangePlanDialog( + organization: Organization, + ): Promise { const reference = openChangePlanDialog(this.dialogService, { data: { organizationId: organization.id, @@ -112,12 +101,16 @@ export class BillingConstraintService { }); const result = await lastValueFrom(reference.closed); - return result === ChangePlanDialogResultType.Submitted; + if (result == null) { + throw new Error("ChangePlanDialog result is null or undefined."); + } + + return result; } private async showSeatLimitReachedDialog(organization: Organization): Promise { - const dialogContent = this.getDialogContent(organization); - const acceptButtonText = this.getAcceptButtonText(organization); + const dialogContent = this.getSeatLimitReachedDialogContent(organization); + const acceptButtonText = this.getSeatLimitReachedDialogAcceptButtonText(organization); const orgUpgradeSimpleDialogOpts = { title: this.i18nService.t("upgradeOrganization"), @@ -147,12 +140,12 @@ export class BillingConstraintService { }); } - private getDialogContent(organization: Organization): string { + private getSeatLimitReachedDialogContent(organization: Organization): string { const productKey = this.getProductKey(organization); return this.i18nService.t(productKey, organization.seats); } - private getAcceptButtonText(organization: Organization): string { + private getSeatLimitReachedDialogAcceptButtonText(organization: Organization): string { if (!organization.canEditSubscription) { return this.i18nService.t("ok"); } diff --git a/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts b/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts index 8cf17ccf4a6e..c67f4aed1757 100644 --- a/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts +++ b/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts @@ -232,7 +232,6 @@ describe("DefaultOrganizationMetadataService", () => { service.refreshMetadataCache(); - // wait to ensure no additional invocations await new Promise((resolve) => setTimeout(resolve, 10)); expect(invocationCount).toBe(2);