[PM-24269] Enable ownership field for personal items (#16069)

* remove global check for personal ownership as `setFormState` now handles it

* ensure that the organizationId is disabled for new ciphers

* only check for personal ownership change for enabling/disabling the entire form - this ensure that it is only applied when the data ownership policy is applied

- The bug was caused by a regular user that wasn't in an organization, their form was getting fully disabled when it shouldn't.

* fix type checking

* do not disable organization id after an organization is selected

libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts

@@ -15,6 +15,7 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
+import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { SelectComponent } from "@bitwarden/components";
 
@@ -62,16 +63,22 @@ describe("ItemDetailsSectionComponent", () => {
   let mockPolicyService: MockProxy<PolicyService>;
 
   const activeAccount$ = new BehaviorSubject<{ email: string }>({ email: "test@example.com" });
-  const getInitialCipherView = jest.fn(() => null);
+  const getInitialCipherView = jest.fn<CipherView | null, []>(() => null);
   const initializedWithCachedCipher = jest.fn(() => false);
+  const disableFormFields = jest.fn();
+  const enableFormFields = jest.fn();
 
   beforeEach(async () => {
     getInitialCipherView.mockClear();
     initializedWithCachedCipher.mockClear();
+    disableFormFields.mockClear();
+    enableFormFields.mockClear();
 
     cipherFormProvider = mock<CipherFormContainer>({
       getInitialCipherView,
       initializedWithCachedCipher,
+      disableFormFields,
+      enableFormFields,
     });
     i18nService = mock<I18nService>();
     i18nService.collator = {
@@ -151,7 +158,7 @@ describe("ItemDetailsSectionComponent", () => {
         folderId: "folder1",
         collectionIds: ["col1"],
         favorite: true,
-      });
+      } as CipherView);
 
       await component.ngOnInit();
       tick();
@@ -420,7 +427,7 @@ describe("ItemDetailsSectionComponent", () => {
         folderId: "folder1",
         collectionIds: ["col1", "col2"],
         favorite: true,
-      });
+      } as CipherView);
       component.config.organizations = [{ id: "org1" } as Organization];
       component.config.collections = [
         createMockCollection("col1", "Collection 1", "org1") as CollectionView,
@@ -467,7 +474,7 @@ describe("ItemDetailsSectionComponent", () => {
         folderId: "folder1",
         collectionIds: ["col1", "col2", "col3"],
         favorite: true,
-      });
+      } as CipherView);
       component.originalCipherView = {
         name: "cipher1",
         organizationId: "org1",
@@ -513,6 +520,7 @@ describe("ItemDetailsSectionComponent", () => {
       expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]);
     });
   });
+
   describe("readonlyCollections", () => {
     beforeEach(() => {
       component.config.mode = "edit";
@@ -594,4 +602,81 @@ describe("ItemDetailsSectionComponent", () => {
       expect(result).toBeUndefined();
     });
   });
+
+  describe("form status when editing a cipher", () => {
+    beforeEach(() => {
+      component.config.mode = "edit";
+      component.config.originalCipher = new Cipher();
+      component.originalCipherView = {
+        name: "cipher1",
+        organizationId: null,
+        folderId: "folder1",
+        collectionIds: ["col1", "col2", "col3"],
+        favorite: true,
+      } as unknown as CipherView;
+    });
+
+    describe("when personal ownership is not allowed", () => {
+      beforeEach(() => {
+        component.config.organizationDataOwnershipDisabled = false; // disallow personal ownership
+        component.config.organizations = [{ id: "orgId" } as Organization];
+      });
+
+      describe("cipher does not belong to an organization", () => {
+        beforeEach(() => {
+          getInitialCipherView.mockReturnValue(component.originalCipherView!);
+        });
+
+        it("enables organizationId", async () => {
+          await component.ngOnInit();
+
+          expect(component.itemDetailsForm.controls.organizationId.disabled).toBe(false);
+        });
+
+        it("disables the rest of the form", async () => {
+          await component.ngOnInit();
+
+          expect(disableFormFields).toHaveBeenCalled();
+          expect(enableFormFields).not.toHaveBeenCalled();
+        });
+      });
+
+      describe("cipher belongs to an organization", () => {
+        beforeEach(() => {
+          component.originalCipherView.organizationId = "org-id";
+          getInitialCipherView.mockReturnValue(component.originalCipherView);
+        });
+
+        it("enables the rest of the form", async () => {
+          await component.ngOnInit();
+
+          expect(disableFormFields).not.toHaveBeenCalled();
+          expect(enableFormFields).toHaveBeenCalled();
+        });
+      });
+    });
+
+    describe("when an ownership change is not allowed", () => {
+      beforeEach(() => {
+        component.config.organizationDataOwnershipDisabled = true; // allow personal ownership
+        component.originalCipherView!.organizationId = undefined;
+      });
+
+      it("disables organizationId when the cipher is owned by an organization", async () => {
+        component.originalCipherView!.organizationId = "orgId";
+
+        await component.ngOnInit();
+
+        expect(component.itemDetailsForm.controls.organizationId.disabled).toBe(true);
+      });
+
+      it("disables organizationId when personal ownership is allowed and the user has no organizations available", async () => {
+        component.config.organizations = [];
+
+        await component.ngOnInit();
+
+        expect(component.itemDetailsForm.controls.organizationId.disabled).toBe(true);
+      });
+    });
+  });
 });

libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts

@@ -225,10 +225,9 @@ 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),
@@ -241,22 +240,28 @@ export class ItemDetailsSectionComponent implements OnInit {
   }
 
   /**
-   * 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. Once the organization is set, enable the form.
-   * Ensure to properly set the collections control state when the form is enabled.
+   * Updates the global form and organizationId control states.
    */
   private setFormState() {
     if (this.config.originalCipher && !this.allowPersonalOwnership) {
+      // When editing a cipher and the user cannot have personal ownership
+      // and the cipher is is not within the organization - force the user to
+      // move the cipher within the organization first before editing any other field
       if (this.itemDetailsForm.controls.organizationId.value === null) {
         this.cipherFormContainer.disableFormFields();
         this.itemDetailsForm.controls.organizationId.enable();
         this.favoriteButtonDisabled = true;
       } else {
+        // The "after" from the above: When editing a cipher and the user cannot have personal ownership
+        // and the organization is populated - re-enable the global form.
         this.cipherFormContainer.enableFormFields();
         this.favoriteButtonDisabled = false;
         this.setCollectionControlState();
       }
+    } else if (!this.allowOwnershipChange) {
+      // When the user cannot change the organization field, disable the organizationId control.
+      // This could be because they aren't a part of an organization
+      this.itemDetailsForm.controls.organizationId.disable({ emitEvent: false });
     }
   }