Skip to content

Commit

Permalink
[PM-3478] Refactor OrganizationUser api (#10949)
Browse files Browse the repository at this point in the history
* User and Group collection dialogs - don't fetch additional associations from the api
* Refactor to use user mini-details endpoint
  • Loading branch information
eliykat authored Sep 30, 2024
1 parent cc0a851 commit 1f85036
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 41 deletions.
7 changes: 5 additions & 2 deletions apps/cli/src/service-container/service-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
);

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) =>
response.data.map((m) => ({
id: m.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -423,16 +426,19 @@ 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,
listName: group.name,
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,
};
}
Expand All @@ -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,
Expand All @@ -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,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]) => {
Expand Down
14 changes: 0 additions & 14 deletions apps/web/src/app/vault/org-vault/vault.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -168,8 +164,6 @@ export class VaultComponent implements OnInit, OnDestroy {
protected editableCollections$: Observable<CollectionAdminView[]>;
protected allCollectionsWithoutUnassigned$: Observable<CollectionAdminView[]>;

protected orgRevokedUsers: OrganizationUserUserDetailsResponse[];

protected get hideVaultFilters(): boolean {
return this.organization?.isProviderUser && !this.organization?.isMember;
}
Expand Down Expand Up @@ -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,
) {}
Expand Down Expand Up @@ -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$,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
OrganizationUserDetailsResponse,
OrganizationUserResetPasswordDetailsResponse,
OrganizationUserUserDetailsResponse,
OrganizationUserUserMiniResponse,
} from "../models/responses";

/**
Expand Down Expand Up @@ -44,7 +45,9 @@ export abstract class OrganizationUserApiService {
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 options - Options for the request
*/
Expand All @@ -56,6 +59,16 @@ export abstract class OrganizationUserApiService {
},
): 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
* @param organizationId - Identifier for the user's organization
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Original file line number Diff line number Diff line change
@@ -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");
}
}
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -84,6 +92,27 @@ export class DefaultOrganizationUserApiService implements OrganizationUserApiSer
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(
organizationId: string,
id: string,
Expand Down
2 changes: 1 addition & 1 deletion libs/angular/src/services/jslib-services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: OrganizationUserApiService,
useClass: DefaultOrganizationUserApiService,
deps: [ApiServiceAbstraction],
deps: [ApiServiceAbstraction, ConfigService],
}),
safeProvider({
provide: PasswordResetEnrollmentServiceAbstraction,
Expand Down
2 changes: 2 additions & 0 deletions libs/common/src/enums/feature-flag.enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -78,6 +79,7 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.AC2476_DeprecateStripeSourcesAPI]: FALSE,
[FeatureFlag.CipherKeyEncryption]: FALSE,
[FeatureFlag.PM11901_RefactorSelfHostingLicenseUploader]: FALSE,
[FeatureFlag.Pm3478RefactorOrganizationUserApi]: FALSE,
} satisfies Record<FeatureFlag, AllowedFeatureFlagTypes>;

export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;
Expand Down

0 comments on commit 1f85036

Please sign in to comment.