diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 51e1203673b..0c894e3b36e 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -4160,15 +4160,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" }, @@ -4900,5 +4891,14 @@ }, "extraWide": { "message": "Extra wide" + }, + "cannotRemoveViewOnlyCollections": { + "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", + "placeholders": { + "collections": { + "content": "$1", + "example": "Work, Personal" + } + } } } 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 4c7067df53a..6e6e30b359b 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 @@ -27,7 +27,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 5d3dee9018e..4057ddc7838 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 @@ -95,6 +95,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/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index 8efb414f5b2..0fb089216b5 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -122,6 +122,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 { diff --git a/apps/cli/src/models/response.ts b/apps/cli/src/models/response.ts index 76d9509226d..ac0977182f4 100644 --- a/apps/cli/src/models/response.ts +++ b/apps/cli/src/models/response.ts @@ -39,6 +39,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); } diff --git a/apps/desktop/src/vault/app/vault/collections.component.html b/apps/desktop/src/vault/app/vault/collections.component.html index 113ebe5ff97..13bd3178899 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]="!cipher.canAssignToCollections" /> 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 556d3d54594..beca0d32c72 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 @@ -42,6 +42,7 @@ export class VaultCipherRowComponent implements OnInit { @Input() collections: CollectionView[]; @Input() viewingOrgVault: boolean; @Input() canEditCipher: boolean; + @Input() canAssignCollections: boolean; @Input() canManageCollection: boolean; @Output() onEvent = new EventEmitter(); @@ -101,7 +102,7 @@ export class VaultCipherRowComponent implements OnInit { } protected get showAssignToCollections() { - return this.organizations?.length && this.canEditCipher && !this.cipher.isDeleted; + return this.organizations?.length && this.canAssignCollections && !this.cipher.isDeleted; } protected get showClone() { @@ -208,6 +209,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); } } 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 653d05ef129..a32def5fc0c 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 @@ -144,6 +144,7 @@ [collections]="allCollections" [checked]="selection.isSelected(item)" [canEditCipher]="canEditCipher(item.cipher)" + [canAssignCollections]="canAssignCollections(item.cipher)" [canManageCollection]="canManageCollection(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 3e1cf173a47..a641c5b5908 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 @@ -236,6 +236,13 @@ export class VaultItemsComponent { return (organization.canEditAllCiphers && this.viewingOrgVault) || cipher.edit; } + protected canAssignCollections(cipher: CipherView) { + const organization = this.allOrganizations.find((o) => o.id === cipher.organizationId); + return ( + (organization?.canEditAllCiphers && this.viewingOrgVault) || cipher.canAssignToCollections + ); + } + protected canManageCollection(cipher: CipherView) { // If the cipher is not part of an organization (personal item), user can manage it if (cipher.organizationId == null) { @@ -461,7 +468,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 { diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 15c5a7fcf6c..4d924da2abc 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -743,15 +743,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'." @@ -10068,6 +10059,15 @@ "descriptorCode": { "message": "Descriptor code" }, + "cannotRemoveViewOnlyCollections": { + "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", + "placeholders": { + "collections": { + "content": "$1", + "example": "Work, Personal" + } + } + }, "importantNotice": { "message": "Important notice" }, diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 20dbd23065c..650a1e9dc45 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -142,6 +142,13 @@ export class CipherView implements View, InitializerMetadata { ); } + get canAssignToCollections(): boolean { + if (this.organizationId == null) { + return true; + } + + return this.edit && this.viewPassword; + } /** * Determines if the cipher can be launched in a new browser tab. */ 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..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 @@ -61,8 +61,8 @@

{{ "itemDetails" | i18n }}

formControlName="collectionIds" [baseItems]="collectionOptions" > - - {{ "cannotRemoveViewOnlyCollections" | i18n: readOnlyCollections.join(", ") }} + + {{ "cannotRemoveViewOnlyCollections" | 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 32c1e7417e4..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 @@ -18,6 +18,29 @@ import { CipherFormContainer } from "../../cipher-form-container"; import { ItemDetailsSectionComponent } from "./item-details-section.component"; +const createMockCollection = ( + id: string, + name: string, + organizationId: string, + readOnly = false, + canEdit = true, +) => { + return { + id, + name, + organizationId, + externalId: "", + readOnly, + hidePasswords: false, + manage: true, + assigned: true, + canEditItems: jest.fn().mockReturnValue(canEdit), + canEdit: jest.fn(), + canDelete: jest.fn(), + canViewCollectionInfo: jest.fn(), + }; +}; + describe("ItemDetailsSectionComponent", () => { let component: ItemDetailsSectionComponent; let fixture: ComponentFixture; @@ -87,13 +110,7 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -122,20 +139,8 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - assigned: true, - readOnly: true, - } as CollectionView, - { - id: "col2", - name: "Collection 2", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, + createMockCollection("col2", "Collection 2", "org1") as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -354,8 +359,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(); @@ -385,27 +390,9 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, - { - id: "col2", - name: "Collection 2", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, - { - id: "col3", - name: "Collection 3", - organizationId: "org1", - assigned: true, - readOnly: false, - } 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(); @@ -423,13 +410,7 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, + createMockCollection("col1", "Collection 1", "org1") as CollectionView, ]; fixture.detectChanges(); @@ -456,27 +437,9 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, - { - id: "col2", - name: "Collection 2", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, - { - id: "col3", - name: "Collection 3", - organizationId: "org1", - readOnly: true, - assigned: true, - } 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, ]; await component.ngOnInit(); @@ -494,27 +457,9 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - readOnly: true, - assigned: false, - } as CollectionView, - { - id: "col2", - name: "Collection 2", - organizationId: "org1", - readOnly: true, - assigned: false, - } as CollectionView, - { - id: "col3", - name: "Collection 3", - organizationId: "org1", - readOnly: false, - assigned: true, - } 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", false, false) as CollectionView, ]; fixture.detectChanges(); @@ -531,26 +476,9 @@ describe("ItemDetailsSectionComponent", () => { component.config.mode = "edit"; component.config.admin = true; component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - readOnly: true, - assigned: false, - } as CollectionView, - { - id: "col2", - name: "Collection 2", - organizationId: "org1", - assigned: false, - } as CollectionView, - { - id: "col3", - name: "Collection 3", - organizationId: "org1", - readOnly: true, - assigned: false, - } as CollectionView, + createMockCollection("col1", "Collection 1", "org1", true, false) as CollectionView, + createMockCollection("col2", "Collection 2", "org1", false, true) as CollectionView, + createMockCollection("col3", "Collection 3", "org1", true, false) as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -563,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(); @@ -574,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 f7fd228232e..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 @@ -67,7 +67,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; @@ -79,6 +79,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. */ @@ -133,7 +137,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; @@ -219,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( @@ -235,20 +245,24 @@ export class ItemDetailsSectionComponent implements OnInit { (this.originalCipherView.collectionIds as CollectionId[]), ); + if (!organization?.canEditAllCiphers && !this.originalCipherView.canAssignToCollections) { + this.itemDetailsForm.controls.collectionIds.disable(); + } + if (this.partialEdit) { this.itemDetailsForm.disable(); this.itemDetailsForm.controls.favorite.enable(); this.itemDetailsForm.controls.folderId.enable(); } else if (this.config.mode === "edit") { - 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.readOnly && - !this.config.admin && this.originalCipherView.collectionIds.includes(c.id as CollectionId), - ) - .map((c) => c.name); + ); + } } } diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index 00852ff101c..9d1c5186d97 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -289,12 +289,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); + }) + .map((c) => ({ + icon: "bwi-collection", + id: c.id, + labelName: c.name, + listName: c.name, + })); // Select assigned collections for a single cipher. this.selectCollectionsAssignedToSingleCipher();