From 1f850363463ad7bfb730d83449817a2c250b5b60 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 1 Oct 2024 07:13:26 +1000 Subject: [PATCH] [PM-3478] Refactor OrganizationUser api (#10949) * User and Group collection dialogs - don't fetch additional associations from the api * Refactor to use user mini-details endpoint --- .../service-container/service-container.ts | 7 +++- .../manage/entity-events.component.ts | 2 +- .../organizations/manage/events.component.ts | 4 +- .../manage/group-add-edit.component.ts | 2 +- .../collection-dialog.component.ts | 42 +++++++++++-------- .../bulk-collections-dialog.component.ts | 2 +- .../app/vault/org-vault/vault.component.ts | 14 ------- .../organization-user-api.service.ts | 15 ++++++- .../models/responses/index.ts | 1 + .../organization-user-mini.response.ts | 24 +++++++++++ .../default-organization-user-api.service.ts | 31 +++++++++++++- .../src/services/jslib-services.module.ts | 2 +- libs/common/src/enums/feature-flag.enum.ts | 2 + 13 files changed, 107 insertions(+), 41 deletions(-) create mode 100644 libs/admin-console/src/common/organization-user/models/responses/organization-user-mini.response.ts diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 6f19081a73..2149b74f61 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -498,8 +498,6 @@ export class ServiceContainer { this.providerService = new ProviderService(this.stateProvider); - this.organizationUserApiService = new DefaultOrganizationUserApiService(this.apiService); - this.policyApiService = new PolicyApiService(this.policyService, this.apiService); this.keyConnectorService = new KeyConnectorService( @@ -778,6 +776,11 @@ export class ServiceContainer { this.organizationApiService = new OrganizationApiService(this.apiService, this.syncService); this.providerApiService = new ProviderApiService(this.apiService); + + this.organizationUserApiService = new DefaultOrganizationUserApiService( + this.apiService, + this.configService, + ); } async logout() { diff --git a/apps/web/src/app/admin-console/organizations/manage/entity-events.component.ts b/apps/web/src/app/admin-console/organizations/manage/entity-events.component.ts index 79ada2b7a5..2caf2e76b7 100644 --- a/apps/web/src/app/admin-console/organizations/manage/entity-events.component.ts +++ b/apps/web/src/app/admin-console/organizations/manage/entity-events.component.ts @@ -78,7 +78,7 @@ export class EntityEventsComponent implements OnInit { async load() { try { if (this.showUser) { - const response = await this.organizationUserApiService.getAllUsers( + const response = await this.organizationUserApiService.getAllMiniUserDetails( this.params.organizationId, ); response.data.forEach((u) => { diff --git a/apps/web/src/app/admin-console/organizations/manage/events.component.ts b/apps/web/src/app/admin-console/organizations/manage/events.component.ts index 574335125e..ef9d5c32d9 100644 --- a/apps/web/src/app/admin-console/organizations/manage/events.component.ts +++ b/apps/web/src/app/admin-console/organizations/manage/events.component.ts @@ -83,7 +83,9 @@ export class EventsComponent extends BaseEventsComponent implements OnInit, OnDe } async load() { - const response = await this.organizationUserApiService.getAllUsers(this.organizationId); + const response = await this.organizationUserApiService.getAllMiniUserDetails( + this.organizationId, + ); response.data.forEach((u) => { const name = this.userNamePipe.transform(u); this.orgUsersUserIdMap.set(u.userId, { name: name, email: u.email }); diff --git a/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts b/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts index 36489e0ab1..cdbc049111 100644 --- a/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts +++ b/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts @@ -131,7 +131,7 @@ export class GroupAddEditComponent implements OnInit, OnDestroy { ); private get orgMembers$(): Observable> { - return from(this.organizationUserApiService.getAllUsers(this.organizationId)).pipe( + return from(this.organizationUserApiService.getAllMiniUserDetails(this.organizationId)).pipe( map((response) => response.data.map((m) => ({ id: m.id, diff --git a/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts index 9dc8a3c0df..5c46a7a029 100644 --- a/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts @@ -15,7 +15,7 @@ import { first } from "rxjs/operators"; import { OrganizationUserApiService, - OrganizationUserUserDetailsResponse, + OrganizationUserUserMiniResponse, } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -156,15 +156,23 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { organization: organization$, collections: this.collectionAdminService.getAll(orgId), groups: groups$, - // Collection(s) needed to map readonlypermission for (potential) access selector disabled state - users: this.organizationUserApiService.getAllUsers(orgId, { includeCollections: true }), + users: this.organizationUserApiService.getAllMiniUserDetails(orgId), }) .pipe(takeUntil(this.formGroup.controls.selectedOrg.valueChanges), takeUntil(this.destroy$)) .subscribe(({ organization, collections: allCollections, groups, users }) => { this.organization = organization; + + if (this.params.collectionId) { + this.collection = allCollections.find((c) => c.id === this.collectionId); + + if (!this.collection) { + throw new Error("Could not find collection to edit."); + } + } + this.accessItems = [].concat( - groups.map((group) => mapGroupToAccessItemView(group, this.collectionId)), - users.data.map((user) => mapUserToAccessItemView(user, this.collectionId)), + groups.map((group) => mapGroupToAccessItemView(group, this.collection)), + users.data.map((user) => mapUserToAccessItemView(user, this.collection)), ); // Force change detection to update the access selector's items @@ -174,15 +182,10 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { ? allCollections.filter((c) => c.manage) : allCollections; - if (this.params.collectionId) { - this.collection = allCollections.find((c) => c.id === this.collectionId); + if (this.collection) { // Ensure we don't allow nesting the current collection within itself this.nestOptions = this.nestOptions.filter((c) => c.id !== this.collectionId); - if (!this.collection) { - throw new Error("Could not find collection to edit."); - } - // Parse the name to find its parent name const { name, parent: parentName } = parseName(this.collection); @@ -423,7 +426,10 @@ function validateCanManagePermission(control: AbstractControl) { * @param collectionId Current collection being viewed/edited * @returns AccessItemView customized to set a readonlyPermission to be displayed if the access selector is in a disabled state */ -function mapGroupToAccessItemView(group: GroupView, collectionId: string): AccessItemView { +function mapGroupToAccessItemView( + group: GroupView, + collection: CollectionAdminView, +): AccessItemView { return { id: group.id, type: AccessItemType.Group, @@ -431,8 +437,8 @@ function mapGroupToAccessItemView(group: GroupView, collectionId: string): Acces labelName: group.name, readonly: false, readonlyPermission: - collectionId != null - ? convertToPermission(group.collections.find((gc) => gc.id == collectionId)) + collection != null + ? convertToPermission(collection.groups.find((g) => g.id === group.id)) : undefined, }; } @@ -444,8 +450,8 @@ function mapGroupToAccessItemView(group: GroupView, collectionId: string): Acces * @returns AccessItemView customized to set a readonlyPermission to be displayed if the access selector is in a disabled state */ function mapUserToAccessItemView( - user: OrganizationUserUserDetailsResponse, - collectionId: string, + user: OrganizationUserUserMiniResponse, + collection: CollectionAdminView, ): AccessItemView { return { id: user.id, @@ -457,9 +463,9 @@ function mapUserToAccessItemView( status: user.status, readonly: false, readonlyPermission: - collectionId != null + collection != null ? convertToPermission( - new CollectionAccessSelectionView(user.collections.find((uc) => uc.id == collectionId)), + new CollectionAccessSelectionView(collection.users.find((u) => u.id === user.id)), ) : undefined, }; diff --git a/apps/web/src/app/vault/org-vault/bulk-collections-dialog/bulk-collections-dialog.component.ts b/apps/web/src/app/vault/org-vault/bulk-collections-dialog/bulk-collections-dialog.component.ts index 76e90097d1..c4b0d8bc2a 100644 --- a/apps/web/src/app/vault/org-vault/bulk-collections-dialog/bulk-collections-dialog.component.ts +++ b/apps/web/src/app/vault/org-vault/bulk-collections-dialog/bulk-collections-dialog.component.ts @@ -79,7 +79,7 @@ export class BulkCollectionsDialogComponent implements OnDestroy { combineLatest([ organization$, groups$, - this.organizationUserApiService.getAllUsers(this.params.organizationId), + this.organizationUserApiService.getAllMiniUserDetails(this.params.organizationId), ]) .pipe(takeUntil(this.destroy$)) .subscribe(([organization, groups, users]) => { diff --git a/apps/web/src/app/vault/org-vault/vault.component.ts b/apps/web/src/app/vault/org-vault/vault.component.ts index 2976bfc8c2..3120b54ed3 100644 --- a/apps/web/src/app/vault/org-vault/vault.component.ts +++ b/apps/web/src/app/vault/org-vault/vault.component.ts @@ -30,10 +30,6 @@ import { withLatestFrom, } from "rxjs/operators"; -import { - OrganizationUserApiService, - OrganizationUserUserDetailsResponse, -} from "@bitwarden/admin-console/common"; import { SearchPipe } from "@bitwarden/angular/pipes/search.pipe"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -168,8 +164,6 @@ export class VaultComponent implements OnInit, OnDestroy { protected editableCollections$: Observable; protected allCollectionsWithoutUnassigned$: Observable; - protected orgRevokedUsers: OrganizationUserUserDetailsResponse[]; - protected get hideVaultFilters(): boolean { return this.organization?.isProviderUser && !this.organization?.isMember; } @@ -206,7 +200,6 @@ export class VaultComponent implements OnInit, OnDestroy { private totpService: TotpService, private apiService: ApiService, private collectionService: CollectionService, - private organizationUserApiService: OrganizationUserApiService, private toastService: ToastService, private accountService: AccountService, ) {} @@ -358,13 +351,6 @@ export class VaultComponent implements OnInit, OnDestroy { shareReplay({ refCount: true, bufferSize: 1 }), ); - // This will be passed into the usersCanManage call - this.orgRevokedUsers = ( - await this.organizationUserApiService.getAllUsers(await firstValueFrom(organizationId$)) - ).data.filter((user: OrganizationUserUserDetailsResponse) => { - return user.status === -1; - }); - const collections$ = combineLatest([ nestedCollections$, filter$, diff --git a/libs/admin-console/src/common/organization-user/abstractions/organization-user-api.service.ts b/libs/admin-console/src/common/organization-user/abstractions/organization-user-api.service.ts index ea5d2185ee..ff7f9c5df6 100644 --- a/libs/admin-console/src/common/organization-user/abstractions/organization-user-api.service.ts +++ b/libs/admin-console/src/common/organization-user/abstractions/organization-user-api.service.ts @@ -16,6 +16,7 @@ import { OrganizationUserDetailsResponse, OrganizationUserResetPasswordDetailsResponse, OrganizationUserUserDetailsResponse, + OrganizationUserUserMiniResponse, } from "../models/responses"; /** @@ -44,7 +45,9 @@ export abstract class OrganizationUserApiService { abstract getOrganizationUserGroups(organizationId: string, id: string): Promise; /** - * Retrieve a list of all users that belong to the specified organization + * Retrieve full details of all users that belong to the specified organization. + * This is only accessible to privileged users, if you need a simple listing of basic details, use + * {@link getAllMiniUserDetails}. * @param organizationId - Identifier for the organization * @param options - Options for the request */ @@ -56,6 +59,16 @@ export abstract class OrganizationUserApiService { }, ): Promise>; + /** + * Retrieve a list of all users that belong to the specified organization, with basic information only. + * This is suitable for lists of names/emails etc. throughout the app and can be accessed by most users. + * @param organizationId - Identifier for the organization + * @param options - Options for the request + */ + abstract getAllMiniUserDetails( + organizationId: string, + ): Promise>; + /** * Retrieve reset password details for the specified organization user * @param organizationId - Identifier for the user's organization diff --git a/libs/admin-console/src/common/organization-user/models/responses/index.ts b/libs/admin-console/src/common/organization-user/models/responses/index.ts index 29c82fb18b..aa0a968f71 100644 --- a/libs/admin-console/src/common/organization-user/models/responses/index.ts +++ b/libs/admin-console/src/common/organization-user/models/responses/index.ts @@ -1,3 +1,4 @@ export * from "./organization-user.response"; export * from "./organization-user-bulk.response"; export * from "./organization-user-bulk-public-key.response"; +export * from "./organization-user-mini.response"; diff --git a/libs/admin-console/src/common/organization-user/models/responses/organization-user-mini.response.ts b/libs/admin-console/src/common/organization-user/models/responses/organization-user-mini.response.ts new file mode 100644 index 0000000000..6ca1bace40 --- /dev/null +++ b/libs/admin-console/src/common/organization-user/models/responses/organization-user-mini.response.ts @@ -0,0 +1,24 @@ +import { + OrganizationUserStatusType, + OrganizationUserType, +} from "@bitwarden/common/admin-console/enums"; +import { BaseResponse } from "@bitwarden/common/models/response/base.response"; + +export class OrganizationUserUserMiniResponse extends BaseResponse { + id: string; + userId: string; + email: string; + name: string; + type: OrganizationUserType; + status: OrganizationUserStatusType; + + constructor(response: any) { + super(response); + this.id = this.getResponseProperty("Id"); + this.userId = this.getResponseProperty("UserId"); + this.email = this.getResponseProperty("Email"); + this.name = this.getResponseProperty("Name"); + this.type = this.getResponseProperty("Type"); + this.status = this.getResponseProperty("Status"); + } +} diff --git a/libs/admin-console/src/common/organization-user/services/default-organization-user-api.service.ts b/libs/admin-console/src/common/organization-user/services/default-organization-user-api.service.ts index 40824550d4..a6438b8b5f 100644 --- a/libs/admin-console/src/common/organization-user/services/default-organization-user-api.service.ts +++ b/libs/admin-console/src/common/organization-user/services/default-organization-user-api.service.ts @@ -1,5 +1,9 @@ +import { firstValueFrom } from "rxjs"; + import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { OrganizationUserApiService } from "../abstractions"; import { @@ -19,10 +23,14 @@ import { OrganizationUserDetailsResponse, OrganizationUserResetPasswordDetailsResponse, OrganizationUserUserDetailsResponse, + OrganizationUserUserMiniResponse, } from "../models/responses"; export class DefaultOrganizationUserApiService implements OrganizationUserApiService { - constructor(private apiService: ApiService) {} + constructor( + private apiService: ApiService, + private configService: ConfigService, + ) {} async getOrganizationUser( organizationId: string, @@ -84,6 +92,27 @@ export class DefaultOrganizationUserApiService implements OrganizationUserApiSer return new ListResponse(r, OrganizationUserUserDetailsResponse); } + async getAllMiniUserDetails( + organizationId: string, + ): Promise> { + const apiEnabled = await firstValueFrom( + this.configService.getFeatureFlag$(FeatureFlag.Pm3478RefactorOrganizationUserApi), + ); + if (!apiEnabled) { + // Keep using the old api until this feature flag is enabled + return this.getAllUsers(organizationId); + } + + const r = await this.apiService.send( + "GET", + `/organizations/${organizationId}/users/mini-details`, + null, + true, + true, + ); + return new ListResponse(r, OrganizationUserUserMiniResponse); + } + async getOrganizationUserResetPasswordDetails( organizationId: string, id: string, diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 1ebaf34306..0cc6e74d5b 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -952,7 +952,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: OrganizationUserApiService, useClass: DefaultOrganizationUserApiService, - deps: [ApiServiceAbstraction], + deps: [ApiServiceAbstraction, ConfigService], }), safeProvider({ provide: PasswordResetEnrollmentServiceAbstraction, diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 505fe33e82..2b7d2bea33 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -34,6 +34,7 @@ export enum FeatureFlag { AC2476_DeprecateStripeSourcesAPI = "AC-2476-deprecate-stripe-sources-api", CipherKeyEncryption = "cipher-key-encryption", PM11901_RefactorSelfHostingLicenseUploader = "PM-11901-refactor-self-hosting-license-uploader", + Pm3478RefactorOrganizationUserApi = "pm-3478-refactor-organizationuser-api", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -78,6 +79,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.AC2476_DeprecateStripeSourcesAPI]: FALSE, [FeatureFlag.CipherKeyEncryption]: FALSE, [FeatureFlag.PM11901_RefactorSelfHostingLicenseUploader]: FALSE, + [FeatureFlag.Pm3478RefactorOrganizationUserApi]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;