[PM-10247] Browser Refresh - Fix save credential banner (#10520)

* [PM-10247] Prioritize initialValues when initiating the CipherForm child forms * [PM-10247] Fetch the addEditCipherInfo when opening the cipher form in Browser and override any initialValues if present * [PM-10247] Fix item details section tests * [PM-10247] Add login details section test * [PM-10247] Add autofill options tests * [PM-10247] Undo webpack config change * [PM-10247] Fix failing tests * [PM-10247] Add additional tests for addEditCipherInfo
2025-12-15 15:53:27 +00:00 · 2024-08-26 12:03:47 -07:00
parent c18a7cb3e8
commit 9e093f88af
10 changed files with 306 additions and 21 deletions
--- a/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts
+++ b/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts
@@ -23,6 +23,7 @@ export type OptionalInitialValues = {
  collectionIds?: CollectionId[];
  loginUri?: string;
  username?: string;
+  password?: string;
  name?: string;
 };

@@ -58,7 +59,8 @@ type BaseCipherFormConfig = {
  originalCipher?: Cipher;

  /**
-   * Optional initial values for the form when creating a new cipher. Useful when creating a cipher in a filtered view.
+   * Optional initial values for the form when opening the cipher form.
+   * Useful when creating a new cipher in a filtered view or modifying a cipher with values from another source (e.g. the notification bar in Browser)
   */
  initialValues?: OptionalInitialValues;

--- a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts
+++ b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts
@@ -128,6 +128,47 @@ describe("AutofillOptionsComponent", () => {
    expect(component.autofillOptionsForm.value.autofillOnPageLoad).toEqual(null);
  });

+  it("initializes 'autoFillOptionsForm' with initialValues when editing an existing cipher", () => {
+    cipherFormContainer.config.initialValues = { loginUri: "https://new-website.com" };
+    const existingLogin = new LoginUriView();
+    existingLogin.uri = "https://example.com";
+    existingLogin.match = UriMatchStrategy.Exact;
+
+    (cipherFormContainer.originalCipherView as CipherView) = new CipherView();
+    cipherFormContainer.originalCipherView.login = {
+      autofillOnPageLoad: true,
+      uris: [existingLogin],
+    } as LoginView;
+
+    fixture.detectChanges();
+
+    expect(component.autofillOptionsForm.value.uris).toEqual([
+      { uri: "https://example.com", matchDetection: UriMatchStrategy.Exact },
+      { uri: "https://new-website.com", matchDetection: null },
+    ]);
+    expect(component.autofillOptionsForm.value.autofillOnPageLoad).toEqual(true);
+  });
+
+  it("initializes 'autoFillOptionsForm' with initialValues without duplicating an existing URI", () => {
+    cipherFormContainer.config.initialValues = { loginUri: "https://example.com" };
+    const existingLogin = new LoginUriView();
+    existingLogin.uri = "https://example.com";
+    existingLogin.match = UriMatchStrategy.Exact;
+
+    (cipherFormContainer.originalCipherView as CipherView) = new CipherView();
+    cipherFormContainer.originalCipherView.login = {
+      autofillOnPageLoad: true,
+      uris: [existingLogin],
+    } as LoginView;
+
+    fixture.detectChanges();
+
+    expect(component.autofillOptionsForm.value.uris).toEqual([
+      { uri: "https://example.com", matchDetection: UriMatchStrategy.Exact },
+    ]);
+    expect(component.autofillOptionsForm.value.autofillOnPageLoad).toEqual(true);
+  });
+
  it("initializes 'autoFillOptionsForm' with an empty URI when creating a new cipher", () => {
    cipherFormContainer.config.initialValues = null;

--- a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts
+++ b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts
@@ -143,6 +143,20 @@ export class AutofillOptionsComponent implements OnInit {
    this.autofillOptionsForm.patchValue({
      autofillOnPageLoad: existingLogin.autofillOnPageLoad,
    });
+
+    if (this.cipherFormContainer.config.initialValues?.loginUri) {
+      // Avoid adding the same uri again if it already exists
+      if (
+        existingLogin.uris?.findIndex(
+          (uri) => uri.uri === this.cipherFormContainer.config.initialValues.loginUri,
+        ) === -1
+      ) {
+        this.addUri({
+          uri: this.cipherFormContainer.config.initialValues.loginUri,
+          matchDetection: null,
+        });
+      }
+    }
  }

  private initNewCipher() {
--- a/libs/vault/src/cipher-form/components/identity/identity.component.ts
+++ b/libs/vault/src/cipher-form/components/identity/identity.component.ts
@@ -127,7 +127,7 @@ export class IdentitySectionComponent implements OnInit {
      firstName: identity.firstName,
      middleName: identity.middleName,
      lastName: identity.lastName,
-      username: identity.username,
+      username: this.cipherFormContainer.config.initialValues?.username ?? identity.username,
      company: identity.company,
      ssn: identity.ssn,
      passportNumber: identity.passportNumber,
--- 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
@@ -5,6 +5,7 @@ import { mock, MockProxy } from "jest-mock-extended";

 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";

@@ -104,6 +105,43 @@ describe("ItemDetailsSectionComponent", () => {
      expect(updatedCipher.favorite).toBe(true);
    }));

+    it("should prioritize initialValues when editing an existing cipher ", fakeAsync(async () => {
+      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,
+      ];
+      component.originalCipherView = {
+        name: "cipher1",
+        organizationId: "org1",
+        folderId: "folder1",
+        collectionIds: ["col1"],
+        favorite: true,
+      } as CipherView;
+
+      component.config.initialValues = {
+        name: "new-name",
+        folderId: "new-folder",
+        organizationId: "bad-org" as OrganizationId, // Should not be set in edit mode
+        collectionIds: ["col2" as CollectionId],
+      };
+
+      await component.ngOnInit();
+      tick();
+
+      expect(cipherFormProvider.patchCipher).toHaveBeenCalled();
+      const patchFn = cipherFormProvider.patchCipher.mock.lastCall[0];
+
+      const updatedCipher = patchFn(new CipherView());
+
+      expect(updatedCipher.name).toBe("new-name");
+      expect(updatedCipher.organizationId).toBe("org1");
+      expect(updatedCipher.folderId).toBe("new-folder");
+      expect(updatedCipher.collectionIds).toEqual(["col2"]);
+      expect(updatedCipher.favorite).toBe(true);
+    }));
+
    it("should disable organizationId control if ownership change is not allowed", async () => {
      component.config.allowPersonalOwnership = false;
      component.config.organizations = [{ id: "org1" } as Organization];
--- 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
@@ -190,9 +190,9 @@ export class ItemDetailsSectionComponent implements OnInit {

  private async initFromExistingCipher() {
    this.itemDetailsForm.setValue({
-      name: this.originalCipherView.name,
-      organizationId: this.originalCipherView.organizationId,
-      folderId: this.originalCipherView.folderId,
+      name: this.initialValues?.name ?? this.originalCipherView.name,
+      organizationId: this.originalCipherView.organizationId, // We do not allow changing ownership of an existing cipher.
+      folderId: this.initialValues?.folderId ?? this.originalCipherView.folderId,
      collectionIds: [],
      favorite: this.originalCipherView.favorite,
    });
@@ -208,7 +208,10 @@ export class ItemDetailsSectionComponent implements OnInit {
      }
    }

-    await this.updateCollectionOptions(this.originalCipherView.collectionIds as CollectionId[]);
+    await this.updateCollectionOptions(
+      this.initialValues?.collectionIds ??
+        (this.originalCipherView.collectionIds as CollectionId[]),
+    );

    if (this.partialEdit) {
      this.itemDetailsForm.disable();
--- a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts
+++ b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts
@@ -125,6 +125,29 @@ describe("LoginDetailsSectionComponent", () => {
    });
  });

+  it("initializes 'loginDetailsForm' with initialValues that override any original cipher view values", async () => {
+    (cipherFormContainer.originalCipherView as CipherView) = {
+      viewPassword: true,
+      login: {
+        password: "original-password",
+        username: "original-username",
+        totp: "original-totp",
+      } as LoginView,
+    } as CipherView;
+    cipherFormContainer.config.initialValues = {
+      username: "new-username",
+      password: "new-password",
+    };
+
+    await component.ngOnInit();
+
+    expect(component.loginDetailsForm.value).toEqual({
+      username: "new-username",
+      password: "new-password",
+      totp: "original-totp",
+    });
+  });
+
  describe("viewHiddenFields", () => {
    beforeEach(() => {
      (cipherFormContainer.originalCipherView as CipherView) = {
--- a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts
+++ b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts
@@ -95,6 +95,10 @@ export class LoginDetailsSectionComponent implements OnInit {
    return true;
  }

+  get initialValues() {
+    return this.cipherFormContainer.config.initialValues;
+  }
+
  constructor(
    private cipherFormContainer: CipherFormContainer,
    private formBuilder: FormBuilder,
@@ -139,8 +143,8 @@ export class LoginDetailsSectionComponent implements OnInit {

  private initFromExistingCipher(existingLogin: LoginView) {
    this.loginDetailsForm.patchValue({
-      username: existingLogin.username,
-      password: existingLogin.password,
+      username: this.initialValues?.username ?? existingLogin.username,
+      password: this.initialValues?.password ?? existingLogin.password,
      totp: existingLogin.totp,
    });

@@ -154,8 +158,8 @@ export class LoginDetailsSectionComponent implements OnInit {

  private async initNewCipher() {
    this.loginDetailsForm.patchValue({
-      username: this.cipherFormContainer.config.initialValues?.username || "",
-      password: "",
+      username: this.initialValues?.username || "",
+      password: this.initialValues?.password || "",
    });
  }