1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 13:53:34 +00:00

[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
This commit is contained in:
Jordan Aasen
2025-07-24 10:59:29 -07:00
committed by GitHub
parent cd33ea0747
commit 7b85870e58
8 changed files with 81 additions and 19 deletions

View File

@@ -4,6 +4,8 @@ import { firstValueFrom } from "rxjs";
import { CollectionRequest } from "@bitwarden/admin-console/common"; import { CollectionRequest } from "@bitwarden/admin-console/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; 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 { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service";
@@ -36,6 +38,7 @@ export class EditCommand {
private folderApiService: FolderApiServiceAbstraction, private folderApiService: FolderApiServiceAbstraction,
private accountService: AccountService, private accountService: AccountService,
private cliRestrictedItemTypesService: CliRestrictedItemTypesService, private cliRestrictedItemTypesService: CliRestrictedItemTypesService,
private policyService: PolicyService,
) {} ) {}
async run( async run(
@@ -104,6 +107,18 @@ export class EditCommand {
return Response.error("Editing this item type is restricted by organizational policy."); 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); const encCipher = await this.cipherService.encrypt(cipherView, activeUserId);
try { try {
const updatedCipher = await this.cipherService.updateWithServer(encCipher); const updatedCipher = await this.cipherService.updateWithServer(encCipher);

View File

@@ -103,6 +103,7 @@ export class OssServeConfigurator {
this.serviceContainer.folderApiService, this.serviceContainer.folderApiService,
this.serviceContainer.accountService, this.serviceContainer.accountService,
this.serviceContainer.cliRestrictedItemTypesService, this.serviceContainer.cliRestrictedItemTypesService,
this.serviceContainer.policyService,
); );
this.generateCommand = new GenerateCommand( this.generateCommand = new GenerateCommand(
this.serviceContainer.passwordGenerationService, this.serviceContainer.passwordGenerationService,

View File

@@ -285,6 +285,7 @@ export class VaultProgram extends BaseProgram {
this.serviceContainer.folderApiService, this.serviceContainer.folderApiService,
this.serviceContainer.accountService, this.serviceContainer.accountService,
this.serviceContainer.cliRestrictedItemTypesService, this.serviceContainer.cliRestrictedItemTypesService,
this.serviceContainer.policyService,
); );
const response = await command.run(object, id, encodedJson, cmd); const response = await command.run(object, id, encodedJson, cmd);
this.processResponse(response); this.processResponse(response);

View File

@@ -70,4 +70,8 @@ export abstract class CipherFormContainer {
/** Returns true when the `CipherFormContainer` was initialized with a cached cipher available. */ /** Returns true when the `CipherFormContainer` was initialized with a cached cipher available. */
abstract initializedWithCachedCipher(): boolean; abstract initializedWithCachedCipher(): boolean;
abstract disableFormFields(): void;
abstract enableFormFields(): void;
} }

View File

@@ -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 * 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. * the parent form for validation.

View File

@@ -22,7 +22,7 @@
<bit-label>{{ "owner" | i18n }}</bit-label> <bit-label>{{ "owner" | i18n }}</bit-label>
<bit-select formControlName="organizationId"> <bit-select formControlName="organizationId">
<bit-option <bit-option
*ngIf="showOrganizationDataOwnershipOption" *ngIf="showPersonalOwnershipOption"
[value]="null" [value]="null"
[label]="userEmail$ | async" [label]="userEmail$ | async"
></bit-option> ></bit-option>

View File

@@ -572,6 +572,7 @@ describe("ItemDetailsSectionComponent", () => {
it("returns matching default when flag & policy match", async () => { it("returns matching default when flag & policy match", async () => {
const def = createMockCollection("def1", "Def", "orgA"); const def = createMockCollection("def1", "Def", "orgA");
component.config.collections = [def] as CollectionView[]; component.config.collections = [def] as CollectionView[];
component.config.organizationDataOwnershipDisabled = false;
component.config.initialValues = { collectionIds: [] } as OptionalInitialValues; component.config.initialValues = { collectionIds: [] } as OptionalInitialValues;
mockConfigService.getFeatureFlag.mockResolvedValue(true); mockConfigService.getFeatureFlag.mockResolvedValue(true);
mockPolicyService.policiesByType$.mockReturnValue(of([{ organizationId: "orgA" } as Policy])); mockPolicyService.policiesByType$.mockReturnValue(of([{ organizationId: "orgA" } as Policy]));

View File

@@ -19,7 +19,7 @@ import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { Utils } from "@bitwarden/common/platform/misc/utils"; 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 { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { import {
CardComponent, CardComponent,
@@ -80,6 +80,8 @@ export class ItemDetailsSectionComponent implements OnInit {
protected organizations: Organization[] = []; protected organizations: Organization[] = [];
protected userId: UserId;
@Input({ required: true }) @Input({ required: true })
config: CipherFormConfig; config: CipherFormConfig;
@@ -96,7 +98,7 @@ export class ItemDetailsSectionComponent implements OnInit {
return this.config.mode === "partial-edit"; return this.config.mode === "partial-edit";
} }
get organizationDataOwnershipDisabled() { get allowPersonalOwnership() {
return this.config.organizationDataOwnershipDisabled; return this.config.organizationDataOwnershipDisabled;
} }
@@ -109,16 +111,19 @@ export class ItemDetailsSectionComponent implements OnInit {
} }
/** /**
* Show the organization data ownership option in the Owner dropdown when: * Show the personal ownership option in the Owner dropdown when any of the following:
* - organization data ownership is disabled * - personal ownership is allowed
* - The `organizationId` control is disabled. This avoids the scenario * - `organizationId` control is disabled
* where a the dropdown is empty because the user personally owns the cipher * - personal ownership is not allowed AND the user is editing a cipher that is not
* but cannot edit the ownership. * currently owned by an organization
*/ */
get showOrganizationDataOwnershipOption() { get showPersonalOwnershipOption() {
return ( return (
this.organizationDataOwnershipDisabled || this.allowPersonalOwnership ||
!this.itemDetailsForm.controls.organizationId.enabled 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 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; return this.organizations.length > 0;
} }
@@ -189,7 +194,7 @@ export class ItemDetailsSectionComponent implements OnInit {
} }
get defaultOwner() { get defaultOwner() {
return this.organizationDataOwnershipDisabled ? null : this.organizations[0].id; return this.allowPersonalOwnership ? null : this.organizations[0].id;
} }
async ngOnInit() { async ngOnInit() {
@@ -197,7 +202,9 @@ export class ItemDetailsSectionComponent implements OnInit {
Utils.getSortFunction(this.i18nService, "name"), 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."); throw new Error("No organizations available for ownership.");
} }
@@ -216,43 +223,68 @@ export class ItemDetailsSectionComponent implements OnInit {
}); });
await this.updateCollectionOptions(this.initialValues?.collectionIds); await this.updateCollectionOptions(this.initialValues?.collectionIds);
} }
this.setFormState();
if (!this.allowOwnershipChange) { if (!this.allowOwnershipChange) {
this.itemDetailsForm.controls.organizationId.disable(); this.itemDetailsForm.controls.organizationId.disable();
} }
this.itemDetailsForm.controls.organizationId.valueChanges this.itemDetailsForm.controls.organizationId.valueChanges
.pipe( .pipe(
takeUntilDestroyed(this.destroyRef), takeUntilDestroyed(this.destroyRef),
concatMap(async () => await this.updateCollectionOptions()), concatMap(async () => {
await this.updateCollectionOptions();
this.setFormState();
}),
) )
.subscribe(); .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. * Gets the default collection IDs for the selected organization.
* Returns null if any of the following apply: * Returns null if any of the following apply:
* - the feature flag is disabled * - the feature flag is disabled
* - the "no private data policy" doesn't apply to the user
* - no org is currently selected * - no org is currently selected
* - the selected org doesn't have the "no private data policy" enabled * - the selected org doesn't have the "no private data policy" enabled
*/ */
private async getDefaultCollectionId(orgId?: OrganizationId) { private async getDefaultCollectionId(orgId?: OrganizationId) {
if (!orgId) { if (!orgId || this.allowPersonalOwnership) {
return; return;
} }
const isFeatureEnabled = await this.configService.getFeatureFlag( const isFeatureEnabled = await this.configService.getFeatureFlag(
FeatureFlag.CreateDefaultLocation, FeatureFlag.CreateDefaultLocation,
); );
if (!isFeatureEnabled) { if (!isFeatureEnabled) {
return; return;
} }
const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
const selectedOrgHasPolicyEnabled = ( const selectedOrgHasPolicyEnabled = (
await firstValueFrom( await firstValueFrom(
this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, userId), this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, this.userId),
) )
).find((p) => p.organizationId); ).find((p) => p.organizationId);
if (!selectedOrgHasPolicyEnabled) { if (!selectedOrgHasPolicyEnabled) {
return; return;
} }
const defaultUserCollection = this.collections.find( const defaultUserCollection = this.collections.find(
(c) => c.organizationId === orgId && c.type === CollectionTypes.DefaultUserCollection, (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); this.itemDetailsForm.controls.organizationId.setValue(this.defaultOwner);
} }
} }