diff --git a/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts b/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts index 7052be5ea6..a11a7d806b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts @@ -74,7 +74,7 @@ export class AssignCollections { combineLatest([cipher$, this.collectionService.decryptedCollections$]) .pipe(takeUntilDestroyed(), first()) .subscribe(([cipherView, collections]) => { - let availableCollections = collections.filter((c) => !c.readOnly); + let availableCollections = collections; const organizationId = (cipherView?.organizationId as OrganizationId) ?? null; // If the cipher is already a part of an organization, diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 35433e8e2e..50b9ed4336 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -3703,6 +3703,15 @@ "changeAtRiskPassword": { "message": "Change at-risk password" }, + "cannotRemoveViewOnlyCollections": { + "message": "You cannot remove collections with View only permissions: $COLLECTIONS$", + "placeholders": { + "collections": { + "content": "$1", + "example": "Work, Personal" + } + } + }, "move": { "message": "Move" }, diff --git a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts index 96c00faceb..a3b62838d6 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts @@ -362,8 +362,7 @@ export class VaultComponent implements OnInit, OnDestroy { if (this.organization.canEditAllCiphers) { return collections; } - // The user is only allowed to add/edit items to assigned collections that are not readonly - return collections.filter((c) => c.assigned && !c.readOnly); + return collections.filter((c) => c.assigned); }), shareReplay({ refCount: true, bufferSize: 1 }), ); diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index 55bbd0c065..6e751f600d 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -933,7 +933,7 @@ export class VaultComponent implements OnInit, OnDestroy { if (orgId && orgId !== "MyVault") { const organization = this.allOrganizations.find((o) => o.id === orgId); availableCollections = this.allCollections.filter( - (c) => c.organizationId === organization.id && !c.readOnly, + (c) => c.organizationId === organization.id, ); } diff --git a/libs/vault/jest.config.js b/libs/vault/jest.config.js index e33c115e8d..16db37527a 100644 --- a/libs/vault/jest.config.js +++ b/libs/vault/jest.config.js @@ -10,7 +10,11 @@ module.exports = { displayName: "libs/vault tests", preset: "jest-preset-angular", setupFilesAfterEnv: ["/test.setup.ts"], - moduleNameMapper: pathsToModuleNameMapper(compilerOptions?.paths || {}, { - prefix: "/", - }), + moduleNameMapper: pathsToModuleNameMapper( + // lets us use @bitwarden/common/spec in tests + { "@bitwarden/common/spec": ["../common/spec"], ...(compilerOptions?.paths ?? {}) }, + { + prefix: "/", + }, + ), }; diff --git a/libs/vault/src/components/assign-collections.component.html b/libs/vault/src/components/assign-collections.component.html index d68799eec6..a82f2cb29a 100644 --- a/libs/vault/src/components/assign-collections.component.html +++ b/libs/vault/src/components/assign-collections.component.html @@ -37,13 +37,16 @@
- + {{ "selectCollectionsToAssign" | i18n }} + + {{ "cannotRemoveViewOnlyCollections" | i18n: readOnlyCollectionNames.join(", ") }} +
diff --git a/libs/vault/src/components/assign-collections.component.spec.ts b/libs/vault/src/components/assign-collections.component.spec.ts new file mode 100644 index 0000000000..d6707cd106 --- /dev/null +++ b/libs/vault/src/components/assign-collections.component.spec.ts @@ -0,0 +1,113 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ProductTierType } from "@bitwarden/common/billing/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { ToastService } from "@bitwarden/components"; + +import { + AssignCollectionsComponent, + CollectionAssignmentParams, +} from "./assign-collections.component"; + +describe("AssignCollectionsComponent", () => { + let component: AssignCollectionsComponent; + let fixture: ComponentFixture; + + const mockUserId = "mock-user-id" as UserId; + const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); + + const editCollection = new CollectionView(); + editCollection.id = "collection-id" as CollectionId; + editCollection.organizationId = "org-id" as OrganizationId; + editCollection.name = "Editable Collection"; + editCollection.readOnly = false; + editCollection.manage = true; + + const readOnlyCollection1 = new CollectionView(); + readOnlyCollection1.id = "read-only-collection-id" as CollectionId; + readOnlyCollection1.organizationId = "org-id" as OrganizationId; + readOnlyCollection1.name = "Read Only Collection"; + readOnlyCollection1.readOnly = true; + + const readOnlyCollection2 = new CollectionView(); + readOnlyCollection2.id = "read-only-collection-id-2" as CollectionId; + readOnlyCollection2.organizationId = "org-id" as OrganizationId; + readOnlyCollection2.name = "Read Only Collection 2"; + readOnlyCollection2.readOnly = true; + + const params = { + organizationId: "org-id" as OrganizationId, + ciphers: [ + { + id: "cipher-id", + name: "Cipher Name", + collectionIds: [readOnlyCollection1.id], + edit: true, + } as unknown as CipherView, + ], + availableCollections: [editCollection, readOnlyCollection1, readOnlyCollection2], + } as CollectionAssignmentParams; + + const org = { + id: "org-id", + name: "Test Org", + productTierType: ProductTierType.Enterprise, + } as Organization; + + const organizations$ = jest.fn().mockReturnValue(of([org])); + + beforeEach(async () => { + await TestBed.configureTestingModule({ + providers: [ + { provide: CipherService, useValue: mock() }, + { provide: OrganizationService, useValue: mock({ organizations$ }) }, + { provide: CollectionService, useValue: mock() }, + { provide: ToastService, useValue: mock() }, + { provide: AccountService, useValue: accountService }, + { provide: I18nService, useValue: { t: (...keys: string[]) => keys.join(" ") } }, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(AssignCollectionsComponent); + component = fixture.componentInstance; + component.params = params; + fixture.detectChanges(); + }); + + describe("read only collections", () => { + beforeEach(async () => { + await component.ngOnInit(); + fixture.detectChanges(); + }); + + it("shows read-only hint for assigned collections", () => { + const hint = fixture.debugElement.query(By.css('[data-testid="view-only-hint"]')); + + expect(hint.nativeElement.textContent.trim()).toBe( + "cannotRemoveViewOnlyCollections Read Only Collection", + ); + }); + + it("does not show read only collections in the list", () => { + expect(component["availableCollections"]).toEqual([ + { + icon: "bwi-collection-shared", + id: editCollection.id, + labelName: editCollection.name, + listName: editCollection.name, + }, + ]); + }); + }); +}); diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index 6a0c45cfbe..db62f096fa 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -126,6 +126,12 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI collections: [[], [Validators.required]], }); + /** + * Collections that are already assigned to the cipher and are read-only. These cannot be removed. + * @protected + */ + protected readOnlyCollectionNames: string[] = []; + protected totalItemCount: number; protected editableItemCount: number; protected readonlyItemCount: number; @@ -301,6 +307,8 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI this.organizationService.organizations$(userId).pipe(getOrganizationById(organizationId)), ); + await this.setReadOnlyCollectionNames(); + this.availableCollections = this.params.availableCollections .filter((collection) => { return collection.canEditItems(org); @@ -503,4 +511,25 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI await this.cipherService.saveCollectionsWithServer(cipher, userId); } } + + /** + * Only display collections that are read-only and are assigned to the ciphers. + */ + private async setReadOnlyCollectionNames() { + const { availableCollections, ciphers } = this.params; + + const organization = await firstValueFrom( + this.organizations$.pipe(map((orgs) => orgs.find((o) => o.id === this.selectedOrgId))), + ); + + this.readOnlyCollectionNames = availableCollections + .filter((c) => { + return ( + c.readOnly && + ciphers.some((cipher) => cipher.collectionIds.includes(c.id)) && + !c.canEditItems(organization) + ); + }) + .map((c) => c.name); + } }