From b42741f313864496cfe30a55231cd63168c4da03 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Thu, 7 Nov 2024 10:22:35 -0600 Subject: [PATCH] [PM-13839][PM-13840] Admin Console Collections (#11649) * allow admin console to see all collections when viewing a cipher - When "manage all" option is selected all collections should be editable * update cipher form service to use admin endpoints * when saving a cipher, choose to move to collections first before saving any other edits - This handles the case where a cipher is moving from unassigned to assigned and needs to have a collection to save any other edits * set admin flag when the original cipher has zero collections - handling the case where the user un-assigns themselves from a cipher * add check for the users ability to edit items within the collection * save cipher edit first to handle when the user unassigns themselves from the cipher * update filter order of collections * use cipher returned from the collections endpoint rather than re-fetching it * fix unit tests by adding canEditItems * re-enable collection control when orgId is present * fetch the updated cipher from the respective service for editing a cipher --- .../vault-item-dialog.component.ts | 19 ++- ...console-cipher-form-config.service.spec.ts | 64 ++++---- ...dmin-console-cipher-form-config.service.ts | 39 +---- libs/common/src/services/api.service.ts | 2 +- .../src/vault/abstractions/cipher.service.ts | 2 +- .../src/vault/services/cipher.service.ts | 6 +- .../item-details-section.component.spec.ts | 153 ++++++++++++++++-- .../item-details-section.component.ts | 22 ++- .../services/default-cipher-form.service.ts | 16 +- .../src/cipher-view/cipher-view.component.ts | 1 + 10 files changed, 240 insertions(+), 84 deletions(-) diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts index ae2cf88fd1..bf623e729a 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts @@ -6,6 +6,7 @@ import { firstValueFrom, Observable, Subject } from "rxjs"; import { map } from "rxjs/operators"; import { CollectionView } from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; @@ -17,6 +18,8 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions/view-password-history.service"; import { CipherType } from "@bitwarden/common/vault/enums"; +import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data"; +import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; import { @@ -231,6 +234,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { private billingAccountProfileStateService: BillingAccountProfileStateService, private premiumUpgradeService: PremiumUpgradePromptService, private cipherAuthorizationService: CipherAuthorizationService, + private apiService: ApiService, ) { this.updateTitle(); } @@ -278,7 +282,20 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { if (this._originalFormMode === "add" || this._originalFormMode === "clone") { this.formConfig.mode = "edit"; } - this.formConfig.originalCipher = await this.cipherService.get(cipherView.id); + + let cipher: Cipher; + + // When the form config is used within the Admin Console, retrieve the cipher from the admin endpoint + if (this.formConfig.isAdminConsole) { + const cipherResponse = await this.apiService.getCipherAdmin(cipherView.id); + const cipherData = new CipherData(cipherResponse); + cipher = new Cipher(cipherData); + } else { + cipher = await this.cipherService.get(cipherView.id); + } + + // Store the updated cipher so any following edits use the most up to date cipher + this.formConfig.originalCipher = cipher; this._cipherModified = true; await this.changeMode("view"); } diff --git a/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts index 02d280f5ff..05c40fe2e7 100644 --- a/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts +++ b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts @@ -1,14 +1,13 @@ import { TestBed } from "@angular/core/testing"; import { BehaviorSubject } from "rxjs"; -import { CollectionAdminService } from "@bitwarden/admin-console/common"; +import { CollectionAdminService, CollectionAdminView } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { CipherId } from "@bitwarden/common/types/guid"; -import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/services/routed-vault-filter.service"; @@ -35,27 +34,41 @@ describe("AdminConsoleCipherFormConfigService", () => { status: OrganizationUserStatusType.Confirmed, }; const policyAppliesToActiveUser$ = new BehaviorSubject(true); + const collection = { + id: "12345-5555", + organizationId: "234534-34334", + name: "Test Collection 1", + assigned: false, + readOnly: true, + } as CollectionAdminView; + const collection2 = { + id: "12345-6666", + organizationId: "22222-2222", + name: "Test Collection 2", + assigned: true, + readOnly: false, + } as CollectionAdminView; + const organization$ = new BehaviorSubject(testOrg as Organization); const organizations$ = new BehaviorSubject([testOrg, testOrg2] as Organization[]); const getCipherAdmin = jest.fn().mockResolvedValue(null); - const getCipher = jest.fn().mockResolvedValue(null); beforeEach(async () => { getCipherAdmin.mockClear(); - getCipher.mockClear(); - getCipher.mockResolvedValue({ id: cipherId, name: "Test Cipher - (non-admin)" }); getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" }); await TestBed.configureTestingModule({ providers: [ AdminConsoleCipherFormConfigService, + { provide: OrganizationService, useValue: { get$: () => organization$, organizations$ } }, + { + provide: CollectionAdminService, + useValue: { getAll: () => Promise.resolve([collection, collection2]) }, + }, { provide: PolicyService, useValue: { policyAppliesToActiveUser$: () => policyAppliesToActiveUser$ }, }, - { provide: OrganizationService, useValue: { get$: () => organization$, organizations$ } }, - { provide: CipherService, useValue: { get: getCipher } }, - { provide: CollectionAdminService, useValue: { getAll: () => Promise.resolve([]) } }, { provide: RoutedVaultFilterService, useValue: { filter$: new BehaviorSubject({ organizationId: testOrg.id }) }, @@ -86,6 +99,12 @@ describe("AdminConsoleCipherFormConfigService", () => { expect(mode).toBe("edit"); }); + it("returns all collections", async () => { + const { collections } = await adminConsoleConfigService.buildConfig("edit", cipherId); + + expect(collections).toEqual([collection, collection2]); + }); + it("sets admin flag based on `canEditAllCiphers`", async () => { // Disable edit all ciphers on org testOrg.canEditAllCiphers = false; @@ -153,33 +172,14 @@ describe("AdminConsoleCipherFormConfigService", () => { expect(result.organizations).toEqual([testOrg, testOrg2]); }); - describe("getCipher", () => { - it("retrieves the cipher from the cipher service", async () => { - testOrg.canEditAllCiphers = false; + it("retrieves the cipher from the admin service", async () => { + getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" }); - adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); - const result = await adminConsoleConfigService.buildConfig("clone", cipherId); + await adminConsoleConfigService.buildConfig("add", cipherId); - expect(getCipher).toHaveBeenCalledWith(cipherId); - expect(result.originalCipher.name).toBe("Test Cipher - (non-admin)"); - - // Admin service not needed when cipher service can return the cipher - expect(getCipherAdmin).not.toHaveBeenCalled(); - }); - - it("retrieves the cipher from the admin service", async () => { - getCipher.mockResolvedValueOnce(null); - getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" }); - - adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); - - await adminConsoleConfigService.buildConfig("add", cipherId); - - expect(getCipherAdmin).toHaveBeenCalledWith(cipherId); - - expect(getCipher).toHaveBeenCalledWith(cipherId); - }); + expect(getCipherAdmin).toHaveBeenCalledWith(cipherId); }); }); }); diff --git a/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts index 328ab4475d..457b4e83d0 100644 --- a/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts +++ b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts @@ -6,9 +6,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType, OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { CipherId } from "@bitwarden/common/types/guid"; -import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -25,7 +23,6 @@ import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/se export class AdminConsoleCipherFormConfigService implements CipherFormConfigService { private policyService: PolicyService = inject(PolicyService); private organizationService: OrganizationService = inject(OrganizationService); - private cipherService: CipherService = inject(CipherService); private routedVaultFilterService: RoutedVaultFilterService = inject(RoutedVaultFilterService); private collectionAdminService: CollectionAdminService = inject(CollectionAdminService); private apiService: ApiService = inject(ApiService); @@ -51,20 +48,8 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ map(([orgs, orgId]) => orgs.find((o) => o.id === orgId)), ); - private editableCollections$ = this.organization$.pipe( - switchMap(async (org) => { - if (!org) { - return []; - } - - const collections = await this.collectionAdminService.getAll(org.id); - // Users that can edit all ciphers can implicitly add to / edit within any collection - if (org.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); - }), + private allCollections$ = this.organization$.pipe( + switchMap(async (org) => await this.collectionAdminService.getAll(org.id)), ); async buildConfig( @@ -72,21 +57,17 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ cipherId?: CipherId, cipherType?: CipherType, ): Promise { + const cipher = await this.getCipher(cipherId); const [organization, allowPersonalOwnership, allOrganizations, allCollections] = await firstValueFrom( combineLatest([ this.organization$, this.allowPersonalOwnership$, this.allOrganizations$, - this.editableCollections$, + this.allCollections$, ]), ); - const cipher = await this.getCipher(organization, cipherId); - - const collections = allCollections.filter( - (c) => c.organizationId === organization.id && c.assigned && !c.readOnly, - ); // When cloning from within the Admin Console, all organizations should be available. // Otherwise only the one in context should be const organizations = mode === "clone" ? allOrganizations : [organization]; @@ -100,7 +81,7 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ admin: organization.canEditAllCiphers ?? false, allowPersonalOwnership: allowPersonalOwnershipOnlyForClone, originalCipher: cipher, - collections, + collections: allCollections, organizations, folders: [], // folders not applicable in the admin console hideIndividualVaultFields: true, @@ -108,19 +89,11 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ }; } - private async getCipher(organization: Organization, id?: CipherId): Promise { + private async getCipher(id?: CipherId): Promise { if (id == null) { return Promise.resolve(null); } - // Check to see if the user has direct access to the cipher - const cipherFromCipherService = await this.cipherService.get(id); - - // If the organization doesn't allow admin/owners to edit all ciphers return the cipher - if (!organization.canEditAllCiphers && cipherFromCipherService != null) { - return cipherFromCipherService; - } - // Retrieve the cipher through the means of an admin const cipherResponse = await this.apiService.getCipherAdmin(id); cipherResponse.edit = true; diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index 2d4a052263..f9e05e7635 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -584,7 +584,7 @@ export class ApiService implements ApiServiceAbstraction { } putCipherCollectionsAdmin(id: string, request: CipherCollectionsRequest): Promise { - return this.send("PUT", "/ciphers/" + id + "/collections-admin", request, true, false); + return this.send("PUT", "/ciphers/" + id + "/collections-admin", request, true, true); } postPurgeCiphers( diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 444c922fe3..5221f4cf0a 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -119,7 +119,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider Promise; + saveCollectionsWithServerAdmin: (cipher: Cipher) => Promise; /** * Bulk update collections for many ciphers with the server * @param orgId diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 154042601e..6b618e2550 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -880,9 +880,11 @@ export class CipherService implements CipherServiceAbstraction { return new Cipher(updated[cipher.id as CipherId], cipher.localData); } - async saveCollectionsWithServerAdmin(cipher: Cipher): Promise { + async saveCollectionsWithServerAdmin(cipher: Cipher): Promise { const request = new CipherCollectionsRequest(cipher.collectionIds); - await this.apiService.putCipherCollectionsAdmin(cipher.id, request); + const response = await this.apiService.putCipherCollectionsAdmin(cipher.id, request); + const data = new CipherData(response); + return new Cipher(data); } /** 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 b62557a432..93229bda6c 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 @@ -87,7 +87,12 @@ 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: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -116,8 +121,18 @@ 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, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -367,9 +382,24 @@ 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, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, + { + id: "col3", + name: "Collection 3", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; fixture.detectChanges(); @@ -387,7 +417,12 @@ 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: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; fixture.detectChanges(); @@ -414,13 +449,24 @@ 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: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, { id: "col3", name: "Collection 3", organizationId: "org1", readOnly: true, + canEditItems: (_org) => true, } as CollectionView, ]; @@ -433,5 +479,94 @@ describe("ItemDetailsSectionComponent", () => { expect(collectionHint).not.toBeNull(); }); + + it("should allow all collections to be altered when `config.admin` is true", async () => { + component.config.admin = true; + component.config.allowPersonalOwnership = true; + component.config.organizations = [{ id: "org1" } as Organization]; + component.config.collections = [ + { + 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(); + await fixture.whenStable(); + + component.itemDetailsForm.controls.organizationId.setValue("org1"); + + expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]); + }); + }); + + describe("readonlyCollections", () => { + beforeEach(() => { + component.config.mode = "edit"; + component.config.admin = true; + component.config.collections = [ + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + readOnly: true, + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col3", + name: "Collection 3", + organizationId: "org1", + readOnly: true, + canEditItems: (_org) => false, + } as CollectionView, + ]; + component.originalCipherView = { + name: "cipher1", + organizationId: "org1", + folderId: "folder1", + collectionIds: ["col1", "col2", "col3"], + favorite: true, + } as CipherView; + component.config.organizations = [{ id: "org1" } as Organization]; + }); + + it("should not show collections as readonly when `config.admin` is true", async () => { + await component.ngOnInit(); + fixture.detectChanges(); + + // Filters out all collections + expect(component["readOnlyCollections"]).toEqual([]); + + // Non-admin, keep readonly collections + component.config.admin = false; + + await component.ngOnInit(); + fixture.detectChanges(); + + expect(component["readOnlyCollections"]).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 86a8818bbe..ea82aa0cae 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 @@ -240,7 +240,11 @@ export class ItemDetailsSectionComponent implements OnInit { } else if (this.config.mode === "edit") { this.readOnlyCollections = this.collections .filter( - (c) => c.readOnly && this.originalCipherView.collectionIds.includes(c.id as CollectionId), + // When the configuration is set up for admins, they can alter read only collections + (c) => + c.readOnly && + !this.config.admin && + this.originalCipherView.collectionIds.includes(c.id as CollectionId), ) .map((c) => c.name); } @@ -262,12 +266,24 @@ export class ItemDetailsSectionComponent implements OnInit { collectionsControl.disable(); this.showCollectionsControl = false; return; + } else { + collectionsControl.enable(); + this.showCollectionsControl = true; } + const organization = this.organizations.find((o) => o.id === orgId); + 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); + // Filter criteria: + // - The collection belongs to the organization + // - When in partial edit mode, show all org collections because the control is disabled. + // - The user can edit items within the collection + // - 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) || this.config.admin) + ); }) .map((c) => ({ id: c.id, diff --git a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts index 8e73d9edd4..1b7e86f82a 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts @@ -1,6 +1,7 @@ import { inject, Injectable } from "@angular/core"; import { firstValueFrom, map } from "rxjs"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -17,6 +18,7 @@ function isSetEqual(a: Set, b: Set) { export class DefaultCipherFormService implements CipherFormService { private cipherService: CipherService = inject(CipherService); private accountService: AccountService = inject(AccountService); + private apiService: ApiService = inject(ApiService); async decryptCipher(cipher: Cipher): Promise { const activeUserId = await firstValueFrom( @@ -66,11 +68,21 @@ export class DefaultCipherFormService implements CipherFormService { // Updating a cipher with collection changes is not supported with a single request currently // First update the cipher with the original collectionIds encryptedCipher.collectionIds = config.originalCipher.collectionIds; - await this.cipherService.updateWithServer(encryptedCipher, config.admin); + await this.cipherService.updateWithServer( + encryptedCipher, + config.admin || originalCollectionIds.size === 0, + config.mode !== "clone", + ); // Then save the new collection changes separately encryptedCipher.collectionIds = cipher.collectionIds; - savedCipher = await this.cipherService.saveCollectionsWithServer(encryptedCipher); + + if (config.admin || originalCollectionIds.size === 0) { + // When using an admin config or the cipher was unassigned, update collections as an admin + savedCipher = await this.cipherService.saveCollectionsWithServerAdmin(encryptedCipher); + } else { + savedCipher = await this.cipherService.saveCollectionsWithServer(encryptedCipher); + } } // Its possible the cipher was made no longer available due to collection assignment changes diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index 5d61caf52f..0871fd8e78 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -98,6 +98,7 @@ export class CipherViewComponent implements OnChanges, OnDestroy { async loadCipherData() { // Load collections if not provided and the cipher has collectionIds if ( + this.cipher.collectionIds && this.cipher.collectionIds.length > 0 && (!this.collections || this.collections.length === 0) ) {