diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts index 1bffcd9ad5..f2c9d47081 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts @@ -381,4 +381,88 @@ describe("AddEditV2Component", () => { expect(navigate).toHaveBeenCalledWith(["/tabs/vault"]); }); }); + + describe("reloadAddEditCipherData", () => { + beforeEach(fakeAsync(() => { + addEditCipherInfo$.next({ + cipher: { + name: "InitialName", + type: CipherType.Login, + login: { + password: "initialPassword", + username: "initialUsername", + uris: [{ uri: "https://initial.com" }], + }, + }, + } as AddEditCipherInfo); + queryParams$.next({}); + tick(); + + cipherServiceMock.setAddEditCipherInfo.mockClear(); + })); + + it("replaces all initialValues with new data, clearing stale fields", fakeAsync(() => { + const newCipherInfo = { + cipher: { + name: "UpdatedName", + type: CipherType.Login, + login: { + password: "updatedPassword", + uris: [{ uri: "https://updated.com" }], + }, + }, + } as AddEditCipherInfo; + + addEditCipherInfo$.next(newCipherInfo); + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(component.config.initialValues).toEqual({ + name: "UpdatedName", + password: "updatedPassword", + loginUri: "https://updated.com", + } as OptionalInitialValues); + + expect(cipherServiceMock.setAddEditCipherInfo).toHaveBeenCalledWith(null, "UserId"); + })); + + it("does not reload data if config is not set", fakeAsync(() => { + component.config = null; + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(cipherServiceMock.setAddEditCipherInfo).not.toHaveBeenCalled(); + })); + + it("does not reload data if latestCipherInfo is null", fakeAsync(() => { + addEditCipherInfo$.next(null); + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(component.config.initialValues).toEqual({ + name: "InitialName", + password: "initialPassword", + username: "initialUsername", + loginUri: "https://initial.com", + } as OptionalInitialValues); + + expect(cipherServiceMock.setAddEditCipherInfo).not.toHaveBeenCalled(); + })); + + it("ignores messages with different commands", fakeAsync(() => { + const initialValues = component.config.initialValues; + + const messageListener = component["messageListener"]; + messageListener({ command: "someOtherCommand" }); + tick(); + + expect(component.config.initialValues).toBe(initialValues); + })); + }); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index 60e44cefbd..22aad854dd 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { CommonModule } from "@angular/common"; -import { Component, OnInit } from "@angular/core"; +import { Component, OnInit, OnDestroy } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormsModule } from "@angular/forms"; import { ActivatedRoute, Params, Router } from "@angular/router"; @@ -158,7 +158,7 @@ export type AddEditQueryParams = Partial>; IconButtonModule, ], }) -export class AddEditV2Component implements OnInit { +export class AddEditV2Component implements OnInit, OnDestroy { headerText: string; config: CipherFormConfig; canDeleteCipher$: Observable; @@ -200,12 +200,58 @@ export class AddEditV2Component implements OnInit { this.subscribeToParams(); } + private messageListener: (message: any) => void; + async ngOnInit() { this.fido2PopoutSessionData = await firstValueFrom(this.fido2PopoutSessionData$); if (BrowserPopupUtils.inPopout(window)) { this.popupCloseWarningService.enable(); } + + // Listen for messages to reload cipher data when the pop up is already open + this.messageListener = async (message: any) => { + if (message?.command === "reloadAddEditCipherData") { + try { + await this.reloadCipherData(); + } catch (error) { + this.logService.error("Failed to reload cipher data", error); + } + } + }; + BrowserApi.addListener(chrome.runtime.onMessage, this.messageListener); + } + + ngOnDestroy() { + if (this.messageListener) { + BrowserApi.removeListener(chrome.runtime.onMessage, this.messageListener); + } + } + + /** + * Reloads the cipher data when the popup is already open and new form data is submitted. + * This completely replaces the initialValues to clear any stale data from the previous submission. + */ + private async reloadCipherData() { + if (!this.config) { + return; + } + + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + const latestCipherInfo = await firstValueFrom( + this.cipherService.addEditCipherInfo$(activeUserId), + ); + + if (latestCipherInfo != null) { + this.config = { + ...this.config, + initialValues: mapAddEditCipherInfoToInitialValues(latestCipherInfo), + }; + + // Be sure to clear the "cached" cipher info, so it doesn't get used again + await this.cipherService.setAddEditCipherInfo(null, activeUserId); + } } /** diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts index 4597c00429..3389228dda 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts @@ -2,6 +2,7 @@ import { mock } from "jest-mock-extended"; import { CipherType } from "@bitwarden/common/vault/enums"; +import { BrowserApi } from "../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; import { @@ -23,6 +24,19 @@ describe("VaultPopoutWindow", () => { .spyOn(BrowserPopupUtils, "closeSingleActionPopout") .mockImplementation(); + beforeEach(() => { + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([]); + jest.spyOn(BrowserApi, "updateWindowProperties").mockResolvedValue(); + global.chrome = { + ...global.chrome, + runtime: { + ...global.chrome?.runtime, + sendMessage: jest.fn().mockResolvedValue(undefined), + getURL: jest.fn((path) => `chrome-extension://extension-id/${path}`), + }, + }; + }); + afterEach(() => { jest.clearAllMocks(); }); @@ -123,6 +137,32 @@ describe("VaultPopoutWindow", () => { }, ); }); + + it("sends a message to refresh data when the popup is already open", async () => { + const existingPopupTab = { + id: 123, + windowId: 456, + url: `chrome-extension://extension-id/popup/index.html#/edit-cipher?singleActionPopout=${VaultPopoutType.addEditVaultItem}_${CipherType.Login}`, + } as chrome.tabs.Tab; + + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([existingPopupTab]); + const sendMessageSpy = jest.spyOn(chrome.runtime, "sendMessage"); + const updateWindowSpy = jest.spyOn(BrowserApi, "updateWindowProperties"); + + await openAddEditVaultItemPopout( + mock({ windowId: 1, url: "https://jest-testing-website.com" }), + { + cipherType: CipherType.Login, + }, + ); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).toHaveBeenCalledWith({ + command: "reloadAddEditCipherData", + data: { cipherId: undefined, cipherType: CipherType.Login }, + }); + expect(updateWindowSpy).toHaveBeenCalledWith(456, { focused: true }); + }); }); describe("closeAddEditVaultItemPopout", () => { diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.ts index 3dae96b6cc..cccf005cd2 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.ts @@ -115,10 +115,26 @@ async function openAddEditVaultItemPopout( addEditCipherUrl += formatQueryString("uri", url); } - await BrowserPopupUtils.openPopout(addEditCipherUrl, { - singleActionKey, - senderWindowId: windowId, - }); + const extensionUrl = chrome.runtime.getURL("popup/index.html"); + const existingPopupTabs = await BrowserApi.tabsQuery({ url: `${extensionUrl}*` }); + const existingPopup = existingPopupTabs.find((tab) => + tab.url?.includes(`singleActionPopout=${singleActionKey}`), + ); + // Check if the an existing popup is already open + try { + await chrome.runtime.sendMessage({ + command: "reloadAddEditCipherData", + data: { cipherId, cipherType }, + }); + await BrowserApi.updateWindowProperties(existingPopup.windowId, { + focused: true, + }); + } catch { + await BrowserPopupUtils.openPopout(addEditCipherUrl, { + singleActionKey, + senderWindowId: windowId, + }); + } } /** diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts b/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts index 1e60ad91fb..9f3102239a 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts @@ -42,9 +42,18 @@ describe("CipherFormComponent", () => { { provide: CipherFormService, useValue: mockAddEditFormService }, { provide: CipherFormCacheService, - useValue: { init: jest.fn(), getCachedCipherView: jest.fn() }, + useValue: { init: jest.fn(), getCachedCipherView: jest.fn(), clearCache: jest.fn() }, + }, + { + provide: ViewCacheService, + useValue: { + signal: jest.fn(() => { + const signalFn = (): any => null; + signalFn.set = jest.fn(); + return signalFn; + }), + }, }, - { provide: ViewCacheService, useValue: { signal: jest.fn(() => (): any => null) } }, { provide: ConfigService, useValue: mock() }, { provide: AccountService, useValue: mockAccountService }, { provide: CipherArchiveService, useValue: mockCipherArchiveService }, 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 f94af25e90..c9e867f8d3 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -304,13 +304,30 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci * Updates `updatedCipherView` based on the value from the cache. */ setInitialCipherFromCache() { + // If we are coming from the overlay/popup flow clear the cache to avoid old cached data + const hasOverlayData = + this.config.initialValues && + (this.config.initialValues.username !== undefined || + this.config.initialValues.password !== undefined); + + if (hasOverlayData) { + this.cipherFormCacheService.clearCache(); + return; + } + const cachedCipher = this.cipherFormCacheService.getCachedCipherView(); if (cachedCipher === null) { return; } - // Use the cached cipher when it matches the cipher being edited - if (this.updatedCipherView.id === cachedCipher.id) { + const isEditingExistingCipher = + this.updatedCipherView.id && this.updatedCipherView.id === cachedCipher.id; + const isCreatingNewCipher = + !this.updatedCipherView.id && + !cachedCipher.id && + this.updatedCipherView.type === cachedCipher.type; + + if (isEditingExistingCipher || isCreatingNewCipher) { this.updatedCipherView = cachedCipher; } } @@ -382,6 +399,9 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci this.config, ); + // Clear the cache after successful save + this.cipherFormCacheService.clearCache(); + this.toastService.showToast({ variant: "success", title: null, diff --git a/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts index 25581ae5ea..d525dcd9af 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts @@ -22,7 +22,6 @@ export class CipherFormCacheService { key: CIPHER_FORM_CACHE_KEY, initialValue: null, deserializer: CipherView.fromJSON, - clearOnTabChange: true, }); constructor() { @@ -45,4 +44,11 @@ export class CipherFormCacheService { getCachedCipherView(): CipherView | null { return this.cipherCache(); } + + /** + * Clear the cached CipherView. + */ + clearCache(): void { + this.cipherCache.set(null); + } }