diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 0cb7c2e4bd4..0eb79ec6b20 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -119,17 +119,19 @@ type NotificationBackgroundExtensionMessageHandlers = { sender, }: BackgroundOnMessageHandlerParams) => Promise; bgCloseNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgTriggerAtRiskPasswordNotification: ({ + bgAdjustNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgTriggerAddLoginNotification: ({ message, sender, - }: BackgroundOnMessageHandlerParams) => Promise; - bgOpenAtRisksPasswords: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgAdjustNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgAddLogin: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + }: BackgroundOnMessageHandlerParams) => Promise; bgTriggerChangedPasswordNotification: ({ message, sender, - }: BackgroundOnMessageHandlerParams) => Promise; + }: BackgroundOnMessageHandlerParams) => Promise; + bgTriggerAtRiskPasswordNotification: ({ + message, + sender, + }: BackgroundOnMessageHandlerParams) => Promise; bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void; bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; bgOpenAddEditVaultItemPopout: ({ diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index 009efd7ff36..1fd1aa34e02 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -274,7 +274,7 @@ describe("NotificationBackground", () => { }); }); - describe("bgAddLogin message handler", () => { + describe("bgTriggerAddLoginNotification message handler", () => { let tab: chrome.tabs.Tab; let sender: chrome.runtime.MessageSender; let getEnableAddedLoginPromptSpy: jest.SpyInstance; @@ -304,7 +304,7 @@ describe("NotificationBackground", () => { it("skips attempting to add the login if the user is logged out", async () => { const message: NotificationBackgroundExtensionMessage = { - command: "bgAddLogin", + command: "bgTriggerAddLoginNotification", login: { username: "test", password: "password", url: "https://example.com" }, }; activeAccountStatusMock$.next(AuthenticationStatus.LoggedOut); @@ -318,7 +318,7 @@ describe("NotificationBackground", () => { it("skips attempting to add the login if the login data does not contain a valid url", async () => { const message: NotificationBackgroundExtensionMessage = { - command: "bgAddLogin", + command: "bgTriggerAddLoginNotification", login: { username: "test", password: "password", url: "" }, }; activeAccountStatusMock$.next(AuthenticationStatus.Locked); @@ -332,7 +332,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerAddLoginNotification", login: { username: "test", password: "password", url: "https://example.com" }, }; activeAccountStatusMock$.next(AuthenticationStatus.Locked); @@ -349,7 +349,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerAddLoginNotification", login: { username: "test", password: "password", url: "https://example.com" }, }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); @@ -367,7 +367,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerAddLoginNotification", login: { username: "test", password: "password", url: "https://example.com" }, }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); @@ -389,7 +389,7 @@ describe("NotificationBackground", () => { it("skips attempting to change the password for an existing login if the password has not changed", async () => { const message: NotificationBackgroundExtensionMessage = { - command: "bgAddLogin", + command: "bgTriggerAddLoginNotification", login: { username: "test", password: "password", url: "https://example.com" }, }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); @@ -409,7 +409,10 @@ describe("NotificationBackground", () => { 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 }; + const message: NotificationBackgroundExtensionMessage = { + command: "bgTriggerAddLoginNotification", + login, + }; activeAccountStatusMock$.next(AuthenticationStatus.Locked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); @@ -425,7 +428,10 @@ describe("NotificationBackground", () => { password: "password", url: "https://example.com", } as any; - const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; + const message: NotificationBackgroundExtensionMessage = { + command: "bgTriggerAddLoginNotification", + login, + }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ @@ -440,7 +446,10 @@ describe("NotificationBackground", () => { 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 }; + const message: NotificationBackgroundExtensionMessage = { + command: "bgTriggerAddLoginNotification", + login, + }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockResolvedValueOnce(true); getEnableChangedPasswordPromptSpy.mockResolvedValueOnce(true); @@ -463,7 +472,7 @@ describe("NotificationBackground", () => { }); }); - describe("bgChangedPassword message handler", () => { + describe("bgTriggerChangedPasswordNotification message handler", () => { let tab: chrome.tabs.Tab; let sender: chrome.runtime.MessageSender; let pushChangePasswordToQueueSpy: jest.SpyInstance; @@ -481,7 +490,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", currentPassword: "currentPassword", url: "" }, }; @@ -493,7 +502,7 @@ describe("NotificationBackground", () => { it("adds a change password message to the queue if the user does not have an unlocked account", async () => { const message: NotificationBackgroundExtensionMessage = { - command: "bgChangedPassword", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", currentPassword: "currentPassword", @@ -516,7 +525,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", currentPassword: "currentPassword", @@ -537,7 +546,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", currentPassword: "currentPassword", @@ -559,7 +568,7 @@ describe("NotificationBackground", () => { it("adds a change password message to the queue if a single cipher matches the passed current password", async () => { const message: NotificationBackgroundExtensionMessage = { - command: "bgChangedPassword", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", currentPassword: "currentPassword", @@ -587,7 +596,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", url: "https://example.com", @@ -608,7 +617,7 @@ describe("NotificationBackground", () => { 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", + command: "bgTriggerChangedPasswordNotification", data: { newPassword: "newPassword", url: "https://example.com", diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 4a8c067a8bb..c6f48a45378 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -85,17 +85,16 @@ export default class NotificationBackground { ExtensionCommand.AutofillIdentity, ]); private readonly extensionMessageHandlers: NotificationBackgroundExtensionMessageHandlers = { - bgAddLogin: ({ message, sender }) => this.triggerAddLoginNotification(message, sender), bgAdjustNotificationBar: ({ message, sender }) => this.handleAdjustNotificationBarMessage(message, sender), + bgTriggerAddLoginNotification: ({ message, sender }) => + this.triggerAddLoginNotification(message, sender), bgTriggerChangedPasswordNotification: ({ message, sender }) => this.triggerChangedPasswordNotification(message, sender), bgTriggerAtRiskPasswordNotification: ({ message, sender }) => this.triggerAtRiskPasswordNotification(message, sender), bgCloseNotificationBar: ({ message, sender }) => this.handleCloseNotificationBarMessage(message, sender), - bgOpenAtRisksPasswords: ({ message, sender }) => - this.handleOpenAtRisksPasswordsMessage(message, sender), bgGetActiveUserServerConfig: () => this.getActiveUserServerConfig(), bgGetDecryptedCiphers: () => this.getNotificationCipherData(), bgGetEnableChangedPasswordPrompt: () => this.getEnableChangedPasswordPrompt(), @@ -397,7 +396,7 @@ export default class NotificationBackground { async triggerAtRiskPasswordNotification( message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, - ) { + ): Promise { const { activeUserId, securityTask, cipher } = message.data; const domain = Utils.getDomain(sender.tab.url); const passwordChangeUri = @@ -426,6 +425,7 @@ export default class NotificationBackground { }; this.notificationQueue.push(queueMessage); await this.checkNotificationQueue(sender.tab); + return true; } /** @@ -439,17 +439,17 @@ export default class NotificationBackground { async triggerAddLoginNotification( message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, - ): Promise { + ): Promise { const authStatus = await this.getAuthStatus(); if (authStatus === AuthenticationStatus.LoggedOut) { - throw Error("Auth status is logged out."); + return false; } const loginInfo = message.login; const normalizedUsername = loginInfo.username ? loginInfo.username.toLowerCase() : ""; const loginDomain = Utils.getDomain(loginInfo.url); if (loginDomain == null) { - throw Error("Login domain is not present."); + return false; } const addLoginIsEnabled = await this.getEnableAddedLoginPrompt(); @@ -459,14 +459,14 @@ export default class NotificationBackground { await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab, true); } - return true; + return false; } const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getOptionalUserId), ); if (activeUserId == null) { - throw Error("No active user."); + return false; } const ciphers = await this.cipherService.getAllDecryptedForUrl(loginInfo.url, activeUserId); @@ -493,7 +493,7 @@ export default class NotificationBackground { ); return true; } - throw Error("No change password notification pushed to queue."); + return false; } private async pushAddLoginToQueue( @@ -530,11 +530,11 @@ export default class NotificationBackground { async triggerChangedPasswordNotification( message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, - ): Promise { + ) { const changeData = message.data as ChangePasswordMessageData; const loginDomain = Utils.getDomain(changeData.url); if (loginDomain == null) { - throw new Error("No login domain present."); + return false; } if ((await this.getAuthStatus()) < AuthenticationStatus.Unlocked) { @@ -553,7 +553,7 @@ export default class NotificationBackground { this.accountService.activeAccount$.pipe(getOptionalUserId), ); if (activeUserId == null) { - throw new Error("No active user."); + return false; } const ciphers = await this.cipherService.getAllDecryptedForUrl(changeData.url, activeUserId); @@ -571,7 +571,7 @@ export default class NotificationBackground { await this.pushChangePasswordToQueue(id, loginDomain, changeData.newPassword, sender.tab); return true; } - throw Error("No change password notification queued."); + return false; } /** @@ -644,7 +644,6 @@ export default class NotificationBackground { }; this.notificationQueue.push(message); await this.checkNotificationQueue(tab); - return true; } private async pushUnlockVaultToQueue(loginDomain: string, tab: chrome.tabs.Tab) { diff --git a/apps/browser/src/autofill/background/overlay-notifications.background.spec.ts b/apps/browser/src/autofill/background/overlay-notifications.background.spec.ts index 774d4194c55..00114330bc4 100644 --- a/apps/browser/src/autofill/background/overlay-notifications.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay-notifications.background.spec.ts @@ -335,7 +335,6 @@ describe("OverlayNotificationsBackground", () => { const pageDetails = mock({ fields: [mock()] }); let notificationChangedPasswordSpy: jest.SpyInstance; let notificationAddLoginSpy: jest.SpyInstance; - let notificationAtRiskPasswordSpy: jest.SpyInstance; beforeEach(async () => { sender = mock({ @@ -347,10 +346,6 @@ describe("OverlayNotificationsBackground", () => { "triggerChangedPasswordNotification", ); notificationAddLoginSpy = jest.spyOn(notificationBackground, "triggerAddLoginNotification"); - notificationAtRiskPasswordSpy = jest.spyOn( - notificationBackground, - "triggerAtRiskPasswordNotification", - ); sendMockExtensionMessage( { command: "collectPageDetailsResponse", details: pageDetails }, @@ -411,7 +406,6 @@ describe("OverlayNotificationsBackground", () => { expect(notificationChangedPasswordSpy).not.toHaveBeenCalled(); expect(notificationAddLoginSpy).not.toHaveBeenCalled(); - expect(notificationAtRiskPasswordSpy).not.toHaveBeenCalled(); }); it("ignores requests for tabs that do not contain stored login data", async () => { @@ -436,7 +430,6 @@ describe("OverlayNotificationsBackground", () => { expect(notificationChangedPasswordSpy).not.toHaveBeenCalled(); expect(notificationAddLoginSpy).not.toHaveBeenCalled(); - expect(notificationAtRiskPasswordSpy).not.toHaveBeenCalled(); }); it("clears the notification fallback timeout if the request is completed with an invalid status code", async () => { diff --git a/apps/browser/src/autofill/background/overlay-notifications.background.ts b/apps/browser/src/autofill/background/overlay-notifications.background.ts index f750215caa1..5d52d52bd19 100644 --- a/apps/browser/src/autofill/background/overlay-notifications.background.ts +++ b/apps/browser/src/autofill/background/overlay-notifications.background.ts @@ -269,7 +269,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg return ( !modifyLoginData || !this.shouldAttemptAddLoginNotification(modifyLoginData) || - !this.shouldAttemptChangePasswordNotification(modifyLoginData) + !this.shouldAttemptChangedPasswordNotification(modifyLoginData) ); }; @@ -401,7 +401,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg }; /** - * Initializes the add login, change, or at risk password notification based on the modified login form data + * Initializes the add login or change password notification based on the modified login form data * and the tab details. This will trigger the notification to be displayed to the user. * * @param requestId - The details of the web response @@ -413,15 +413,14 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg modifyLoginData: ModifyLoginCipherFormData, tab: chrome.tabs.Tab, ) => { - let error: Error; - - if (this.shouldAttemptChangePasswordNotification(modifyLoginData)) { + let error: string; + if (this.shouldAttemptChangedPasswordNotification(modifyLoginData)) { // These notifications are temporarily setup as "messages" to the notification background. // This will be structured differently in a future refactor. try { - await this.notificationBackground.triggerChangedPasswordNotification( + const result = await this.notificationBackground.triggerChangedPasswordNotification( { - command: "bgTriggerChangedPasswordNotification", + command: "bgChangedPassword", data: { url: modifyLoginData.uri, currentPassword: modifyLoginData.password, @@ -430,18 +429,21 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg }, { tab }, ); - this.clearCompletedWebRequest(requestId, tab); - return; + if (!result) { + throw Error("Didn't trigger changedPassword "); + } } catch (e) { error = e; } + this.clearCompletedWebRequest(requestId, tab); + return; } if (this.shouldAttemptAddLoginNotification(modifyLoginData)) { try { - await this.notificationBackground.triggerAddLoginNotification( + const result = await this.notificationBackground.triggerAddLoginNotification( { - command: "bgAddLogin", + command: "bgTriggerAddLoginNotification", login: { url: modifyLoginData.uri, username: modifyLoginData.username, @@ -450,11 +452,14 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg }, { tab }, ); - this.clearCompletedWebRequest(requestId, tab); - return; + if (!result) { + throw Error("Didn't trigger addLogin notification"); + } } catch (e) { error = e; } + this.clearCompletedWebRequest(requestId, tab); + return error; } const shouldGetTasks: boolean = await this.notificationBackground.getNotificationFlag(); @@ -482,25 +487,23 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg { tab }, ); this.clearCompletedWebRequest(requestId, tab); - return; } } - return error; }; /** - * Determines if the change password notification should be attempted. + * Determines if the change password notification should be triggered. * * @param modifyLoginData - The modified login form data */ - private shouldAttemptChangePasswordNotification = ( + private shouldAttemptChangedPasswordNotification = ( modifyLoginData: ModifyLoginCipherFormData, ) => { return modifyLoginData?.newPassword && !modifyLoginData.username; }; /** - * Determines if the add login notification should be attempted. + * Determines if the add login notification should be triggered. * * @param modifyLoginData - The modified login form data */