From 46789392c28058f65083be199f4d69da261fb9b2 Mon Sep 17 00:00:00 2001 From: Miles Blackwood Date: Tue, 20 May 2025 18:05:40 -0400 Subject: [PATCH] Allow notification trigger attempts with tracking of last error. --- .../abstractions/notification.background.ts | 9 +- .../background/notification.background.ts | 40 +++++---- .../overlay-notifications.background.spec.ts | 14 +++- .../overlay-notifications.background.ts | 82 +++++++++++-------- 4 files changed, 88 insertions(+), 57 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 2a908eb8832..0cb7c2e4bd4 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -119,14 +119,17 @@ type NotificationBackgroundExtensionMessageHandlers = { sender, }: BackgroundOnMessageHandlerParams) => Promise; bgCloseNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgOpenAtRisksPasswordNotification: ({ + bgTriggerAtRiskPasswordNotification: ({ message, sender, }: BackgroundOnMessageHandlerParams) => Promise; bgOpenAtRisksPasswords: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgAdjustNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgAddLogin: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgChangedPassword: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgAddLogin: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; + bgTriggerChangedPasswordNotification: ({ + message, + sender, + }: BackgroundOnMessageHandlerParams) => Promise; bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void; bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; bgOpenAddEditVaultItemPopout: ({ diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index cb495af4755..4a8c067a8bb 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -85,12 +85,13 @@ export default class NotificationBackground { ExtensionCommand.AutofillIdentity, ]); private readonly extensionMessageHandlers: NotificationBackgroundExtensionMessageHandlers = { - bgAddLogin: ({ message, sender }) => this.addLogin(message, sender), + bgAddLogin: ({ message, sender }) => this.triggerAddLoginNotification(message, sender), bgAdjustNotificationBar: ({ message, sender }) => this.handleAdjustNotificationBarMessage(message, sender), - bgChangedPassword: ({ message, sender }) => this.changedPassword(message, sender), - bgOpenAtRisksPasswordNotification: ({ message, sender }) => - this.openAtRisksPasswordNotification(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 }) => @@ -393,7 +394,7 @@ export default class NotificationBackground { * @param message - The extension message * @param sender - The contextual sender of the message */ - async openAtRisksPasswordNotification( + async triggerAtRiskPasswordNotification( message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, ) { @@ -435,20 +436,20 @@ export default class NotificationBackground { * @param message - The message to add to the queue * @param sender - The contextual sender of the message */ - async addLogin( + async triggerAddLoginNotification( message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, - ) { + ): Promise { const authStatus = await this.getAuthStatus(); if (authStatus === AuthenticationStatus.LoggedOut) { - return; + throw Error("Auth status is logged out."); } const loginInfo = message.login; const normalizedUsername = loginInfo.username ? loginInfo.username.toLowerCase() : ""; const loginDomain = Utils.getDomain(loginInfo.url); if (loginDomain == null) { - return; + throw Error("Login domain is not present."); } const addLoginIsEnabled = await this.getEnableAddedLoginPrompt(); @@ -458,14 +459,14 @@ export default class NotificationBackground { await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab, true); } - return; + return true; } const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getOptionalUserId), ); if (activeUserId == null) { - return; + throw Error("No active user."); } const ciphers = await this.cipherService.getAllDecryptedForUrl(loginInfo.url, activeUserId); @@ -474,7 +475,7 @@ export default class NotificationBackground { ); if (addLoginIsEnabled && usernameMatches.length === 0) { await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab); - return; + return true; } const changePasswordIsEnabled = await this.getEnableChangedPasswordPrompt(); @@ -490,7 +491,9 @@ export default class NotificationBackground { loginInfo.password, sender.tab, ); + return true; } + throw Error("No change password notification pushed to queue."); } private async pushAddLoginToQueue( @@ -524,14 +527,14 @@ export default class NotificationBackground { * @param message - The message to add to the queue * @param sender - The contextual sender of the message */ - async changedPassword( + async triggerChangedPasswordNotification( message: NotificationBackgroundExtensionMessage, sender: chrome.runtime.MessageSender, - ) { + ): Promise { const changeData = message.data as ChangePasswordMessageData; const loginDomain = Utils.getDomain(changeData.url); if (loginDomain == null) { - return; + throw new Error("No login domain present."); } if ((await this.getAuthStatus()) < AuthenticationStatus.Unlocked) { @@ -542,7 +545,7 @@ export default class NotificationBackground { sender.tab, true, ); - return; + return true; } let id: string = null; @@ -550,7 +553,7 @@ export default class NotificationBackground { this.accountService.activeAccount$.pipe(getOptionalUserId), ); if (activeUserId == null) { - return; + throw new Error("No active user."); } const ciphers = await this.cipherService.getAllDecryptedForUrl(changeData.url, activeUserId); @@ -566,7 +569,9 @@ export default class NotificationBackground { } if (id != null) { await this.pushChangePasswordToQueue(id, loginDomain, changeData.newPassword, sender.tab); + return true; } + throw Error("No change password notification queued."); } /** @@ -639,6 +644,7 @@ 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 27a6f85e745..774d4194c55 100644 --- a/apps/browser/src/autofill/background/overlay-notifications.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay-notifications.background.spec.ts @@ -335,14 +335,22 @@ describe("OverlayNotificationsBackground", () => { const pageDetails = mock({ fields: [mock()] }); let notificationChangedPasswordSpy: jest.SpyInstance; let notificationAddLoginSpy: jest.SpyInstance; + let notificationAtRiskPasswordSpy: jest.SpyInstance; beforeEach(async () => { sender = mock({ tab: { id: 1 }, url: "https://example.com", }); - notificationChangedPasswordSpy = jest.spyOn(notificationBackground, "changedPassword"); - notificationAddLoginSpy = jest.spyOn(notificationBackground, "addLogin"); + notificationChangedPasswordSpy = jest.spyOn( + notificationBackground, + "triggerChangedPasswordNotification", + ); + notificationAddLoginSpy = jest.spyOn(notificationBackground, "triggerAddLoginNotification"); + notificationAtRiskPasswordSpy = jest.spyOn( + notificationBackground, + "triggerAtRiskPasswordNotification", + ); sendMockExtensionMessage( { command: "collectPageDetailsResponse", details: pageDetails }, @@ -403,6 +411,7 @@ 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 () => { @@ -427,6 +436,7 @@ 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 abc5b491047..f750215caa1 100644 --- a/apps/browser/src/autofill/background/overlay-notifications.background.ts +++ b/apps/browser/src/autofill/background/overlay-notifications.background.ts @@ -268,8 +268,8 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg const modifyLoginData = this.modifyLoginCipherFormData.get(tabId); return ( !modifyLoginData || - !this.shouldTriggerAddLoginNotification(modifyLoginData) || - !this.shouldTriggerChangePasswordNotification(modifyLoginData) + !this.shouldAttemptAddLoginNotification(modifyLoginData) || + !this.shouldAttemptChangePasswordNotification(modifyLoginData) ); }; @@ -401,7 +401,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg }; /** - * Initializes the add login or change password notification based on the modified login form data + * Initializes the add login, change, or at risk 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,38 +413,48 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg modifyLoginData: ModifyLoginCipherFormData, tab: chrome.tabs.Tab, ) => { - if (this.shouldTriggerChangePasswordNotification(modifyLoginData)) { + let error: Error; + + if (this.shouldAttemptChangePasswordNotification(modifyLoginData)) { // These notifications are temporarily setup as "messages" to the notification background. // This will be structured differently in a future refactor. - await this.notificationBackground.changedPassword( - { - command: "bgChangedPassword", - data: { - url: modifyLoginData.uri, - currentPassword: modifyLoginData.password, - newPassword: modifyLoginData.newPassword, + try { + await this.notificationBackground.triggerChangedPasswordNotification( + { + command: "bgTriggerChangedPasswordNotification", + data: { + url: modifyLoginData.uri, + currentPassword: modifyLoginData.password, + newPassword: modifyLoginData.newPassword, + }, }, - }, - { tab }, - ); - this.clearCompletedWebRequest(requestId, tab); - return; + { tab }, + ); + this.clearCompletedWebRequest(requestId, tab); + return; + } catch (e) { + error = e; + } } - if (this.shouldTriggerAddLoginNotification(modifyLoginData)) { - await this.notificationBackground.addLogin( - { - command: "bgAddLogin", - login: { - url: modifyLoginData.uri, - username: modifyLoginData.username, - password: modifyLoginData.password || modifyLoginData.newPassword, + if (this.shouldAttemptAddLoginNotification(modifyLoginData)) { + try { + await this.notificationBackground.triggerAddLoginNotification( + { + command: "bgAddLogin", + login: { + url: modifyLoginData.uri, + username: modifyLoginData.username, + password: modifyLoginData.password || modifyLoginData.newPassword, + }, }, - }, - { tab }, - ); - this.clearCompletedWebRequest(requestId, tab); - return; + { tab }, + ); + this.clearCompletedWebRequest(requestId, tab); + return; + } catch (e) { + error = e; + } } const shouldGetTasks: boolean = await this.notificationBackground.getNotificationFlag(); @@ -460,9 +470,9 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg const shouldTriggerAtRiskPasswordNotification: boolean = typeof securityTask !== "undefined"; if (shouldTriggerAtRiskPasswordNotification) { - await this.notificationBackground.openAtRisksPasswordNotification( + await this.notificationBackground.triggerAtRiskPasswordNotification( { - command: "bgOpenAtRisksPasswordNotification", + command: "bgTriggerAtRiskPasswordNotification", data: { activeUserId, cipher, @@ -472,27 +482,29 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg { tab }, ); this.clearCompletedWebRequest(requestId, tab); + return; } } + return error; }; /** - * Determines if the change password notification should be triggered. + * Determines if the change password notification should be attempted. * * @param modifyLoginData - The modified login form data */ - private shouldTriggerChangePasswordNotification = ( + private shouldAttemptChangePasswordNotification = ( modifyLoginData: ModifyLoginCipherFormData, ) => { return modifyLoginData?.newPassword && !modifyLoginData.username; }; /** - * Determines if the add login notification should be triggered. + * Determines if the add login notification should be attempted. * * @param modifyLoginData - The modified login form data */ - private shouldTriggerAddLoginNotification = (modifyLoginData: ModifyLoginCipherFormData) => { + private shouldAttemptAddLoginNotification = (modifyLoginData: ModifyLoginCipherFormData) => { return modifyLoginData?.username && (modifyLoginData.password || modifyLoginData.newPassword); };