From 56f80539b9451bcd132482acc17a162e31974e9c Mon Sep 17 00:00:00 2001 From: Miles Blackwood Date: Fri, 1 Aug 2025 11:30:40 -0400 Subject: [PATCH] Combine notif check/attempt, end msg pass syntax. (#15771) --- .../abstractions/notification.background.ts | 34 +-- .../overlay-notifications.background.ts | 9 + .../notification.background.spec.ts | 233 ++++++++---------- .../background/notification.background.ts | 163 ++++++++---- .../overlay-notifications.background.ts | 231 ++++++----------- 5 files changed, 303 insertions(+), 367 deletions(-) diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 9c9c5c0e243..f2152b44862 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -1,9 +1,6 @@ import { NeverDomains } from "@bitwarden/common/models/domain/domain-service"; import { ServerConfig } from "@bitwarden/common/platform/abstractions/config/server-config"; -import { UserId } from "@bitwarden/common/types/guid"; -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; -import { SecurityTask } from "@bitwarden/common/vault/tasks"; import { CollectionView } from "../../content/components/common-types"; import { NotificationQueueMessageTypes } from "../../enums/notification-queue-message-type.enum"; @@ -60,23 +57,10 @@ type LockedVaultPendingNotificationsData = { target: string; }; -type AtRiskPasswordNotificationsData = { - activeUserId: UserId; - cipher: CipherView; - securityTask: SecurityTask; - uri: string; -}; - type AdjustNotificationBarMessageData = { height: number; }; -type ChangePasswordMessageData = { - currentPassword: string; - newPassword: string; - url: string; -}; - type AddLoginMessageData = { username: string; password: string; @@ -92,10 +76,7 @@ type NotificationBackgroundExtensionMessage = { command: string; data?: Partial & Partial & - Partial & - Partial & - Partial; - login?: AddLoginMessageData; + Partial; folder?: string; edit?: boolean; details?: AutofillPageDetails; @@ -121,18 +102,6 @@ type NotificationBackgroundExtensionMessageHandlers = { bgCloseNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgOpenAtRiskPasswords: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgAdjustNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; - bgTriggerAddLoginNotification: ({ - message, - sender, - }: BackgroundOnMessageHandlerParams) => Promise; - bgTriggerChangedPasswordNotification: ({ - message, - sender, - }: BackgroundOnMessageHandlerParams) => Promise; - bgTriggerAtRiskPasswordNotification: ({ - message, - sender, - }: BackgroundOnMessageHandlerParams) => Promise; bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void; bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; bgOpenAddEditVaultItemPopout: ({ @@ -162,7 +131,6 @@ export { NotificationQueueMessageItem, LockedVaultPendingNotificationsData, AdjustNotificationBarMessageData, - ChangePasswordMessageData, UnlockVaultMessageData, AddLoginMessageData, NotificationBackgroundExtensionMessage, diff --git a/apps/browser/src/autofill/background/abstractions/overlay-notifications.background.ts b/apps/browser/src/autofill/background/abstractions/overlay-notifications.background.ts index 0ec6a9ae04a..d446e18b480 100644 --- a/apps/browser/src/autofill/background/abstractions/overlay-notifications.background.ts +++ b/apps/browser/src/autofill/background/abstractions/overlay-notifications.background.ts @@ -1,3 +1,6 @@ +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { SecurityTask } from "@bitwarden/common/vault/tasks"; + import AutofillPageDetails from "../../models/autofill-page-details"; export type NotificationTypeData = { @@ -8,6 +11,12 @@ export type NotificationTypeData = { launchTimestamp?: number; }; +export type LoginSecurityTaskInfo = { + securityTask: SecurityTask; + cipher: CipherView; + uri: ModifyLoginCipherFormData["uri"]; +}; + export type WebsiteOriginsWithFields = Map>; export type ActiveFormSubmissionRequests = Set; diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index 5e7e3ed30f5..7302ae7d705 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -3,17 +3,18 @@ import { BehaviorSubject, firstValueFrom, of } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { DefaultPolicyService } from "@bitwarden/common/admin-console/services/policy/default-policy.service"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { ExtensionCommand } from "@bitwarden/common/autofill/constants"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; -import { UserNotificationSettingsService } from "@bitwarden/common/autofill/services/user-notification-settings.service"; +import { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { ThemeTypes } from "@bitwarden/common/platform/enums"; import { SelfHostedEnvironment } from "@bitwarden/common/platform/services/default-environment.service"; import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; import { UserId } from "@bitwarden/common/types/guid"; @@ -38,6 +39,7 @@ import { LockedVaultPendingNotificationsData, NotificationBackgroundExtensionMessage, } from "./abstractions/notification.background"; +import { ModifyLoginCipherFormData } from "./abstractions/overlay-notifications.background"; import NotificationBackground from "./notification.background"; jest.mock("rxjs", () => { @@ -58,13 +60,21 @@ describe("NotificationBackground", () => { const collectionService = mock(); let activeAccountStatusMock$: BehaviorSubject; let authService: MockProxy; - const policyService = mock(); + const policyAppliesToUser$ = new BehaviorSubject(true); + const policyService = mock({ + policyAppliesToUser$: jest.fn().mockReturnValue(policyAppliesToUser$), + }); const folderService = mock(); - const userNotificationSettingsService = mock(); + const enableChangedPasswordPromptMock$ = new BehaviorSubject(true); + const userNotificationSettingsService = mock(); + userNotificationSettingsService.enableChangedPasswordPrompt$ = enableChangedPasswordPromptMock$; + const domainSettingsService = mock(); const environmentService = mock(); const logService = mock(); + const selectedThemeMock$ = new BehaviorSubject(ThemeTypes.Light); const themeStateService = mock(); + themeStateService.selectedTheme$ = selectedThemeMock$; const configService = mock(); const accountService = mock(); const organizationService = mock(); @@ -164,7 +174,7 @@ describe("NotificationBackground", () => { }); }); - describe("notification bar extension message handlers", () => { + describe("notification bar extension message handlers and triggers", () => { beforeEach(() => { notificationBackground.init(); }); @@ -283,7 +293,12 @@ describe("NotificationBackground", () => { let pushAddLoginToQueueSpy: jest.SpyInstance; let pushChangePasswordToQueueSpy: jest.SpyInstance; let getAllDecryptedForUrlSpy: jest.SpyInstance; - + const mockModifyLoginCipherFormData: ModifyLoginCipherFormData = { + username: "test", + password: "password", + uri: "https://example.com", + newPassword: null, + }; beforeEach(() => { tab = createChromeTabMock(); sender = mock({ tab }); @@ -304,43 +319,34 @@ describe("NotificationBackground", () => { }); it("skips attempting to add the login if the user is logged out", async () => { - const message: NotificationBackgroundExtensionMessage = { - command: "bgTriggerAddLoginNotification", - login: { username: "test", password: "password", url: "https://example.com" }, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; activeAccountStatusMock$.next(AuthenticationStatus.LoggedOut); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(getEnableAddedLoginPromptSpy).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: "bgTriggerAddLoginNotification", - login: { username: "test", password: "password", url: "" }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "", }; activeAccountStatusMock$.next(AuthenticationStatus.Locked); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(getEnableAddedLoginPromptSpy).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: "bgTriggerAddLoginNotification", - login: { username: "test", password: "password", url: "https://example.com" }, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; activeAccountStatusMock$.next(AuthenticationStatus.Locked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(false); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).not.toHaveBeenCalled(); @@ -349,16 +355,12 @@ 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: "bgTriggerAddLoginNotification", - login: { username: "test", password: "password", url: "https://example.com" }, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(false); getAllDecryptedForUrlSpy.mockResolvedValueOnce([]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); @@ -367,10 +369,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: "bgTriggerAddLoginNotification", - login: { username: "test", password: "password", url: "https://example.com" }, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); getEnableChangedPasswordPromptSpy.mockReturnValueOnce(false); @@ -378,8 +377,7 @@ describe("NotificationBackground", () => { mock({ login: { username: "test", password: "oldPassword" } }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); @@ -389,18 +387,14 @@ describe("NotificationBackground", () => { }); it("skips attempting to change the password for an existing login if the password has not changed", async () => { - const message: NotificationBackgroundExtensionMessage = { - command: "bgTriggerAddLoginNotification", - login: { username: "test", password: "password", url: "https://example.com" }, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ login: { username: "test", password: "password" } }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); @@ -409,48 +403,55 @@ 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: "bgTriggerAddLoginNotification", - login, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; activeAccountStatusMock$.next(AuthenticationStatus.Locked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); - expect(pushAddLoginToQueueSpy).toHaveBeenCalledWith("example.com", login, sender.tab, true); + expect(pushAddLoginToQueueSpy).toHaveBeenCalledWith( + "example.com", + { + url: data.uri, + username: data.username, + password: data.password, + }, + 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: "bgTriggerAddLoginNotification", - login, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + username: null, }; + activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ login: { username: "anotherTestUsername", password: "password" } }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); - expect(pushAddLoginToQueueSpy).toHaveBeenCalledWith("example.com", login, sender.tab); + expect(pushAddLoginToQueueSpy).toHaveBeenCalledWith( + "example.com", + { + url: data.uri, + username: data.username, + password: data.password, + }, + 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: "bgTriggerAddLoginNotification", - login, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + username: "tEsT", }; + activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getEnableAddedLoginPromptSpy.mockResolvedValueOnce(true); getEnableChangedPasswordPromptSpy.mockResolvedValueOnce(true); @@ -461,13 +462,12 @@ describe("NotificationBackground", () => { }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerAddLoginNotification(data, tab); expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( "cipher-id", "example.com", - login.password, + data.password, sender.tab, ); }); @@ -478,6 +478,12 @@ describe("NotificationBackground", () => { let sender: chrome.runtime.MessageSender; let pushChangePasswordToQueueSpy: jest.SpyInstance; let getAllDecryptedForUrlSpy: jest.SpyInstance; + const mockModifyLoginCipherFormData: ModifyLoginCipherFormData = { + username: null, + uri: null, + password: "currentPassword", + newPassword: "newPassword", + }; beforeEach(() => { tab = createChromeTabMock(); @@ -490,69 +496,51 @@ 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: "bgTriggerChangedPasswordNotification", - data: { newPassword: "newPassword", currentPassword: "currentPassword", url: "" }, - }; + const data: ModifyLoginCipherFormData = mockModifyLoginCipherFormData; - sendMockExtensionMessage(message); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); 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: "bgTriggerChangedPasswordNotification", - data: { - newPassword: "newPassword", - currentPassword: "currentPassword", - url: "https://example.com", - }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "https://example.com", }; + activeAccountStatusMock$.next(AuthenticationStatus.Locked); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( null, "example.com", - message.data?.newPassword, + 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: "bgTriggerChangedPasswordNotification", - data: { - newPassword: "newPassword", - currentPassword: "currentPassword", - url: "https://example.com", - }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "https://example.com", }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ login: { username: "test", password: "password" } }), ]); - - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); 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: "bgTriggerChangedPasswordNotification", - data: { - newPassword: "newPassword", - currentPassword: "currentPassword", - url: "https://example.com", - }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "https://example.com", }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ @@ -560,21 +548,16 @@ describe("NotificationBackground", () => { mock({ login: { username: "test2", password: "password" } }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); 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: "bgTriggerChangedPasswordNotification", - data: { - newPassword: "newPassword", - currentPassword: "currentPassword", - url: "https://example.com", - }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "https://example.com", }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ @@ -584,24 +567,20 @@ describe("NotificationBackground", () => { }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( "cipher-id", "example.com", - message.data?.newPassword, + 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: "bgTriggerChangedPasswordNotification", - data: { - newPassword: "newPassword", - url: "https://example.com", - }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "https://example.com", }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ @@ -609,20 +588,17 @@ describe("NotificationBackground", () => { mock({ login: { username: "test2", password: "password" } }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); 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: "bgTriggerChangedPasswordNotification", - data: { - newPassword: "newPassword", - url: "https://example.com", - }, + const data: ModifyLoginCipherFormData = { + ...mockModifyLoginCipherFormData, + uri: "https://example.com", + password: null, }; activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ @@ -632,13 +608,12 @@ describe("NotificationBackground", () => { }), ]); - sendMockExtensionMessage(message, sender); - await flushPromises(); + await notificationBackground.triggerChangedPasswordNotification(data, tab); expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith( "cipher-id", "example.com", - message.data?.newPassword, + data?.newPassword, sender.tab, ); }); diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 15baccf3560..3f6e93d8454 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -41,7 +41,7 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view"; import { LoginView } from "@bitwarden/common/vault/models/view/login.view"; import { TaskService } from "@bitwarden/common/vault/tasks"; -import { SecurityTaskType } from "@bitwarden/common/vault/tasks/enums"; +import { SecurityTaskStatus, SecurityTaskType } from "@bitwarden/common/vault/tasks/enums"; import { SecurityTask } from "@bitwarden/common/vault/tasks/models/security-task"; // FIXME (PM-22628): Popup imports are forbidden in background @@ -68,14 +68,17 @@ import { AddChangePasswordQueueMessage, AddLoginQueueMessage, AddUnlockVaultQueueMessage, - ChangePasswordMessageData, AddLoginMessageData, NotificationQueueMessageItem, LockedVaultPendingNotificationsData, NotificationBackgroundExtensionMessage, NotificationBackgroundExtensionMessageHandlers, } from "./abstractions/notification.background"; -import { NotificationTypeData } from "./abstractions/overlay-notifications.background"; +import { + LoginSecurityTaskInfo, + ModifyLoginCipherFormData, + NotificationTypeData, +} from "./abstractions/overlay-notifications.background"; import { OverlayBackgroundExtensionMessage } from "./abstractions/overlay.background"; export default class NotificationBackground { @@ -91,12 +94,6 @@ export default class NotificationBackground { private readonly extensionMessageHandlers: NotificationBackgroundExtensionMessageHandlers = { 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), bgOpenAtRiskPasswords: ({ message, sender }) => @@ -286,6 +283,62 @@ export default class NotificationBackground { }; } + /** + * If there is a security task for this cipher at login, return the task, cipher view, and uri. + * + * @param modifyLoginData - The modified login form data + * @param activeUserId - The currently logged in user ID + */ + private async getSecurityTaskAndCipherForLoginData( + modifyLoginData: ModifyLoginCipherFormData, + activeUserId: UserId, + ): Promise { + const tasks: SecurityTask[] = await this.getSecurityTasks(activeUserId); + if (!tasks?.length) { + return null; + } + + const urlCiphers: CipherView[] = await this.cipherService.getAllDecryptedForUrl( + modifyLoginData.uri, + activeUserId, + ); + if (!urlCiphers?.length) { + return null; + } + + const securityTaskForLogin = urlCiphers.reduce( + (taskInfo: LoginSecurityTaskInfo | null, cipher: CipherView) => { + if ( + // exit early if info was found already + taskInfo || + // exit early if the cipher was deleted + cipher.deletedDate || + // exit early if the entered login info doesn't match an existing cipher + modifyLoginData.username !== cipher.login.username || + modifyLoginData.password !== cipher.login.password + ) { + return taskInfo; + } + + // Find the first security task for the cipherId belonging to the entered login + const cipherSecurityTask = tasks.find( + ({ cipherId, status }) => + cipher.id === cipherId && // match security task cipher id to url cipher id + status === SecurityTaskStatus.Pending, // security task has not been completed + ); + + if (cipherSecurityTask) { + return { securityTask: cipherSecurityTask, cipher, uri: modifyLoginData.uri }; + } + + return taskInfo; + }, + null, + ); + + return securityTaskForLogin; + } + /** * Gets the active user server config from the config service. */ @@ -302,6 +355,10 @@ export default class NotificationBackground { return flagValue; } + /** + * Gets the current authentication status of the user. + * @returns Promise - The current authentication status of the user. + */ private async getAuthStatus() { return await firstValueFrom(this.authService.activeAccountStatus$); } @@ -400,11 +457,32 @@ export default class NotificationBackground { * @param sender - The contextual sender of the message */ async triggerAtRiskPasswordNotification( - message: NotificationBackgroundExtensionMessage, - sender: chrome.runtime.MessageSender, + data: ModifyLoginCipherFormData, + tab: chrome.tabs.Tab, ): Promise { - const { activeUserId, securityTask, cipher } = message.data; - const domain = Utils.getDomain(sender.tab.url); + const flag = await this.getNotificationFlag(); + if (!flag) { + return false; + } + + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(getOptionalUserId), + ); + + if (!activeUserId) { + return false; + } + const loginSecurityTaskInfo = await this.getSecurityTaskAndCipherForLoginData( + data, + activeUserId, + ); + + if (!loginSecurityTaskInfo) { + return false; + } + + const { securityTask, cipher } = loginSecurityTaskInfo; + const domain = Utils.getDomain(tab.url); const passwordChangeUri = await new TemporaryNotificationChangeLoginService().getChangePasswordUrl(cipher); @@ -418,7 +496,7 @@ export default class NotificationBackground { .pipe(getOrganizationById(securityTask.organizationId)), ); - this.removeTabFromNotificationQueue(sender.tab); + this.removeTabFromNotificationQueue(tab); const launchTimestamp = new Date().getTime(); const queueMessage: NotificationQueueMessageItem = { domain, @@ -426,12 +504,12 @@ export default class NotificationBackground { type: NotificationQueueMessageType.AtRiskPassword, passwordChangeUri, organizationName: organization.name, - tab: sender.tab, + tab: tab, launchTimestamp, expires: new Date(launchTimestamp + NOTIFICATION_BAR_LIFESPAN_MS), }; this.notificationQueue.push(queueMessage); - await this.checkNotificationQueue(sender.tab); + await this.checkNotificationQueue(tab); return true; } @@ -444,17 +522,22 @@ export default class NotificationBackground { * @param sender - The contextual sender of the message */ async triggerAddLoginNotification( - message: NotificationBackgroundExtensionMessage, - sender: chrome.runtime.MessageSender, + data: ModifyLoginCipherFormData, + tab: chrome.tabs.Tab, ): Promise { + const login = { + url: data.uri, + username: data.username, + password: data.password || data.newPassword, + }; + const authStatus = await this.getAuthStatus(); if (authStatus === AuthenticationStatus.LoggedOut) { return false; } - const loginInfo = message.login; - const normalizedUsername = loginInfo.username ? loginInfo.username.toLowerCase() : ""; - const loginDomain = Utils.getDomain(loginInfo.url); + const normalizedUsername = login.username ? login.username.toLowerCase() : ""; + const loginDomain = Utils.getDomain(login.url); if (loginDomain == null) { return false; } @@ -463,7 +546,7 @@ export default class NotificationBackground { if (authStatus === AuthenticationStatus.Locked) { if (addLoginIsEnabled) { - await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab, true); + await this.pushAddLoginToQueue(loginDomain, login, tab, true); } return false; @@ -476,12 +559,12 @@ export default class NotificationBackground { return false; } - const ciphers = await this.cipherService.getAllDecryptedForUrl(loginInfo.url, activeUserId); + const ciphers = await this.cipherService.getAllDecryptedForUrl(login.url, activeUserId); const usernameMatches = ciphers.filter( (c) => c.login.username != null && c.login.username.toLowerCase() === normalizedUsername, ); if (addLoginIsEnabled && usernameMatches.length === 0) { - await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab); + await this.pushAddLoginToQueue(loginDomain, login, tab); return true; } @@ -490,14 +573,9 @@ export default class NotificationBackground { if ( changePasswordIsEnabled && usernameMatches.length === 1 && - usernameMatches[0].login.password !== loginInfo.password + usernameMatches[0].login.password !== login.password ) { - await this.pushChangePasswordToQueue( - usernameMatches[0].id, - loginDomain, - loginInfo.password, - sender.tab, - ); + await this.pushChangePasswordToQueue(usernameMatches[0].id, loginDomain, login.password, tab); return true; } return false; @@ -535,23 +613,22 @@ export default class NotificationBackground { * @param sender - The contextual sender of the message */ async triggerChangedPasswordNotification( - message: NotificationBackgroundExtensionMessage, - sender: chrome.runtime.MessageSender, - ) { - const changeData = message.data as ChangePasswordMessageData; + data: ModifyLoginCipherFormData, + tab: chrome.tabs.Tab, + ): Promise { + const changeData = { + url: data.uri, + currentPassword: data.password, + newPassword: data.newPassword, + }; + const loginDomain = Utils.getDomain(changeData.url); if (loginDomain == null) { return false; } if ((await this.getAuthStatus()) < AuthenticationStatus.Unlocked) { - await this.pushChangePasswordToQueue( - null, - loginDomain, - changeData.newPassword, - sender.tab, - true, - ); + await this.pushChangePasswordToQueue(null, loginDomain, changeData.newPassword, tab, true); return true; } @@ -575,7 +652,7 @@ export default class NotificationBackground { id = ciphers[0].id; } if (id != null) { - await this.pushChangePasswordToQueue(id, loginDomain, changeData.newPassword, sender.tab); + await this.pushChangePasswordToQueue(id, loginDomain, changeData.newPassword, tab); return true; } return false; diff --git a/apps/browser/src/autofill/background/overlay-notifications.background.ts b/apps/browser/src/autofill/background/overlay-notifications.background.ts index 93357113fc4..1d7f2b1f9d8 100644 --- a/apps/browser/src/autofill/background/overlay-notifications.background.ts +++ b/apps/browser/src/autofill/background/overlay-notifications.background.ts @@ -1,17 +1,15 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, Subject, switchMap, timer } from "rxjs"; +import { Subject, switchMap, timer } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { getOptionalUserId } from "@bitwarden/common/auth/services/account.service"; import { CLEAR_NOTIFICATION_LOGIN_DATA_DURATION } from "@bitwarden/common/autofill/constants"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; -import { SecurityTask, SecurityTaskStatus, TaskService } from "@bitwarden/common/vault/tasks"; +import { TaskService } from "@bitwarden/common/vault/tasks"; import { BrowserApi } from "../../platform/browser/browser-api"; +import { NotificationType, NotificationTypes } from "../notification/abstractions/notification-bar"; import { generateDomainMatchPatterns, isInvalidResponseStatusCode } from "../utils"; import { @@ -25,12 +23,6 @@ import { } from "./abstractions/overlay-notifications.background"; import NotificationBackground from "./notification.background"; -type LoginSecurityTaskInfo = { - securityTask: SecurityTask; - cipher: CipherView; - uri: ModifyLoginCipherFormData["uri"]; -}; - export class OverlayNotificationsBackground implements OverlayNotificationsBackgroundInterface { private websiteOriginsWithFields: WebsiteOriginsWithFields = new Map(); private activeFormSubmissionRequests: ActiveFormSubmissionRequests = new Set(); @@ -274,8 +266,8 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg const modifyLoginData = this.modifyLoginCipherFormData.get(tabId); return ( !modifyLoginData || - !this.shouldAttemptAddLoginNotification(modifyLoginData) || - !this.shouldAttemptChangedPasswordNotification(modifyLoginData) + !this.shouldAttemptNotification(modifyLoginData, NotificationTypes.Add) || + !this.shouldAttemptNotification(modifyLoginData, NotificationTypes.Change) ); }; @@ -381,7 +373,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg return; } - await this.triggerNotificationInit(requestId, modifyLoginData, tab); + await this.processNotifications(requestId, modifyLoginData, tab); }; /** @@ -401,171 +393,86 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg const handleWebNavigationOnCompleted = async () => { chrome.webNavigation.onCompleted.removeListener(handleWebNavigationOnCompleted); const tab = await BrowserApi.getTab(tabId); - await this.triggerNotificationInit(requestId, modifyLoginData, tab); + await this.processNotifications(requestId, modifyLoginData, tab); }; chrome.webNavigation.onCompleted.addListener(handleWebNavigationOnCompleted); }; /** - * 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. + * This method attempts to trigger the add login, change password, or at-risk password notifications + * based on the modified login data and the tab details. * * @param requestId - The details of the web response * @param modifyLoginData - The modified login form data * @param tab - The tab details */ - private triggerNotificationInit = async ( + private processNotifications = async ( requestId: chrome.webRequest.ResourceRequest["requestId"], modifyLoginData: ModifyLoginCipherFormData, tab: chrome.tabs.Tab, + config: { skippable: NotificationType[] } = { skippable: [] }, ) => { - let result: 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. - const success = await this.notificationBackground.triggerChangedPasswordNotification( - { - command: "bgChangedPassword", - data: { - url: modifyLoginData.uri, - currentPassword: modifyLoginData.password, - newPassword: modifyLoginData.newPassword, - }, - }, - { tab }, - ); - if (!success) { - result = "Unqualified changedPassword notification attempt."; - } - } - - if (this.shouldAttemptAddLoginNotification(modifyLoginData)) { - const success = await this.notificationBackground.triggerAddLoginNotification( - { - command: "bgTriggerAddLoginNotification", - login: { - url: modifyLoginData.uri, - username: modifyLoginData.username, - password: modifyLoginData.password || modifyLoginData.newPassword, - }, - }, - { tab }, - ); - if (!success) { - result = "Unqualified addLogin notification attempt."; - } - } - - const shouldGetTasks = - (await this.notificationBackground.getNotificationFlag()) && !modifyLoginData.newPassword; - - if (shouldGetTasks) { - const activeUserId = await firstValueFrom( - this.accountService.activeAccount$.pipe(getOptionalUserId), - ); - - if (activeUserId) { - const loginSecurityTaskInfo = await this.getSecurityTaskAndCipherForLoginData( - modifyLoginData, - activeUserId, - ); - - if (loginSecurityTaskInfo) { - await this.notificationBackground.triggerAtRiskPasswordNotification( - { - command: "bgTriggerAtRiskPasswordNotification", - data: { - activeUserId, - cipher: loginSecurityTaskInfo.cipher, - securityTask: loginSecurityTaskInfo.securityTask, - }, - }, - { tab }, - ); - } else { - result = "Unqualified atRiskPassword notification attempt."; - } - } - } - this.clearCompletedWebRequest(requestId, tab); - return result; - }; - - /** - * Determines if the change password notification should be triggered. - * - * @param modifyLoginData - The modified login form data - */ - private shouldAttemptChangedPasswordNotification = ( - modifyLoginData: ModifyLoginCipherFormData, - ) => { - return modifyLoginData?.newPassword && !modifyLoginData.username; - }; - - /** - * Determines if the add login notification should be triggered. - * - * @param modifyLoginData - The modified login form data - */ - private shouldAttemptAddLoginNotification = (modifyLoginData: ModifyLoginCipherFormData) => { - return modifyLoginData?.username && (modifyLoginData.password || modifyLoginData.newPassword); - }; - - /** - * If there is a security task for this cipher at login, return the task, cipher view, and uri. - * - * @param modifyLoginData - The modified login form data - * @param activeUserId - The currently logged in user ID - */ - private async getSecurityTaskAndCipherForLoginData( - modifyLoginData: ModifyLoginCipherFormData, - activeUserId: UserId, - ): Promise { - const tasks: SecurityTask[] = await this.notificationBackground.getSecurityTasks(activeUserId); - if (!tasks?.length) { - return null; - } - - const urlCiphers: CipherView[] = await this.cipherService.getAllDecryptedForUrl( - modifyLoginData.uri, - activeUserId, - ); - if (!urlCiphers?.length) { - return null; - } - - const securityTaskForLogin = urlCiphers.reduce( - (taskInfo: LoginSecurityTaskInfo | null, cipher: CipherView) => { - if ( - // exit early if info was found already - taskInfo || - // exit early if the cipher was deleted - cipher.deletedDate || - // exit early if the entered login info doesn't match an existing cipher - modifyLoginData.username !== cipher.login.username || - modifyLoginData.password !== cipher.login.password - ) { - return taskInfo; - } - - // Find the first security task for the cipherId belonging to the entered login - const cipherSecurityTask = tasks.find( - ({ cipherId, status }) => - cipher.id === cipherId && // match security task cipher id to url cipher id - status === SecurityTaskStatus.Pending, // security task has not been completed - ); - - if (cipherSecurityTask) { - return { securityTask: cipherSecurityTask, cipher, uri: modifyLoginData.uri }; - } - - return taskInfo; + const notificationCandidates = [ + { + type: NotificationTypes.Change, + trigger: this.notificationBackground.triggerChangedPasswordNotification, }, - null, + { + type: NotificationTypes.Add, + trigger: this.notificationBackground.triggerAddLoginNotification, + }, + { + type: NotificationTypes.AtRiskPassword, + trigger: this.notificationBackground.triggerAtRiskPasswordNotification, + }, + ].filter( + (candidate) => + this.shouldAttemptNotification(modifyLoginData, candidate.type) || + config.skippable.includes(candidate.type), ); - return securityTaskForLogin; - } + const results: string[] = []; + for (const { trigger, type } of notificationCandidates) { + const success = await trigger.bind(this.notificationBackground)(modifyLoginData, tab); + if (success) { + results.push(`Success: ${type}`); + break; + } else { + results.push(`Unqualified ${type} notification attempt.`); + } + } + + this.clearCompletedWebRequest(requestId, tab); + return results.join(" "); + }; + + /** + * Determines if the add login notification should be attempted based on the modified login form data. + * @param modifyLoginData modified login form data + * @param notificationType The type of notification to be triggered + * @returns true if the notification should be attempted, false otherwise + */ + private shouldAttemptNotification = ( + modifyLoginData: ModifyLoginCipherFormData, + notificationType: NotificationType, + ): boolean => { + switch (notificationType) { + case NotificationTypes.Change: + return modifyLoginData?.newPassword && !modifyLoginData.username; + case NotificationTypes.Add: + return ( + modifyLoginData?.username && !!(modifyLoginData.password || modifyLoginData.newPassword) + ); + case NotificationTypes.AtRiskPassword: + return !modifyLoginData.newPassword; + case NotificationTypes.Unlock: + // Unlock notifications are handled separately and do not require form data + return false; + default: + this.logService.error(`Unknown notification type: ${notificationType}`); + return false; + } + }; /** * Clears the completed web request and removes the modified login form data for the tab.