From 7b85870e58d9e07ee78e00f35a417fa67a24e248 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Thu, 24 Jul 2025 10:59:29 -0700 Subject: [PATCH] [PM-22377] - [Vault] [Clients] Update cipher form component to restrict editing old My Vault items (#15687) * disable cipher form for "My Items" ciphers * use correct property * prevent changing non org fields in cli for org owned vaults * update var name * fix tests * fix stories * revert changes to item details section. update comment in edit command * remove unused props * fix test * re-apply logic to enforce org ownership * re-apply logic to enforce org ownership * fix logic and test * add empty line to comment * remove unused var * delegate form enabling/disabling to cipherFormContainer * rename var and getter back to original. update comment --- apps/cli/src/commands/edit.command.ts | 15 ++++ apps/cli/src/oss-serve-configurator.ts | 1 + apps/cli/src/vault.program.ts | 1 + .../src/cipher-form/cipher-form-container.ts | 4 ++ .../components/cipher-form.component.ts | 8 +++ .../item-details-section.component.html | 2 +- .../item-details-section.component.spec.ts | 1 + .../item-details-section.component.ts | 68 ++++++++++++++----- 8 files changed, 81 insertions(+), 19 deletions(-) diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index ebf877011b7..6b8c5811056 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -4,6 +4,8 @@ import { firstValueFrom } from "rxjs"; import { CollectionRequest } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; @@ -36,6 +38,7 @@ export class EditCommand { private folderApiService: FolderApiServiceAbstraction, private accountService: AccountService, private cliRestrictedItemTypesService: CliRestrictedItemTypesService, + private policyService: PolicyService, ) {} async run( @@ -104,6 +107,18 @@ export class EditCommand { return Response.error("Editing this item type is restricted by organizational policy."); } + const isPersonalVaultItem = cipherView.organizationId == null; + + const organizationOwnershipPolicyApplies = await firstValueFrom( + this.policyService.policyAppliesToUser$(PolicyType.OrganizationDataOwnership, activeUserId), + ); + + if (isPersonalVaultItem && organizationOwnershipPolicyApplies) { + return Response.error( + "An organization policy restricts editing this cipher. Please use the share command first before modifying it.", + ); + } + const encCipher = await this.cipherService.encrypt(cipherView, activeUserId); try { const updatedCipher = await this.cipherService.updateWithServer(encCipher); diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts index 848627b703f..a460fa270a8 100644 --- a/apps/cli/src/oss-serve-configurator.ts +++ b/apps/cli/src/oss-serve-configurator.ts @@ -103,6 +103,7 @@ export class OssServeConfigurator { this.serviceContainer.folderApiService, this.serviceContainer.accountService, this.serviceContainer.cliRestrictedItemTypesService, + this.serviceContainer.policyService, ); this.generateCommand = new GenerateCommand( this.serviceContainer.passwordGenerationService, diff --git a/apps/cli/src/vault.program.ts b/apps/cli/src/vault.program.ts index 2b08bc67ec1..bdcc52393ca 100644 --- a/apps/cli/src/vault.program.ts +++ b/apps/cli/src/vault.program.ts @@ -285,6 +285,7 @@ export class VaultProgram extends BaseProgram { this.serviceContainer.folderApiService, this.serviceContainer.accountService, this.serviceContainer.cliRestrictedItemTypesService, + this.serviceContainer.policyService, ); const response = await command.run(object, id, encodedJson, cmd); this.processResponse(response); diff --git a/libs/vault/src/cipher-form/cipher-form-container.ts b/libs/vault/src/cipher-form/cipher-form-container.ts index cef5e102afe..628b4c07f6c 100644 --- a/libs/vault/src/cipher-form/cipher-form-container.ts +++ b/libs/vault/src/cipher-form/cipher-form-container.ts @@ -70,4 +70,8 @@ export abstract class CipherFormContainer { /** Returns true when the `CipherFormContainer` was initialized with a cached cipher available. */ abstract initializedWithCachedCipher(): boolean; + + abstract disableFormFields(): void; + + abstract enableFormFields(): void; } 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 b8815235ee8..47ab9977f64 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -150,6 +150,14 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } } + disableFormFields(): void { + this.cipherForm.disable({ emitEvent: false }); + } + + enableFormFields(): void { + this.cipherForm.enable({ emitEvent: false }); + } + /** * Registers a child form group with the parent form group. Used by child components to add their form groups to * the parent form for validation. 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 c61312c13eb..4d575634b1c 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 @@ -22,7 +22,7 @@ {{ "owner" | i18n }} 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 db8e2007c61..3c513a2f067 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 @@ -572,6 +572,7 @@ describe("ItemDetailsSectionComponent", () => { it("returns matching default when flag & policy match", async () => { const def = createMockCollection("def1", "Def", "orgA"); component.config.collections = [def] as CollectionView[]; + component.config.organizationDataOwnershipDisabled = false; component.config.initialValues = { collectionIds: [] } as OptionalInitialValues; mockConfigService.getFeatureFlag.mockResolvedValue(true); mockPolicyService.policiesByType$.mockReturnValue(of([{ organizationId: "orgA" } as Policy])); 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 1064980050f..4fd999ae601 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 @@ -19,7 +19,7 @@ import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CardComponent, @@ -80,6 +80,8 @@ export class ItemDetailsSectionComponent implements OnInit { protected organizations: Organization[] = []; + protected userId: UserId; + @Input({ required: true }) config: CipherFormConfig; @@ -96,7 +98,7 @@ export class ItemDetailsSectionComponent implements OnInit { return this.config.mode === "partial-edit"; } - get organizationDataOwnershipDisabled() { + get allowPersonalOwnership() { return this.config.organizationDataOwnershipDisabled; } @@ -109,16 +111,19 @@ export class ItemDetailsSectionComponent implements OnInit { } /** - * Show the organization data ownership option in the Owner dropdown when: - * - organization data ownership is disabled - * - The `organizationId` control is disabled. This avoids the scenario - * where a the dropdown is empty because the user personally owns the cipher - * but cannot edit the ownership. + * Show the personal ownership option in the Owner dropdown when any of the following: + * - personal ownership is allowed + * - `organizationId` control is disabled + * - personal ownership is not allowed AND the user is editing a cipher that is not + * currently owned by an organization */ - get showOrganizationDataOwnershipOption() { + get showPersonalOwnershipOption() { return ( - this.organizationDataOwnershipDisabled || - !this.itemDetailsForm.controls.organizationId.enabled + this.allowPersonalOwnership || + this.itemDetailsForm.controls.organizationId.disabled || + (!this.allowPersonalOwnership && + this.config.originalCipher && + this.itemDetailsForm.controls.organizationId.value === null) ); } @@ -170,7 +175,7 @@ export class ItemDetailsSectionComponent implements OnInit { } // If personal ownership is allowed and there is at least one organization, allow ownership change. - if (this.organizationDataOwnershipDisabled) { + if (this.allowPersonalOwnership) { return this.organizations.length > 0; } @@ -189,7 +194,7 @@ export class ItemDetailsSectionComponent implements OnInit { } get defaultOwner() { - return this.organizationDataOwnershipDisabled ? null : this.organizations[0].id; + return this.allowPersonalOwnership ? null : this.organizations[0].id; } async ngOnInit() { @@ -197,7 +202,9 @@ export class ItemDetailsSectionComponent implements OnInit { Utils.getSortFunction(this.i18nService, "name"), ); - if (!this.organizationDataOwnershipDisabled && this.organizations.length === 0) { + this.userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + if (!this.allowPersonalOwnership && this.organizations.length === 0) { throw new Error("No organizations available for ownership."); } @@ -216,43 +223,68 @@ export class ItemDetailsSectionComponent implements OnInit { }); await this.updateCollectionOptions(this.initialValues?.collectionIds); } + this.setFormState(); if (!this.allowOwnershipChange) { this.itemDetailsForm.controls.organizationId.disable(); } this.itemDetailsForm.controls.organizationId.valueChanges .pipe( takeUntilDestroyed(this.destroyRef), - concatMap(async () => await this.updateCollectionOptions()), + concatMap(async () => { + await this.updateCollectionOptions(); + this.setFormState(); + }), ) .subscribe(); } + /** + * When the cipher does not belong to an organization but the user's organization + * requires all ciphers to be owned by an organization, disable the entire form + * until the user selects an organization. + */ + private setFormState() { + if (this.config.originalCipher && !this.allowPersonalOwnership) { + if (this.itemDetailsForm.controls.organizationId.value === null) { + this.cipherFormContainer.disableFormFields(); + this.itemDetailsForm.controls.organizationId.enable(); + } else { + this.cipherFormContainer.enableFormFields(); + } + } + } + /** * Gets the default collection IDs for the selected organization. * Returns null if any of the following apply: * - the feature flag is disabled + * - the "no private data policy" doesn't apply to the user * - no org is currently selected * - the selected org doesn't have the "no private data policy" enabled */ private async getDefaultCollectionId(orgId?: OrganizationId) { - if (!orgId) { + if (!orgId || this.allowPersonalOwnership) { return; } + const isFeatureEnabled = await this.configService.getFeatureFlag( FeatureFlag.CreateDefaultLocation, ); + if (!isFeatureEnabled) { return; } - const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + const selectedOrgHasPolicyEnabled = ( await firstValueFrom( - this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, userId), + this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, this.userId), ) ).find((p) => p.organizationId); + if (!selectedOrgHasPolicyEnabled) { return; } + const defaultUserCollection = this.collections.find( (c) => c.organizationId === orgId && c.type === CollectionTypes.DefaultUserCollection, ); @@ -284,7 +316,7 @@ export class ItemDetailsSectionComponent implements OnInit { ); } - if (!this.organizationDataOwnershipDisabled && prefillCipher.organizationId == null) { + if (!this.allowPersonalOwnership && prefillCipher.organizationId == null) { this.itemDetailsForm.controls.organizationId.setValue(this.defaultOwner); } }