From 08b850085ceae9b6657e1719a55e51226e2c0e4a Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Wed, 21 Feb 2024 12:47:10 -0600 Subject: [PATCH] [PM-673] Safari browser extension does not list folders when offering to save new or last-used credentials (#7729) * [PM-673] Safari Notification Bar Does Not Show Folders * [PM-673] Refactoring Context Menu Implementations to Ensure Pages with No Logins Can Dismiss Notification Bar * [PM-673] Refactoring typing information for the LockedVaultPendingNotificationsItem typing * [PM-673] Refactoring typing information for notification background * [PM-673] Finishing out typing for potential extension messages to the notification bar; * [PM-673] Working through implementation details for the notification background * [PM-673] Fixing issues present with messaging re-implementation * [PM-673] Fixing issue with folders not populating within Safari notification bar * [PM-673] Fixing jest test issues present within implementation * [PM-673] Fixing issue present with webVaultUrl vulnerability * [PM-673] Fixing XSS Vulnerability within Notification Bar; * [PM-5670] Putting together a partial implementation for having messages appear on network error within the notification bar * [PM-673] Incorporating status update for when user has successfully saved credentials * [PM-673] Incorporating status update for when user has successfully saved credentials * [PM-5949] Refactor typing information for notification bar * [PM-5949] Fix jest tests for overlay background * [PM-5949] Removing unnused typing data * [PM-5949] Fixing lint error * [PM-5949] Adding jest tests for convertAddLoginQueueMessageToCipherView method * [PM-5949] Fixing jest test for overlay * [PM-5950] Fix Context Menu Update Race Condition and Refactor Implementation * [PM-5950] Adding jest test for cipherContextMenu.update method * [PM-5950] Adding documentation for method within MainContextMenuHandler * [PM-5950] Adding jest tests for the mainContextMenuHandler * [PM-673] Stripping unnecessary work for network drop issue * [PM-673] Stripping unnecessary work for network drop issue * [PM-673] Stripping out work done for another ticket * [PM-5950] Removing unnecessary return value from MainContextMenuHandler.create method * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-673] Implementing unit test coverage for newly introduced logic * [PM-5950] Fixing unawaited context menu promise * [PM-673] Merging changes in from main and fixing merge conflicts * [PM-673] Fixing issue where updates to the added login were not triggering correctly * [PM-673] Merging changes in from main and fixing merge conflicts --- apps/browser/src/_locales/en/messages.json | 4 + .../abstractions/notification.background.ts | 52 ++ .../notification.background.spec.ts | 838 +++++++++++++++++- .../background/notification.background.ts | 382 ++++---- .../src/autofill/content/notification-bar.ts | 22 +- apps/browser/src/autofill/notification/bar.ts | 73 +- .../shared/autofill-overlay-page-element.ts | 4 + .../browser/src/background/main.background.ts | 1 + 8 files changed, 1150 insertions(+), 226 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index b25acf8ce3..981a3e56d2 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2004,6 +2004,10 @@ "selectFolder": { "message": "Select folder..." }, + "noFoldersFound": { + "message": "No folders found", + "description": "Used as a message within the notification bar when no folders are found" + }, "orgPermissionsUpdatedMustSetPassword": { "message": "Your organization permissions were updated, requiring you to set a master password.", "description": "Used as a card title description on the set password page to explain why the user is there" diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 20270828f7..8b7cbf5059 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -1,4 +1,7 @@ +import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; + import { NotificationQueueMessageTypes } from "../../enums/notification-queue-message-type.enum"; +import AutofillPageDetails from "../../models/autofill-page-details"; interface NotificationQueueMessage { type: NotificationQueueMessageTypes; @@ -49,6 +52,10 @@ type LockedVaultPendingNotificationsData = { target: string; }; +type AdjustNotificationBarMessageData = { + height: number; +}; + type ChangePasswordMessageData = { currentPassword: string; newPassword: string; @@ -61,6 +68,47 @@ type AddLoginMessageData = { url: string; }; +type UnlockVaultMessageData = { + skipNotification?: boolean; +}; + +type NotificationBackgroundExtensionMessage = { + [key: string]: any; + command: string; + data?: Partial & + Partial & + Partial & + Partial; + login?: AddLoginMessageData; + folder?: string; + edit?: boolean; + details?: AutofillPageDetails; + tab?: chrome.tabs.Tab; + sender?: string; + notificationType?: string; +}; + +type BackgroundMessageParam = { message: NotificationBackgroundExtensionMessage }; +type BackgroundSenderParam = { sender: chrome.runtime.MessageSender }; +type BackgroundOnMessageHandlerParams = BackgroundMessageParam & BackgroundSenderParam; + +type NotificationBackgroundExtensionMessageHandlers = { + [key: string]: CallableFunction; + unlockCompleted: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgGetFolderData: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgCloseNotificationBar: ({ sender }: BackgroundSenderParam) => Promise; + bgAdjustNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgAddLogin: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgChangedPassword: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void; + bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; + bgNeverSave: ({ sender }: BackgroundSenderParam) => Promise; + bgUnlockPopoutOpened: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgReopenUnlockPopout: ({ sender }: BackgroundSenderParam) => Promise; + checkNotificationQueue: ({ sender }: BackgroundSenderParam) => Promise; + collectPageDetailsResponse: ({ message }: BackgroundMessageParam) => Promise; +}; + export { AddChangePasswordQueueMessage, AddLoginQueueMessage, @@ -68,6 +116,10 @@ export { AddRequestFilelessImportQueueMessage, NotificationQueueMessageItem, LockedVaultPendingNotificationsData, + AdjustNotificationBarMessageData, ChangePasswordMessageData, + UnlockVaultMessageData, AddLoginMessageData, + NotificationBackgroundExtensionMessage, + NotificationBackgroundExtensionMessageHandlers, }; diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index ceb6035026..5bee7d4093 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -1,18 +1,41 @@ import { mock } from "jest-mock-extended"; +import { firstValueFrom } from "rxjs"; import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { EnvironmentService } from "@bitwarden/common/platform/services/environment.service"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; import { FolderService } from "@bitwarden/common/vault/services/folder/folder.service"; +import { BrowserApi } from "../../platform/browser/browser-api"; import { BrowserStateService } from "../../platform/services/browser-state.service"; +import { NotificationQueueMessageType } from "../enums/notification-queue-message-type.enum"; +import { FormData } from "../services/abstractions/autofill.service"; import AutofillService from "../services/autofill.service"; -import { createChromeTabMock } from "../spec/autofill-mocks"; +import { createAutofillPageDetailsMock, createChromeTabMock } from "../spec/autofill-mocks"; +import { flushPromises, sendExtensionRuntimeMessage } from "../spec/testing-utils"; -import { AddLoginQueueMessage } from "./abstractions/notification.background"; +import { + AddLoginQueueMessage, + AddUnlockVaultQueueMessage, + LockedVaultPendingNotificationsData, + NotificationBackgroundExtensionMessage, +} from "./abstractions/notification.background"; import NotificationBackground from "./notification.background"; +jest.mock("rxjs", () => { + const rxjs = jest.requireActual("rxjs"); + const { firstValueFrom } = rxjs; + return { + ...rxjs, + firstValueFrom: jest.fn(firstValueFrom), + }; +}); + describe("NotificationBackground", () => { let notificationBackground: NotificationBackground; const autofillService = mock(); @@ -22,6 +45,7 @@ describe("NotificationBackground", () => { const folderService = mock(); const stateService = mock(); const environmentService = mock(); + const logService = mock(); beforeEach(() => { notificationBackground = new NotificationBackground( @@ -32,6 +56,7 @@ describe("NotificationBackground", () => { folderService, stateService, environmentService, + logService, ); }); @@ -39,20 +64,6 @@ describe("NotificationBackground", () => { jest.clearAllMocks(); }); - describe("unlockVault", () => { - it("returns early if the message indicates that the notification should be skipped", async () => { - const tabMock = createChromeTabMock(); - const message = { data: { skipNotification: true } }; - jest.spyOn(notificationBackground["authService"], "getAuthStatus"); - jest.spyOn(notificationBackground as any, "pushUnlockVaultToQueue"); - - await notificationBackground["unlockVault"](message, tabMock); - - expect(notificationBackground["authService"].getAuthStatus).not.toHaveBeenCalled(); - expect(notificationBackground["pushUnlockVaultToQueue"]).not.toHaveBeenCalled(); - }); - }); - describe("convertAddLoginQueueMessageToCipherView", () => { it("returns a cipher view when passed an `AddLoginQueueMessage`", () => { const message: AddLoginQueueMessage = { @@ -108,4 +119,799 @@ describe("NotificationBackground", () => { expect(cipherView.folderId).toEqual(folderId); }); }); + + describe("notification bar extension message handlers", () => { + beforeEach(async () => { + await notificationBackground.init(); + }); + + it("ignores messages whose command does not match the expected handlers", () => { + const message: NotificationBackgroundExtensionMessage = { command: "unknown" }; + jest.spyOn(notificationBackground as any, "handleSaveCipherMessage"); + + sendExtensionRuntimeMessage(message); + + expect(notificationBackground["handleSaveCipherMessage"]).not.toHaveBeenCalled(); + }); + + describe("unlockCompleted message handler", () => { + it("sends a `closeNotificationBar` message if the retryCommand is for `autofill_login", async () => { + const sender = mock({ tab: { id: 1 } }); + const message: NotificationBackgroundExtensionMessage = { + command: "unlockCompleted", + data: { + commandToRetry: { message: { command: "autofill_login" } }, + } as LockedVaultPendingNotificationsData, + }; + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + sender.tab, + "closeNotificationBar", + ); + }); + + it("triggers a retryHandler if the message target is `notification.background` and a handler exists", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "unlockCompleted", + data: { + commandToRetry: { message: { command: "bgSaveCipher" } }, + target: "notification.background", + } as LockedVaultPendingNotificationsData, + }; + jest.spyOn(notificationBackground as any, "handleSaveCipherMessage").mockImplementation(); + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(notificationBackground["handleSaveCipherMessage"]).toHaveBeenCalledWith( + message.data.commandToRetry.message, + message.data.commandToRetry.sender, + ); + }); + }); + + describe("bgGetFolderData message handler", () => { + it("returns a list of folders", async () => { + const folderView = mock({ id: "folder-id" }); + const folderViews = [folderView]; + const message: NotificationBackgroundExtensionMessage = { + command: "bgGetFolderData", + }; + jest.spyOn(notificationBackground as any, "getFolderData"); + (firstValueFrom as jest.Mock).mockResolvedValueOnce(folderViews); + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(notificationBackground["getFolderData"]).toHaveBeenCalled(); + expect(firstValueFrom).toHaveBeenCalled(); + }); + }); + + describe("bgCloseNotificationBar message handler", () => { + it("sends a `closeNotificationBar` message to the sender tab", async () => { + const sender = mock({ tab: { id: 1 } }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgCloseNotificationBar", + }; + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + sender.tab, + "closeNotificationBar", + ); + }); + }); + + describe("bgAdjustNotificationBar message handler", () => { + it("sends a `adjustNotificationBar` message to the sender tab", async () => { + const sender = mock({ tab: { id: 1 } }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgAdjustNotificationBar", + data: { height: 100 }, + }; + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + sender.tab, + "adjustNotificationBar", + message.data, + ); + }); + }); + + describe("bgAddLogin message handler", () => { + let tab: chrome.tabs.Tab; + let sender: chrome.runtime.MessageSender; + let getAuthStatusSpy: jest.SpyInstance; + let getDisableAddLoginNotificationSpy: jest.SpyInstance; + let getDisableChangedPasswordNotificationSpy: jest.SpyInstance; + let pushAddLoginToQueueSpy: jest.SpyInstance; + let pushChangePasswordToQueueSpy: jest.SpyInstance; + let getAllDecryptedForUrlSpy: jest.SpyInstance; + + beforeEach(() => { + tab = createChromeTabMock(); + sender = mock({ tab }); + getAuthStatusSpy = jest.spyOn(authService, "getAuthStatus"); + getDisableAddLoginNotificationSpy = jest.spyOn( + stateService, + "getDisableAddLoginNotification", + ); + getDisableChangedPasswordNotificationSpy = jest.spyOn( + stateService, + "getDisableChangedPasswordNotification", + ); + pushAddLoginToQueueSpy = jest.spyOn(notificationBackground as any, "pushAddLoginToQueue"); + pushChangePasswordToQueueSpy = jest.spyOn( + notificationBackground as any, + "pushChangePasswordToQueue", + ); + getAllDecryptedForUrlSpy = jest.spyOn(cipherService, "getAllDecryptedForUrl"); + }); + + it("skips attempting to add the login if the user is logged out", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgAddLogin", + login: { username: "test", password: "password", url: "https://example.com" }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.LoggedOut); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getDisableAddLoginNotificationSpy).not.toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips attempting to add the login if the login data does not contain a valid url", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgAddLogin", + login: { username: "test", password: "password", url: "" }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getDisableAddLoginNotificationSpy).not.toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips attempting to add the login if the user with a locked vault has disabled the login notification", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgAddLogin", + login: { username: "test", password: "password", url: "https://example.com" }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(true); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).not.toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips attempting to add the login if the user with an unlocked vault has disabled the login notification", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgAddLogin", + login: { username: "test", password: "password", url: "https://example.com" }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(true); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips attempting to change the password for an existing login if the user has disabled changing the password notification", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgAddLogin", + login: { username: "test", password: "password", url: "https://example.com" }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getDisableChangedPasswordNotificationSpy.mockReturnValueOnce(true); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ login: { username: "test", password: "oldPassword" } }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); + expect(getDisableChangedPasswordNotificationSpy).toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips attempting to change the password for an existing login if the password has not changed", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgAddLogin", + login: { username: "test", password: "password", url: "https://example.com" }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ login: { username: "test", password: "password" } }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("adds the login to the queue if the user has a locked account", async () => { + const login = { username: "test", password: "password", url: "https://example.com" }; + const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).toHaveBeenCalledWith("example.com", login, sender.tab, true); + }); + + it("adds the login to the queue if the user has an unlocked account and the login is new", async () => { + const login = { + username: undefined, + password: "password", + url: "https://example.com", + } as any; + const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ login: { username: "anotherTestUsername", password: "password" } }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(pushAddLoginToQueueSpy).toHaveBeenCalledWith("example.com", login, sender.tab); + }); + + it("adds a change password message to the queue if the user has changed an existing cipher's password", async () => { + const login = { username: "tEsT", password: "password", url: "https://example.com" }; + const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ + id: "cipher-id", + login: { username: "test", password: "oldPassword" }, + }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( + "cipher-id", + "example.com", + login.password, + sender.tab, + ); + }); + }); + + describe("bgChangedPassword message handler", () => { + let tab: chrome.tabs.Tab; + let sender: chrome.runtime.MessageSender; + let getAuthStatusSpy: jest.SpyInstance; + let pushChangePasswordToQueueSpy: jest.SpyInstance; + let getAllDecryptedForUrlSpy: jest.SpyInstance; + + beforeEach(() => { + tab = createChromeTabMock(); + sender = mock({ tab }); + getAuthStatusSpy = jest.spyOn(authService, "getAuthStatus"); + pushChangePasswordToQueueSpy = jest.spyOn( + notificationBackground as any, + "pushChangePasswordToQueue", + ); + getAllDecryptedForUrlSpy = jest.spyOn(cipherService, "getAllDecryptedForUrl"); + }); + + it("skips attempting to add the change password message to the queue if the passed url is not valid", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { newPassword: "newPassword", currentPassword: "currentPassword", url: "" }, + }; + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("adds a change password message to the queue if the user does not have an unlocked account", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { + newPassword: "newPassword", + currentPassword: "currentPassword", + url: "https://example.com", + }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( + null, + "example.com", + message.data.newPassword, + sender.tab, + true, + ); + }); + + it("skips adding a change password message to the queue if the multiple ciphers exist for the passed URL and the current password is not found within the list of ciphers", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { + newPassword: "newPassword", + currentPassword: "currentPassword", + url: "https://example.com", + }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ login: { username: "test", password: "password" } }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips adding a change password message if more than one existing cipher is found with a matching password ", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { + newPassword: "newPassword", + currentPassword: "currentPassword", + url: "https://example.com", + }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ login: { username: "test", password: "password" } }), + mock({ login: { username: "test2", password: "password" } }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("adds a change password message to the queue if a single cipher matches the passed current password", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { + newPassword: "newPassword", + currentPassword: "currentPassword", + url: "https://example.com", + }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ + id: "cipher-id", + login: { username: "test", password: "currentPassword" }, + }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( + "cipher-id", + "example.com", + message.data.newPassword, + sender.tab, + ); + }); + + it("skips adding a change password message if no current password is passed in the message and more than one cipher is found for a url", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { + newPassword: "newPassword", + url: "https://example.com", + }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ login: { username: "test", password: "password" } }), + mock({ login: { username: "test2", password: "password" } }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); + expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); + }); + + it("adds a change password message to the queue if no current password is passed with the message, but a single cipher is matched for the uri", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgChangedPassword", + data: { + newPassword: "newPassword", + url: "https://example.com", + }, + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); + getAllDecryptedForUrlSpy.mockResolvedValueOnce([ + mock({ + id: "cipher-id", + login: { username: "test", password: "password" }, + }), + ]); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( + "cipher-id", + "example.com", + message.data.newPassword, + sender.tab, + ); + }); + }); + + describe("bgRemoveTabFromNotificationQueue message handler", () => { + it("splices a notification queue item based on the passed tab", async () => { + const tab = createChromeTabMock({ id: 2 }); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgRemoveTabFromNotificationQueue", + }; + const removeTabFromNotificationQueueSpy = jest.spyOn( + notificationBackground as any, + "removeTabFromNotificationQueue", + ); + const firstQueueMessage = mock({ + tab: createChromeTabMock({ id: 1 }), + }); + const secondQueueMessage = mock({ tab }); + const thirdQueueMessage = mock({ + tab: createChromeTabMock({ id: 3 }), + }); + notificationBackground["notificationQueue"] = [ + firstQueueMessage, + secondQueueMessage, + thirdQueueMessage, + ]; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(removeTabFromNotificationQueueSpy).toHaveBeenCalledWith(tab); + expect(notificationBackground["notificationQueue"]).toEqual([ + firstQueueMessage, + thirdQueueMessage, + ]); + }); + }); + + describe("bgNeverSave message handler", () => { + let tabSendMessageDataSpy: jest.SpyInstance; + + beforeEach(() => { + tabSendMessageDataSpy = jest.spyOn(BrowserApi, "tabSendMessageData"); + }); + + it("skips saving the domain as a never value if the passed tab does not exist within the notification queue", async () => { + const tab = createChromeTabMock({ id: 2 }); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { command: "bgNeverSave" }; + notificationBackground["notificationQueue"] = [ + mock({ + tab: createChromeTabMock({ id: 1 }), + }), + mock({ + tab: createChromeTabMock({ id: 3 }), + }), + ]; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(tabSendMessageDataSpy).not.toHaveBeenCalled(); + }); + + it("skips saving the domain as a never value if the tab does not contain an addLogin message within the NotificationQueue", async () => { + const tab = createChromeTabMock({ id: 2 }); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { command: "bgNeverSave" }; + notificationBackground["notificationQueue"] = [ + mock({ type: NotificationQueueMessageType.UnlockVault, tab }), + ]; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(tabSendMessageDataSpy).not.toHaveBeenCalled(); + }); + + it("skips saving the domain as a never value if the tab url does not match the queue message domain", async () => { + const tab = createChromeTabMock({ id: 2, url: "https://example.com" }); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { command: "bgNeverSave" }; + notificationBackground["notificationQueue"] = [ + mock({ + type: NotificationQueueMessageType.AddLogin, + tab, + domain: "another.com", + }), + ]; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(tabSendMessageDataSpy).not.toHaveBeenCalled(); + }); + + it("saves the tabs domain as a never value and closes the notification bar", async () => { + const tab = createChromeTabMock({ id: 2, url: "https://example.com" }); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { command: "bgNeverSave" }; + const firstNotification = mock({ + type: NotificationQueueMessageType.AddLogin, + tab, + domain: "example.com", + }); + const secondNotification = mock({ + type: NotificationQueueMessageType.AddLogin, + tab: createChromeTabMock({ id: 3 }), + domain: "another.com", + }); + notificationBackground["notificationQueue"] = [firstNotification, secondNotification]; + jest.spyOn(cipherService, "saveNeverDomain").mockImplementation(); + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(tabSendMessageDataSpy).toHaveBeenCalledWith(tab, "closeNotificationBar"); + expect(cipherService.saveNeverDomain).toHaveBeenCalledWith("example.com"); + expect(notificationBackground["notificationQueue"]).toEqual([secondNotification]); + }); + }); + + describe("collectPageDetailsResponse", () => { + let tabSendMessageDataSpy: jest.SpyInstance; + + beforeEach(() => { + tabSendMessageDataSpy = jest.spyOn(BrowserApi, "tabSendMessageData"); + }); + + it("skips sending the `notificationBarPageDetails` message if the message sender is not `notificationBar`", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "collectPageDetailsResponse", + sender: "not-notificationBar", + }; + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(tabSendMessageDataSpy).not.toHaveBeenCalled(); + }); + + it("sends a `notificationBarPageDetails` message with the forms with password fields", async () => { + const tab = createChromeTabMock(); + const message: NotificationBackgroundExtensionMessage = { + command: "collectPageDetailsResponse", + sender: "notificationBar", + details: createAutofillPageDetailsMock(), + tab, + }; + const formData = [mock()]; + jest.spyOn(autofillService, "getFormsWithPasswordFields").mockReturnValueOnce(formData); + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(tabSendMessageDataSpy).toHaveBeenCalledWith( + message.tab, + "notificationBarPageDetails", + { + details: message.details, + forms: formData, + }, + ); + }); + }); + + describe("bgUnlockPopoutOpened message handler", () => { + let getAuthStatusSpy: jest.SpyInstance; + let pushUnlockVaultToQueueSpy: jest.SpyInstance; + + beforeEach(() => { + getAuthStatusSpy = jest.spyOn(authService, "getAuthStatus"); + pushUnlockVaultToQueueSpy = jest.spyOn( + notificationBackground as any, + "pushUnlockVaultToQueue", + ); + }); + + it("skips pushing the unlock vault message to the queue if the message indicates that the notification should be skipped", async () => { + const tabMock = createChromeTabMock(); + const sender = mock({ tab: tabMock }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgUnlockPopoutOpened", + data: { skipNotification: true }, + }; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).not.toHaveBeenCalled(); + expect(pushUnlockVaultToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips pushing the unlock vault message to the queue if the auth status is not `Locked`", async () => { + const tabMock = createChromeTabMock(); + const sender = mock({ tab: tabMock }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgUnlockPopoutOpened", + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.LoggedOut); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(getAuthStatusSpy).toHaveBeenCalled(); + expect(pushUnlockVaultToQueueSpy).not.toHaveBeenCalled(); + }); + + it("skips pushing the unlock vault message to the queue if the notification queue already has an item", async () => { + const tabMock = createChromeTabMock(); + const sender = mock({ tab: tabMock }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgUnlockPopoutOpened", + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); + notificationBackground["notificationQueue"] = [mock()]; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(pushUnlockVaultToQueueSpy).not.toHaveBeenCalled(); + }); + + it("sends an unlock vault message to the queue if the user has a locked vault", async () => { + const tabMock = createChromeTabMock({ url: "https://example.com" }); + const sender = mock({ tab: tabMock }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgUnlockPopoutOpened", + }; + getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(pushUnlockVaultToQueueSpy).toHaveBeenCalledWith("example.com", sender.tab); + }); + }); + + describe("checkNotificationQueue", () => { + let doNotificationQueueCheckSpy: jest.SpyInstance; + let getTabFromCurrentWindowSpy: jest.SpyInstance; + + beforeEach(() => { + doNotificationQueueCheckSpy = jest.spyOn( + notificationBackground as any, + "doNotificationQueueCheck", + ); + getTabFromCurrentWindowSpy = jest.spyOn(BrowserApi, "getTabFromCurrentWindow"); + }); + + it("skips checking the notification queue if the queue does not contain any items", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "checkNotificationQueue", + }; + notificationBackground["notificationQueue"] = []; + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(doNotificationQueueCheckSpy).not.toHaveBeenCalled(); + }); + + it("checks the notification queue for the sender tab", async () => { + const tab = createChromeTabMock(); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { + command: "checkNotificationQueue", + }; + notificationBackground["notificationQueue"] = [ + mock({ tab }), + mock({ tab: createChromeTabMock({ id: 2 }) }), + ]; + + sendExtensionRuntimeMessage(message, sender); + await flushPromises(); + + expect(doNotificationQueueCheckSpy).toHaveBeenCalledWith(tab); + }); + + it("checks the notification queue for the current tab if the sender does not send a tab", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "checkNotificationQueue", + }; + const currenTab = createChromeTabMock({ id: 2 }); + notificationBackground["notificationQueue"] = [ + mock({ tab: currenTab }), + ]; + getTabFromCurrentWindowSpy.mockResolvedValueOnce(currenTab); + + sendExtensionRuntimeMessage(message, mock({ tab: null })); + await flushPromises(); + + expect(getTabFromCurrentWindowSpy).toHaveBeenCalledWith(); + expect(doNotificationQueueCheckSpy).toHaveBeenCalledWith(currenTab); + }); + }); + + describe("bgReopenUnlockPopout message handler", () => { + it("opens the unlock popout window", async () => { + const message: NotificationBackgroundExtensionMessage = { + command: "bgReopenUnlockPopout", + }; + const openUnlockWindowSpy = jest.spyOn(notificationBackground as any, "openUnlockPopout"); + + sendExtensionRuntimeMessage(message); + await flushPromises(); + + expect(openUnlockWindowSpy).toHaveBeenCalled(); + }); + }); + }); }); diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 4da80b7695..cb49e47eb6 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -5,6 +5,7 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ThemeType } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -31,10 +32,32 @@ import { AddLoginMessageData, NotificationQueueMessageItem, LockedVaultPendingNotificationsData, + NotificationBackgroundExtensionMessage, + NotificationBackgroundExtensionMessageHandlers, } from "./abstractions/notification.background"; +import { OverlayBackgroundExtensionMessage } from "./abstractions/overlay.background"; export default class NotificationBackground { + private openUnlockPopout = openUnlockPopout; private notificationQueue: NotificationQueueMessageItem[] = []; + private readonly extensionMessageHandlers: NotificationBackgroundExtensionMessageHandlers = { + unlockCompleted: ({ message, sender }) => this.handleUnlockCompleted(message, sender), + bgGetFolderData: () => this.getFolderData(), + bgCloseNotificationBar: ({ sender }) => this.handleCloseNotificationBarMessage(sender), + bgAdjustNotificationBar: ({ message, sender }) => + this.handleAdjustNotificationBarMessage(message, sender), + bgAddLogin: ({ message, sender }) => this.addLogin(message, sender), + bgChangedPassword: ({ message, sender }) => this.changedPassword(message, sender), + bgRemoveTabFromNotificationQueue: ({ sender }) => + this.removeTabFromNotificationQueue(sender.tab), + bgSaveCipher: ({ message, sender }) => this.handleSaveCipherMessage(message, sender), + bgNeverSave: ({ sender }) => this.saveNever(sender.tab), + collectPageDetailsResponse: ({ message }) => + this.handleCollectPageDetailsResponseMessage(message), + bgUnlockPopoutOpened: ({ message, sender }) => this.unlockVault(message, sender.tab), + checkNotificationQueue: ({ sender }) => this.checkNotificationQueue(sender.tab), + bgReopenUnlockPopout: ({ sender }) => this.openUnlockPopout(sender.tab), + }; constructor( private autofillService: AutofillService, @@ -44,6 +67,7 @@ export default class NotificationBackground { private folderService: FolderService, private stateService: BrowserStateService, private environmentService: EnvironmentService, + private logService: LogService, ) {} async init() { @@ -51,95 +75,17 @@ export default class NotificationBackground { return; } - BrowserApi.messageListener( - "notification.background", - (msg: any, sender: chrome.runtime.MessageSender) => { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.processMessage(msg, sender); - }, - ); + this.setupExtensionMessageListener(); this.cleanupNotificationQueue(); } - async processMessage(msg: any, sender: chrome.runtime.MessageSender) { - switch (msg.command) { - case "unlockCompleted": - await this.handleUnlockCompleted(msg.data, sender); - break; - case "bgGetDataForTab": - await this.getDataForTab(sender.tab, msg.responseCommand); - break; - case "bgCloseNotificationBar": - await BrowserApi.tabSendMessageData(sender.tab, "closeNotificationBar"); - break; - case "bgAdjustNotificationBar": - await BrowserApi.tabSendMessageData(sender.tab, "adjustNotificationBar", msg.data); - break; - case "bgAddLogin": - await this.addLogin(msg.login, sender.tab); - break; - case "bgChangedPassword": - await this.changedPassword(msg.data, sender.tab); - break; - case "bgAddClose": - case "bgChangeClose": - this.removeTabFromNotificationQueue(sender.tab); - break; - case "bgAddSave": - case "bgChangeSave": - if ((await this.authService.getAuthStatus()) < AuthenticationStatus.Unlocked) { - const retryMessage: LockedVaultPendingNotificationsData = { - commandToRetry: { - message: { - command: msg, - }, - sender: sender, - }, - target: "notification.background", - }; - await BrowserApi.tabSendMessageData( - sender.tab, - "addToLockedVaultPendingNotifications", - retryMessage, - ); - await openUnlockPopout(sender.tab); - return; - } - await this.saveOrUpdateCredentials(sender.tab, msg.edit, msg.folder); - break; - case "bgNeverSave": - await this.saveNever(sender.tab); - break; - case "collectPageDetailsResponse": - switch (msg.sender) { - case "notificationBar": { - const forms = this.autofillService.getFormsWithPasswordFields(msg.details); - await BrowserApi.tabSendMessageData(msg.tab, "notificationBarPageDetails", { - details: msg.details, - forms: forms, - }); - break; - } - default: - break; - } - break; - case "bgUnlockPopoutOpened": - await this.unlockVault(msg, sender.tab); - break; - case "checkNotificationQueue": - await this.checkNotificationQueue(sender.tab); - break; - case "bgReopenUnlockPopout": - await openUnlockPopout(sender.tab); - break; - default: - break; - } - } - + /** + * Checks the notification queue for any messages that need to be sent to the + * specified tab. If no tab is specified, the current tab will be used. + * + * @param tab - The tab to check the notification queue for + */ async checkNotificationQueue(tab: chrome.tabs.Tab = null): Promise { if (this.notificationQueue.length === 0) { return; @@ -159,9 +105,9 @@ export default class NotificationBackground { private cleanupNotificationQueue() { for (let i = this.notificationQueue.length - 1; i >= 0; i--) { if (this.notificationQueue[i].expires < new Date()) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - BrowserApi.tabSendMessageData(this.notificationQueue[i].tab, "closeNotificationBar"); + BrowserApi.tabSendMessageData(this.notificationQueue[i].tab, "closeNotificationBar").catch( + (error) => this.logService.error(error), + ); this.notificationQueue.splice(i, 1); } } @@ -178,9 +124,7 @@ export default class NotificationBackground { (message) => message.tab.id === tab.id && message.domain === tabDomain, ); if (queueMessage) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.sendNotificationQueueMessage(tab, queueMessage); + await this.sendNotificationQueueMessage(tab, queueMessage); } } @@ -225,6 +169,12 @@ export default class NotificationBackground { : ThemeType.Light; } + /** + * Removes any login messages from the notification queue that + * are associated with the specified tab. + * + * @param tab - The tab to remove messages for + */ private removeTabFromNotificationQueue(tab: chrome.tabs.Tab) { for (let i = this.notificationQueue.length - 1; i >= 0; i--) { if (this.notificationQueue[i].tab.id === tab.id) { @@ -233,31 +183,36 @@ export default class NotificationBackground { } } - private async addLogin(loginInfo: AddLoginMessageData, tab: chrome.tabs.Tab) { + /** + * Adds a login message to the notification queue, prompting the user to save + * the login if it does not already exist in the vault. If the cipher exists + * but the password has changed, the user will be prompted to update the password. + * + * @param message - The message to add to the queue + * @param sender - The contextual sender of the message + */ + private async addLogin( + message: NotificationBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { const authStatus = await this.authService.getAuthStatus(); if (authStatus === AuthenticationStatus.LoggedOut) { return; } + const loginInfo = message.login; + const normalizedUsername = loginInfo.username ? loginInfo.username.toLowerCase() : ""; const loginDomain = Utils.getDomain(loginInfo.url); if (loginDomain == null) { return; } - let normalizedUsername = loginInfo.username; - if (normalizedUsername != null) { - normalizedUsername = normalizedUsername.toLowerCase(); - } - const disabledAddLogin = await this.stateService.getDisableAddLoginNotification(); if (authStatus === AuthenticationStatus.Locked) { - if (disabledAddLogin) { - return; + if (!disabledAddLogin) { + await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab, true); } - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushAddLoginToQueue(loginDomain, loginInfo, tab, true); return; } @@ -265,26 +220,23 @@ export default class NotificationBackground { const usernameMatches = ciphers.filter( (c) => c.login.username != null && c.login.username.toLowerCase() === normalizedUsername, ); - if (usernameMatches.length === 0) { - if (disabledAddLogin) { - return; - } + if (!disabledAddLogin && usernameMatches.length === 0) { + await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab); + return; + } - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushAddLoginToQueue(loginDomain, loginInfo, tab); - } else if ( + const disabledChangePassword = await this.stateService.getDisableChangedPasswordNotification(); + if ( + !disabledChangePassword && usernameMatches.length === 1 && usernameMatches[0].login.password !== loginInfo.password ) { - const disabledChangePassword = - await this.stateService.getDisableChangedPasswordNotification(); - if (disabledChangePassword) { - return; - } - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushChangePasswordToQueue(usernameMatches[0].id, loginDomain, loginInfo.password, tab); + await this.pushChangePasswordToQueue( + usernameMatches[0].id, + loginDomain, + loginInfo.password, + sender.tab, + ); } } @@ -310,16 +262,31 @@ export default class NotificationBackground { await this.checkNotificationQueue(tab); } - private async changedPassword(changeData: ChangePasswordMessageData, tab: chrome.tabs.Tab) { + /** + * Adds a change password message to the notification queue, prompting the user + * to update the password for a login that has changed. + * + * @param message - The message to add to the queue + * @param sender - The contextual sender of the message + */ + private async changedPassword( + message: NotificationBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { + const changeData = message.data as ChangePasswordMessageData; const loginDomain = Utils.getDomain(changeData.url); if (loginDomain == null) { return; } if ((await this.authService.getAuthStatus()) < AuthenticationStatus.Unlocked) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushChangePasswordToQueue(null, loginDomain, changeData.newPassword, tab, true); + await this.pushChangePasswordToQueue( + null, + loginDomain, + changeData.newPassword, + sender.tab, + true, + ); return; } @@ -336,12 +303,30 @@ export default class NotificationBackground { id = ciphers[0].id; } if (id != null) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushChangePasswordToQueue(id, loginDomain, changeData.newPassword, tab); + await this.pushChangePasswordToQueue(id, loginDomain, changeData.newPassword, sender.tab); } } + /** + * Sends the page details to the notification bar. Will query all + * forms with a password field and pass them to the notification bar. + * + * @param message - The extension message + */ + private async handleCollectPageDetailsResponseMessage( + message: NotificationBackgroundExtensionMessage, + ) { + if (message.sender !== "notificationBar") { + return; + } + + const forms = this.autofillService.getFormsWithPasswordFields(message.details); + await BrowserApi.tabSendMessageData(message.tab, "notificationBarPageDetails", { + details: message.details, + forms: forms, + }); + } + /** * Sets up a notification to unlock the vault when the user * attempts to autofill a cipher while the vault is locked. @@ -349,10 +334,7 @@ export default class NotificationBackground { * @param message - Extension message, determines if the notification should be skipped * @param tab - The tab that the message was sent from */ - private async unlockVault( - message: { data?: { skipNotification?: boolean } }, - tab: chrome.tabs.Tab, - ) { + private async unlockVault(message: NotificationBackgroundExtensionMessage, tab: chrome.tabs.Tab) { if (message.data?.skipNotification) { return; } @@ -364,9 +346,7 @@ export default class NotificationBackground { const loginDomain = Utils.getDomain(tab.url); if (loginDomain) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushUnlockVaultToQueue(loginDomain, tab); + await this.pushUnlockVaultToQueue(loginDomain, tab); } } @@ -385,9 +365,7 @@ export default class NotificationBackground { const loginDomain = Utils.getDomain(tab.url); if (loginDomain) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.pushRequestFilelessImportToQueue(loginDomain, tab, importType); + await this.pushRequestFilelessImportToQueue(loginDomain, tab, importType); } } @@ -453,6 +431,29 @@ export default class NotificationBackground { this.removeTabFromNotificationQueue(tab); } + private async handleSaveCipherMessage( + message: NotificationBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { + if ((await this.authService.getAuthStatus()) < AuthenticationStatus.Unlocked) { + await BrowserApi.tabSendMessageData(sender.tab, "addToLockedVaultPendingNotifications", { + commandToRetry: { + message: { + command: message.command, + edit: message.edit, + folder: message.folder, + }, + sender: sender, + }, + target: "notification.background", + } as LockedVaultPendingNotificationsData); + await this.openUnlockPopout(sender.tab); + return; + } + + await this.saveOrUpdateCredentials(sender.tab, message.edit, message.folder); + } + private async saveOrUpdateCredentials(tab: chrome.tabs.Tab, edit: boolean, folderId?: string) { for (let i = this.notificationQueue.length - 1; i >= 0; i--) { const queueMessage = this.notificationQueue[i]; @@ -470,9 +471,9 @@ export default class NotificationBackground { } this.notificationQueue.splice(i, 1); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - BrowserApi.tabSendMessageData(tab, "closeNotificationBar"); + BrowserApi.tabSendMessageData(tab, "closeNotificationBar").catch((error) => + this.logService.error(error), + ); if (queueMessage.type === NotificationQueueMessageType.ChangePassword) { const cipherView = await this.getDecryptedCipherById(queueMessage.cipherId); @@ -505,9 +506,9 @@ export default class NotificationBackground { const cipher = await this.cipherService.encrypt(newCipher); await this.cipherService.createWithServer(cipher); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - BrowserApi.tabSendMessageData(tab, "addedCipher"); + BrowserApi.tabSendMessageData(tab, "addedCipher").catch((error) => + this.logService.error(error), + ); } } } @@ -522,9 +523,7 @@ export default class NotificationBackground { if (edit) { await this.editItem(cipherView, tab); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - BrowserApi.tabSendMessage(tab, "editedCipher"); + await BrowserApi.tabSendMessage(tab, { command: "editedCipher" }); return; } @@ -560,6 +559,11 @@ export default class NotificationBackground { return null; } + /** + * Saves the current tab's domain to the never save list. + * + * @param tab - The tab that sent the neverSave message + */ private async saveNever(tab: chrome.tabs.Tab) { for (let i = this.notificationQueue.length - 1; i >= 0; i--) { const queueMessage = this.notificationQueue[i]; @@ -576,22 +580,18 @@ export default class NotificationBackground { } this.notificationQueue.splice(i, 1); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - BrowserApi.tabSendMessageData(tab, "closeNotificationBar"); + await BrowserApi.tabSendMessageData(tab, "closeNotificationBar"); const hostname = Utils.getHostname(tab.url); await this.cipherService.saveNeverDomain(hostname); } } - private async getDataForTab(tab: chrome.tabs.Tab, responseCommand: string) { - const responseData: any = {}; - if (responseCommand === "notificationBarGetFoldersList") { - responseData.folders = await firstValueFrom(this.folderService.folderViews$); - } - - await BrowserApi.tabSendMessageData(tab, responseCommand, responseData); + /** + * Returns the first value found from the folder service's folderViews$ observable. + */ + private async getFolderData() { + return await firstValueFrom(this.folderService.folderViews$); } private async removeIndividualVault(): Promise { @@ -600,11 +600,21 @@ export default class NotificationBackground { ); } + /** + * Handles the unlockCompleted extension message. Will close the notification bar + * after an attempted autofill action, and retry the autofill action if the message + * contains a follow-up command. + * + * @param message - The extension message + * @param sender - The contextual sender of the message + */ private async handleUnlockCompleted( - messageData: LockedVaultPendingNotificationsData, + message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, ): Promise { - if (messageData.commandToRetry.message.command === "autofill_login") { + const messageData = message.data as LockedVaultPendingNotificationsData; + const retryCommand = messageData.commandToRetry.message.command; + if (retryCommand === "autofill_login") { await BrowserApi.tabSendMessageData(sender.tab, "closeNotificationBar"); } @@ -612,10 +622,37 @@ export default class NotificationBackground { return; } - await this.processMessage( - messageData.commandToRetry.message.command, - messageData.commandToRetry.sender, - ); + const retryHandler: CallableFunction | undefined = this.extensionMessageHandlers[retryCommand]; + if (retryHandler) { + retryHandler({ + message: messageData.commandToRetry.message, + sender: messageData.commandToRetry.sender, + }); + } + } + + /** + * Sends a message back to the sender tab which + * triggers closure of the notification bar. + * + * @param sender - The contextual sender of the message + */ + private async handleCloseNotificationBarMessage(sender: chrome.runtime.MessageSender) { + await BrowserApi.tabSendMessageData(sender.tab, "closeNotificationBar"); + } + + /** + * Sends a message back to the sender tab which triggers + * an CSS adjustment of the notification bar. + * + * @param message - The extension message + * @param sender - The contextual sender of the message + */ + private async handleAdjustNotificationBarMessage( + message: NotificationBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + ) { + await BrowserApi.tabSendMessageData(sender.tab, "adjustNotificationBar", message.data); } /** @@ -645,4 +682,29 @@ export default class NotificationBackground { return cipherView; } + + private setupExtensionMessageListener() { + BrowserApi.messageListener("notification.background", this.handleExtensionMessage); + } + + private handleExtensionMessage = ( + message: OverlayBackgroundExtensionMessage, + sender: chrome.runtime.MessageSender, + sendResponse: (response?: any) => void, + ) => { + const handler: CallableFunction | undefined = this.extensionMessageHandlers[message?.command]; + if (!handler) { + return; + } + + const messageResponse = handler({ message, sender }); + if (!messageResponse) { + return; + } + + Promise.resolve(messageResponse) + .then((response) => sendResponse(response)) + .catch((error) => this.logService.error(error)); + return true; + }; } diff --git a/apps/browser/src/autofill/content/notification-bar.ts b/apps/browser/src/autofill/content/notification-bar.ts index 568f22ff9c..175f5ada2c 100644 --- a/apps/browser/src/autofill/content/notification-bar.ts +++ b/apps/browser/src/autofill/content/notification-bar.ts @@ -30,6 +30,8 @@ interface HTMLElementWithFormOpId extends HTMLElement { * and async scripts to finish loading. * https://developer.mozilla.org/en-US/docs/Web/API/Window/DOMContentLoaded_event */ +let notificationBarIframe: HTMLIFrameElement = null; + if (document.readyState === "loading") { document.addEventListener("DOMContentLoaded", loadNotificationBar); } else { @@ -871,10 +873,11 @@ async function loadNotificationBar() { const barPageUrl: string = chrome.runtime.getURL(barPage); - const iframe = document.createElement("iframe"); - iframe.style.cssText = "height: 42px; width: 100%; border: 0; min-height: initial;"; - iframe.id = "bit-notification-bar-iframe"; - iframe.src = barPageUrl; + notificationBarIframe = document.createElement("iframe"); + notificationBarIframe.style.cssText = + "height: 42px; width: 100%; border: 0; min-height: initial;"; + notificationBarIframe.id = "bit-notification-bar-iframe"; + notificationBarIframe.src = barPageUrl; const frameDiv = document.createElement("div"); frameDiv.setAttribute("aria-live", "polite"); @@ -882,10 +885,10 @@ async function loadNotificationBar() { frameDiv.style.cssText = "height: 42px; width: 100%; top: 0; left: 0; padding: 0; position: fixed; " + "z-index: 2147483647; visibility: visible;"; - frameDiv.appendChild(iframe); + frameDiv.appendChild(notificationBarIframe); document.body.appendChild(frameDiv); - (iframe.contentWindow.location as any) = barPageUrl; + (notificationBarIframe.contentWindow.location as any) = barPageUrl; const spacer = document.createElement("div"); spacer.id = "bit-notification-bar-spacer"; @@ -897,6 +900,7 @@ async function loadNotificationBar() { const barEl = document.getElementById("bit-notification-bar"); if (barEl != null) { barEl.parentElement.removeChild(barEl); + notificationBarIframe = null; } const spacerEl = document.getElementById("bit-notification-bar-spacer"); @@ -910,13 +914,9 @@ async function loadNotificationBar() { switch (barType) { case "add": - sendPlatformMessage({ - command: "bgAddClose", - }); - break; case "change": sendPlatformMessage({ - command: "bgChangeClose", + command: "bgRemoveTabFromNotificationQueue", }); break; default: diff --git a/apps/browser/src/autofill/notification/bar.ts b/apps/browser/src/autofill/notification/bar.ts index 7ac04ba2c0..0dae1ab1fa 100644 --- a/apps/browser/src/autofill/notification/bar.ts +++ b/apps/browser/src/autofill/notification/bar.ts @@ -1,9 +1,8 @@ -import type { Jsonify } from "type-fest"; - import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import type { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { FilelessImportPort, FilelessImportType } from "../../tools/enums/fileless-import.enums"; +import { AdjustNotificationBarMessageData } from "../background/abstractions/notification.background"; require("./bar.scss"); @@ -155,11 +154,7 @@ function handleTypeAdd() { e.preventDefault(); // If Remove Individual Vault policy applies, "Add" opens the edit tab - sendPlatformMessage({ - command: "bgAddSave", - folder: getSelectedFolder(), - edit: removeIndividualVault(), - }); + sendSaveCipherMessage(removeIndividualVault(), getSelectedFolder()); }); if (removeIndividualVault()) { @@ -171,11 +166,7 @@ function handleTypeAdd() { editButton.addEventListener("click", (e) => { e.preventDefault(); - sendPlatformMessage({ - command: "bgAddSave", - folder: getSelectedFolder(), - edit: true, - }); + sendSaveCipherMessage(true, getSelectedFolder()); }); const neverButton = document.getElementById("never-save"); @@ -195,20 +186,22 @@ function handleTypeChange() { changeButton.addEventListener("click", (e) => { e.preventDefault(); - sendPlatformMessage({ - command: "bgChangeSave", - edit: false, - }); + sendSaveCipherMessage(false); }); const editButton = document.getElementById("change-edit"); editButton.addEventListener("click", (e) => { e.preventDefault(); - sendPlatformMessage({ - command: "bgChangeSave", - edit: true, - }); + sendSaveCipherMessage(true); + }); +} + +function sendSaveCipherMessage(edit: boolean, folder?: string) { + sendPlatformMessage({ + command: "bgSaveCipher", + folder, + edit, }); } @@ -281,33 +274,34 @@ function setContent(template: HTMLTemplateElement) { content.appendChild(newElement); } -function sendPlatformMessage(msg: Record) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - chrome.runtime.sendMessage(msg); +function sendPlatformMessage( + msg: Record, + responseCallback?: (response: any) => void, +) { + chrome.runtime.sendMessage(msg, (response) => { + if (responseCallback) { + responseCallback(response); + } + }); } function loadFolderSelector() { - const responseFoldersCommand = "notificationBarGetFoldersList"; - - chrome.runtime.onMessage.addListener((msg) => { - if (msg.command !== responseFoldersCommand || msg.data == null) { + const populateFolderData = (folderData: FolderView[]) => { + const select = document.getElementById("select-folder"); + if (!folderData?.length) { + select.appendChild(new Option(chrome.i18n.getMessage("noFoldersFound"), null, true)); + select.setAttribute("disabled", "true"); return; } - const folders = msg.data.folders as Jsonify; - const select = document.getElementById("select-folder"); select.appendChild(new Option(chrome.i18n.getMessage("selectFolder"), null, true)); - folders.forEach((folder) => { + folderData.forEach((folder: FolderView) => { // Select "No Folder" (id=null) folder by default select.appendChild(new Option(folder.name, folder.id || "", false)); }); - }); + }; - sendPlatformMessage({ - command: "bgGetDataForTab", - responseCommand: responseFoldersCommand, - }); + sendPlatformMessage({ command: "bgGetFolderData" }, populateFolderData); } function getSelectedFolder(): string { @@ -319,10 +313,11 @@ function removeIndividualVault(): boolean { } function adjustHeight() { + const data: AdjustNotificationBarMessageData = { + height: document.querySelector("body").scrollHeight, + }; sendPlatformMessage({ command: "bgAdjustNotificationBar", - data: { - height: document.querySelector("body").scrollHeight, - }, + data, }); } diff --git a/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts b/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts index cca96555b2..d556e5d52a 100644 --- a/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts +++ b/apps/browser/src/autofill/overlay/pages/shared/autofill-overlay-page-element.ts @@ -93,6 +93,10 @@ class AutofillOverlayPageElement extends HTMLElement { this.messageOrigin = event.origin; } + if (event.origin !== this.messageOrigin) { + return; + } + const message = event?.data; const handler = this.windowMessageHandlers[message?.command]; if (!handler) { diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 00dc3e3ebc..8388136202 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -815,6 +815,7 @@ export default class MainBackground { this.folderService, this.stateService, this.environmentService, + this.logService, ); this.overlayBackground = new OverlayBackground( this.cipherService,