From 51d15cd515d7afe9a8ae15dac4c3dfe57a320645 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 2 Oct 2024 12:22:04 -0400 Subject: [PATCH 01/26] updated canEditCipher permission logic inside vault-items --- .../vault/components/vault-items/vault-items.component.html | 3 +-- .../vault/components/vault-items/vault-items.component.ts | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.html b/apps/web/src/app/vault/components/vault-items/vault-items.component.html index de7bc0ec169..f6e571601ab 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.html +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.html @@ -131,8 +131,7 @@ [organizations]="allOrganizations" [collections]="allCollections" [checked]="selection.isSelected(item)" - [canEditCipher]="canEditCipher(item.cipher) && vaultBulkManagementActionEnabled" - [vaultBulkManagementActionEnabled]="vaultBulkManagementActionEnabled" + [canEditCipher]="canEditCipher(item.cipher)" (checkedToggled)="selection.toggle(item)" (onEvent)="event($event)" > 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 591c132e1e6..2a1a0da15fd 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 @@ -211,7 +211,10 @@ export class VaultItemsComponent { } const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); - return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.edit; + return ( + (organization.canEditAllCiphers && this.viewingOrgVault) || + (cipher.edit && cipher.viewPassword) + ); } private refreshItems() { From a7eb84c6d2ab505981d9b98ab9b64eca635d10d3 Mon Sep 17 00:00:00 2001 From: jng Date: Fri, 4 Oct 2024 13:42:02 -0400 Subject: [PATCH 02/26] add permission logic to assign to collections for browser cipher edit --- .../item-details-section.component.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index b0716218b59..c77cfefbdef 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -258,7 +258,27 @@ export class ItemDetailsSectionComponent implements OnInit { collectionsControl.reset(); collectionsControl.enable(); - this.showCollectionsControl = true; + + const organization = this.organizations.find((org) => { + return org.id === this.originalCipherView.organizationId; + }); + + const filteredCollections = this.originalCipherView.collectionIds.find((id) => { + return this.collections.find((collection) => { + return collection.id === id && collection.manage; + }); + }); + + // Show Assign To Collections if + // The user has "Can Manage" or "Can Edit" permissions + // The Organization has "Owners and Admins Can Manage" setting on + if ( + organization.canEditAllCiphers || + filteredCollections?.length > 0 || + (this.originalCipherView.edit && this.originalCipherView.viewPassword) + ) { + this.showCollectionsControl = true; + } // If there is only one collection, select it by default. if (this.collectionOptions.length === 1) { From 3812613ea43c3347a8a324fdab5119ec8c8a9679 Mon Sep 17 00:00:00 2001 From: jng Date: Fri, 4 Oct 2024 15:00:44 -0400 Subject: [PATCH 03/26] update spec for new perm logic in item details section --- .../item-details-section.component.spec.ts | 29 +++++++++++++++++++ .../item-details-section.component.ts | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) 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 d7678aa596a..0b89f8f0a4e 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 @@ -145,6 +145,15 @@ describe("ItemDetailsSectionComponent", () => { it("should disable organizationId control if ownership change is not allowed", async () => { component.config.allowPersonalOwnership = false; component.config.organizations = [{ id: "org1" } as Organization]; + component.originalCipherView = { + name: "cipher1", + organizationId: "org1", + edit: true, + folderId: "folder1", + collectionIds: ["col1", "col2"], + favorite: true, + viewPassword: true, + } as CipherView; jest.spyOn(component, "allowOwnershipChange", "get").mockReturnValue(false); await component.ngOnInit(); @@ -303,6 +312,15 @@ describe("ItemDetailsSectionComponent", () => { { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, ]; + component.originalCipherView = { + name: "cipher1", + organizationId: "org1", + edit: true, + folderId: "folder1", + collectionIds: ["col1", "col2"], + favorite: true, + viewPassword: true, + } as CipherView; fixture.detectChanges(); await fixture.whenStable(); @@ -353,6 +371,15 @@ describe("ItemDetailsSectionComponent", () => { component.config.collections = [ { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, ]; + component.originalCipherView = { + name: "cipher1", + organizationId: "org1", + edit: true, + folderId: "folder1", + collectionIds: ["col1", "col2"], + favorite: true, + viewPassword: true, + } as CipherView; fixture.detectChanges(); await fixture.whenStable(); @@ -372,9 +399,11 @@ describe("ItemDetailsSectionComponent", () => { component.originalCipherView = { name: "cipher1", organizationId: "org1", + edit: true, folderId: "folder1", collectionIds: ["col1", "col2", "col3"], favorite: true, + viewPassword: true, } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index c77cfefbdef..d01d5ed0031 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -273,7 +273,7 @@ export class ItemDetailsSectionComponent implements OnInit { // The user has "Can Manage" or "Can Edit" permissions // The Organization has "Owners and Admins Can Manage" setting on if ( - organization.canEditAllCiphers || + organization?.canEditAllCiphers || filteredCollections?.length > 0 || (this.originalCipherView.edit && this.originalCipherView.viewPassword) ) { From 6308ecad569533e1ba0185cd4ae1f34d57f3b7ca Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 9 Oct 2024 14:27:30 -0400 Subject: [PATCH 04/26] updates to vault items and item details section to only disable and not remove assign to collections based on permissions --- .../vault-items/vault-cipher-row.component.ts | 3 +- .../vault-items/vault-items.component.html | 1 + .../vault-items/vault-items.component.ts | 9 ++++++ .../item-details-section.component.spec.ts | 29 ------------------- .../item-details-section.component.ts | 26 ++++------------- 5 files changed, 17 insertions(+), 51 deletions(-) diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts index 221a22421b0..8faa6045e4f 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts @@ -35,6 +35,7 @@ export class VaultCipherRowComponent implements OnInit { @Input() collections: CollectionView[]; @Input() viewingOrgVault: boolean; @Input() canEditCipher: boolean; + @Input() canAssignCollections: boolean; @Output() onEvent = new EventEmitter(); @@ -71,7 +72,7 @@ export class VaultCipherRowComponent implements OnInit { } protected get showAssignToCollections() { - return this.canEditCipher && !this.cipher.isDeleted; + return this.canAssignCollections && !this.cipher.isDeleted; } protected get showClone() { diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.html b/apps/web/src/app/vault/components/vault-items/vault-items.component.html index 99516907bbb..15ecc25727e 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.html +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.html @@ -123,6 +123,7 @@ [collections]="allCollections" [checked]="selection.isSelected(item)" [canEditCipher]="canEditCipher(item.cipher)" + [canAssignCollections]="canAssignCollections(item.cipher)" (checkedToggled)="selection.toggle(item)" (onEvent)="event($event)" > 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 5e8401fe2b2..e7950ec5e12 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 @@ -201,6 +201,15 @@ export class VaultItemsComponent { ); } + protected canAssignCollections(cipher: CipherView) { + if (cipher.organizationId == null) { + return true; + } + + const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); + return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.edit; + } + private refreshItems() { const collections: VaultItem[] = this.collections.map((collection) => ({ collection })); const ciphers: VaultItem[] = this.ciphers.map((cipher) => ({ cipher })); 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 69aa3f858e6..fe1710ccade 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 @@ -145,15 +145,6 @@ describe("ItemDetailsSectionComponent", () => { it("should disable organizationId control if ownership change is not allowed", async () => { component.config.allowPersonalOwnership = false; component.config.organizations = [{ id: "org1" } as Organization]; - component.originalCipherView = { - name: "cipher1", - organizationId: "org1", - edit: true, - folderId: "folder1", - collectionIds: ["col1", "col2"], - favorite: true, - viewPassword: true, - } as CipherView; jest.spyOn(component, "allowOwnershipChange", "get").mockReturnValue(false); await component.ngOnInit(); @@ -312,15 +303,6 @@ describe("ItemDetailsSectionComponent", () => { { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, ]; - component.originalCipherView = { - name: "cipher1", - organizationId: "org1", - edit: true, - folderId: "folder1", - collectionIds: ["col1", "col2"], - favorite: true, - viewPassword: true, - } as CipherView; fixture.detectChanges(); await fixture.whenStable(); @@ -371,15 +353,6 @@ describe("ItemDetailsSectionComponent", () => { component.config.collections = [ { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, ]; - component.originalCipherView = { - name: "cipher1", - organizationId: "org1", - edit: true, - folderId: "folder1", - collectionIds: ["col1", "col2"], - favorite: true, - viewPassword: true, - } as CipherView; fixture.detectChanges(); await fixture.whenStable(); @@ -399,11 +372,9 @@ describe("ItemDetailsSectionComponent", () => { component.originalCipherView = { name: "cipher1", organizationId: "org1", - edit: true, folderId: "folder1", collectionIds: ["col1", "col2", "col3"], favorite: true, - viewPassword: true, } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 582ea555f80..b46e09aeb35 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -213,6 +213,10 @@ export class ItemDetailsSectionComponent implements OnInit { (this.originalCipherView.collectionIds as CollectionId[]), ); + if (!(this.originalCipherView?.edit && this.originalCipherView?.viewPassword)) { + this.itemDetailsForm.controls.collectionIds.disable(); + } + if (this.partialEdit) { this.itemDetailsForm.disable(); this.itemDetailsForm.controls.favorite.enable(); @@ -258,27 +262,7 @@ export class ItemDetailsSectionComponent implements OnInit { collectionsControl.reset(); collectionsControl.enable(); - - const organization = this.organizations.find((org) => { - return org.id === this.originalCipherView.organizationId; - }); - - const filteredCollections = this.originalCipherView.collectionIds.find((id) => { - return this.collections.find((collection) => { - return collection.id === id && collection.manage; - }); - }); - - // Show Assign To Collections if - // The user has "Can Manage" or "Can Edit" permissions - // The Organization has "Owners and Admins Can Manage" setting on - if ( - organization?.canEditAllCiphers || - filteredCollections?.length > 0 || - (this.originalCipherView.edit && this.originalCipherView.viewPassword) - ) { - this.showCollectionsControl = true; - } + this.showCollectionsControl = true; // If there is only one collection, select it by default. if (this.collectionOptions.length === 1) { From 97efd35b339224d1f63d5afbb459b65fd1b37672 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 9 Oct 2024 14:31:18 -0400 Subject: [PATCH 05/26] fix reversed logic for canEdit and canAssign in vault-items --- .../components/vault-items/vault-items.component.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 e7950ec5e12..6fe72774ac9 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 @@ -195,10 +195,7 @@ export class VaultItemsComponent { } const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); - return ( - (organization.canEditAllCiphers && this.viewingOrgVault) || - (cipher.edit && cipher.viewPassword) - ); + return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.edit; } protected canAssignCollections(cipher: CipherView) { @@ -207,7 +204,10 @@ export class VaultItemsComponent { } const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); - return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.edit; + return ( + (organization.canEditAllCiphers && this.viewingOrgVault) || + (cipher.edit && cipher.viewPassword) + ); } private refreshItems() { From 2a93e72ef9932414cbb2838cc885edb2ecaaf521 Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 14 Oct 2024 15:30:48 -0400 Subject: [PATCH 06/26] update collections component to disable collections based on permissions in desktop client --- .../vault/app/vault/collections.component.html | 1 + .../components/collections.component.ts | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/vault/app/vault/collections.component.html b/apps/desktop/src/vault/app/vault/collections.component.html index 113ebe5ff97..1250a02b991 100644 --- a/apps/desktop/src/vault/app/vault/collections.component.html +++ b/apps/desktop/src/vault/app/vault/collections.component.html @@ -21,6 +21,7 @@

type="checkbox" [(ngModel)]="$any(c).checked" name="Collection[{{ i }}].Checked" + [disabled]="!canAssignCollections" /> diff --git a/libs/angular/src/admin-console/components/collections.component.ts b/libs/angular/src/admin-console/components/collections.component.ts index 304ff4411cb..1eebb6911f6 100644 --- a/libs/angular/src/admin-console/components/collections.component.ts +++ b/libs/angular/src/admin-console/components/collections.component.ts @@ -25,6 +25,9 @@ export class CollectionsComponent implements OnInit { collections: CollectionView[] = []; organization: Organization; + // Using to disable collections edit in desktop unless user has "Can Manage" or "Can Edit" permissions + canAssignCollections: boolean; + protected cipherDomain: Cipher; constructor( @@ -52,7 +55,7 @@ export class CollectionsComponent implements OnInit { await this.cipherService.getKeyForCipherKeyDecryption(this.cipherDomain, activeUserId), ); this.collections = await this.loadCollections(); - + this.subscribeAssignCollectionsCheck(this.cipher); this.collections.forEach((c) => ((c as any).checked = false)); if (this.collectionIds != null) { this.collections.forEach((c) => { @@ -65,6 +68,16 @@ export class CollectionsComponent implements OnInit { } } + protected subscribeAssignCollectionsCheck(cipher: CipherView): void { + if (cipher?.organizationId == null) { + this.canAssignCollections = true; + } + + this.organizationService.get$(cipher.organizationId).subscribe((org) => { + this.canAssignCollections = org.canEditAllCiphers || (cipher.edit && cipher.viewPassword); + }); + } + async submit(): Promise { const selectedCollectionIds = this.collections .filter((c) => { From 8cd5cacc6962c5b89d3963a7144c77086b28b919 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 16 Oct 2024 12:02:40 -0400 Subject: [PATCH 07/26] reduce logic for can assign for desktop --- .../src/vault/app/vault/collections.component.html | 2 +- .../components/collections.component.ts | 14 -------------- libs/common/src/vault/models/view/cipher.view.ts | 4 ++++ 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/apps/desktop/src/vault/app/vault/collections.component.html b/apps/desktop/src/vault/app/vault/collections.component.html index 1250a02b991..54f9a09e597 100644 --- a/apps/desktop/src/vault/app/vault/collections.component.html +++ b/apps/desktop/src/vault/app/vault/collections.component.html @@ -21,7 +21,7 @@

type="checkbox" [(ngModel)]="$any(c).checked" name="Collection[{{ i }}].Checked" - [disabled]="!canAssignCollections" + [disabled]="!cipher.canAssignCollections" /> diff --git a/libs/angular/src/admin-console/components/collections.component.ts b/libs/angular/src/admin-console/components/collections.component.ts index 1eebb6911f6..5ec7ad0354a 100644 --- a/libs/angular/src/admin-console/components/collections.component.ts +++ b/libs/angular/src/admin-console/components/collections.component.ts @@ -25,9 +25,6 @@ export class CollectionsComponent implements OnInit { collections: CollectionView[] = []; organization: Organization; - // Using to disable collections edit in desktop unless user has "Can Manage" or "Can Edit" permissions - canAssignCollections: boolean; - protected cipherDomain: Cipher; constructor( @@ -55,7 +52,6 @@ export class CollectionsComponent implements OnInit { await this.cipherService.getKeyForCipherKeyDecryption(this.cipherDomain, activeUserId), ); this.collections = await this.loadCollections(); - this.subscribeAssignCollectionsCheck(this.cipher); this.collections.forEach((c) => ((c as any).checked = false)); if (this.collectionIds != null) { this.collections.forEach((c) => { @@ -68,16 +64,6 @@ export class CollectionsComponent implements OnInit { } } - protected subscribeAssignCollectionsCheck(cipher: CipherView): void { - if (cipher?.organizationId == null) { - this.canAssignCollections = true; - } - - this.organizationService.get$(cipher.organizationId).subscribe((org) => { - this.canAssignCollections = org.canEditAllCiphers || (cipher.edit && cipher.viewPassword); - }); - } - async submit(): Promise { const selectedCollectionIds = this.collections .filter((c) => { diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 028b582db26..d3f1b224690 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -132,6 +132,10 @@ export class CipherView implements View, InitializerMetadata { ); } + get canAssignCollections(): boolean { + return this.edit && this.viewPassword; + } + linkedFieldValue(id: LinkedIdType) { const linkedFieldOption = this.linkedFieldOptions?.get(id); if (linkedFieldOption == null) { From b94ee53a774e334df71d9d5937886aeba8b60c0c Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 16 Oct 2024 17:10:13 -0400 Subject: [PATCH 08/26] update naming for new cipherView getter --- apps/desktop/src/vault/app/vault/collections.component.html | 2 +- .../vault/components/vault-items/vault-items.component.ts | 5 +---- libs/common/src/vault/models/view/cipher.view.ts | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/vault/app/vault/collections.component.html b/apps/desktop/src/vault/app/vault/collections.component.html index 54f9a09e597..53bccc9813a 100644 --- a/apps/desktop/src/vault/app/vault/collections.component.html +++ b/apps/desktop/src/vault/app/vault/collections.component.html @@ -21,7 +21,7 @@

type="checkbox" [(ngModel)]="$any(c).checked" name="Collection[{{ i }}].Checked" - [disabled]="!cipher.canAssignCollections" + [disabled]="!cipher.canEditWithPassword" /> 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 6fe72774ac9..af701eee6bc 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 @@ -204,10 +204,7 @@ export class VaultItemsComponent { } const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); - return ( - (organization.canEditAllCiphers && this.viewingOrgVault) || - (cipher.edit && cipher.viewPassword) - ); + return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.canEditWithPassword; } private refreshItems() { diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index d3f1b224690..20196354277 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -132,7 +132,7 @@ export class CipherView implements View, InitializerMetadata { ); } - get canAssignCollections(): boolean { + get canEditWithPassword(): boolean { return this.edit && this.viewPassword; } From 5c598a4c67ba4f793d59014c0b54ee82f83ab9a7 Mon Sep 17 00:00:00 2001 From: jng Date: Thu, 31 Oct 2024 16:14:31 -0400 Subject: [PATCH 09/26] do not show checkbox for cipher that has viewPassword false --- .../vault/components/vault-items/vault-cipher-row.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts index 2f01a434fb6..852936ab41a 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts @@ -200,6 +200,6 @@ export class VaultCipherRowComponent implements OnInit { return true; // Always show checkbox in individual vault or for non-org items } - return this.organization.canEditAllCiphers || this.cipher.edit; + return this.organization.canEditAllCiphers || (this.cipher.edit && this.cipher.viewPassword); } } From 03806ce81904db00f1f099a3421a3042b4c8e893 Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 4 Nov 2024 10:26:31 -0500 Subject: [PATCH 10/26] remove assign in browser for can edit except password --- .../item-more-options.component.html | 2 +- .../item-more-options.component.ts | 3 ++ .../assign-collections.component.ts | 28 ++++++++++++------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html index 8b70896e019..7e1ccaa8633 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html @@ -21,7 +21,7 @@ - + {{ "clone" | i18n }} diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts index f175c55c826..e91bddc92a3 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -70,6 +70,9 @@ export class ItemMoreOptionsComponent implements OnInit { return this.cipher.edit; } + get canViewPassword() { + return this.cipher.viewPassword; + } /** * Determines if the cipher can be autofilled. */ diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index fed1dde28ef..286b5fbb712 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -256,16 +256,24 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI if (cipherIds.length > 0) { const isSingleOrgCipher = cipherIds.length === 1 && this.personalItemsCount === 0; - // Update assigned collections for single org cipher or bulk update collections for multiple org ciphers - await (isSingleOrgCipher - ? this.updateAssignedCollections(this.editableItems[0]) - : this.bulkUpdateCollections(cipherIds)); - - this.toastService.showToast({ - variant: "success", - title: null, - message: this.i18nService.t("successfullyAssignedCollections"), - }); + try { + // Update assigned collections for single org cipher or bulk update collections for multiple org ciphers + await (isSingleOrgCipher + ? this.updateAssignedCollections(this.editableItems[0]) + : this.bulkUpdateCollections(cipherIds)); + + this.toastService.showToast({ + variant: "success", + title: null, + message: this.i18nService.t("successfullyAssignedCollections"), + }); + } catch (e) { + this.toastService.showToast({ + variant: "error", + title: null, + message: this.i18nService.t("noEditPermissions"), + }); + } } this.onCollectionAssign.emit(CollectionAssignmentResult.Saved); From 6589aa161247e1d89b8a954608cc8b0ee61e7f37 Mon Sep 17 00:00:00 2001 From: jng Date: Tue, 5 Nov 2024 09:08:52 -0500 Subject: [PATCH 11/26] update error message for cli edit collections --- apps/cli/src/commands/edit.command.ts | 5 +++++ apps/cli/src/models/response.ts | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index 2e97f683801..f22a63c0f99 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -129,6 +129,11 @@ export class EditCommand { const res = new CipherResponse(decCipher); return Response.success(res); } catch (e) { + // If a Resource not found error is returned here, the user did not have ViewPassword permissions + // Show No Edit Permission message + if (e.message === "Resource not found.") { + return Response.noEditPermission(); + } return Response.error(e); } } diff --git a/apps/cli/src/models/response.ts b/apps/cli/src/models/response.ts index cbeb9f17273..2b611be2614 100644 --- a/apps/cli/src/models/response.ts +++ b/apps/cli/src/models/response.ts @@ -22,6 +22,10 @@ export class Response { return Response.error("Not found."); } + static noEditPermission(): Response { + return Response.error("You do not have permission to edit this item"); + } + static badRequest(message: string): Response { return Response.error(message); } From fcb68ce6b9f4c48532d60e092033a82a8a76f1ad Mon Sep 17 00:00:00 2001 From: jng Date: Tue, 5 Nov 2024 09:34:51 -0500 Subject: [PATCH 12/26] remove excess space change --- .../src/admin-console/components/collections.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/angular/src/admin-console/components/collections.component.ts b/libs/angular/src/admin-console/components/collections.component.ts index 5ec7ad0354a..304ff4411cb 100644 --- a/libs/angular/src/admin-console/components/collections.component.ts +++ b/libs/angular/src/admin-console/components/collections.component.ts @@ -52,6 +52,7 @@ export class CollectionsComponent implements OnInit { await this.cipherService.getKeyForCipherKeyDecryption(this.cipherDomain, activeUserId), ); this.collections = await this.loadCollections(); + this.collections.forEach((c) => ((c as any).checked = false)); if (this.collectionIds != null) { this.collections.forEach((c) => { From 873e16cc8ddcee85edee8ea11349419841973e3e Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 6 Nov 2024 13:17:13 -0500 Subject: [PATCH 13/26] updates to catch permissions before API call --- apps/cli/src/commands/edit.command.ts | 8 ++---- .../assign-collections.component.ts | 28 +++++++------------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index f22a63c0f99..415594c9252 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -116,6 +116,9 @@ export class EditCommand { "Item does not belong to an organization. Consider moving it first.", ); } + if (!cipher.viewPassword) { + return Response.noEditPermission(); + } cipher.collectionIds = req; try { @@ -129,11 +132,6 @@ export class EditCommand { const res = new CipherResponse(decCipher); return Response.success(res); } catch (e) { - // If a Resource not found error is returned here, the user did not have ViewPassword permissions - // Show No Edit Permission message - if (e.message === "Resource not found.") { - return Response.noEditPermission(); - } return Response.error(e); } } diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index 286b5fbb712..fed1dde28ef 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -256,24 +256,16 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI if (cipherIds.length > 0) { const isSingleOrgCipher = cipherIds.length === 1 && this.personalItemsCount === 0; - try { - // Update assigned collections for single org cipher or bulk update collections for multiple org ciphers - await (isSingleOrgCipher - ? this.updateAssignedCollections(this.editableItems[0]) - : this.bulkUpdateCollections(cipherIds)); - - this.toastService.showToast({ - variant: "success", - title: null, - message: this.i18nService.t("successfullyAssignedCollections"), - }); - } catch (e) { - this.toastService.showToast({ - variant: "error", - title: null, - message: this.i18nService.t("noEditPermissions"), - }); - } + // Update assigned collections for single org cipher or bulk update collections for multiple org ciphers + await (isSingleOrgCipher + ? this.updateAssignedCollections(this.editableItems[0]) + : this.bulkUpdateCollections(cipherIds)); + + this.toastService.showToast({ + variant: "success", + title: null, + message: this.i18nService.t("successfullyAssignedCollections"), + }); } this.onCollectionAssign.emit(CollectionAssignmentResult.Saved); From 2b8afdb070e4c97738dda3d2be57e0b9ae9a0209 Mon Sep 17 00:00:00 2001 From: jng Date: Thu, 7 Nov 2024 17:51:42 -0500 Subject: [PATCH 14/26] updated vault items so can edit except password does not get to assign in bulk menu --- .../app/vault/components/vault-items/vault-items.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 65012319eff..05d160c92a4 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 @@ -348,7 +348,7 @@ export class VaultItemsComponent { private allCiphersHaveEditAccess(): boolean { return this.selection.selected .filter(({ cipher }) => cipher) - .every(({ cipher }) => cipher?.edit); + .every(({ cipher }) => cipher?.edit && cipher?.viewPassword); } private getUniqueOrganizationIds(): Set { From 2824e788579cbb13222a14081d6bb04efc9989d1 Mon Sep 17 00:00:00 2001 From: jng Date: Thu, 7 Nov 2024 19:14:59 -0500 Subject: [PATCH 15/26] remove can edit except pw from assign to collections modal --- .../collections/models/collection.view.ts | 11 +++++++++-- .../components/assign-collections.component.ts | 18 ++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) 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 037509634b4..3456593ff43 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -39,14 +39,21 @@ export class CollectionView implements View, ITreeNodeObject { } } - canEditItems(org: Organization): boolean { + // Applying excludeHidePassword false will remove collections with Can Edit Except Password Permissions + canEditItems(org: Organization, excludeHidePassword = true): boolean { if (org != null && org.id !== this.organizationId) { throw new Error( "Id of the organization provided does not match the org id of the collection.", ); } - return org?.canEditAllCiphers || this.manage || (this.assigned && !this.readOnly); + return ( + org?.canEditAllCiphers || + this.manage || + (this.assigned && + !this.readOnly && + (excludeHidePassword ? true : this.hidePasswords === false)) + ); } /** diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index fed1dde28ef..0e5bda2d1a5 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -287,12 +287,18 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI return; } - this.availableCollections = this.params.availableCollections.map((c) => ({ - icon: "bwi-collection", - id: c.id, - labelName: c.name, - listName: c.name, - })); + const org = await this.organizationService.get(this.selectedOrgId); + + this.availableCollections = this.params.availableCollections + .filter((collection) => { + return collection.canEditItems(org, false); + }) + .map((c) => ({ + icon: "bwi-collection", + id: c.id, + labelName: c.name, + listName: c.name, + })); // Select assigned collections for a single cipher. this.selectCollectionsAssignedToSingleCipher(); From 4be508aec17117ad9582e4fa3060b6c1fe307587 Mon Sep 17 00:00:00 2001 From: jng Date: Fri, 8 Nov 2024 12:28:01 -0500 Subject: [PATCH 16/26] remove canEdit Except PW collections from edit item dropdown --- .../item-details-section.component.spec.ts | 51 ++++++++++++------- .../item-details-section.component.ts | 8 ++- 2 files changed, 41 insertions(+), 18 deletions(-) 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 b62557a4329..b6d0770eb4a 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 @@ -18,6 +18,28 @@ import { CipherFormContainer } from "../../cipher-form-container"; import { ItemDetailsSectionComponent } from "./item-details-section.component"; +const createMockCollection = ( + id: string, + name: string, + organizationId: string, + readOnly = false, +) => { + return { + id, + name, + organizationId, + externalId: "", + readOnly, + hidePasswords: false, + manage: true, + assigned: true, + canEditItems: jest.fn().mockReturnValue(true), + canEdit: jest.fn(), + canDelete: jest.fn(), + canViewCollectionInfo: jest.fn(), + }; +}; + describe("ItemDetailsSectionComponent", () => { let component: ItemDetailsSectionComponent; let fixture: ComponentFixture; @@ -87,7 +109,7 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -116,8 +138,8 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, + createMockCollection("col2", "Collection 2", "org1") as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -336,8 +358,8 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, + createMockCollection("col2", "Collection 2", "org1") as CollectionView, ]; fixture.detectChanges(); @@ -367,9 +389,9 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, - { id: "col3", name: "Collection 3", organizationId: "org1" } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, + createMockCollection("col2", "Collection 2", "org1") as CollectionView, + createMockCollection("col3", "Collection 3", "org1") as CollectionView, ]; fixture.detectChanges(); @@ -387,7 +409,7 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, ]; fixture.detectChanges(); @@ -414,14 +436,9 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, - { - id: "col3", - name: "Collection 3", - organizationId: "org1", - readOnly: true, - } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, + createMockCollection("col2", "Collection 2", "org1") as CollectionView, + createMockCollection("col3", "Collection 3", "org1", true) as CollectionView, ]; await component.ngOnInit(); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index cb547be18cf..61499a467ac 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -268,10 +268,16 @@ export class ItemDetailsSectionComponent implements OnInit { return; } + const org = this.config.organizations[0]; + this.collectionOptions = this.collections .filter((c) => { // If partial edit mode, show all org collections because the control is disabled. - return c.organizationId === orgId && (this.partialEdit || !c.readOnly); + return ( + c.organizationId === orgId && + (this.partialEdit || !c.readOnly) && + c.canEditItems(org, false) + ); }) .map((c) => ({ id: c.id, From 9d2b12516322a53a587147d60742953f08cc5764 Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 11 Nov 2024 16:29:37 -0500 Subject: [PATCH 17/26] updated CanEditItems helper --- .../app/vault/org-vault/vault.component.html | 2 +- .../collections/models/collection.view.ts | 6 ++---- .../item-details-section.component.spec.ts | 21 ------------------- .../item-details-section.component.ts | 2 +- .../assign-collections.component.ts | 2 +- 5 files changed, 5 insertions(+), 28 deletions(-) diff --git a/apps/web/src/app/vault/org-vault/vault.component.html b/apps/web/src/app/vault/org-vault/vault.component.html index 9e9264e77cd..ff7aaf013ae 100644 --- a/apps/web/src/app/vault/org-vault/vault.component.html +++ b/apps/web/src/app/vault/org-vault/vault.component.html @@ -107,7 +107,7 @@ *ngIf=" filter.type !== 'trash' && filter.collectionId !== Unassigned && - selectedCollection?.node?.canEditItems(organization) + selectedCollection?.node?.canEditItems(organization, false) " > {{ "newItem" | i18n }} 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 3456593ff43..85624558eca 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -40,7 +40,7 @@ export class CollectionView implements View, ITreeNodeObject { } // Applying excludeHidePassword false will remove collections with Can Edit Except Password Permissions - canEditItems(org: Organization, excludeHidePassword = true): boolean { + canEditItems(org: Organization, editPassword = true): boolean { if (org != null && org.id !== this.organizationId) { throw new Error( "Id of the organization provided does not match the org id of the collection.", @@ -50,9 +50,7 @@ export class CollectionView implements View, ITreeNodeObject { return ( org?.canEditAllCiphers || this.manage || - (this.assigned && - !this.readOnly && - (excludeHidePassword ? true : this.hidePasswords === false)) + (this.assigned && !this.readOnly && (!editPassword || !this.hidePasswords)) ); } 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 9e885cf6c21..520ebae2fba 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 @@ -460,27 +460,6 @@ describe("ItemDetailsSectionComponent", () => { createMockCollection("col1", "Collection 1", "org1", true, false) as CollectionView, createMockCollection("col2", "Collection 2", "org1", true, false) as CollectionView, createMockCollection("col3", "Collection 3", "org1", false, false) as CollectionView, - // { - // id: "col1", - // name: "Collection 1", - // organizationId: "org1", - // readOnly: true, - // canEditItems: (_org) => false, - // } as CollectionView, - // { - // id: "col2", - // name: "Collection 2", - // organizationId: "org1", - // readOnly: true, - // canEditItems: (_org) => false, - // } as CollectionView, - // { - // id: "col3", - // name: "Collection 3", - // organizationId: "org1", - // readOnly: false, - // canEditItems: (_org) => false, - // } as CollectionView, ]; fixture.detectChanges(); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 92cd864d22f..692cd75ed3a 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -286,7 +286,7 @@ export class ItemDetailsSectionComponent implements OnInit { // - When viewing as an admin, all collections should be shown, even readonly. When non-admin, filter out readonly collections return ( c.organizationId === orgId && - (this.partialEdit || c.canEditItems(organization, false) || this.config.admin) + (this.partialEdit || c.canEditItems(organization) || this.config.admin) ); }) .map((c) => ({ diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index 0e5bda2d1a5..cb949c3ad5b 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -291,7 +291,7 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI this.availableCollections = this.params.availableCollections .filter((collection) => { - return collection.canEditItems(org, false); + return collection.canEditItems(org); }) .map((c) => ({ icon: "bwi-collection", From 0fa9f531dea780b34cf9035e09404c2b3987113f Mon Sep 17 00:00:00 2001 From: jng Date: Tue, 12 Nov 2024 09:51:44 -0500 Subject: [PATCH 18/26] update comment --- .../src/common/collections/models/collection.view.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 85624558eca..527c8403928 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -39,7 +39,7 @@ export class CollectionView implements View, ITreeNodeObject { } } - // Applying excludeHidePassword false will remove collections with Can Edit Except Password Permissions + // Applying editPassword=false will include items with Can Edit Except Password Permissions canEditItems(org: Organization, editPassword = true): boolean { if (org != null && org.id !== this.organizationId) { throw new Error( From bca5ad54d1cb2688eacc83eed81e82f69eb056f3 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 20 Nov 2024 13:43:25 -0500 Subject: [PATCH 19/26] updated to persist collections with can edit except password permissions --- .../components/cipher-form.component.ts | 28 +++++++++++++++++++ .../assign-collections.component.ts | 24 +++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index d1bbbef0910..2dd135b6278 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -17,7 +17,9 @@ import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder, FormGroup, ReactiveFormsModule } from "@angular/forms"; import { Subject } from "rxjs"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { CollectionId } from "@bitwarden/common/types/guid"; import { CipherType, SecureNoteType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { @@ -77,6 +79,7 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci private bitSubmit: BitSubmitDirective; private destroyRef = inject(DestroyRef); private _firstInitialized = false; + private cannotEditCollections: CollectionId[] = []; /** * The form ID to use for the form. Used to connect it to a submit button. @@ -218,6 +221,14 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } } + const org = await this.organizationService.get(this.originalCipherView.organizationId); + + this.config.collections.map((collection) => { + if (collection.organizationId === org.id && !collection.canEditItems(org)) { + this.cannotEditCollections.push(collection.id as CollectionId); + } + }); + this.loading = false; this.formReadySubject.next(); } @@ -228,6 +239,7 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci private toastService: ToastService, private i18nService: I18nService, private changeDetectorRef: ChangeDetectorRef, + private organizationService: OrganizationService, ) {} /** @@ -245,6 +257,22 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } submit = async () => { + /** + * If a cipher is part of a collection this user cannot edit. + * We will remove that collection from the dropdown and add it back before the saveCipher call + * This will persist any Cannot Edit collections for the cipher without allowing the user to change it's status + */ + const cannotEditOwnedCollection = this.originalCipherView.collectionIds.filter((id) => { + return this.cannotEditCollections.includes(id as CollectionId); + }); + + if (cannotEditOwnedCollection.length > 0) { + this.updatedCipherView.collectionIds = [ + ...this.updatedCipherView.collectionIds, + ...cannotEditOwnedCollection, + ]; + } + if (this.cipherForm.invalid) { this.cipherForm.markAllAsTouched(); diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index cb949c3ad5b..f4ac8832788 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -123,6 +123,7 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI protected readonlyItemCount: number; protected personalItemsCount: number; protected availableCollections: SelectItemView[] = []; + protected unavailableCollections: CollectionId[] = []; protected orgName: string; protected showOrgSelector: boolean = false; @@ -291,6 +292,9 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI this.availableCollections = this.params.availableCollections .filter((collection) => { + if (!collection.canEditItems(org)) { + this.unavailableCollections.push(collection.id as CollectionId); + } return collection.canEditItems(org); }) .map((c) => ({ @@ -475,7 +479,25 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI private async updateAssignedCollections(cipherView: CipherView) { const { collections } = this.formGroup.getRawValue(); - cipherView.collectionIds = collections.map((i) => i.id as CollectionId); + /** + * If a cipher is part of a collection this user cannot edit. + * We will remove that collection from the dropdown and add it back before the saveCipher call + * This will persist any Cannot Edit collections for the cipher without allowing the user to change it's status + */ + const assignedCollectionIds = this.params.ciphers[0].collectionIds; + const assignedCannotEditCollection: CollectionId[] = []; + + if (this.unavailableCollections.length > 0) { + this.unavailableCollections.map((id) => { + if (assignedCollectionIds.includes(id) && id !== this.params.activeCollection?.id) { + assignedCannotEditCollection.push(id as CollectionId); + } + }); + } + cipherView.collectionIds = [ + ...collections.map((i) => i.id as CollectionId), + ...assignedCannotEditCollection, + ]; const cipher = await this.cipherService.encrypt(cipherView, this.activeUserId); if (this.params.isSingleCipherAdmin) { await this.cipherService.saveCollectionsWithServerAdmin(cipher); From 2594eb0bdadfe85a0e838d0132bb0420e2826eda Mon Sep 17 00:00:00 2001 From: jng Date: Thu, 21 Nov 2024 13:11:06 -0500 Subject: [PATCH 20/26] add check in edit item modal for null value of cipher --- .../components/cipher-form.component.ts | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index 2dd135b6278..ff648c027f6 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -221,13 +221,15 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } } - const org = await this.organizationService.get(this.originalCipherView.organizationId); + if (this.originalCipherView?.organizationId) { + const org = await this.organizationService.get(this.originalCipherView.organizationId); - this.config.collections.map((collection) => { - if (collection.organizationId === org.id && !collection.canEditItems(org)) { - this.cannotEditCollections.push(collection.id as CollectionId); - } - }); + this.config.collections.map((collection) => { + if (collection.organizationId === org.id && !collection.canEditItems(org)) { + this.cannotEditCollections.push(collection.id as CollectionId); + } + }); + } this.loading = false; this.formReadySubject.next(); @@ -262,15 +264,17 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci * We will remove that collection from the dropdown and add it back before the saveCipher call * This will persist any Cannot Edit collections for the cipher without allowing the user to change it's status */ - const cannotEditOwnedCollection = this.originalCipherView.collectionIds.filter((id) => { - return this.cannotEditCollections.includes(id as CollectionId); - }); + if (this.originalCipherView?.collectionIds) { + const cannotEditOwnedCollection = this.originalCipherView.collectionIds.filter((id) => { + return this.cannotEditCollections.includes(id as CollectionId); + }); - if (cannotEditOwnedCollection.length > 0) { - this.updatedCipherView.collectionIds = [ - ...this.updatedCipherView.collectionIds, - ...cannotEditOwnedCollection, - ]; + if (cannotEditOwnedCollection.length > 0) { + this.updatedCipherView.collectionIds = [ + ...this.updatedCipherView.collectionIds, + ...cannotEditOwnedCollection, + ]; + } } if (this.cipherForm.invalid) { From f3ecbad9e6d54464360aed23a60649229de046fc Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 2 Dec 2024 18:19:05 -0500 Subject: [PATCH 21/26] remove unavailableCollections logic. Putting that logic inside the server side. add canEditItems logic to readOnlyCollections in item details --- .../components/cipher-form.component.ts | 30 ------------------- .../item-details-section.component.spec.ts | 6 ++-- .../item-details-section.component.ts | 4 ++- .../assign-collections.component.ts | 24 +-------------- 4 files changed, 7 insertions(+), 57 deletions(-) diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index ff648c027f6..2236c8cf6a0 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -19,7 +19,6 @@ import { Subject } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { CollectionId } from "@bitwarden/common/types/guid"; import { CipherType, SecureNoteType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { @@ -79,7 +78,6 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci private bitSubmit: BitSubmitDirective; private destroyRef = inject(DestroyRef); private _firstInitialized = false; - private cannotEditCollections: CollectionId[] = []; /** * The form ID to use for the form. Used to connect it to a submit button. @@ -221,16 +219,6 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } } - if (this.originalCipherView?.organizationId) { - const org = await this.organizationService.get(this.originalCipherView.organizationId); - - this.config.collections.map((collection) => { - if (collection.organizationId === org.id && !collection.canEditItems(org)) { - this.cannotEditCollections.push(collection.id as CollectionId); - } - }); - } - this.loading = false; this.formReadySubject.next(); } @@ -259,24 +247,6 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } submit = async () => { - /** - * If a cipher is part of a collection this user cannot edit. - * We will remove that collection from the dropdown and add it back before the saveCipher call - * This will persist any Cannot Edit collections for the cipher without allowing the user to change it's status - */ - if (this.originalCipherView?.collectionIds) { - const cannotEditOwnedCollection = this.originalCipherView.collectionIds.filter((id) => { - return this.cannotEditCollections.includes(id as CollectionId); - }); - - if (cannotEditOwnedCollection.length > 0) { - this.updatedCipherView.collectionIds = [ - ...this.updatedCipherView.collectionIds, - ...cannotEditOwnedCollection, - ]; - } - } - if (this.cipherForm.invalid) { this.cipherForm.markAllAsTouched(); 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 520ebae2fba..0d8daa653d6 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 @@ -437,8 +437,8 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - createMockCollection("col1", "Collection 1", "org1") as CollectionView, - createMockCollection("col2", "Collection 2", "org1") as CollectionView, + createMockCollection("col1", "Collection 1", "org1", true, false) as CollectionView, + createMockCollection("col2", "Collection 2", "org1", true, false) as CollectionView, createMockCollection("col3", "Collection 3", "org1", true) as CollectionView, ]; @@ -477,7 +477,7 @@ describe("ItemDetailsSectionComponent", () => { component.config.admin = true; component.config.collections = [ createMockCollection("col1", "Collection 1", "org1", true, false) as CollectionView, - createMockCollection("col2", "Collection 2", "org1", false, false) as CollectionView, + createMockCollection("col2", "Collection 2", "org1", false, true) as CollectionView, createMockCollection("col3", "Collection 3", "org1", true, false) as CollectionView, ]; component.originalCipherView = { diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 692cd75ed3a..d2ce4c9dcd8 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -242,11 +242,13 @@ export class ItemDetailsSectionComponent implements OnInit { this.itemDetailsForm.controls.favorite.enable(); this.itemDetailsForm.controls.folderId.enable(); } else if (this.config.mode === "edit") { + const orgId = this.itemDetailsForm.controls.organizationId.value as OrganizationId; + const organization = this.organizations.find((o) => o.id === orgId); this.readOnlyCollections = this.collections .filter( // When the configuration is set up for admins, they can alter read only collections (c) => - c.readOnly && + !c.canEditItems(organization) && !this.config.admin && this.originalCipherView.collectionIds.includes(c.id as CollectionId), ) diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index f4ac8832788..cb949c3ad5b 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -123,7 +123,6 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI protected readonlyItemCount: number; protected personalItemsCount: number; protected availableCollections: SelectItemView[] = []; - protected unavailableCollections: CollectionId[] = []; protected orgName: string; protected showOrgSelector: boolean = false; @@ -292,9 +291,6 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI this.availableCollections = this.params.availableCollections .filter((collection) => { - if (!collection.canEditItems(org)) { - this.unavailableCollections.push(collection.id as CollectionId); - } return collection.canEditItems(org); }) .map((c) => ({ @@ -479,25 +475,7 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI private async updateAssignedCollections(cipherView: CipherView) { const { collections } = this.formGroup.getRawValue(); - /** - * If a cipher is part of a collection this user cannot edit. - * We will remove that collection from the dropdown and add it back before the saveCipher call - * This will persist any Cannot Edit collections for the cipher without allowing the user to change it's status - */ - const assignedCollectionIds = this.params.ciphers[0].collectionIds; - const assignedCannotEditCollection: CollectionId[] = []; - - if (this.unavailableCollections.length > 0) { - this.unavailableCollections.map((id) => { - if (assignedCollectionIds.includes(id) && id !== this.params.activeCollection?.id) { - assignedCannotEditCollection.push(id as CollectionId); - } - }); - } - cipherView.collectionIds = [ - ...collections.map((i) => i.id as CollectionId), - ...assignedCannotEditCollection, - ]; + cipherView.collectionIds = collections.map((i) => i.id as CollectionId); const cipher = await this.cipherService.encrypt(cipherView, this.activeUserId); if (this.params.isSingleCipherAdmin) { await this.cipherService.saveCollectionsWithServerAdmin(cipher); From 5b875c8463997927c4f6d4df4466c3c48ed750fd Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 2 Dec 2024 18:28:50 -0500 Subject: [PATCH 22/26] update translation key for no permissions hint in item details collections --- apps/browser/src/_locales/en/messages.json | 18 +++++++++--------- apps/web/src/locales/en/messages.json | 18 +++++++++--------- .../item-details-section.component.html | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index ab1c6377e6a..7202659ff0a 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -4217,15 +4217,6 @@ "itemName": { "message": "Item name" }, - "cannotRemoveViewOnlyCollections": { - "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", - "placeholders": { - "collections": { - "content": "$1", - "example": "Work, Personal" - } - } - }, "organizationIsDeactivated": { "message": "Organization is deactivated" }, @@ -4897,5 +4888,14 @@ }, "extraWide": { "message": "Extra wide" + }, + "noPermissionToRemoveCollections": { + "message": "You do not have permission to remove these collections: $COLLECTIONS$", + "placeholders": { + "collections": { + "content": "$1", + "example": "Work, Personal" + } + } } } diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index eb98a1d7577..08b5bbfab96 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -680,15 +680,6 @@ "itemName": { "message": "Item name" }, - "cannotRemoveViewOnlyCollections": { - "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", - "placeholders": { - "collections": { - "content": "$1", - "example": "Work, Personal" - } - } - }, "ex": { "message": "ex.", "description": "Short abbreviation for 'example'." @@ -9787,5 +9778,14 @@ }, "descriptorCode": { "message": "Descriptor code" + }, + "noPermissionToRemoveCollections": { + "message": "You do not have permission to remove these collections: $COLLECTIONS$", + "placeholders": { + "collections": { + "content": "$1", + "example": "Work, Personal" + } + } } } diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index 648539932de..6cc8a5e5297 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -62,7 +62,7 @@

{{ "itemDetails" | i18n }}

[baseItems]="collectionOptions" > - {{ "cannotRemoveViewOnlyCollections" | i18n: readOnlyCollections.join(", ") }} + {{ "noPermissionToRemoveCollections" | i18n: readOnlyCollections.join(", ") }}
From 284dc8a4ece4a0e8ed34e32e91f8ce856747699d Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 9 Dec 2024 19:36:19 -0500 Subject: [PATCH 23/26] address resource not found error when user updates a item with only can edit except pw collection --- .../item-details-section.component.html | 4 ++-- .../item-details-section.component.spec.ts | 4 ++-- .../item-details-section.component.ts | 22 +++++++++++++------ 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index 6cc8a5e5297..a7ec68bbeea 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -61,8 +61,8 @@

{{ "itemDetails" | i18n }}

formControlName="collectionIds" [baseItems]="collectionOptions" > - - {{ "noPermissionToRemoveCollections" | i18n: readOnlyCollections.join(", ") }} + + {{ "noPermissionToRemoveCollections" | i18n: readOnlyCollectionsNames.join(", ") }}
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 0d8daa653d6..b76fc4d3cfc 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 @@ -491,6 +491,7 @@ describe("ItemDetailsSectionComponent", () => { }); it("should not show collections as readonly when `config.admin` is true", async () => { + component.config.isAdminConsole = true; await component.ngOnInit(); fixture.detectChanges(); @@ -502,8 +503,7 @@ describe("ItemDetailsSectionComponent", () => { await component.ngOnInit(); fixture.detectChanges(); - - expect(component["readOnlyCollections"]).toEqual(["Collection 1", "Collection 3"]); + expect(component["readOnlyCollectionsNames"]).toEqual(["Collection 1", "Collection 3"]); }); }); }); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index d2ce4c9dcd8..b8bbc53bc00 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -65,7 +65,7 @@ export class ItemDetailsSectionComponent implements OnInit { * Collections that are already assigned to the cipher and are read-only. These cannot be removed. * @protected */ - protected readOnlyCollections: string[] = []; + protected readOnlyCollections: CollectionView[] = []; protected showCollectionsControl: boolean; @@ -77,6 +77,10 @@ export class ItemDetailsSectionComponent implements OnInit { @Input() originalCipherView: CipherView; + + get readOnlyCollectionsNames(): string[] { + return this.readOnlyCollections.map((c) => c.name); + } /** * Whether the form is in partial edit mode. Only the folder and favorite controls are available. */ @@ -131,7 +135,10 @@ export class ItemDetailsSectionComponent implements OnInit { name: value.name, organizationId: value.organizationId, folderId: value.folderId, - collectionIds: value.collectionIds?.map((c) => c.id) || [], + collectionIds: [ + ...(value.collectionIds?.map((c) => c.id) || []), + ...this.readOnlyCollections.map((c) => c.id), + ], favorite: value.favorite, } as CipherView); return cipher; @@ -244,15 +251,16 @@ export class ItemDetailsSectionComponent implements OnInit { } else if (this.config.mode === "edit") { const orgId = this.itemDetailsForm.controls.organizationId.value as OrganizationId; const organization = this.organizations.find((o) => o.id === orgId); - this.readOnlyCollections = this.collections - .filter( + + if (!this.config.isAdminConsole || !this.config.admin) { + this.readOnlyCollections = this.collections.filter( // When the configuration is set up for admins, they can alter read only collections (c) => + c.organizationId === orgId && !c.canEditItems(organization) && - !this.config.admin && this.originalCipherView.collectionIds.includes(c.id as CollectionId), - ) - .map((c) => c.name); + ); + } } } From 8d711d952054d6b26f574d497a4d2aedd09a1806 Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 9 Dec 2024 19:44:55 -0500 Subject: [PATCH 24/26] remove excess import in cipher form --- libs/vault/src/cipher-form/components/cipher-form.component.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index 2236c8cf6a0..d1bbbef0910 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -17,7 +17,6 @@ import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder, FormGroup, ReactiveFormsModule } from "@angular/forms"; import { Subject } from "rxjs"; -import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherType, SecureNoteType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -229,7 +228,6 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci private toastService: ToastService, private i18nService: I18nService, private changeDetectorRef: ChangeDetectorRef, - private organizationService: OrganizationService, ) {} /** From a1136b0a00eac98802dd8fee8023f4ccbed24f7e Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 23 Dec 2024 16:13:06 -0500 Subject: [PATCH 25/26] revert changes. keeping can edit except pw collections inside collection input if user can edit that item. will otherwise have input as disabled. updated method names for clarity. revert translation key for collection input footer --- apps/browser/src/_locales/en/messages.json | 4 ++-- .../vault/app/vault/collections.component.html | 2 +- .../vault-items/vault-items.component.ts | 4 +++- .../src/app/vault/org-vault/vault.component.html | 2 +- apps/web/src/locales/en/messages.json | 4 ++-- .../common/collections/models/collection.view.ts | 11 ++--------- libs/common/src/vault/models/view/cipher.view.ts | 2 +- .../item-details-section.component.html | 2 +- .../item-details-section.component.ts | 16 ++++++++++------ 9 files changed, 23 insertions(+), 24 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index d326f83d51b..596e3523033 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -4836,8 +4836,8 @@ "extraWide": { "message": "Extra wide" }, - "noPermissionToRemoveCollections": { - "message": "You do not have permission to remove these collections: $COLLECTIONS$", + "cannotRemoveViewOnlyCollections": { + "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", "placeholders": { "collections": { "content": "$1", diff --git a/apps/desktop/src/vault/app/vault/collections.component.html b/apps/desktop/src/vault/app/vault/collections.component.html index 53bccc9813a..13bd3178899 100644 --- a/apps/desktop/src/vault/app/vault/collections.component.html +++ b/apps/desktop/src/vault/app/vault/collections.component.html @@ -21,7 +21,7 @@

type="checkbox" [(ngModel)]="$any(c).checked" name="Collection[{{ i }}].Checked" - [disabled]="!cipher.canEditWithPassword" + [disabled]="!cipher.canAssignToCollections" /> 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 6e37645e3ba..acc7a74061b 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 @@ -241,7 +241,9 @@ export class VaultItemsComponent { return true; } const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); - return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.canEditWithPassword; + return ( + (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.canAssignToCollections + ); } protected canManageCollection(cipher: CipherView) { diff --git a/apps/web/src/app/vault/org-vault/vault.component.html b/apps/web/src/app/vault/org-vault/vault.component.html index e49db0140d1..52f3ea026ff 100644 --- a/apps/web/src/app/vault/org-vault/vault.component.html +++ b/apps/web/src/app/vault/org-vault/vault.component.html @@ -107,7 +107,7 @@ *ngIf=" filter.type !== 'trash' && filter.collectionId !== Unassigned && - selectedCollection?.node?.canEditItems(organization, false) + selectedCollection?.node?.canEditItems(organization) " > {{ "newItem" | i18n }} diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 01862c81756..06fd88e194b 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -9879,8 +9879,8 @@ "descriptorCode": { "message": "Descriptor code" }, - "noPermissionToRemoveCollections": { - "message": "You do not have permission to remove these collections: $COLLECTIONS$", + "cannotRemoveViewOnlyCollections": { + "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", "placeholders": { "collections": { "content": "$1", 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 ceb2d90f648..1ce76608df1 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -41,21 +41,14 @@ export class CollectionView implements View, ITreeNodeObject { } } - // Setting editPassword=false will determine what types of permissions are returned - // if editPassword is true only return permissions manage / edit - // if editPassword is false return permissions manage / edit / edit except pw - canEditItems(org: Organization, editPassword = true): boolean { + canEditItems(org: Organization): boolean { if (org != null && org.id !== this.organizationId) { throw new Error( "Id of the organization provided does not match the org id of the collection.", ); } - return ( - org?.canEditAllCiphers || - this.manage || - (this.assigned && !this.readOnly && (!editPassword || !this.hidePasswords)) - ); + return org?.canEditAllCiphers || this.manage || (this.assigned && !this.readOnly); } /** diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index a5f38f21bdb..85a24fdb4a3 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -137,7 +137,7 @@ export class CipherView implements View, InitializerMetadata { ); } - get canEditWithPassword(): boolean { + get canAssignToCollections(): boolean { return this.edit && this.viewPassword; } /** diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index a7ec68bbeea..9750983bba1 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -62,7 +62,7 @@

{{ "itemDetails" | i18n }}

[baseItems]="collectionOptions" > - {{ "noPermissionToRemoveCollections" | i18n: readOnlyCollectionsNames.join(", ") }} + {{ "cannotRemoveViewOnlyCollections" | i18n: readOnlyCollectionsNames.join(", ") }} diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index e7745ade08f..5f317c58110 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -226,6 +226,9 @@ export class ItemDetailsSectionComponent implements OnInit { favorite: this.originalCipherView.favorite, }); + const orgId = this.itemDetailsForm.controls.organizationId.value as OrganizationId; + const organization = this.organizations.find((o) => o.id === orgId); + // Configure form for clone mode. if (this.config.mode === "clone") { this.itemDetailsForm.controls.name.setValue( @@ -242,7 +245,11 @@ export class ItemDetailsSectionComponent implements OnInit { (this.originalCipherView.collectionIds as CollectionId[]), ); - if (!(this.originalCipherView?.edit && this.originalCipherView?.viewPassword)) { + if ( + organization != null && + !organization.canEditAllCiphers && + !(this.originalCipherView?.edit && this.originalCipherView?.viewPassword) + ) { this.itemDetailsForm.controls.collectionIds.disable(); } @@ -251,15 +258,12 @@ export class ItemDetailsSectionComponent implements OnInit { this.itemDetailsForm.controls.favorite.enable(); this.itemDetailsForm.controls.folderId.enable(); } else if (this.config.mode === "edit") { - const orgId = this.itemDetailsForm.controls.organizationId.value as OrganizationId; - const organization = this.organizations.find((o) => o.id === orgId); - if (!this.config.isAdminConsole || !this.config.admin) { this.readOnlyCollections = this.collections.filter( // When the configuration is set up for admins, they can alter read only collections (c) => c.organizationId === orgId && - !c.canEditItems(organization) && + c.readOnly && this.originalCipherView.collectionIds.includes(c.id as CollectionId), ); } @@ -305,7 +309,7 @@ export class ItemDetailsSectionComponent implements OnInit { } // Non-admins can only select assigned collections that are not read only. (Non-AC) - return c.assigned && !c.readOnly && !c.hidePasswords; + return c.assigned && !c.readOnly; }) .map((c) => ({ id: c.id, From 736f756aec2bea6b482c46e5962621bed11100d4 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 15 Jan 2025 13:15:30 -0500 Subject: [PATCH 26/26] move org null check to cipher view --- .../vault/components/vault-items/vault-items.component.ts | 3 --- libs/common/src/vault/models/view/cipher.view.ts | 4 ++++ .../item-details/item-details-section.component.ts | 6 +----- 3 files changed, 5 insertions(+), 8 deletions(-) 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 acc7a74061b..04970a34b0f 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 @@ -237,9 +237,6 @@ export class VaultItemsComponent { } protected canAssignCollections(cipher: CipherView) { - if (cipher.organizationId == null) { - return true; - } const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); return ( (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.canAssignToCollections diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index de3b82b960e..650a1e9dc45 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -143,6 +143,10 @@ export class CipherView implements View, InitializerMetadata { } get canAssignToCollections(): boolean { + if (this.organizationId == null) { + return true; + } + return this.edit && this.viewPassword; } /** diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 5f317c58110..2a239322b97 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -245,11 +245,7 @@ export class ItemDetailsSectionComponent implements OnInit { (this.originalCipherView.collectionIds as CollectionId[]), ); - if ( - organization != null && - !organization.canEditAllCiphers && - !(this.originalCipherView?.edit && this.originalCipherView?.viewPassword) - ) { + if (!organization?.canEditAllCiphers && !this.originalCipherView.canAssignToCollections) { this.itemDetailsForm.controls.collectionIds.disable(); }