diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index e1f4f33149e..2e273d041d6 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -225,14 +225,15 @@ export class EditCommand { : req.users.map( (u) => new SelectionReadOnlyRequest(u.id, u.readOnly, u.hidePasswords, u.manage), ); - const request = new CollectionRequest(); - request.name = (await this.encryptService.encryptString(req.name, orgKey)).encryptedString; - request.externalId = req.externalId; - request.groups = groups; - request.users = users; + const request = new CollectionRequest({ + name: await this.encryptService.encryptString(req.name, orgKey), + externalId: req.externalId, + users, + groups, + }); + const response = await this.apiService.putCollection(req.organizationId, id, request); - const view = CollectionExport.toView(req); - view.id = response.id; + const view = CollectionExport.toView(req, response.id); const res = new OrganizationCollectionResponse(view, groups, users); return Response.success(res); } catch (e) { diff --git a/apps/cli/src/commands/get.command.ts b/apps/cli/src/commands/get.command.ts index b0f0c1f7d17..58181cff6ce 100644 --- a/apps/cli/src/commands/get.command.ts +++ b/apps/cli/src/commands/get.command.ts @@ -13,7 +13,6 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { CardExport } from "@bitwarden/common/models/export/card.export"; import { CipherExport } from "@bitwarden/common/models/export/cipher.export"; import { CollectionExport } from "@bitwarden/common/models/export/collection.export"; @@ -452,6 +451,7 @@ export class GetCommand extends DownloadCommand { const orgKeys = await firstValueFrom(this.keyService.activeUserOrgKeys$); decCollection = await collection.decrypt( orgKeys[collection.organizationId as OrganizationId], + this.encryptService, ); } } else if (id.trim() !== "") { @@ -497,9 +497,9 @@ export class GetCommand extends DownloadCommand { } const response = await this.apiService.getCollectionAccessDetails(options.organizationId, id); - const decCollection = new CollectionView(response); - decCollection.name = await this.encryptService.decryptString( - new EncString(response.name), + const decCollection = await CollectionView.fromCollectionAccessDetails( + response, + this.encryptService, orgKey, ); const groups = diff --git a/apps/cli/src/commands/list.command.ts b/apps/cli/src/commands/list.command.ts index 94abd97d6eb..d8b4cfcfd10 100644 --- a/apps/cli/src/commands/list.command.ts +++ b/apps/cli/src/commands/list.command.ts @@ -211,7 +211,9 @@ export class ListCommand { } const collections = response.data .filter((c) => c.organizationId === options.organizationId) - .map((r) => new Collection(new CollectionData(r as ApiCollectionDetailsResponse))); + .map((r) => + Collection.fromCollectionData(new CollectionData(r as ApiCollectionDetailsResponse)), + ); const orgKeys = await firstValueFrom(this.keyService.orgKeys$(userId)); if (orgKeys == null) { throw new Error("Organization keys not found."); diff --git a/apps/cli/src/vault/create.command.ts b/apps/cli/src/vault/create.command.ts index 33ec52eeca8..cbaae4d2544 100644 --- a/apps/cli/src/vault/create.command.ts +++ b/apps/cli/src/vault/create.command.ts @@ -233,14 +233,14 @@ export class CreateCommand { : req.users.map( (u) => new SelectionReadOnlyRequest(u.id, u.readOnly, u.hidePasswords, u.manage), ); - const request = new CollectionRequest(); - request.name = (await this.encryptService.encryptString(req.name, orgKey)).encryptedString; - request.externalId = req.externalId; - request.groups = groups; - request.users = users; + const request = new CollectionRequest({ + name: await this.encryptService.encryptString(req.name, orgKey), + externalId: req.externalId, + groups, + users, + }); const response = await this.apiService.postCollection(req.organizationId, request); - const view = CollectionExport.toView(req); - view.id = response.id; + const view = CollectionExport.toView(req, response.id); const res = new OrganizationCollectionResponse(view, groups, users); return Response.success(res); } catch (e) { diff --git a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts index abd99d37355..ad3d0d8169a 100644 --- a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts +++ b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.spec.ts @@ -1,5 +1,7 @@ import { CollectionView } from "@bitwarden/admin-console/common"; +import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; +import { newGuid } from "@bitwarden/guid"; import { getNestedCollectionTree, getFlatCollectionTree } from "./collection-utils"; @@ -9,11 +11,17 @@ describe("CollectionUtils Service", () => { // Arrange const collections: CollectionView[] = []; - const parentCollection = new CollectionView(); - parentCollection.name = "Parent"; + const parentCollection = new CollectionView({ + name: "Parent", + organizationId: "orgId" as OrganizationId, + id: newGuid() as CollectionId, + }); - const childCollection = new CollectionView(); - childCollection.name = "Parent/Child"; + const childCollection = new CollectionView({ + name: "Parent/Child", + organizationId: "orgId" as OrganizationId, + id: newGuid() as CollectionId, + }); collections.push(childCollection); collections.push(parentCollection); @@ -41,12 +49,14 @@ describe("CollectionUtils Service", () => { describe("getFlatCollectionTree", () => { it("should flatten a tree node with no children", () => { // Arrange - const collection = new CollectionView(); - collection.name = "Test Collection"; - collection.id = "test-id"; + const collection = new CollectionView({ + name: "Test Collection", + id: "test-id" as CollectionId, + organizationId: "orgId" as OrganizationId, + }); const treeNodes: TreeNode[] = [ - new TreeNode(collection, null), + new TreeNode(collection, {} as TreeNode), ]; // Act @@ -59,23 +69,34 @@ describe("CollectionUtils Service", () => { it("should flatten a tree node with children", () => { // Arrange - const parentCollection = new CollectionView(); - parentCollection.name = "Parent"; - parentCollection.id = "parent-id"; + const parentCollection = new CollectionView({ + name: "Parent", + id: "parent-id" as CollectionId, + organizationId: "orgId" as OrganizationId, + }); - const child1Collection = new CollectionView(); - child1Collection.name = "Child 1"; - child1Collection.id = "child1-id"; + const child1Collection = new CollectionView({ + name: "Child 1", + id: "child1-id" as CollectionId, + organizationId: "orgId" as OrganizationId, + }); - const child2Collection = new CollectionView(); - child2Collection.name = "Child 2"; - child2Collection.id = "child2-id"; + const child2Collection = new CollectionView({ + name: "Child 2", + id: "child2-id" as CollectionId, + organizationId: "orgId" as OrganizationId, + }); - const grandchildCollection = new CollectionView(); - grandchildCollection.name = "Grandchild"; - grandchildCollection.id = "grandchild-id"; + const grandchildCollection = new CollectionView({ + name: "Grandchild", + id: "grandchild-id" as CollectionId, + organizationId: "orgId" as OrganizationId, + }); - const parentNode = new TreeNode(parentCollection, null); + const parentNode = new TreeNode( + parentCollection, + {} as TreeNode, + ); const child1Node = new TreeNode(child1Collection, parentNode); const child2Node = new TreeNode(child2Collection, parentNode); const grandchildNode = new TreeNode(grandchildCollection, child1Node); 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 0697659c976..8a0fab520e3 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 @@ -22,7 +22,7 @@ export function getNestedCollectionTree( // Collections need to be cloned because ServiceUtils.nestedTraverse actively // modifies the names of collections. // These changes risk affecting collections store in StateService. - const clonedCollections = collections + const clonedCollections: CollectionView[] | CollectionAdminView[] = collections .sort((a, b) => a.name.localeCompare(b.name)) .map(cloneCollection); @@ -37,6 +37,21 @@ export function getNestedCollectionTree( return nodes; } +export function cloneCollection(collection: CollectionView): CollectionView; +export function cloneCollection(collection: CollectionAdminView): CollectionAdminView; +export function cloneCollection( + collection: CollectionView | CollectionAdminView, +): CollectionView | CollectionAdminView { + let cloned; + + if (collection instanceof CollectionAdminView) { + cloned = Object.assign(new CollectionAdminView({ ...collection }), collection); + } else { + cloned = Object.assign(new CollectionView({ ...collection }), collection); + } + return cloned; +} + export function getFlatCollectionTree( nodes: TreeNode[], ): CollectionAdminView[]; @@ -57,32 +72,3 @@ export function getFlatCollectionTree( return [node.node, ...children]; }); } - -function cloneCollection(collection: CollectionView): CollectionView; -function cloneCollection(collection: CollectionAdminView): CollectionAdminView; -function cloneCollection( - collection: CollectionView | CollectionAdminView, -): CollectionView | CollectionAdminView { - let cloned; - - if (collection instanceof CollectionAdminView) { - cloned = new CollectionAdminView(); - cloned.groups = [...collection.groups]; - cloned.users = [...collection.users]; - cloned.assigned = collection.assigned; - cloned.unmanaged = collection.unmanaged; - } else { - cloned = new CollectionView(); - } - - cloned.id = collection.id; - cloned.externalId = collection.externalId; - cloned.hidePasswords = collection.hidePasswords; - cloned.name = collection.name; - cloned.organizationId = collection.organizationId; - cloned.readOnly = collection.readOnly; - cloned.manage = collection.manage; - cloned.type = collection.type; - - return cloned; -} diff --git a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts index fe63830b891..5d2460abdc1 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts @@ -391,11 +391,13 @@ export class VaultComponent implements OnInit, OnDestroy { // FIXME: We should not assert that the Unassigned type is a CollectionId. // Instead we should consider representing the Unassigned collection as a different object, given that // it is not actually a collection. - const noneCollection = new CollectionAdminView(); - noneCollection.name = this.i18nService.t("unassigned"); - noneCollection.id = Unassigned as CollectionId; - noneCollection.organizationId = organizationId; - return allCollections.concat(noneCollection); + return allCollections.concat( + new CollectionAdminView({ + name: this.i18nService.t("unassigned"), + id: Unassigned as CollectionId, + organizationId, + }), + ); }), ); diff --git a/apps/web/src/app/admin-console/organizations/manage/groups.component.ts b/apps/web/src/app/admin-console/organizations/manage/groups.component.ts index f48860c69a6..23e92056c95 100644 --- a/apps/web/src/app/admin-console/organizations/manage/groups.component.ts +++ b/apps/web/src/app/admin-console/organizations/manage/groups.component.ts @@ -253,8 +253,8 @@ export class GroupsComponent { private toCollectionMap( response: ListResponse, ): Observable> { - const collections = response.data.map( - (r) => new Collection(new CollectionData(r as CollectionDetailsResponse)), + const collections = response.data.map((r) => + Collection.fromCollectionData(new CollectionData(r as CollectionDetailsResponse)), ); return this.accountService.activeAccount$.pipe( 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 c34b42a61bd..b4542be8d26 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 @@ -312,7 +312,9 @@ export class MembersComponent extends BaseMembersComponent async getCollectionNameMap() { const response = from(this.apiService.getCollections(this.organization.id)).pipe( map((res) => - res.data.map((r) => new Collection(new CollectionData(r as CollectionDetailsResponse))), + res.data.map((r) => + Collection.fromCollectionData(new CollectionData(r as CollectionDetailsResponse)), + ), ), ); 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 003de9a54e1..59d042cae52 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 @@ -399,9 +399,14 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { return; } - const collectionView = new CollectionAdminView(); - collectionView.id = this.params.collectionId; - collectionView.organizationId = this.formGroup.controls.selectedOrg.value; + const parent = this.formGroup.controls.parent?.value; + const collectionView = new CollectionAdminView({ + id: this.params.collectionId as CollectionId, + organizationId: this.formGroup.controls.selectedOrg.value, + name: parent + ? `${parent}/${this.formGroup.controls.name.value}` + : this.formGroup.controls.name.value, + }); collectionView.externalId = this.formGroup.controls.externalId.value; collectionView.groups = this.formGroup.controls.access.value .filter((v) => v.type === AccessItemType.Group) @@ -410,13 +415,6 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { .filter((v) => v.type === AccessItemType.Member) .map(convertToSelectionView); - const parent = this.formGroup.controls.parent.value; - if (parent) { - collectionView.name = `${parent}/${this.formGroup.controls.name.value}`; - } else { - collectionView.name = this.formGroup.controls.name.value; - } - const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const savedCollection = await this.collectionAdminService.save(collectionView, userId); diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts index a8dd0056806..8fde2eb44e4 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts @@ -224,7 +224,7 @@ export class VaultItemsComponent { } protected canEditCollection(collection: CollectionView): boolean { - // Only allow allow deletion if collection editing is enabled and not deleting "Unassigned" + // Only allow deletion if collection editing is enabled and not deleting "Unassigned" if (collection.id === Unassigned) { return false; } @@ -235,7 +235,7 @@ export class VaultItemsComponent { } protected canDeleteCollection(collection: CollectionView): boolean { - // Only allow allow deletion if collection editing is enabled and not deleting "Unassigned" + // Only allow deletion if collection editing is enabled and not deleting "Unassigned" if (collection.id === Unassigned) { return false; } diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts index c114cb6d7c2..043ae900b40 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts @@ -262,9 +262,11 @@ export const OrganizationTrash: Story = { }, }; -const unassignedCollection = new CollectionAdminView(); -unassignedCollection.id = Unassigned as CollectionId; -unassignedCollection.name = "Unassigned"; +const unassignedCollection = new CollectionAdminView({ + id: Unassigned as CollectionId, + name: "Unassigned", + organizationId: "org id" as OrganizationId, +}); export const OrganizationTopLevelCollection: Story = { args: { ciphers: [], @@ -327,11 +329,11 @@ function createCipherView(i: number, deleted = false): CipherView { function createCollectionView(i: number): CollectionAdminView { const organization = organizations[i % (organizations.length + 1)]; const group = groups[i % (groups.length + 1)]; - const view = new CollectionAdminView(); - view.id = `collection-${i}` as CollectionId; - view.name = `Collection ${i}`; - view.organizationId = organization?.id; - view.manage = true; + const view = new CollectionAdminView({ + id: `collection-${i}` as CollectionId, + name: `Collection ${i}`, + organizationId: organization?.id ?? ("orgId" as OrganizationId), + }); if (group !== undefined) { view.groups = [ @@ -344,6 +346,7 @@ function createCollectionView(i: number): CollectionAdminView { ]; } + view.manage = true; return view; } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts index b7a19bf2e76..f2c60651ed9 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts @@ -21,7 +21,7 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { UserId } from "@bitwarden/common/types/guid"; +import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -368,11 +368,16 @@ describe("vault filter service", () => { orgId: string, type?: CollectionType, ): CollectionView { - const collection = new CollectionView(); - collection.id = id; - collection.name = name; - collection.organizationId = orgId; - collection.type = type || CollectionTypes.SharedCollection; + const collection = new CollectionView({ + id: id as CollectionId, + name, + organizationId: orgId as OrganizationId, + }); + + if (type) { + collection.type = type; + } + return collection; } }); 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 11e074db985..ec77ff97a11 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 @@ -12,11 +12,7 @@ import { switchMap, } from "rxjs"; -import { - CollectionAdminView, - CollectionService, - CollectionView, -} from "@bitwarden/admin-console/common"; +import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import { sortDefaultCollections } from "@bitwarden/angular/vault/vault-filter/services/vault-filter.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -38,6 +34,7 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { ServiceUtils } from "@bitwarden/common/vault/service-utils"; import { COLLAPSED_GROUPINGS } from "@bitwarden/common/vault/services/key-state/collapsed-groupings.state"; import { CipherListView } from "@bitwarden/sdk-internal"; +import { cloneCollection } from "@bitwarden/web-vault/app/admin-console/organizations/collections"; import { CipherTypeFilter, @@ -253,14 +250,8 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { } collections.forEach((c) => { - const collectionCopy = new CollectionView() as CollectionFilter; - collectionCopy.id = c.id; - collectionCopy.organizationId = c.organizationId; + const collectionCopy = cloneCollection(new CollectionView({ ...c })) as CollectionFilter; collectionCopy.icon = "bwi-collection-shared"; - if (c instanceof CollectionAdminView) { - collectionCopy.groups = c.groups; - collectionCopy.assigned = c.assigned; - } const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : []; ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, null, NestingDelimiter); }); @@ -274,7 +265,7 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { } protected getCollectionFilterHead(): TreeNode { - const head = new CollectionView() as CollectionFilter; + const head = CollectionView.vaultFilterHead() as CollectionFilter; return new TreeNode(head, null, "collections", "AllCollections"); } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter.model.spec.ts b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter.model.spec.ts index bd4f3d29b34..51fe837468c 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter.model.spec.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter.model.spec.ts @@ -2,6 +2,7 @@ // @ts-strict-ignore import { CollectionView } from "@bitwarden/admin-console/common"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { CollectionId } from "@bitwarden/common/types/guid"; import { CipherType } from "@bitwarden/common/vault/enums"; import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -224,7 +225,9 @@ describe("VaultFilter", () => { it("should return false when filtering by All Collections", () => { const filterFunction = createFilterFunction({ - selectedCollectionNode: createCollectionFilterNode({ id: "AllCollections" }), + selectedCollectionNode: createCollectionFilterNode({ + id: "AllCollections" as CollectionId, + }), }); const result = filterFunction(cipher); @@ -309,15 +312,12 @@ function createFolderFilterNode(options: Partial): TreeNode, ): TreeNode { - const collection = new CollectionView() as CollectionFilter; - collection.id = options.id; - collection.name = options.name ?? ""; - collection.icon = options.icon ?? ""; - collection.organizationId = options.organizationId; - collection.externalId = options.externalId ?? ""; - collection.readOnly = options.readOnly ?? false; - collection.hidePasswords = options.hidePasswords ?? false; - return new TreeNode(collection, null); + const collection = new CollectionView({ + name: options.name ?? "Test Name", + id: options.id ?? null, + organizationId: options.organizationId ?? "Org Id", + }) as CollectionFilter; + return new TreeNode(collection, {} as TreeNode); } function createCipher(options: Partial = {}) { 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 dcc88551551..1cec9c3d39d 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 @@ -1,7 +1,10 @@ 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 { OrgKey } from "@bitwarden/common/types/key"; import { CollectionAccessSelectionView } from "./collection-access-selection.view"; -import { CollectionAccessDetailsResponse } from "./collection.response"; +import { CollectionAccessDetailsResponse, CollectionResponse } from "./collection.response"; import { CollectionView } from "./collection.view"; // TODO: this is used to represent the pseudo "Unassigned" collection as well as @@ -24,24 +27,6 @@ export class CollectionAdminView extends CollectionView { */ assigned: boolean = false; - constructor(response?: CollectionAccessDetailsResponse) { - super(response); - - if (!response) { - return; - } - - this.groups = response.groups - ? response.groups.map((g) => new CollectionAccessSelectionView(g)) - : []; - - this.users = response.users - ? response.users.map((g) => new CollectionAccessSelectionView(g)) - : []; - - this.assigned = response.assigned; - } - /** * Returns true if the user can edit a collection (including user and group access) from the Admin Console. */ @@ -115,4 +100,46 @@ export class CollectionAdminView extends CollectionView { get isUnassignedCollection() { return this.id === Unassigned; } + + static async fromCollectionAccessDetails( + collection: CollectionAccessDetailsResponse, + encryptService: EncryptService, + orgKey: OrgKey, + ): Promise { + const view = new CollectionAdminView({ ...collection }); + view.name = await encryptService.decryptString(new EncString(view.name), orgKey); + view.assigned = collection.assigned; + view.readOnly = collection.readOnly; + view.hidePasswords = collection.hidePasswords; + view.manage = collection.manage; + view.unmanaged = collection.unmanaged; + view.type = collection.type; + view.externalId = collection.externalId; + + view.groups = collection.groups + ? collection.groups.map((g) => new CollectionAccessSelectionView(g)) + : []; + + view.users = collection.users + ? collection.users.map((g) => new CollectionAccessSelectionView(g)) + : []; + + return view; + } + + static async fromCollectionResponse( + collection: CollectionResponse, + encryptService: EncryptService, + orgKey: OrgKey, + ): Promise { + const collectionAdminView = new CollectionAdminView({ + id: collection.id, + name: await encryptService.decryptString(new EncString(collection.name), orgKey), + organizationId: collection.organizationId, + }); + + collectionAdminView.externalId = collection.externalId; + + return collectionAdminView; + } } 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 ca24e139517..f1fccda0cfa 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 @@ -10,7 +10,10 @@ export class CollectionWithIdRequest extends CollectionRequest { if (collection == null) { return; } - super(collection); + super({ + name: collection.name, + externalId: collection.externalId, + }); 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 27c5c0c0efa..dac8f41b2ba 100644 --- a/libs/admin-console/src/common/collections/models/collection.data.ts +++ b/libs/admin-console/src/common/collections/models/collection.data.ts @@ -2,18 +2,18 @@ import { Jsonify } from "type-fest"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; -import { CollectionType } from "./collection"; +import { CollectionType, CollectionTypes } from "./collection"; import { CollectionDetailsResponse } from "./collection.response"; export class CollectionData { id: CollectionId; organizationId: OrganizationId; name: string; - externalId: string; - readOnly: boolean; - manage: boolean; - hidePasswords: boolean; - type: CollectionType; + externalId: string | undefined; + readOnly: boolean = false; + manage: boolean = false; + hidePasswords: boolean = false; + type: CollectionType = CollectionTypes.SharedCollection; constructor(response: CollectionDetailsResponse) { this.id = response.id; 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 4244bf1e780..39ed03d81fa 100644 --- a/libs/admin-console/src/common/collections/models/collection.request.ts +++ b/libs/admin-console/src/common/collections/models/collection.request.ts @@ -1,20 +1,30 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request"; - -import { Collection } from "./collection"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; export class CollectionRequest { name: string; - externalId: string; + externalId: string | undefined; groups: SelectionReadOnlyRequest[] = []; users: SelectionReadOnlyRequest[] = []; - constructor(collection?: Collection) { - if (collection == null) { - return; + constructor(c: { + name: EncString; + 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) { + this.groups = c.groups; + } + if (c.users) { + this.users = c.users; } - this.name = collection.name ? collection.name.encryptedString : null; - this.externalId = collection.externalId; } } 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 c9b8ccf0456..ad0eed53ddf 100644 --- a/libs/admin-console/src/common/collections/models/collection.response.ts +++ b/libs/admin-console/src/common/collections/models/collection.response.ts @@ -2,14 +2,14 @@ import { SelectionReadOnlyResponse } from "@bitwarden/common/admin-console/model import { BaseResponse } from "@bitwarden/common/models/response/base.response"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; -import { CollectionType } from "./collection"; +import { CollectionType, CollectionTypes } from "./collection"; export class CollectionResponse extends BaseResponse { id: CollectionId; organizationId: OrganizationId; name: string; - externalId: string; - type: CollectionType; + externalId: string | undefined; + type: CollectionType = CollectionTypes.SharedCollection; constructor(response: any) { super(response); @@ -17,7 +17,7 @@ export class CollectionResponse extends BaseResponse { this.organizationId = this.getResponseProperty("OrganizationId"); this.name = this.getResponseProperty("Name"); this.externalId = this.getResponseProperty("ExternalId"); - this.type = this.getResponseProperty("Type"); + 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 fb38f1507f9..1cb1d39ed57 100644 --- a/libs/admin-console/src/common/collections/models/collection.spec.ts +++ b/libs/admin-console/src/common/collections/models/collection.spec.ts @@ -1,50 +1,62 @@ -import { makeSymmetricCryptoKey, mockEnc } from "@bitwarden/common/spec"; +import { MockProxy, mock } from "jest-mock-extended"; + +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { makeSymmetricCryptoKey } from "@bitwarden/common/spec"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; import { Collection, CollectionTypes } from "./collection"; import { CollectionData } from "./collection.data"; +import { CollectionDetailsResponse } from "./collection.response"; describe("Collection", () => { let data: CollectionData; + let encService: MockProxy; beforeEach(() => { - data = { - id: "id" as CollectionId, - organizationId: "orgId" as OrganizationId, - name: "encName", - externalId: "extId", - readOnly: true, - manage: true, - hidePasswords: true, - type: CollectionTypes.DefaultUserCollection, - }; + data = new CollectionData( + new CollectionDetailsResponse({ + id: "id" as CollectionId, + organizationId: "orgId" as OrganizationId, + name: "encName", + externalId: "extId", + readOnly: true, + manage: true, + hidePasswords: true, + type: CollectionTypes.DefaultUserCollection, + }), + ); + encService = mock(); + encService.decryptString.mockResolvedValue("encName"); }); - it("Convert from empty", () => { - const data = new CollectionData({} as any); - const card = new Collection(data); - - expect(card).toEqual({ - externalId: null, - hidePasswords: null, - id: null, - name: null, - organizationId: null, - readOnly: null, - manage: null, - type: null, + it("Convert from partial", () => { + const card = new Collection({ + name: new EncString("name"), + organizationId: "orgId" as OrganizationId, + id: "id" as CollectionId, }); + expect(() => card).not.toThrow(); + + expect(card.name).not.toBe(null); + expect(card.organizationId).not.toBe(null); + expect(card.id).not.toBe(null); + expect(card.externalId).toBe(undefined); + expect(card.readOnly).toBe(false); + expect(card.manage).toBe(false); + expect(card.hidePasswords).toBe(false); + expect(card.type).toEqual(CollectionTypes.SharedCollection); }); it("Convert", () => { - const collection = new Collection(data); + const collection = Collection.fromCollectionData(data); expect(collection).toEqual({ id: "id", organizationId: "orgId", name: { encryptedString: "encName", encryptionType: 0 }, - externalId: { encryptedString: "extId", encryptionType: 0 }, + externalId: "extId", readOnly: true, manage: true, hidePasswords: true, @@ -53,10 +65,11 @@ describe("Collection", () => { }); it("Decrypt", async () => { - const collection = new Collection(); - collection.id = "id" as CollectionId; - collection.organizationId = "orgId" as OrganizationId; - collection.name = mockEnc("encName"); + const collection = new Collection({ + name: new EncString("encName"), + organizationId: "orgId" as OrganizationId, + id: "id" as CollectionId, + }); collection.externalId = "extId"; collection.readOnly = false; collection.hidePasswords = false; @@ -65,7 +78,7 @@ describe("Collection", () => { const key = makeSymmetricCryptoKey(); - const view = await collection.decrypt(key); + const view = await collection.decrypt(key, encService); expect(view).toEqual({ externalId: "extId", diff --git a/libs/admin-console/src/common/collections/models/collection.ts b/libs/admin-console/src/common/collections/models/collection.ts index d1709d1751d..196b15ef5e9 100644 --- a/libs/admin-console/src/common/collections/models/collection.ts +++ b/libs/admin-console/src/common/collections/models/collection.ts @@ -1,5 +1,6 @@ +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; -import Domain, { EncryptableKeys } from "@bitwarden/common/platform/models/domain/domain-base"; +import Domain from "@bitwarden/common/platform/models/domain/domain-base"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; @@ -14,45 +15,63 @@ export const CollectionTypes = { export type CollectionType = (typeof CollectionTypes)[keyof typeof CollectionTypes]; export class Collection extends Domain { - id: CollectionId | undefined; - organizationId: OrganizationId | undefined; - name: EncString | undefined; + id: CollectionId; + organizationId: OrganizationId; + name: EncString; externalId: string | undefined; readOnly: boolean = false; hidePasswords: boolean = false; manage: boolean = false; type: CollectionType = CollectionTypes.SharedCollection; - constructor(obj?: CollectionData | null) { + constructor(c: { id: CollectionId; name: EncString; organizationId: OrganizationId }) { super(); - if (obj == null) { - return; + this.id = c.id; + this.name = c.name; + this.organizationId = c.organizationId; + } + + static fromCollectionData(obj: CollectionData): Collection { + if (obj == null || obj.name == null || obj.organizationId == null) { + throw new Error("CollectionData must contain name and organizationId."); } - this.buildDomainModel( - this, - obj, - { - id: null, - organizationId: null, - name: null, - externalId: null, - readOnly: null, - hidePasswords: null, - manage: null, - type: null, - }, - ["id", "organizationId", "readOnly", "hidePasswords", "manage", "type"], + const collection = new Collection({ + ...obj, + name: new EncString(obj.name), + }); + + collection.externalId = obj.externalId; + collection.readOnly = obj.readOnly; + collection.hidePasswords = obj.hidePasswords; + collection.manage = obj.manage; + collection.type = obj.type; + + return collection; + } + + static async fromCollectionView( + view: CollectionView, + encryptService: EncryptService, + orgKey: OrgKey, + ): Promise { + return Object.assign( + new Collection({ + name: await encryptService.encryptString(view.name, orgKey), + id: view.id, + organizationId: view.organizationId, + }), + view, ); } - decrypt(orgKey: OrgKey): Promise { - return this.decryptObj( - this, - new CollectionView(this), - ["name"] as EncryptableKeys[], - this.organizationId ?? null, - orgKey, - ); + decrypt(orgKey: OrgKey, encryptService: EncryptService): Promise { + return CollectionView.fromCollection(this, encryptService, orgKey); + } + + // @TODO: This would be better off in Collection.Utils. Move this there when + // refactoring to a shared lib. + static isCollectionId(id: any): id is CollectionId { + return typeof id === "string" && id != null; } } 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 3a60320856d..d8039820845 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -1,8 +1,11 @@ import { Jsonify } from "type-fest"; 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 { View } from "@bitwarden/common/models/view/view"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { OrgKey } from "@bitwarden/common/types/key"; import { ITreeNodeObject } from "@bitwarden/common/vault/models/domain/tree-node"; import { Collection, CollectionType, CollectionTypes } from "./collection"; @@ -11,9 +14,9 @@ import { CollectionAccessDetailsResponse } from "./collection.response"; export const NestingDelimiter = "/"; export class CollectionView implements View, ITreeNodeObject { - id: CollectionId | undefined; - organizationId: OrganizationId | undefined; - name: string = ""; + id: CollectionId; + organizationId: OrganizationId; + name: string; externalId: string | undefined; // readOnly applies to the items within a collection readOnly: boolean = false; @@ -22,24 +25,10 @@ export class CollectionView implements View, ITreeNodeObject { assigned: boolean = false; type: CollectionType = CollectionTypes.SharedCollection; - constructor(c?: Collection | CollectionAccessDetailsResponse) { - if (!c) { - return; - } - + constructor(c: { id: CollectionId; organizationId: OrganizationId; name: string }) { this.id = c.id; this.organizationId = c.organizationId; - this.externalId = c.externalId; - if (c instanceof Collection) { - this.readOnly = c.readOnly; - this.hidePasswords = c.hidePasswords; - this.manage = c.manage; - this.assigned = true; - } - if (c instanceof CollectionAccessDetailsResponse) { - this.assigned = c.assigned; - } - this.type = c.type; + this.name = c.name; } canEditItems(org: Organization): boolean { @@ -94,11 +83,53 @@ export class CollectionView implements View, ITreeNodeObject { return false; } - static fromJSON(obj: Jsonify) { - return Object.assign(new CollectionView(new Collection()), obj); - } - get isDefaultCollection() { return this.type == CollectionTypes.DefaultUserCollection; } + + // FIXME: we should not use a CollectionView object for the vault filter header because it is not a real + // CollectionView and this violates ts-strict rules. + static vaultFilterHead(): CollectionView { + return new CollectionView({ + id: "" as CollectionId, + organizationId: "" as OrganizationId, + name: "", + }); + } + + static async fromCollection( + collection: Collection, + encryptService: EncryptService, + key: OrgKey, + ): Promise { + const view: CollectionView = Object.assign( + new CollectionView({ ...collection, name: "" }), + collection, + ); + view.name = await encryptService.decryptString(collection.name, key); + view.assigned = true; + return view; + } + + static async fromCollectionAccessDetails( + collection: CollectionAccessDetailsResponse, + encryptService: EncryptService, + orgKey: OrgKey, + ): Promise { + const view = new CollectionView({ ...collection }); + + view.name = await encryptService.decryptString(new EncString(collection.name), orgKey); + view.externalId = collection.externalId; + view.type = collection.type; + view.assigned = collection.assigned; + return view; + } + + static fromJSON(obj: Jsonify) { + return Object.assign(new CollectionView({ ...obj }), obj); + } + + encrypt(orgKey: OrgKey, encryptService: EncryptService): Promise { + return Collection.fromCollectionView(this, encryptService, orgKey); + } } diff --git a/libs/admin-console/src/common/collections/services/collection.state.ts b/libs/admin-console/src/common/collections/services/collection.state.ts index ebb620c2354..9ca6faac75b 100644 --- a/libs/admin-console/src/common/collections/services/collection.state.ts +++ b/libs/admin-console/src/common/collections/services/collection.state.ts @@ -9,13 +9,14 @@ import { CollectionId } from "@bitwarden/common/types/guid"; import { CollectionData, CollectionView } from "../models"; -export const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record< - CollectionData | null, - CollectionId ->(COLLECTION_DISK, "collections", { - deserializer: (jsonData: Jsonify) => CollectionData.fromJSON(jsonData), - clearOn: ["logout"], -}); +export const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record( + COLLECTION_DISK, + "collections", + { + deserializer: (jsonData: Jsonify) => CollectionData.fromJSON(jsonData), + clearOn: ["logout"], + }, +); export const DECRYPTED_COLLECTION_DATA_KEY = new UserKeyDefinition( COLLECTION_MEMORY, 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 10ca6eb1531..3f7e8e31e0b 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,12 +1,8 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore - import { combineLatest, firstValueFrom, from, map, Observable, of, switchMap } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; 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 { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; import { KeyService } from "@bitwarden/key-management"; @@ -36,12 +32,15 @@ export class DefaultCollectionAdminService implements CollectionAdminService { this.keyService.orgKeys$(userId), from(this.apiService.getManyCollectionsWithAccessDetails(organizationId)), ]).pipe( - switchMap(([orgKey, res]) => { + switchMap(([orgKeys, res]) => { if (res?.data == null || res.data.length === 0) { return of([]); } + if (orgKeys == null) { + throw new Error("No org keys found."); + } - return this.decryptMany(organizationId, res.data, orgKey); + return this.decryptMany(organizationId, res.data, orgKeys); }), ); } @@ -104,55 +103,65 @@ export class DefaultCollectionAdminService implements CollectionAdminService { orgKeys: Record, ): Promise { const promises = collections.map(async (c) => { - const view = new CollectionAdminView(); - view.id = c.id; - view.name = await this.encryptService.decryptString( - new EncString(c.name), - orgKeys[organizationId as OrganizationId], - ); - view.externalId = c.externalId; - view.organizationId = c.organizationId; - if (isCollectionAccessDetailsResponse(c)) { - view.groups = c.groups; - view.users = c.users; - view.assigned = c.assigned; - view.readOnly = c.readOnly; - view.hidePasswords = c.hidePasswords; - view.manage = c.manage; - view.unmanaged = c.unmanaged; + return CollectionAdminView.fromCollectionAccessDetails( + c, + this.encryptService, + orgKeys[organizationId as OrganizationId], + ); } - return view; + return await CollectionAdminView.fromCollectionResponse( + c, + this.encryptService, + orgKeys[organizationId as OrganizationId], + ); }); return await Promise.all(promises); } private async encrypt(model: CollectionAdminView, userId: UserId): Promise { - if (model.organizationId == null) { + if (!model.organizationId) { throw new Error("Collection has no organization id."); } + const key = await firstValueFrom( - this.keyService - .orgKeys$(userId) - .pipe(map((orgKeys) => orgKeys[model.organizationId] ?? null)), + this.keyService.orgKeys$(userId).pipe( + map((orgKeys) => { + if (!orgKeys) { + throw new Error("No keys for the provided userId."); + } + + const key = orgKeys[model.organizationId]; + + if (key == null) { + throw new Error("No key for this collection's organization."); + } + + return key; + }), + ), ); - if (key == null) { - throw new Error("No key for this collection's organization."); - } - const collection = new CollectionRequest(); - collection.externalId = model.externalId; - collection.name = (await this.encryptService.encryptString(model.name, key)).encryptedString; - collection.groups = model.groups.map( + + const groups = model.groups.map( (group) => new SelectionReadOnlyRequest(group.id, group.readOnly, group.hidePasswords, group.manage), ); - collection.users = model.users.map( + + const users = model.users.map( (user) => new SelectionReadOnlyRequest(user.id, user.readOnly, user.hidePasswords, user.manage), ); - return collection; + + const collectionRequest = new CollectionRequest({ + 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.spec.ts b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts index c2c0332a486..a0332cfb501 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts @@ -390,9 +390,11 @@ const collectionDataFactory = (orgId?: OrganizationId) => { }; function collectionViewDataFactory(orgId?: OrganizationId): CollectionView { - const collectionView = new CollectionView(); - collectionView.id = Utils.newGuid() as CollectionId; - collectionView.organizationId = orgId ?? (Utils.newGuid() as OrganizationId); - collectionView.name = "DEC_NAME_" + collectionView.id; + const id = Utils.newGuid() as CollectionId; + const collectionView = new CollectionView({ + id, + organizationId: orgId ?? (Utils.newGuid() as OrganizationId), + name: "DEC_NAME_" + id, + }); return collectionView; } 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 4978b06df35..aa25cdfa1e7 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 @@ -42,9 +42,7 @@ export class DefaultCollectionService implements CollectionService { /** * @returns a SingleUserState for encrypted collection data. */ - private encryptedState( - userId: UserId, - ): SingleUserState> { + private encryptedState(userId: UserId): SingleUserState> { return this.stateProvider.getUser(userId, ENCRYPTED_COLLECTION_DATA_KEY); } @@ -62,7 +60,7 @@ export class DefaultCollectionService implements CollectionService { return null; } - return Object.values(collections).map((c) => new Collection(c)); + return Object.values(collections).map((c) => Collection.fromCollectionData(c)); }), ); } @@ -110,8 +108,8 @@ export class DefaultCollectionService implements CollectionService { if (collections == null) { collections = {}; } - collections[toUpdate.id] = toUpdate; + collections[toUpdate.id] = toUpdate; return collections; }); @@ -121,7 +119,7 @@ export class DefaultCollectionService implements CollectionService { if (!orgKeys) { throw new Error("No key for this collection's organization."); } - return this.decryptMany$([new Collection(toUpdate)], orgKeys); + return this.decryptMany$([Collection.fromCollectionData(toUpdate)], orgKeys); }), ), ); @@ -177,10 +175,6 @@ export class DefaultCollectionService implements CollectionService { } async encrypt(model: CollectionView, userId: UserId): Promise { - if (model.organizationId == null) { - throw new Error("Collection has no organization id."); - } - const key = await firstValueFrom( this.keyService.orgKeys$(userId).pipe( filter((orgKeys) => !!orgKeys), @@ -188,13 +182,7 @@ export class DefaultCollectionService implements CollectionService { ), ); - const collection = new Collection(); - collection.id = model.id; - collection.organizationId = model.organizationId; - collection.readOnly = model.readOnly; - collection.externalId = model.externalId; - collection.name = await this.encryptService.encryptString(model.name, key); - return collection; + return await model.encrypt(key, this.encryptService); } // TODO: this should be private. @@ -211,7 +199,12 @@ export class DefaultCollectionService implements CollectionService { collections.forEach((collection) => { decCollections.push( - from(collection.decrypt(orgKeys[collection.organizationId as OrganizationId])), + from( + collection.decrypt( + orgKeys[collection.organizationId as OrganizationId], + this.encryptService, + ), + ), ); }); @@ -223,9 +216,8 @@ export class DefaultCollectionService implements CollectionService { getAllNested(collections: CollectionView[]): TreeNode[] { const nodes: TreeNode[] = []; collections.forEach((c) => { - const collectionCopy = new CollectionView(); - collectionCopy.id = c.id; - collectionCopy.organizationId = c.organizationId; + const collectionCopy = Object.assign(new CollectionView({ ...c }), 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/models/export/collection-with-id.export.ts b/libs/common/src/models/export/collection-with-id.export.ts index c973472e0bb..9a372fbdfa9 100644 --- a/libs/common/src/models/export/collection-with-id.export.ts +++ b/libs/common/src/models/export/collection-with-id.export.ts @@ -3,20 +3,18 @@ // 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 import { Collection as CollectionDomain, CollectionView } from "@bitwarden/admin-console/common"; - -import { CollectionId } from "../../types/guid"; +import { CollectionId } from "@bitwarden/common/types/guid"; import { CollectionExport } from "./collection.export"; export class CollectionWithIdExport extends CollectionExport { id: CollectionId; - static toView(req: CollectionWithIdExport, view = new CollectionView()) { - view.id = req.id; - return super.toView(req, view); + static toView(req: CollectionWithIdExport) { + return super.toView(req, req.id); } - static toDomain(req: CollectionWithIdExport, domain = new CollectionDomain()) { + static toDomain(req: CollectionWithIdExport, domain: CollectionDomain) { domain.id = req.id; return super.toDomain(req, domain); } diff --git a/libs/common/src/models/export/collection.export.ts b/libs/common/src/models/export/collection.export.ts index b141346d03f..631b31d8b7b 100644 --- a/libs/common/src/models/export/collection.export.ts +++ b/libs/common/src/models/export/collection.export.ts @@ -5,7 +5,7 @@ import { Collection as CollectionDomain, CollectionView } from "@bitwarden/admin-console/common"; import { EncString } from "../../key-management/crypto/models/enc-string"; -import { emptyGuid, OrganizationId } from "../../types/guid"; +import { CollectionId, emptyGuid, OrganizationId } from "../../types/guid"; import { safeGetString } from "./utils"; @@ -18,16 +18,17 @@ export class CollectionExport { return req; } - static toView(req: CollectionExport, view = new CollectionView()) { - view.name = req.name; + static toView(req: CollectionExport, id: CollectionId) { + const view = new CollectionView({ + name: req.name, + organizationId: req.organizationId, + id, + }); view.externalId = req.externalId; - if (view.organizationId == null) { - view.organizationId = req.organizationId; - } return view; } - static toDomain(req: CollectionExport, domain = new CollectionDomain()) { + static toDomain(req: CollectionExport, domain: CollectionDomain) { domain.name = req.name != null ? new EncString(req.name) : null; domain.externalId = req.externalId; if (domain.organizationId == null) { diff --git a/libs/common/src/platform/misc/rxjs-operators.ts b/libs/common/src/platform/misc/rxjs-operators.ts index 423bcbb790f..b3c4423c36f 100644 --- a/libs/common/src/platform/misc/rxjs-operators.ts +++ b/libs/common/src/platform/misc/rxjs-operators.ts @@ -13,8 +13,8 @@ export const getById = (id: TId) => * @param id The IDs of the objects to return. * @returns An array containing objects with matching IDs, or an empty array if there are no matching objects. */ -export const getByIds = (ids: TId[]) => { - const idSet = new Set(ids.filter((id) => id != null)); +export const getByIds = (ids: TId[]) => { + const idSet = new Set(ids); return map((objects) => { return objects.filter((o) => o.id && idSet.has(o.id)); }); diff --git a/libs/common/src/platform/models/domain/domain-base.ts b/libs/common/src/platform/models/domain/domain-base.ts index b999a5e5d15..bab9f0f8ac7 100644 --- a/libs/common/src/platform/models/domain/domain-base.ts +++ b/libs/common/src/platform/models/domain/domain-base.ts @@ -13,7 +13,7 @@ export type DecryptedObject< > = Record & Omit; // extracts shared keys from the domain and view types -export type EncryptableKeys = (keyof D & +type EncryptableKeys = (keyof D & ConditionalKeys) & (keyof V & ConditionalKeys); diff --git a/libs/importer/src/importers/base-importer.ts b/libs/importer/src/importers/base-importer.ts index 4c25a01f965..19a8a4828e1 100644 --- a/libs/importer/src/importers/base-importer.ts +++ b/libs/importer/src/importers/base-importer.ts @@ -4,12 +4,12 @@ import * as papa from "papaparse"; // 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 -import { CollectionView } from "@bitwarden/admin-console/common"; +import { Collection, CollectionView } from "@bitwarden/admin-console/common"; import { normalizeExpiryYearFormat } from "@bitwarden/common/autofill/utils"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; -import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { OrganizationId } from "@bitwarden/common/types/guid"; import { FieldType, SecureNoteType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; @@ -278,9 +278,12 @@ export abstract class BaseImporter { protected moveFoldersToCollections(result: ImportResult) { result.folderRelationships.forEach((r) => result.collectionRelationships.push(r)); result.collections = result.folders.map((f) => { - const collection = new CollectionView(); - collection.name = f.name; - collection.id = (f.id as CollectionId) ?? undefined; // folder id may be null, which is not suitable for collections. + const collection = new CollectionView({ + name: f.name, + organizationId: this.organizationId, + // FIXME: Folder.id may be null, this should be changed when refactoring Folders to be ts-strict + id: Collection.isCollectionId(f.id) ? f.id : null, + }); return collection; }); result.folderRelationships = []; diff --git a/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts b/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts index 1636d867193..70c783df52a 100644 --- a/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts +++ b/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts @@ -1,10 +1,10 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, map } from "rxjs"; +import { concatMap, firstValueFrom, map } from "rxjs"; // 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 -import { CollectionView } from "@bitwarden/admin-console/common"; +import { Collection, CollectionView } from "@bitwarden/admin-console/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; @@ -206,11 +206,20 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer { for (const c of data.collections) { let collectionView: CollectionView; if (data.encrypted) { - const collection = CollectionWithIdExport.toDomain(c); - collection.organizationId = this.organizationId; - collectionView = await firstValueFrom(this.keyService.activeUserOrgKeys$).then((orgKeys) => - collection.decrypt(orgKeys[c.organizationId as OrganizationId]), + const collection = CollectionWithIdExport.toDomain( + c, + new Collection({ + id: c.id, + name: new EncString(c.name), + organizationId: this.organizationId, + }), ); + const collection$ = this.keyService.activeUserOrgKeys$.pipe( + // FIXME: replace type assertion with narrowing + map((keys) => keys[c.organizationId as OrganizationId]), + concatMap((key) => collection.decrypt(key, this.encryptService)), + ); + collectionView = await firstValueFrom(collection$); } else { collectionView = CollectionWithIdExport.toView(c); collectionView.organizationId = null; diff --git a/libs/importer/src/importers/padlock-csv-importer.ts b/libs/importer/src/importers/padlock-csv-importer.ts index 86b569fbc52..ec781170c4d 100644 --- a/libs/importer/src/importers/padlock-csv-importer.ts +++ b/libs/importer/src/importers/padlock-csv-importer.ts @@ -46,8 +46,11 @@ export class PadlockCsvImporter extends BaseImporter implements Importer { } if (addCollection) { - const collection = new CollectionView(); - collection.name = tag; + // FIXME use a different model if ID is not required. + // @ts-expect-error current functionality creates this view with no Id since its being imported. + const collection = new CollectionView({ + name: tag, + }); result.collections.push(collection); } diff --git a/libs/importer/src/importers/passpack-csv-importer.ts b/libs/importer/src/importers/passpack-csv-importer.ts index 17b0c148896..09ff841b8a4 100644 --- a/libs/importer/src/importers/passpack-csv-importer.ts +++ b/libs/importer/src/importers/passpack-csv-importer.ts @@ -47,8 +47,11 @@ export class PasspackCsvImporter extends BaseImporter implements Importer { } if (addCollection) { - const collection = new CollectionView(); - collection.name = tag; + // FIXME use a different model if ID is not required. + // @ts-expect-error current functionality creates this view with no Id since its being imported. + const collection = new CollectionView({ + name: tag, + }); result.collections.push(collection); } diff --git a/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts b/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts index dc438c0edee..1f14d05c51e 100644 --- a/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts +++ b/libs/importer/src/importers/password-depot/password-depot-17-xml-importer.spec.ts @@ -487,8 +487,11 @@ describe("Password Depot 17 Xml Importer", () => { it("should parse groups nodes into collections when importing into an organization", async () => { const importer = new PasswordDepot17XmlImporter(); importer.organizationId = "someOrgId" as OrganizationId; - const collection = new CollectionView(); - collection.name = "tempDB"; + const collection = new CollectionView({ + name: "tempDB", + organizationId: importer.organizationId, + id: null, + }); const actual = [collection]; const result = await importer.parse(PasswordTestData); diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts index ac560ed6f7f..ad6e6ebf016 100644 --- a/libs/importer/src/services/import.service.spec.ts +++ b/libs/importer/src/services/import.service.spec.ts @@ -145,20 +145,29 @@ describe("ImportService", () => { ); }); - const mockImportTargetCollection = new CollectionView(); - mockImportTargetCollection.id = "myImportTarget" as CollectionId; - mockImportTargetCollection.name = "myImportTarget"; - mockImportTargetCollection.organizationId = organizationId; + const mockName = "myImportTarget"; + const mockId = "myImportTarget" as CollectionId; + const mockImportTargetCollection = new CollectionView({ + name: mockName, + id: mockId, + organizationId, + }); - const mockCollection1 = new CollectionView(); - mockCollection1.id = "collection1" as CollectionId; - mockCollection1.name = "collection1"; - mockCollection1.organizationId = organizationId; + const mockName1 = "collection1"; + const mockId1 = "collection1" as CollectionId; + const mockCollection1 = new CollectionView({ + name: mockName1, + id: mockId1, + organizationId, + }); - const mockCollection2 = new CollectionView(); - mockCollection2.id = "collection2" as CollectionId; - mockCollection2.name = "collection2"; - mockCollection2.organizationId = organizationId; + const mockName2 = "collection2"; + const mockId2 = "collection2" as CollectionId; + const mockCollection2 = new CollectionView({ + name: mockName2, + id: mockId2, + organizationId, + }); it("passing importTarget adds it to collections", async () => { await importService["setImportTarget"]( diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts index d3880f63bd5..133607251c3 100644 --- a/libs/importer/src/services/import.service.ts +++ b/libs/importer/src/services/import.service.ts @@ -501,7 +501,7 @@ export class ImportService implements ImportServiceAbstraction { const collections: CollectionView[] = [...importResult.collections]; importResult.collections = [importTarget as CollectionView]; collections.map((x) => { - const f = new CollectionView(); + const f = new CollectionView(x); f.name = `${importTarget.name}/${x.name}`; importResult.collections.push(f); }); diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts index ba04748b864..404a5d6cf3c 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts @@ -143,10 +143,14 @@ export class OrganizationVaultExportService if (exportData != null) { if (exportData.collections != null && exportData.collections.length > 0) { exportData.collections.forEach((c) => { - const collection = new Collection(new CollectionData(c as CollectionDetailsResponse)); + const collection = Collection.fromCollectionData( + new CollectionData(c as CollectionDetailsResponse), + ); exportPromises.push( firstValueFrom(this.keyService.activeUserOrgKeys$) - .then((keys) => collection.decrypt(keys[organizationId as OrganizationId])) + .then((keys) => + collection.decrypt(keys[organizationId as OrganizationId], this.encryptService), + ) .then((decCol) => { decCollections.push(decCol); }), @@ -191,7 +195,9 @@ export class OrganizationVaultExportService this.apiService.getCollections(organizationId).then((c) => { if (c != null && c.data != null && c.data.length > 0) { c.data.forEach((r) => { - const collection = new Collection(new CollectionData(r as CollectionDetailsResponse)); + const collection = Collection.fromCollectionData( + new CollectionData(r as CollectionDetailsResponse), + ); collections.push(collection); }); } 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 1f2aaa8904b..6aa7175d373 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 @@ -48,6 +48,7 @@ const createMockCollection = ( canEdit: jest.fn(), canDelete: jest.fn(), canViewCollectionInfo: jest.fn(), + encrypt: jest.fn(), }; }; diff --git a/libs/vault/src/components/assign-collections.component.spec.ts b/libs/vault/src/components/assign-collections.component.spec.ts index 800bf5393ad..e54bada30ba 100644 --- a/libs/vault/src/components/assign-collections.component.spec.ts +++ b/libs/vault/src/components/assign-collections.component.spec.ts @@ -29,23 +29,27 @@ describe("AssignCollectionsComponent", () => { const mockUserId = "mock-user-id" as UserId; const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); - const editCollection = new CollectionView(); - editCollection.id = "collection-id" as CollectionId; - editCollection.organizationId = "org-id" as OrganizationId; - editCollection.name = "Editable Collection"; + const editCollection = new CollectionView({ + id: "collection-id" as CollectionId, + organizationId: "org-id" as OrganizationId, + name: "Editable Collection", + }); + editCollection.readOnly = false; editCollection.manage = true; - const readOnlyCollection1 = new CollectionView(); - readOnlyCollection1.id = "read-only-collection-id" as CollectionId; - readOnlyCollection1.organizationId = "org-id" as OrganizationId; - readOnlyCollection1.name = "Read Only Collection"; + const readOnlyCollection1 = new CollectionView({ + id: "read-only-collection-id" as CollectionId, + organizationId: "org-id" as OrganizationId, + name: "Read Only Collection", + }); readOnlyCollection1.readOnly = true; - const readOnlyCollection2 = new CollectionView(); - readOnlyCollection2.id = "read-only-collection-id-2" as CollectionId; - readOnlyCollection2.organizationId = "org-id" as OrganizationId; - readOnlyCollection2.name = "Read Only Collection 2"; + const readOnlyCollection2 = new CollectionView({ + id: "read-only-collection-id-2" as CollectionId, + organizationId: "org-id" as OrganizationId, + name: "Read Only Collection 2", + }); readOnlyCollection2.readOnly = true; const params = {