[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
This commit is contained in:
Thomas Rittson 2024-10-01 07:13:26 +10:00 committed by GitHub
parent cc0a851c0e
commit 1f85036346
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 107 additions and 41 deletions

View File

@ -498,8 +498,6 @@ export class ServiceContainer {
this.providerService = new ProviderService(this.stateProvider); this.providerService = new ProviderService(this.stateProvider);
this.organizationUserApiService = new DefaultOrganizationUserApiService(this.apiService);
this.policyApiService = new PolicyApiService(this.policyService, this.apiService); this.policyApiService = new PolicyApiService(this.policyService, this.apiService);
this.keyConnectorService = new KeyConnectorService( this.keyConnectorService = new KeyConnectorService(
@ -778,6 +776,11 @@ export class ServiceContainer {
this.organizationApiService = new OrganizationApiService(this.apiService, this.syncService); this.organizationApiService = new OrganizationApiService(this.apiService, this.syncService);
this.providerApiService = new ProviderApiService(this.apiService); this.providerApiService = new ProviderApiService(this.apiService);
this.organizationUserApiService = new DefaultOrganizationUserApiService(
this.apiService,
this.configService,
);
} }
async logout() { async logout() {

View File

@ -78,7 +78,7 @@ export class EntityEventsComponent implements OnInit {
async load() { async load() {
try { try {
if (this.showUser) { if (this.showUser) {
const response = await this.organizationUserApiService.getAllUsers( const response = await this.organizationUserApiService.getAllMiniUserDetails(
this.params.organizationId, this.params.organizationId,
); );
response.data.forEach((u) => { response.data.forEach((u) => {

View File

@ -83,7 +83,9 @@ export class EventsComponent extends BaseEventsComponent implements OnInit, OnDe
} }
async load() { async load() {
const response = await this.organizationUserApiService.getAllUsers(this.organizationId); const response = await this.organizationUserApiService.getAllMiniUserDetails(
this.organizationId,
);
response.data.forEach((u) => { response.data.forEach((u) => {
const name = this.userNamePipe.transform(u); const name = this.userNamePipe.transform(u);
this.orgUsersUserIdMap.set(u.userId, { name: name, email: u.email }); this.orgUsersUserIdMap.set(u.userId, { name: name, email: u.email });

View File

@ -131,7 +131,7 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
); );
private get orgMembers$(): Observable<Array<AccessItemView & { userId: UserId }>> { private get orgMembers$(): Observable<Array<AccessItemView & { userId: UserId }>> {
return from(this.organizationUserApiService.getAllUsers(this.organizationId)).pipe( return from(this.organizationUserApiService.getAllMiniUserDetails(this.organizationId)).pipe(
map((response) => map((response) =>
response.data.map((m) => ({ response.data.map((m) => ({
id: m.id, id: m.id,

View File

@ -15,7 +15,7 @@ import { first } from "rxjs/operators";
import { import {
OrganizationUserApiService, OrganizationUserApiService,
OrganizationUserUserDetailsResponse, OrganizationUserUserMiniResponse,
} from "@bitwarden/admin-console/common"; } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
@ -156,15 +156,23 @@ export class CollectionDialogComponent implements OnInit, OnDestroy {
organization: organization$, organization: organization$,
collections: this.collectionAdminService.getAll(orgId), collections: this.collectionAdminService.getAll(orgId),
groups: groups$, groups: groups$,
// Collection(s) needed to map readonlypermission for (potential) access selector disabled state users: this.organizationUserApiService.getAllMiniUserDetails(orgId),
users: this.organizationUserApiService.getAllUsers(orgId, { includeCollections: true }),
}) })
.pipe(takeUntil(this.formGroup.controls.selectedOrg.valueChanges), takeUntil(this.destroy$)) .pipe(takeUntil(this.formGroup.controls.selectedOrg.valueChanges), takeUntil(this.destroy$))
.subscribe(({ organization, collections: allCollections, groups, users }) => { .subscribe(({ organization, collections: allCollections, groups, users }) => {
this.organization = organization; 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( this.accessItems = [].concat(
groups.map((group) => mapGroupToAccessItemView(group, this.collectionId)), groups.map((group) => mapGroupToAccessItemView(group, this.collection)),
users.data.map((user) => mapUserToAccessItemView(user, this.collectionId)), users.data.map((user) => mapUserToAccessItemView(user, this.collection)),
); );
// Force change detection to update the access selector's items // 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.filter((c) => c.manage)
: allCollections; : allCollections;
if (this.params.collectionId) { if (this.collection) {
this.collection = allCollections.find((c) => c.id === this.collectionId);
// Ensure we don't allow nesting the current collection within itself // Ensure we don't allow nesting the current collection within itself
this.nestOptions = this.nestOptions.filter((c) => c.id !== this.collectionId); 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 // Parse the name to find its parent name
const { name, parent: parentName } = parseName(this.collection); const { name, parent: parentName } = parseName(this.collection);
@ -423,7 +426,10 @@ function validateCanManagePermission(control: AbstractControl) {
* @param collectionId Current collection being viewed/edited * @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 * @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 { return {
id: group.id, id: group.id,
type: AccessItemType.Group, type: AccessItemType.Group,
@ -431,8 +437,8 @@ function mapGroupToAccessItemView(group: GroupView, collectionId: string): Acces
labelName: group.name, labelName: group.name,
readonly: false, readonly: false,
readonlyPermission: readonlyPermission:
collectionId != null collection != null
? convertToPermission(group.collections.find((gc) => gc.id == collectionId)) ? convertToPermission(collection.groups.find((g) => g.id === group.id))
: undefined, : 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 * @returns AccessItemView customized to set a readonlyPermission to be displayed if the access selector is in a disabled state
*/ */
function mapUserToAccessItemView( function mapUserToAccessItemView(
user: OrganizationUserUserDetailsResponse, user: OrganizationUserUserMiniResponse,
collectionId: string, collection: CollectionAdminView,
): AccessItemView { ): AccessItemView {
return { return {
id: user.id, id: user.id,
@ -457,9 +463,9 @@ function mapUserToAccessItemView(
status: user.status, status: user.status,
readonly: false, readonly: false,
readonlyPermission: readonlyPermission:
collectionId != null collection != null
? convertToPermission( ? convertToPermission(
new CollectionAccessSelectionView(user.collections.find((uc) => uc.id == collectionId)), new CollectionAccessSelectionView(collection.users.find((u) => u.id === user.id)),
) )
: undefined, : undefined,
}; };

View File

@ -79,7 +79,7 @@ export class BulkCollectionsDialogComponent implements OnDestroy {
combineLatest([ combineLatest([
organization$, organization$,
groups$, groups$,
this.organizationUserApiService.getAllUsers(this.params.organizationId), this.organizationUserApiService.getAllMiniUserDetails(this.params.organizationId),
]) ])
.pipe(takeUntil(this.destroy$)) .pipe(takeUntil(this.destroy$))
.subscribe(([organization, groups, users]) => { .subscribe(([organization, groups, users]) => {

View File

@ -30,10 +30,6 @@ import {
withLatestFrom, withLatestFrom,
} from "rxjs/operators"; } from "rxjs/operators";
import {
OrganizationUserApiService,
OrganizationUserUserDetailsResponse,
} from "@bitwarden/admin-console/common";
import { SearchPipe } from "@bitwarden/angular/pipes/search.pipe"; import { SearchPipe } from "@bitwarden/angular/pipes/search.pipe";
import { ModalService } from "@bitwarden/angular/services/modal.service"; import { ModalService } from "@bitwarden/angular/services/modal.service";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service";
@ -168,8 +164,6 @@ export class VaultComponent implements OnInit, OnDestroy {
protected editableCollections$: Observable<CollectionAdminView[]>; protected editableCollections$: Observable<CollectionAdminView[]>;
protected allCollectionsWithoutUnassigned$: Observable<CollectionAdminView[]>; protected allCollectionsWithoutUnassigned$: Observable<CollectionAdminView[]>;
protected orgRevokedUsers: OrganizationUserUserDetailsResponse[];
protected get hideVaultFilters(): boolean { protected get hideVaultFilters(): boolean {
return this.organization?.isProviderUser && !this.organization?.isMember; return this.organization?.isProviderUser && !this.organization?.isMember;
} }
@ -206,7 +200,6 @@ export class VaultComponent implements OnInit, OnDestroy {
private totpService: TotpService, private totpService: TotpService,
private apiService: ApiService, private apiService: ApiService,
private collectionService: CollectionService, private collectionService: CollectionService,
private organizationUserApiService: OrganizationUserApiService,
private toastService: ToastService, private toastService: ToastService,
private accountService: AccountService, private accountService: AccountService,
) {} ) {}
@ -358,13 +351,6 @@ export class VaultComponent implements OnInit, OnDestroy {
shareReplay({ refCount: true, bufferSize: 1 }), 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([ const collections$ = combineLatest([
nestedCollections$, nestedCollections$,
filter$, filter$,

View File

@ -16,6 +16,7 @@ import {
OrganizationUserDetailsResponse, OrganizationUserDetailsResponse,
OrganizationUserResetPasswordDetailsResponse, OrganizationUserResetPasswordDetailsResponse,
OrganizationUserUserDetailsResponse, OrganizationUserUserDetailsResponse,
OrganizationUserUserMiniResponse,
} from "../models/responses"; } from "../models/responses";
/** /**
@ -44,7 +45,9 @@ export abstract class OrganizationUserApiService {
abstract getOrganizationUserGroups(organizationId: string, id: string): Promise<string[]>; abstract getOrganizationUserGroups(organizationId: string, id: string): Promise<string[]>;
/** /**
* 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 organizationId - Identifier for the organization
* @param options - Options for the request * @param options - Options for the request
*/ */
@ -56,6 +59,16 @@ export abstract class OrganizationUserApiService {
}, },
): Promise<ListResponse<OrganizationUserUserDetailsResponse>>; ): Promise<ListResponse<OrganizationUserUserDetailsResponse>>;
/**
* 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<ListResponse<OrganizationUserUserMiniResponse>>;
/** /**
* Retrieve reset password details for the specified organization user * Retrieve reset password details for the specified organization user
* @param organizationId - Identifier for the user's organization * @param organizationId - Identifier for the user's organization

View File

@ -1,3 +1,4 @@
export * from "./organization-user.response"; export * from "./organization-user.response";
export * from "./organization-user-bulk.response"; export * from "./organization-user-bulk.response";
export * from "./organization-user-bulk-public-key.response"; export * from "./organization-user-bulk-public-key.response";
export * from "./organization-user-mini.response";

View File

@ -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");
}
}

View File

@ -1,5 +1,9 @@
import { firstValueFrom } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; 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 { ListResponse } from "@bitwarden/common/models/response/list.response";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { OrganizationUserApiService } from "../abstractions"; import { OrganizationUserApiService } from "../abstractions";
import { import {
@ -19,10 +23,14 @@ import {
OrganizationUserDetailsResponse, OrganizationUserDetailsResponse,
OrganizationUserResetPasswordDetailsResponse, OrganizationUserResetPasswordDetailsResponse,
OrganizationUserUserDetailsResponse, OrganizationUserUserDetailsResponse,
OrganizationUserUserMiniResponse,
} from "../models/responses"; } from "../models/responses";
export class DefaultOrganizationUserApiService implements OrganizationUserApiService { export class DefaultOrganizationUserApiService implements OrganizationUserApiService {
constructor(private apiService: ApiService) {} constructor(
private apiService: ApiService,
private configService: ConfigService,
) {}
async getOrganizationUser( async getOrganizationUser(
organizationId: string, organizationId: string,
@ -84,6 +92,27 @@ export class DefaultOrganizationUserApiService implements OrganizationUserApiSer
return new ListResponse(r, OrganizationUserUserDetailsResponse); return new ListResponse(r, OrganizationUserUserDetailsResponse);
} }
async getAllMiniUserDetails(
organizationId: string,
): Promise<ListResponse<OrganizationUserUserMiniResponse>> {
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( async getOrganizationUserResetPasswordDetails(
organizationId: string, organizationId: string,
id: string, id: string,

View File

@ -952,7 +952,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({ safeProvider({
provide: OrganizationUserApiService, provide: OrganizationUserApiService,
useClass: DefaultOrganizationUserApiService, useClass: DefaultOrganizationUserApiService,
deps: [ApiServiceAbstraction], deps: [ApiServiceAbstraction, ConfigService],
}), }),
safeProvider({ safeProvider({
provide: PasswordResetEnrollmentServiceAbstraction, provide: PasswordResetEnrollmentServiceAbstraction,

View File

@ -34,6 +34,7 @@ export enum FeatureFlag {
AC2476_DeprecateStripeSourcesAPI = "AC-2476-deprecate-stripe-sources-api", AC2476_DeprecateStripeSourcesAPI = "AC-2476-deprecate-stripe-sources-api",
CipherKeyEncryption = "cipher-key-encryption", CipherKeyEncryption = "cipher-key-encryption",
PM11901_RefactorSelfHostingLicenseUploader = "PM-11901-refactor-self-hosting-license-uploader", PM11901_RefactorSelfHostingLicenseUploader = "PM-11901-refactor-self-hosting-license-uploader",
Pm3478RefactorOrganizationUserApi = "pm-3478-refactor-organizationuser-api",
} }
export type AllowedFeatureFlagTypes = boolean | number | string; export type AllowedFeatureFlagTypes = boolean | number | string;
@ -78,6 +79,7 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.AC2476_DeprecateStripeSourcesAPI]: FALSE, [FeatureFlag.AC2476_DeprecateStripeSourcesAPI]: FALSE,
[FeatureFlag.CipherKeyEncryption]: FALSE, [FeatureFlag.CipherKeyEncryption]: FALSE,
[FeatureFlag.PM11901_RefactorSelfHostingLicenseUploader]: FALSE, [FeatureFlag.PM11901_RefactorSelfHostingLicenseUploader]: FALSE,
[FeatureFlag.Pm3478RefactorOrganizationUserApi]: FALSE,
} satisfies Record<FeatureFlag, AllowedFeatureFlagTypes>; } satisfies Record<FeatureFlag, AllowedFeatureFlagTypes>;
export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;