diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index 2e273d041d..92674aa3dc 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -2,7 +2,7 @@ // @ts-strict-ignore import { firstValueFrom, map, switchMap } from "rxjs"; -import { CollectionRequest } from "@bitwarden/admin-console/common"; +import { UpdateCollectionRequest } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; @@ -225,7 +225,7 @@ export class EditCommand { : req.users.map( (u) => new SelectionReadOnlyRequest(u.id, u.readOnly, u.hidePasswords, u.manage), ); - const request = new CollectionRequest({ + const request = new UpdateCollectionRequest({ name: await this.encryptService.encryptString(req.name, orgKey), externalId: req.externalId, users, diff --git a/apps/cli/src/vault/create.command.ts b/apps/cli/src/vault/create.command.ts index cbaae4d254..0892bb4221 100644 --- a/apps/cli/src/vault/create.command.ts +++ b/apps/cli/src/vault/create.command.ts @@ -5,7 +5,7 @@ import * as path from "path"; import { firstValueFrom, map } from "rxjs"; -import { CollectionRequest } from "@bitwarden/admin-console/common"; +import { CreateCollectionRequest } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request"; @@ -233,7 +233,7 @@ export class CreateCommand { : req.users.map( (u) => new SelectionReadOnlyRequest(u.id, u.readOnly, u.hidePasswords, u.manage), ); - const request = new CollectionRequest({ + const request = new CreateCollectionRequest({ name: await this.encryptService.encryptString(req.name, orgKey), externalId: req.externalId, groups, diff --git a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts index 8a0fab520e..67cb4c7cdc 100644 --- a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts +++ b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts @@ -45,9 +45,15 @@ export function cloneCollection( let cloned; if (collection instanceof CollectionAdminView) { - cloned = Object.assign(new CollectionAdminView({ ...collection }), collection); + cloned = Object.assign( + new CollectionAdminView({ ...collection, name: collection.name }), + collection, + ); } else { - cloned = Object.assign(new CollectionView({ ...collection }), collection); + cloned = Object.assign( + new CollectionView({ ...collection, name: collection.name }), + collection, + ); } return cloned; } diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts index 59d042cae5..52c418a521 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts @@ -398,6 +398,13 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { } return; } + if ( + this.editMode && + !this.collection.canEditName(this.organization) && + this.formGroup.controls.name.dirty + ) { + throw new Error("Cannot change readonly field: Name"); + } const parent = this.formGroup.controls.parent?.value; const collectionView = new CollectionAdminView({ @@ -414,9 +421,13 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { collectionView.users = this.formGroup.controls.access.value .filter((v) => v.type === AccessItemType.Member) .map(convertToSelectionView); + collectionView.defaultUserCollectionEmail = this.collection.defaultUserCollectionEmail; const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - const savedCollection = await this.collectionAdminService.save(collectionView, userId); + + const collectionResponse = this.editMode + ? await this.collectionAdminService.update(collectionView, userId) + : await this.collectionAdminService.create(collectionView, userId); this.toastService.showToast({ variant: "success", @@ -426,7 +437,7 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { ), }); - this.close(CollectionDialogAction.Saved, savedCollection); + this.close(CollectionDialogAction.Saved, collectionResponse); }; protected delete = async () => { @@ -483,14 +494,23 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { private handleFormGroupReadonly(readonly: boolean) { if (readonly) { + this.formGroup.controls.access.disable(); this.formGroup.controls.name.disable(); this.formGroup.controls.parent.disable(); - this.formGroup.controls.access.disable(); - } else { + return; + } + + this.formGroup.controls.access.enable(); + + if (!this.editMode) { this.formGroup.controls.name.enable(); this.formGroup.controls.parent.enable(); - this.formGroup.controls.access.enable(); + return; } + + const canEditName = this.collection.canEditName(this.organization); + this.formGroup.controls.name[canEditName ? "enable" : "disable"](); + this.formGroup.controls.parent[canEditName ? "enable" : "disable"](); } private close(action: CollectionDialogAction, collection?: CollectionResponse | CollectionView) { diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index cccbe26c52..2238614473 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -45,6 +45,7 @@ import { import { OrganizationIntegrationApiService } from "@bitwarden/bit-common/dirt/integrations"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService, @@ -317,7 +318,13 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: CollectionAdminService, useClass: DefaultCollectionAdminService, - deps: [ApiService, KeyServiceAbstraction, EncryptService, CollectionService], + deps: [ + ApiService, + KeyServiceAbstraction, + EncryptService, + CollectionService, + OrganizationService, + ], }), safeProvider({ provide: SdkLoadService, diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts index ec77ff97a1..eeecccc87d 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts @@ -250,7 +250,9 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { } collections.forEach((c) => { - const collectionCopy = cloneCollection(new CollectionView({ ...c })) as CollectionFilter; + const collectionCopy = cloneCollection( + new CollectionView({ ...c, name: c.name }), + ) as CollectionFilter; collectionCopy.icon = "bwi-collection-shared"; const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : []; ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, null, NestingDelimiter); diff --git a/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts b/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts index b322825e5e..9fde1b2090 100644 --- a/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts +++ b/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts @@ -10,7 +10,11 @@ export abstract class CollectionAdminService { organizationId: string, userId: UserId, ): Observable; - abstract save( + abstract update( + collection: CollectionAdminView, + userId: UserId, + ): Promise; + abstract create( collection: CollectionAdminView, userId: UserId, ): Promise; diff --git a/libs/admin-console/src/common/collections/models/collection-admin.view.ts b/libs/admin-console/src/common/collections/models/collection-admin.view.ts index 1cec9c3d39..fcd2be17b8 100644 --- a/libs/admin-console/src/common/collections/models/collection-admin.view.ts +++ b/libs/admin-console/src/common/collections/models/collection-admin.view.ts @@ -101,6 +101,17 @@ export class CollectionAdminView extends CollectionView { return this.id === Unassigned; } + /** + * Returns true if the collection name can be edited. Editing the collection name is restricted for collections + * that were DefaultUserCollections but where the relevant user has been offboarded. + * When this occurs, the offboarded user's email is treated as the collection name, and cannot be edited. + * This is important for security so that the server cannot ask the client to encrypt arbitrary data. + * WARNING! This is an IMPORTANT restriction that MUST be maintained for security purposes. + * Do not edit or remove this unless you understand why. + */ + override canEditName(org: Organization): boolean { + return (this.canEdit(org) && !this.defaultUserCollectionEmail) || super.canEditName(org); + } static async fromCollectionAccessDetails( collection: CollectionAccessDetailsResponse, encryptService: EncryptService, @@ -115,6 +126,7 @@ export class CollectionAdminView extends CollectionView { view.unmanaged = collection.unmanaged; view.type = collection.type; view.externalId = collection.externalId; + view.defaultUserCollectionEmail = collection.defaultUserCollectionEmail; view.groups = collection.groups ? collection.groups.map((g) => new CollectionAccessSelectionView(g)) diff --git a/libs/admin-console/src/common/collections/models/collection-with-id.request.ts b/libs/admin-console/src/common/collections/models/collection-with-id.request.ts index f1fccda0cf..0f7b83705e 100644 --- a/libs/admin-console/src/common/collections/models/collection-with-id.request.ts +++ b/libs/admin-console/src/common/collections/models/collection-with-id.request.ts @@ -1,19 +1,18 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Collection } from "./collection"; -import { CollectionRequest } from "./collection.request"; +import { BaseCollectionRequest } from "./collection.request"; -export class CollectionWithIdRequest extends CollectionRequest { +export class CollectionWithIdRequest extends BaseCollectionRequest { id: string; + name: string; - constructor(collection?: Collection) { - if (collection == null) { - return; + constructor(collection: Collection) { + if (collection == null || collection.name == null || collection.name.encryptedString == null) { + throw new Error("CollectionWithIdRequest must contain name."); } super({ - name: collection.name, externalId: collection.externalId, }); + this.name = collection.name.encryptedString; this.id = collection.id; } } diff --git a/libs/admin-console/src/common/collections/models/collection.data.ts b/libs/admin-console/src/common/collections/models/collection.data.ts index dac8f41b2b..a783a3c9ab 100644 --- a/libs/admin-console/src/common/collections/models/collection.data.ts +++ b/libs/admin-console/src/common/collections/models/collection.data.ts @@ -9,6 +9,7 @@ export class CollectionData { id: CollectionId; organizationId: OrganizationId; name: string; + defaultUserCollectionEmail: string | undefined; externalId: string | undefined; readOnly: boolean = false; manage: boolean = false; @@ -24,6 +25,7 @@ export class CollectionData { this.manage = response.manage; this.hidePasswords = response.hidePasswords; this.type = response.type; + this.defaultUserCollectionEmail = response.defaultUserCollectionEmail; } static fromJSON(obj: Jsonify): CollectionData | null { diff --git a/libs/admin-console/src/common/collections/models/collection.request.ts b/libs/admin-console/src/common/collections/models/collection.request.ts index 39ed03d81f..310bb08ea7 100644 --- a/libs/admin-console/src/common/collections/models/collection.request.ts +++ b/libs/admin-console/src/common/collections/models/collection.request.ts @@ -1,23 +1,20 @@ import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; -export class CollectionRequest { - name: string; +export abstract class BaseCollectionRequest { externalId: string | undefined; groups: SelectionReadOnlyRequest[] = []; users: SelectionReadOnlyRequest[] = []; - constructor(c: { - name: EncString; + static isUpdate = (request: BaseCollectionRequest): request is UpdateCollectionRequest => { + return request instanceof UpdateCollectionRequest; + }; + + protected constructor(c: { users?: SelectionReadOnlyRequest[]; groups?: SelectionReadOnlyRequest[]; externalId?: string; }) { - if (!c.name || !c.name.encryptedString) { - throw new Error("Name not provided for CollectionRequest."); - } - - this.name = c.name.encryptedString; this.externalId = c.externalId; if (c.groups) { @@ -28,3 +25,36 @@ export class CollectionRequest { } } } + +export class CreateCollectionRequest extends BaseCollectionRequest { + name: string; + + constructor(c: { + name: EncString; + users?: SelectionReadOnlyRequest[]; + groups?: SelectionReadOnlyRequest[]; + externalId?: string; + }) { + super(c); + + if (!c.name || !c.name.encryptedString) { + throw new Error("Name not provided for CollectionRequest."); + } + + this.name = c.name.encryptedString; + } +} + +export class UpdateCollectionRequest extends BaseCollectionRequest { + name: string | null; + + constructor(c: { + name: EncString | null; + users?: SelectionReadOnlyRequest[]; + groups?: SelectionReadOnlyRequest[]; + externalId?: string; + }) { + super(c); + this.name = c.name?.encryptedString ?? null; + } +} diff --git a/libs/admin-console/src/common/collections/models/collection.response.ts b/libs/admin-console/src/common/collections/models/collection.response.ts index ad0eed53dd..e672263598 100644 --- a/libs/admin-console/src/common/collections/models/collection.response.ts +++ b/libs/admin-console/src/common/collections/models/collection.response.ts @@ -8,6 +8,7 @@ export class CollectionResponse extends BaseResponse { id: CollectionId; organizationId: OrganizationId; name: string; + defaultUserCollectionEmail: string | undefined; externalId: string | undefined; type: CollectionType = CollectionTypes.SharedCollection; @@ -17,6 +18,7 @@ export class CollectionResponse extends BaseResponse { this.organizationId = this.getResponseProperty("OrganizationId"); this.name = this.getResponseProperty("Name"); this.externalId = this.getResponseProperty("ExternalId"); + this.defaultUserCollectionEmail = this.getResponseProperty("DefaultUserCollectionEmail"); this.type = this.getResponseProperty("Type") ?? CollectionTypes.SharedCollection; } } diff --git a/libs/admin-console/src/common/collections/models/collection.spec.ts b/libs/admin-console/src/common/collections/models/collection.spec.ts index 1cb1d39ed5..16066f88ce 100644 --- a/libs/admin-console/src/common/collections/models/collection.spec.ts +++ b/libs/admin-console/src/common/collections/models/collection.spec.ts @@ -25,6 +25,7 @@ describe("Collection", () => { manage: true, hidePasswords: true, type: CollectionTypes.DefaultUserCollection, + defaultUserCollectionEmail: "defaultCollectionEmail", }), ); encService = mock(); @@ -61,6 +62,7 @@ describe("Collection", () => { manage: true, hidePasswords: true, type: CollectionTypes.DefaultUserCollection, + defaultUserCollectionEmail: "defaultCollectionEmail", }); }); @@ -75,6 +77,7 @@ describe("Collection", () => { collection.hidePasswords = false; collection.manage = true; collection.type = CollectionTypes.DefaultUserCollection; + collection.defaultUserCollectionEmail = "defaultCollectionEmail"; const key = makeSymmetricCryptoKey(); @@ -84,12 +87,13 @@ describe("Collection", () => { externalId: "extId", hidePasswords: false, id: "id", - name: "encName", + _name: "encName", organizationId: "orgId", readOnly: false, manage: true, assigned: true, type: CollectionTypes.DefaultUserCollection, + defaultUserCollectionEmail: "defaultCollectionEmail", }); }); }); diff --git a/libs/admin-console/src/common/collections/models/collection.ts b/libs/admin-console/src/common/collections/models/collection.ts index 7de4462b0c..cf5573b8f4 100644 --- a/libs/admin-console/src/common/collections/models/collection.ts +++ b/libs/admin-console/src/common/collections/models/collection.ts @@ -23,6 +23,7 @@ export class Collection extends Domain { hidePasswords: boolean = false; manage: boolean = false; type: CollectionType = CollectionTypes.SharedCollection; + defaultUserCollectionEmail: string | undefined; constructor(c: { id: CollectionId; name: EncString; organizationId: OrganizationId }) { super(); @@ -46,6 +47,7 @@ export class Collection extends Domain { collection.hidePasswords = obj.hidePasswords; collection.manage = obj.manage; collection.type = obj.type; + collection.defaultUserCollectionEmail = obj.defaultUserCollectionEmail; return collection; } diff --git a/libs/admin-console/src/common/collections/models/collection.view.ts b/libs/admin-console/src/common/collections/models/collection.view.ts index c4470fe13f..d3e15806c8 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -16,7 +16,6 @@ export const NestingDelimiter = "/"; export class CollectionView implements View, ITreeNodeObject { id: CollectionId; organizationId: OrganizationId; - name: string; externalId: string | undefined; // readOnly applies to the items within a collection readOnly: boolean = false; @@ -24,11 +23,22 @@ export class CollectionView implements View, ITreeNodeObject { manage: boolean = false; assigned: boolean = false; type: CollectionType = CollectionTypes.SharedCollection; + defaultUserCollectionEmail: string | undefined; + + private _name: string; constructor(c: { id: CollectionId; organizationId: OrganizationId; name: string }) { this.id = c.id; this.organizationId = c.organizationId; - this.name = c.name; + this._name = c.name; + } + + set name(name: string) { + this._name = name; + } + + get name(): string { + return this.defaultUserCollectionEmail ?? this._name; } canEditItems(org: Organization): boolean { @@ -83,6 +93,18 @@ export class CollectionView implements View, ITreeNodeObject { return false; } + /** + * Returns true if the collection name can be edited. Editing the collection name is restricted for collections + * that were DefaultUserCollections but where the relevant user has been offboarded. + * When this occurs, the offboarded user's email is treated as the collection name, and cannot be edited. + * This is important for security so that the server cannot ask the client to encrypt arbitrary data. + * WARNING! This is an IMPORTANT restriction that MUST be maintained for security purposes. + * Do not edit or remove this unless you understand why. + */ + canEditName(org: Organization): boolean { + return this.canEdit(org) && !this.defaultUserCollectionEmail; + } + get isDefaultCollection() { return this.type == CollectionTypes.DefaultUserCollection; } @@ -111,6 +133,7 @@ export class CollectionView implements View, ITreeNodeObject { view.hidePasswords = collection.hidePasswords; view.manage = collection.manage; view.type = collection.type; + view.defaultUserCollectionEmail = collection.defaultUserCollectionEmail; return view; } @@ -125,6 +148,7 @@ export class CollectionView implements View, ITreeNodeObject { view.externalId = collection.externalId; view.type = collection.type; view.assigned = collection.assigned; + view.defaultUserCollectionEmail = collection.defaultUserCollectionEmail; return view; } diff --git a/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts b/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts index 3f7e8e31e0..ca797a0f9a 100644 --- a/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts +++ b/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts @@ -1,6 +1,10 @@ import { combineLatest, firstValueFrom, from, map, Observable, of, switchMap } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { + getOrganizationById, + OrganizationService, +} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; @@ -10,13 +14,15 @@ import { KeyService } from "@bitwarden/key-management"; import { CollectionAdminService, CollectionService } from "../abstractions"; import { CollectionData, - CollectionRequest, CollectionAccessDetailsResponse, CollectionDetailsResponse, CollectionResponse, BulkCollectionAccessRequest, CollectionAccessSelectionView, CollectionAdminView, + BaseCollectionRequest, + UpdateCollectionRequest, + CreateCollectionRequest, } from "../models"; export class DefaultCollectionAdminService implements CollectionAdminService { @@ -25,6 +31,7 @@ export class DefaultCollectionAdminService implements CollectionAdminService { private keyService: KeyService, private encryptService: EncryptService, private collectionService: CollectionService, + private organizationService: OrganizationService, ) {} collectionAdminViews$(organizationId: string, userId: UserId): Observable { @@ -45,27 +52,40 @@ export class DefaultCollectionAdminService implements CollectionAdminService { ); } - async save(collection: CollectionAdminView, userId: UserId): Promise { - const request = await this.encrypt(collection, userId); - - let response: CollectionDetailsResponse; - if (collection.id == null) { - response = await this.apiService.postCollection(collection.organizationId, request); - collection.id = response.id; - } else { - response = await this.apiService.putCollection( - collection.organizationId, - collection.id, - request, - ); + async update( + collection: CollectionAdminView, + userId: UserId, + ): Promise { + const request = await this.encrypt(collection, userId, true); + if (!BaseCollectionRequest.isUpdate(request)) { + throw new Error("Cannot update collection with CreateCollectionRequest."); } - if (response.assigned) { - await this.collectionService.upsert(new CollectionData(response), userId); - } else { - await this.collectionService.delete([collection.id as CollectionId], userId); + const response = await this.apiService.putCollection( + collection.organizationId, + collection.id, + request, + ); + + await this.updateLocalCollections(response, collection, userId); + + return response; + } + + async create( + collection: CollectionAdminView, + userId: UserId, + ): Promise { + const request = await this.encrypt(collection, userId, false); + if (BaseCollectionRequest.isUpdate(request)) { + throw new Error("Cannot create collection with UpdateCollectionRequest."); } + const response = await this.apiService.postCollection(collection.organizationId, request); + collection.id = response.id; + + await this.updateLocalCollections(response, collection, userId); + return response; } @@ -73,6 +93,16 @@ export class DefaultCollectionAdminService implements CollectionAdminService { await this.apiService.deleteCollection(organizationId, collectionId); } + private async updateLocalCollections( + response: CollectionDetailsResponse, + collection: CollectionAdminView, + userId: UserId, + ) { + response.assigned + ? await this.collectionService.upsert(new CollectionData(response), userId) + : await this.collectionService.delete([collection.id as CollectionId], userId); + } + async bulkAssignAccess( organizationId: string, collectionIds: string[], @@ -118,10 +148,15 @@ export class DefaultCollectionAdminService implements CollectionAdminService { ); }); - return await Promise.all(promises); + const r = await Promise.all(promises); + return r; } - private async encrypt(model: CollectionAdminView, userId: UserId): Promise { + private async encrypt( + model: CollectionAdminView, + userId: UserId, + editMode: boolean, + ): Promise { if (!model.organizationId) { throw new Error("Collection has no organization id."); } @@ -154,14 +189,31 @@ export class DefaultCollectionAdminService implements CollectionAdminService { new SelectionReadOnlyRequest(user.id, user.readOnly, user.hidePasswords, user.manage), ); - const collectionRequest = new CollectionRequest({ + if (editMode) { + const org = await firstValueFrom( + this.organizationService + .organizations$(userId) + .pipe(getOrganizationById(model.organizationId)), + ); + if (org == null) { + throw new Error("No Organization found."); + } + return new UpdateCollectionRequest({ + name: model.canEditName(org) + ? await this.encryptService.encryptString(model.name, key) + : null, + externalId: model.externalId, + users, + groups, + }); + } + + return new CreateCollectionRequest({ name: await this.encryptService.encryptString(model.name, key), externalId: model.externalId, users, groups, }); - - return collectionRequest; } } diff --git a/libs/admin-console/src/common/collections/services/default-collection.service.ts b/libs/admin-console/src/common/collections/services/default-collection.service.ts index aa25cdfa1e..39b3b49186 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.ts @@ -216,7 +216,7 @@ export class DefaultCollectionService implements CollectionService { getAllNested(collections: CollectionView[]): TreeNode[] { const nodes: TreeNode[] = []; collections.forEach((c) => { - const collectionCopy = Object.assign(new CollectionView({ ...c }), c); + const collectionCopy = Object.assign(new CollectionView({ ...c, name: c.name }), c); const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : []; ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, undefined, NestingDelimiter); diff --git a/libs/common/src/abstractions/api.service.ts b/libs/common/src/abstractions/api.service.ts index d8b8c1c429..726b04534a 100644 --- a/libs/common/src/abstractions/api.service.ts +++ b/libs/common/src/abstractions/api.service.ts @@ -3,8 +3,9 @@ import { CollectionAccessDetailsResponse, CollectionDetailsResponse, - CollectionRequest, CollectionResponse, + CreateCollectionRequest, + UpdateCollectionRequest, } from "@bitwarden/admin-console/common"; import { OrganizationConnectionType } from "../admin-console/enums"; @@ -270,12 +271,12 @@ export abstract class ApiService { ): Promise>; abstract postCollection( organizationId: string, - request: CollectionRequest, + request: CreateCollectionRequest, ): Promise; abstract putCollection( organizationId: string, id: string, - request: CollectionRequest, + request: UpdateCollectionRequest, ): Promise; abstract deleteCollection(organizationId: string, id: string): Promise; abstract deleteManyCollections(organizationId: string, collectionIds: string[]): Promise; diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index e48b941d4a..6a670368b1 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -7,8 +7,9 @@ import { firstValueFrom } from "rxjs"; import { CollectionAccessDetailsResponse, CollectionDetailsResponse, - CollectionRequest, CollectionResponse, + CreateCollectionRequest, + UpdateCollectionRequest, } from "@bitwarden/admin-console/common"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports @@ -727,7 +728,7 @@ export class ApiService implements ApiServiceAbstraction { async postCollection( organizationId: string, - request: CollectionRequest, + request: CreateCollectionRequest, ): Promise { const r = await this.send( "POST", @@ -742,7 +743,7 @@ export class ApiService implements ApiServiceAbstraction { async putCollection( organizationId: string, id: string, - request: CollectionRequest, + request: UpdateCollectionRequest, ): Promise { const r = await this.send( "PUT", diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts index 6aa7175d37..e3d863a0af 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts @@ -33,23 +33,24 @@ const createMockCollection = ( readOnly = false, canEdit = true, ): CollectionView => { - return { - id: id as CollectionId, + const cv = new CollectionView({ name, organizationId: organizationId as OrganizationId, - externalId: "", - readOnly, - hidePasswords: false, - manage: true, - assigned: true, - type: CollectionTypes.DefaultUserCollection, - isDefaultCollection: true, - canEditItems: jest.fn().mockReturnValue(canEdit), - canEdit: jest.fn(), - canDelete: jest.fn(), - canViewCollectionInfo: jest.fn(), - encrypt: jest.fn(), - }; + id: id as CollectionId, + }); + cv.readOnly = readOnly; + cv.manage = true; + cv.type = CollectionTypes.DefaultUserCollection; + cv.externalId = ""; + cv.hidePasswords = false; + cv.assigned = true; + cv.canEditName = jest.fn().mockReturnValue(true); + cv.canEditItems = jest.fn().mockReturnValue(canEdit); + cv.canEdit = jest.fn(); + cv.canDelete = jest.fn(); + cv.canViewCollectionInfo = jest.fn(); + + return cv; }; describe("ItemDetailsSectionComponent", () => {