diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 94ab1a398d9..322ccd732d0 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -109,6 +109,8 @@ type NotificationBackgroundExtensionMessageHandlers = { bgReopenUnlockPopout: ({ sender }: BackgroundSenderParam) => Promise; checkNotificationQueue: ({ sender }: BackgroundSenderParam) => Promise; collectPageDetailsResponse: ({ message }: BackgroundMessageParam) => Promise; + bgGetEnableChangedPasswordPrompt: () => Promise; + bgGetEnableAddedLoginPrompt: () => Promise; getWebVaultUrlForNotification: () => string; }; diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index 29c8481e5f2..7dde61b6fd7 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -4,6 +4,7 @@ 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 { UserNotificationSettingsService } from "@bitwarden/common/autofill/services/user-notification-settings.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"; @@ -45,6 +46,7 @@ describe("NotificationBackground", () => { const policyService = mock(); const folderService = mock(); const stateService = mock(); + const userNotificationSettingsService = mock(); const environmentService = mock(); const logService = mock(); @@ -56,6 +58,7 @@ describe("NotificationBackground", () => { policyService, folderService, stateService, + userNotificationSettingsService, environmentService, logService, ); @@ -235,8 +238,8 @@ describe("NotificationBackground", () => { let tab: chrome.tabs.Tab; let sender: chrome.runtime.MessageSender; let getAuthStatusSpy: jest.SpyInstance; - let getDisableAddLoginNotificationSpy: jest.SpyInstance; - let getDisableChangedPasswordNotificationSpy: jest.SpyInstance; + let getEnableAddedLoginPromptSpy: jest.SpyInstance; + let getEnableChangedPasswordPromptSpy: jest.SpyInstance; let pushAddLoginToQueueSpy: jest.SpyInstance; let pushChangePasswordToQueueSpy: jest.SpyInstance; let getAllDecryptedForUrlSpy: jest.SpyInstance; @@ -245,13 +248,13 @@ describe("NotificationBackground", () => { tab = createChromeTabMock(); sender = mock({ tab }); getAuthStatusSpy = jest.spyOn(authService, "getAuthStatus"); - getDisableAddLoginNotificationSpy = jest.spyOn( - stateService, - "getDisableAddLoginNotification", + getEnableAddedLoginPromptSpy = jest.spyOn( + notificationBackground as any, + "getEnableAddedLoginPrompt", ); - getDisableChangedPasswordNotificationSpy = jest.spyOn( - stateService, - "getDisableChangedPasswordNotification", + getEnableChangedPasswordPromptSpy = jest.spyOn( + notificationBackground as any, + "getEnableChangedPasswordPrompt", ); pushAddLoginToQueueSpy = jest.spyOn(notificationBackground as any, "pushAddLoginToQueue"); pushChangePasswordToQueueSpy = jest.spyOn( @@ -272,7 +275,7 @@ describe("NotificationBackground", () => { await flushPromises(); expect(getAuthStatusSpy).toHaveBeenCalled(); - expect(getDisableAddLoginNotificationSpy).not.toHaveBeenCalled(); + expect(getEnableAddedLoginPromptSpy).not.toHaveBeenCalled(); expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); }); @@ -287,7 +290,7 @@ describe("NotificationBackground", () => { await flushPromises(); expect(getAuthStatusSpy).toHaveBeenCalled(); - expect(getDisableAddLoginNotificationSpy).not.toHaveBeenCalled(); + expect(getEnableAddedLoginPromptSpy).not.toHaveBeenCalled(); expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); }); @@ -297,13 +300,13 @@ describe("NotificationBackground", () => { login: { username: "test", password: "password", url: "https://example.com" }, }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(true); + getEnableAddedLoginPromptSpy.mockReturnValueOnce(false); sendExtensionRuntimeMessage(message, sender); await flushPromises(); expect(getAuthStatusSpy).toHaveBeenCalled(); - expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).not.toHaveBeenCalled(); expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); @@ -315,14 +318,14 @@ describe("NotificationBackground", () => { login: { username: "test", password: "password", url: "https://example.com" }, }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(true); + getEnableAddedLoginPromptSpy.mockReturnValueOnce(false); getAllDecryptedForUrlSpy.mockResolvedValueOnce([]); sendExtensionRuntimeMessage(message, sender); await flushPromises(); expect(getAuthStatusSpy).toHaveBeenCalled(); - expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); @@ -334,8 +337,8 @@ describe("NotificationBackground", () => { login: { username: "test", password: "password", url: "https://example.com" }, }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); - getDisableChangedPasswordNotificationSpy.mockReturnValueOnce(true); + getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); + getEnableChangedPasswordPromptSpy.mockReturnValueOnce(false); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ login: { username: "test", password: "oldPassword" } }), ]); @@ -344,9 +347,9 @@ describe("NotificationBackground", () => { await flushPromises(); expect(getAuthStatusSpy).toHaveBeenCalled(); - expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); - expect(getDisableChangedPasswordNotificationSpy).toHaveBeenCalled(); + expect(getEnableChangedPasswordPromptSpy).toHaveBeenCalled(); expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); }); @@ -357,7 +360,7 @@ describe("NotificationBackground", () => { login: { username: "test", password: "password", url: "https://example.com" }, }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ login: { username: "test", password: "password" } }), ]); @@ -366,7 +369,7 @@ describe("NotificationBackground", () => { await flushPromises(); expect(getAuthStatusSpy).toHaveBeenCalled(); - expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled(); + expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled(); expect(getAllDecryptedForUrlSpy).toHaveBeenCalled(); expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled(); expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled(); @@ -376,7 +379,7 @@ describe("NotificationBackground", () => { const login = { username: "test", password: "password", url: "https://example.com" }; const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); sendExtensionRuntimeMessage(message, sender); await flushPromises(); @@ -393,7 +396,7 @@ describe("NotificationBackground", () => { } as any; const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getEnableAddedLoginPromptSpy.mockReturnValueOnce(true); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ login: { username: "anotherTestUsername", password: "password" } }), ]); @@ -409,7 +412,8 @@ describe("NotificationBackground", () => { const login = { username: "tEsT", password: "password", url: "https://example.com" }; const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login }; getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked); - getDisableAddLoginNotificationSpy.mockReturnValueOnce(false); + getEnableAddedLoginPromptSpy.mockResolvedValueOnce(true); + getEnableChangedPasswordPromptSpy.mockResolvedValueOnce(true); getAllDecryptedForUrlSpy.mockResolvedValueOnce([ mock({ id: "cipher-id", diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 8ebf1bd26e3..6b5f1a8297f 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -4,6 +4,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli 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 { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -57,6 +58,8 @@ export default class NotificationBackground { bgUnlockPopoutOpened: ({ message, sender }) => this.unlockVault(message, sender.tab), checkNotificationQueue: ({ sender }) => this.checkNotificationQueue(sender.tab), bgReopenUnlockPopout: ({ sender }) => this.openUnlockPopout(sender.tab), + bgGetEnableChangedPasswordPrompt: () => this.getEnableChangedPasswordPrompt(), + bgGetEnableAddedLoginPrompt: () => this.getEnableAddedLoginPrompt(), getWebVaultUrlForNotification: () => this.getWebVaultUrl(), }; @@ -67,6 +70,7 @@ export default class NotificationBackground { private policyService: PolicyService, private folderService: FolderService, private stateService: BrowserStateService, + private userNotificationSettingsService: UserNotificationSettingsServiceAbstraction, private environmentService: EnvironmentService, private logService: LogService, ) {} @@ -81,6 +85,20 @@ export default class NotificationBackground { this.cleanupNotificationQueue(); } + /** + * Gets the enableChangedPasswordPrompt setting from the user notification settings service. + */ + async getEnableChangedPasswordPrompt(): Promise { + return await firstValueFrom(this.userNotificationSettingsService.enableChangedPasswordPrompt$); + } + + /** + * Gets the enableAddedLoginPrompt setting from the user notification settings service. + */ + async getEnableAddedLoginPrompt(): Promise { + return await firstValueFrom(this.userNotificationSettingsService.enableAddedLoginPrompt$); + } + /** * 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. @@ -194,9 +212,10 @@ export default class NotificationBackground { return; } - const disabledAddLogin = await this.stateService.getDisableAddLoginNotification(); + const addLoginIsEnabled = await this.getEnableAddedLoginPrompt(); + if (authStatus === AuthenticationStatus.Locked) { - if (!disabledAddLogin) { + if (addLoginIsEnabled) { await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab, true); } @@ -207,14 +226,15 @@ export default class NotificationBackground { const usernameMatches = ciphers.filter( (c) => c.login.username != null && c.login.username.toLowerCase() === normalizedUsername, ); - if (!disabledAddLogin && usernameMatches.length === 0) { + if (addLoginIsEnabled && usernameMatches.length === 0) { await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab); return; } - const disabledChangePassword = await this.stateService.getDisableChangedPasswordNotification(); + const changePasswordIsEnabled = await this.getEnableChangedPasswordPrompt(); + if ( - !disabledChangePassword && + changePasswordIsEnabled && usernameMatches.length === 1 && usernameMatches[0].login.password !== loginInfo.password ) { diff --git a/apps/browser/src/autofill/background/service_factories/user-notification-settings-service.factory.ts b/apps/browser/src/autofill/background/service_factories/user-notification-settings-service.factory.ts new file mode 100644 index 00000000000..5e19795e0e6 --- /dev/null +++ b/apps/browser/src/autofill/background/service_factories/user-notification-settings-service.factory.ts @@ -0,0 +1,25 @@ +import { UserNotificationSettingsService } from "@bitwarden/common/autofill/services/user-notification-settings.service"; + +import { + CachedServices, + factory, + FactoryOptions, +} from "../../../platform/background/service-factories/factory-options"; +import { + stateProviderFactory, + StateProviderInitOptions, +} from "../../../platform/background/service-factories/state-provider.factory"; + +export type UserNotificationSettingsServiceInitOptions = FactoryOptions & StateProviderInitOptions; + +export function userNotificationSettingsServiceFactory( + cache: { userNotificationSettingsService?: UserNotificationSettingsService } & CachedServices, + opts: UserNotificationSettingsServiceInitOptions, +): Promise { + return factory( + cache, + "userNotificationSettingsService", + opts, + async () => new UserNotificationSettingsService(await stateProviderFactory(cache, opts)), + ); +} diff --git a/apps/browser/src/autofill/content/notification-bar.ts b/apps/browser/src/autofill/content/notification-bar.ts index 5766017ebe8..f2a74310c29 100644 --- a/apps/browser/src/autofill/content/notification-bar.ts +++ b/apps/browser/src/autofill/content/notification-bar.ts @@ -7,7 +7,11 @@ import { WatchedForm } from "../models/watched-form"; import { NotificationBarIframeInitData } from "../notification/abstractions/notification-bar"; import { FormData } from "../services/abstractions/autofill.service"; import { GlobalSettings, UserSettings } from "../types"; -import { getFromLocalStorage, setupExtensionDisconnectAction } from "../utils"; +import { + getFromLocalStorage, + sendExtensionMessage, + setupExtensionDisconnectAction, +} from "../utils"; interface HTMLElementWithFormOpId extends HTMLElement { formOpId: string; @@ -86,12 +90,11 @@ async function loadNotificationBar() { ]); const changePasswordButtonContainsNames = new Set(["pass", "change", "contras", "senha"]); - // These are preferences for whether to show the notification bar based on the user's settings - // and they are set in the Settings > Options page in the browser extension. - let disabledAddLoginNotification = false; - let disabledChangedPasswordNotification = false; + const enableChangedPasswordPrompt = await sendExtensionMessage( + "bgGetEnableChangedPasswordPrompt", + ); + const enableAddedLoginPrompt = await sendExtensionMessage("bgGetEnableAddedLoginPrompt"); let showNotificationBar = true; - // Look up the active user id from storage const activeUserIdKey = "activeUserId"; const globalStorageKey = "global"; @@ -121,11 +124,7 @@ async function loadNotificationBar() { // Example: '{"bitwarden.com":null}' const excludedDomainsDict = globalSettings.neverDomains; if (!excludedDomainsDict || !(window.location.hostname in excludedDomainsDict)) { - // Set local disabled preferences - disabledAddLoginNotification = globalSettings.disableAddLoginNotification; - disabledChangedPasswordNotification = globalSettings.disableChangedPasswordNotification; - - if (!disabledAddLoginNotification || !disabledChangedPasswordNotification) { + if (enableAddedLoginPrompt || enableChangedPasswordPrompt) { // If the user has not disabled both notifications, then handle the initial page change (null -> actual page) handlePageChange(); } @@ -352,9 +351,7 @@ async function loadNotificationBar() { // to avoid missing any forms that are added after the page loads observeDom(); - sendPlatformMessage({ - command: "checkNotificationQueue", - }); + void sendExtensionMessage("checkNotificationQueue"); } // This is a safeguard in case the observer misses a SPA page change. @@ -392,10 +389,7 @@ async function loadNotificationBar() { * * */ function collectPageDetails() { - sendPlatformMessage({ - command: "bgCollectPageDetails", - sender: "notificationBar", - }); + void sendExtensionMessage("bgCollectPageDetails", { sender: "notificationBar" }); } // End Page Detail Collection Methods @@ -620,10 +614,9 @@ async function loadNotificationBar() { continue; } - const disabledBoth = disabledChangedPasswordNotification && disabledAddLoginNotification; - // if user has not disabled both notifications and we have a username and password field, + // if user has enabled either add login or change password notification, and we have a username and password field if ( - !disabledBoth && + (enableChangedPasswordPrompt || enableAddedLoginPrompt) && watchedForms[i].usernameEl != null && watchedForms[i].passwordEl != null ) { @@ -639,10 +632,7 @@ async function loadNotificationBar() { const passwordPopulated = login.password != null && login.password !== ""; if (userNamePopulated && passwordPopulated) { processedForm(form); - sendPlatformMessage({ - command: "bgAddLogin", - login, - }); + void sendExtensionMessage("bgAddLogin", { login }); break; } else if ( userNamePopulated && @@ -659,7 +649,7 @@ async function loadNotificationBar() { // if user has not disabled the password changed notification and we have multiple password fields, // then check if the user has changed their password - if (!disabledChangedPasswordNotification && watchedForms[i].passwordEls != null) { + if (enableChangedPasswordPrompt && watchedForms[i].passwordEls != null) { // Get the values of the password fields const passwords: string[] = watchedForms[i].passwordEls .filter((el: HTMLInputElement) => el.value != null && el.value !== "") @@ -716,7 +706,7 @@ async function loadNotificationBar() { currentPassword: curPass, url: document.URL, }; - sendPlatformMessage({ command: "bgChangedPassword", data }); + void sendExtensionMessage("bgChangedPassword", { data }); break; } } @@ -954,9 +944,7 @@ async function loadNotificationBar() { switch (barType) { case "add": case "change": - sendPlatformMessage({ - command: "bgRemoveTabFromNotificationQueue", - }); + void sendExtensionMessage("bgRemoveTabFromNotificationQueue"); break; default: break; @@ -981,12 +969,6 @@ async function loadNotificationBar() { // End Notification Bar Functions (open, close, height adjustment, etc.) // Helper Functions - function sendPlatformMessage(msg: any) { - // 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 isInIframe() { try { return window.self !== window.top; diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 3fd0bf54342..055e92a31c4 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -166,11 +166,10 @@ describe("AutofillService", () => { jest .spyOn(autofillService, "getOverlayVisibility") .mockResolvedValue(AutofillOverlayVisibility.OnFieldFocus); + jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); }); it("accepts an extension message sender and injects the autofill scripts into the tab of the sender", async () => { - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); - await autofillService.injectAutofillScripts(sender.tab, sender.frameId, true); [autofillOverlayBootstrapScript, ...defaultAutofillScripts].forEach((scriptName) => { @@ -195,11 +194,6 @@ describe("AutofillService", () => { }); it("will inject the bootstrap-autofill-overlay script if the user has the autofill overlay enabled", async () => { - jest - .spyOn(autofillService, "getOverlayVisibility") - .mockResolvedValue(AutofillOverlayVisibility.OnFieldFocus); - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); - await autofillService.injectAutofillScripts(sender.tab, sender.frameId); expect(BrowserApi.executeScriptInTab).toHaveBeenCalledWith(tabMock.id, { @@ -218,7 +212,6 @@ describe("AutofillService", () => { jest .spyOn(autofillService, "getOverlayVisibility") .mockResolvedValue(AutofillOverlayVisibility.Off); - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); await autofillService.injectAutofillScripts(sender.tab, sender.frameId); @@ -235,8 +228,6 @@ describe("AutofillService", () => { }); it("injects the content-message-handler script if not injecting on page load", async () => { - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); - await autofillService.injectAutofillScripts(sender.tab, sender.frameId, false); expect(BrowserApi.executeScriptInTab).toHaveBeenCalledWith(tabMock.id, { diff --git a/apps/browser/src/autofill/types/index.ts b/apps/browser/src/autofill/types/index.ts index b3b803b7ab4..d9ae0b16f47 100644 --- a/apps/browser/src/autofill/types/index.ts +++ b/apps/browser/src/autofill/types/index.ts @@ -39,10 +39,7 @@ export type UserSettings = { vaultTimeoutAction: VaultTimeoutAction; }; -export type GlobalSettings = Pick< - GlobalState, - "disableAddLoginNotification" | "disableChangedPasswordNotification" | "neverDomains" ->; +export type GlobalSettings = Pick; /** * A HTMLElement (usually a form element) with additional custom properties added by this script diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 9e7fc3f08b7..b214ba0d390 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -55,6 +55,10 @@ import { BadgeSettingsServiceAbstraction, BadgeSettingsService, } from "@bitwarden/common/autofill/services/badge-settings.service"; +import { + UserNotificationSettingsService, + UserNotificationSettingsServiceAbstraction, +} from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { AppIdService as AppIdServiceAbstraction } from "@bitwarden/common/platform/abstractions/app-id.service"; import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction"; import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto-function.service"; @@ -248,6 +252,7 @@ export default class MainBackground { searchService: SearchServiceAbstraction; notificationsService: NotificationsServiceAbstraction; stateService: StateServiceAbstraction; + userNotificationSettingsService: UserNotificationSettingsServiceAbstraction; autofillSettingsService: AutofillSettingsServiceAbstraction; badgeSettingsService: BadgeSettingsServiceAbstraction; systemService: SystemServiceAbstraction; @@ -419,6 +424,7 @@ export default class MainBackground { this.environmentService, migrationRunner, ); + this.userNotificationSettingsService = new UserNotificationSettingsService(this.stateProvider); this.platformUtilsService = new BrowserPlatformUtilsService( this.messagingService, (clipboardValue, clearMs) => { @@ -846,6 +852,7 @@ export default class MainBackground { this.policyService, this.folderService, this.stateService, + this.userNotificationSettingsService, this.environmentService, this.logService, ); @@ -1088,10 +1095,12 @@ export default class MainBackground { this.keyConnectorService.clear(), this.vaultFilterService.clear(), this.biometricStateService.logout(userId), - /* We intentionally do not clear: - * - autofillSettingsService - * - badgeSettingsService - */ + /* + We intentionally do not clear: + - autofillSettingsService + - badgeSettingsService + - userNotificationSettingsService + */ ]); //Needs to be checked before state is cleaned diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 35849a98e1d..dd081b7877f 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -42,6 +42,10 @@ import { AutofillSettingsService, AutofillSettingsServiceAbstraction, } from "@bitwarden/common/autofill/services/autofill-settings.service"; +import { + UserNotificationSettingsService, + UserNotificationSettingsServiceAbstraction, +} from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; @@ -551,6 +555,11 @@ function getBgService(service: keyof MainBackground) { useClass: AutofillSettingsService, deps: [StateProvider, PolicyService], }, + { + provide: UserNotificationSettingsServiceAbstraction, + useClass: UserNotificationSettingsService, + deps: [StateProvider], + }, ], }) export class ServicesModule {} diff --git a/apps/browser/src/popup/settings/options.component.ts b/apps/browser/src/popup/settings/options.component.ts index 2a68fe73ac0..d798714b5f4 100644 --- a/apps/browser/src/popup/settings/options.component.ts +++ b/apps/browser/src/popup/settings/options.component.ts @@ -5,6 +5,7 @@ import { AbstractThemingService } from "@bitwarden/angular/platform/services/the import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { BadgeSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/badge-settings.service"; +import { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -47,6 +48,7 @@ export class OptionsComponent implements OnInit { constructor( private messagingService: MessagingService, private stateService: StateService, + private userNotificationSettingsService: UserNotificationSettingsServiceAbstraction, private autofillSettingsService: AutofillSettingsServiceAbstraction, private badgeSettingsService: BadgeSettingsServiceAbstraction, i18nService: I18nService, @@ -95,10 +97,13 @@ export class OptionsComponent implements OnInit { this.autofillSettingsService.autofillOnPageLoadDefault$, ); - this.enableAddLoginNotification = !(await this.stateService.getDisableAddLoginNotification()); + this.enableAddLoginNotification = await firstValueFrom( + this.userNotificationSettingsService.enableAddedLoginPrompt$, + ); - this.enableChangedPasswordNotification = - !(await this.stateService.getDisableChangedPasswordNotification()); + this.enableChangedPasswordNotification = await firstValueFrom( + this.userNotificationSettingsService.enableChangedPasswordPrompt$, + ); this.enableContextMenuItem = !(await this.stateService.getDisableContextMenuItem()); @@ -122,12 +127,14 @@ export class OptionsComponent implements OnInit { } async updateAddLoginNotification() { - await this.stateService.setDisableAddLoginNotification(!this.enableAddLoginNotification); + await this.userNotificationSettingsService.setEnableAddedLoginPrompt( + this.enableAddLoginNotification, + ); } async updateChangedPasswordNotification() { - await this.stateService.setDisableChangedPasswordNotification( - !this.enableChangedPasswordNotification, + await this.userNotificationSettingsService.setEnableChangedPasswordPrompt( + this.enableChangedPasswordNotification, ); } diff --git a/libs/common/src/autofill/services/user-notification-settings.service.ts b/libs/common/src/autofill/services/user-notification-settings.service.ts new file mode 100644 index 00000000000..1a3a387df6b --- /dev/null +++ b/libs/common/src/autofill/services/user-notification-settings.service.ts @@ -0,0 +1,60 @@ +import { map, Observable } from "rxjs"; + +import { + USER_NOTIFICATION_SETTINGS_DISK, + GlobalState, + KeyDefinition, + StateProvider, +} from "../../platform/state"; + +const ENABLE_ADDED_LOGIN_PROMPT = new KeyDefinition( + USER_NOTIFICATION_SETTINGS_DISK, + "enableAddedLoginPrompt", + { + deserializer: (value: boolean) => value ?? true, + }, +); +const ENABLE_CHANGED_PASSWORD_PROMPT = new KeyDefinition( + USER_NOTIFICATION_SETTINGS_DISK, + "enableChangedPasswordPrompt", + { + deserializer: (value: boolean) => value ?? true, + }, +); + +export abstract class UserNotificationSettingsServiceAbstraction { + enableAddedLoginPrompt$: Observable; + setEnableAddedLoginPrompt: (newValue: boolean) => Promise; + enableChangedPasswordPrompt$: Observable; + setEnableChangedPasswordPrompt: (newValue: boolean) => Promise; +} + +export class UserNotificationSettingsService implements UserNotificationSettingsServiceAbstraction { + private enableAddedLoginPromptState: GlobalState; + readonly enableAddedLoginPrompt$: Observable; + + private enableChangedPasswordPromptState: GlobalState; + readonly enableChangedPasswordPrompt$: Observable; + + constructor(private stateProvider: StateProvider) { + this.enableAddedLoginPromptState = this.stateProvider.getGlobal(ENABLE_ADDED_LOGIN_PROMPT); + this.enableAddedLoginPrompt$ = this.enableAddedLoginPromptState.state$.pipe( + map((x) => x ?? true), + ); + + this.enableChangedPasswordPromptState = this.stateProvider.getGlobal( + ENABLE_CHANGED_PASSWORD_PROMPT, + ); + this.enableChangedPasswordPrompt$ = this.enableChangedPasswordPromptState.state$.pipe( + map((x) => x ?? true), + ); + } + + async setEnableAddedLoginPrompt(newValue: boolean): Promise { + await this.enableAddedLoginPromptState.update(() => newValue); + } + + async setEnableChangedPasswordPrompt(newValue: boolean): Promise { + await this.enableChangedPasswordPromptState.update(() => newValue); + } +} diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 7f079be45f8..338e3b6df3d 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -200,13 +200,6 @@ export abstract class StateService { setDecryptedSends: (value: SendView[], options?: StorageOptions) => Promise; getDefaultUriMatch: (options?: StorageOptions) => Promise; setDefaultUriMatch: (value: UriMatchType, options?: StorageOptions) => Promise; - getDisableAddLoginNotification: (options?: StorageOptions) => Promise; - setDisableAddLoginNotification: (value: boolean, options?: StorageOptions) => Promise; - getDisableChangedPasswordNotification: (options?: StorageOptions) => Promise; - setDisableChangedPasswordNotification: ( - value: boolean, - options?: StorageOptions, - ) => Promise; getDisableContextMenuItem: (options?: StorageOptions) => Promise; setDisableContextMenuItem: (value: boolean, options?: StorageOptions) => Promise; /** diff --git a/libs/common/src/platform/models/domain/global-state.ts b/libs/common/src/platform/models/domain/global-state.ts index 3b7799ee5da..d7c1d93d035 100644 --- a/libs/common/src/platform/models/domain/global-state.ts +++ b/libs/common/src/platform/models/domain/global-state.ts @@ -26,8 +26,6 @@ export class GlobalState { enableBrowserIntegrationFingerprint?: boolean; enableDuckDuckGoBrowserIntegration?: boolean; neverDomains?: { [id: string]: unknown }; - disableAddLoginNotification?: boolean; - disableChangedPasswordNotification?: boolean; disableContextMenuItem?: boolean; deepLinkRedirectUrl?: string; } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index cb3b3c8c870..22a3c884f15 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -853,45 +853,6 @@ export class StateService< ); } - async getDisableAddLoginNotification(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.disableAddLoginNotification ?? false - ); - } - - async setDisableAddLoginNotification(value: boolean, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.disableAddLoginNotification = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - - async getDisableChangedPasswordNotification(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.disableChangedPasswordNotification ?? false - ); - } - - async setDisableChangedPasswordNotification( - value: boolean, - options?: StorageOptions, - ): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.disableChangedPasswordNotification = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getDisableContextMenuItem(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index c5be07023e8..fe419f00cbb 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -63,6 +63,10 @@ export const VAULT_FILTER_DISK = new StateDefinition("vaultFilter", "disk", { web: "disk-local", }); +export const USER_NOTIFICATION_SETTINGS_DISK = new StateDefinition( + "userNotificationSettings", + "disk", +); export const CLEAR_EVENT_DISK = new StateDefinition("clearEvent", "disk"); export const NEW_WEB_LAYOUT_BANNER_DISK = new StateDefinition("newWebLayoutBanner", "disk", { diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 7ed50c4206d..3b217135394 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -23,6 +23,7 @@ import { ClearClipboardDelayMigrator } from "./migrations/25-move-clear-clipboar import { RevertLastSyncMigrator } from "./migrations/26-revert-move-last-sync-to-state-provider"; import { BadgeSettingsMigrator } from "./migrations/27-move-badge-settings-to-state-providers"; import { MoveBiometricUnlockToStateProviders } from "./migrations/28-move-biometric-unlock-to-state-providers"; +import { UserNotificationSettingsKeyMigrator } from "./migrations/29-move-user-notification-settings-to-state-provider"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; @@ -33,7 +34,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 28; +export const CURRENT_VERSION = 29; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -64,7 +65,8 @@ export function createMigrationBuilder() { .with(ClearClipboardDelayMigrator, 24, 25) .with(RevertLastSyncMigrator, 25, 26) .with(BadgeSettingsMigrator, 26, 27) - .with(MoveBiometricUnlockToStateProviders, 27, CURRENT_VERSION); + .with(MoveBiometricUnlockToStateProviders, 27, 28) + .with(UserNotificationSettingsKeyMigrator, 28, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/29-move-user-notification-settings-to-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/29-move-user-notification-settings-to-state-provider.spec.ts new file mode 100644 index 00000000000..3dbf68b35b8 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/29-move-user-notification-settings-to-state-provider.spec.ts @@ -0,0 +1,102 @@ +import { MockProxy } from "jest-mock-extended"; + +import { StateDefinitionLike, MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { UserNotificationSettingsKeyMigrator } from "./29-move-user-notification-settings-to-state-provider"; + +function exampleJSON() { + return { + global: { + disableAddLoginNotification: false, + disableChangedPasswordNotification: false, + otherStuff: "otherStuff1", + }, + }; +} + +function rollbackJSON() { + return { + global_userNotificationSettings_enableAddedLoginPrompt: true, + global_userNotificationSettings_enableChangedPasswordPrompt: true, + global: { + otherStuff: "otherStuff1", + }, + }; +} + +const userNotificationSettingsLocalStateDefinition: { + stateDefinition: StateDefinitionLike; +} = { + stateDefinition: { + name: "userNotificationSettings", + }, +}; + +describe("ProviderKeysMigrator", () => { + let helper: MockProxy; + let sut: UserNotificationSettingsKeyMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 28); + sut = new UserNotificationSettingsKeyMigrator(28, 29); + }); + + it("should remove disableAddLoginNotification and disableChangedPasswordNotification global setting", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledTimes(2); + expect(helper.set).toHaveBeenCalledWith("global", { otherStuff: "otherStuff1" }); + expect(helper.set).toHaveBeenCalledWith("global", { otherStuff: "otherStuff1" }); + }); + + it("should set global user notification setting values", async () => { + await sut.migrate(helper); + + expect(helper.setToGlobal).toHaveBeenCalledTimes(2); + expect(helper.setToGlobal).toHaveBeenCalledWith( + { ...userNotificationSettingsLocalStateDefinition, key: "enableAddedLoginPrompt" }, + true, + ); + expect(helper.setToGlobal).toHaveBeenCalledWith( + { ...userNotificationSettingsLocalStateDefinition, key: "enableChangedPasswordPrompt" }, + true, + ); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 29); + sut = new UserNotificationSettingsKeyMigrator(28, 29); + }); + + it("should null out new global values", async () => { + await sut.rollback(helper); + + expect(helper.setToGlobal).toHaveBeenCalledTimes(2); + expect(helper.setToGlobal).toHaveBeenCalledWith( + { ...userNotificationSettingsLocalStateDefinition, key: "enableAddedLoginPrompt" }, + null, + ); + expect(helper.setToGlobal).toHaveBeenCalledWith( + { ...userNotificationSettingsLocalStateDefinition, key: "enableChangedPasswordPrompt" }, + null, + ); + }); + + it("should add explicit global values back", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledTimes(2); + expect(helper.set).toHaveBeenCalledWith("global", { + disableAddLoginNotification: false, + otherStuff: "otherStuff1", + }); + expect(helper.set).toHaveBeenCalledWith("global", { + disableChangedPasswordNotification: false, + otherStuff: "otherStuff1", + }); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/29-move-user-notification-settings-to-state-provider.ts b/libs/common/src/state-migrations/migrations/29-move-user-notification-settings-to-state-provider.ts new file mode 100644 index 00000000000..6c26882e66a --- /dev/null +++ b/libs/common/src/state-migrations/migrations/29-move-user-notification-settings-to-state-provider.ts @@ -0,0 +1,105 @@ +import { MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedGlobalState = { + disableAddLoginNotification?: boolean; + disableChangedPasswordNotification?: boolean; +}; + +export class UserNotificationSettingsKeyMigrator extends Migrator<28, 29> { + async migrate(helper: MigrationHelper): Promise { + const globalState = await helper.get("global"); + + // disableAddLoginNotification -> enableAddedLoginPrompt + if (globalState?.disableAddLoginNotification != null) { + await helper.setToGlobal( + { + stateDefinition: { + name: "userNotificationSettings", + }, + key: "enableAddedLoginPrompt", + }, + !globalState.disableAddLoginNotification, + ); + + // delete `disableAddLoginNotification` from state global + delete globalState.disableAddLoginNotification; + + await helper.set("global", globalState); + } + + // disableChangedPasswordNotification -> enableChangedPasswordPrompt + if (globalState?.disableChangedPasswordNotification != null) { + await helper.setToGlobal( + { + stateDefinition: { + name: "userNotificationSettings", + }, + key: "enableChangedPasswordPrompt", + }, + !globalState.disableChangedPasswordNotification, + ); + + // delete `disableChangedPasswordNotification` from state global + delete globalState.disableChangedPasswordNotification; + + await helper.set("global", globalState); + } + } + + async rollback(helper: MigrationHelper): Promise { + const globalState = (await helper.get("global")) || {}; + + const enableAddedLoginPrompt: boolean = await helper.getFromGlobal({ + stateDefinition: { + name: "userNotificationSettings", + }, + key: "enableAddedLoginPrompt", + }); + + const enableChangedPasswordPrompt: boolean = await helper.getFromGlobal({ + stateDefinition: { + name: "userNotificationSettings", + }, + key: "enableChangedPasswordPrompt", + }); + + // enableAddedLoginPrompt -> disableAddLoginNotification + if (enableAddedLoginPrompt) { + await helper.set("global", { + ...globalState, + disableAddLoginNotification: !enableAddedLoginPrompt, + }); + + // remove the global state provider framework key for `enableAddedLoginPrompt` + await helper.setToGlobal( + { + stateDefinition: { + name: "userNotificationSettings", + }, + key: "enableAddedLoginPrompt", + }, + null, + ); + } + + // enableChangedPasswordPrompt -> disableChangedPasswordNotification + if (enableChangedPasswordPrompt) { + await helper.set("global", { + ...globalState, + disableChangedPasswordNotification: !enableChangedPasswordPrompt, + }); + + // remove the global state provider framework key for `enableChangedPasswordPrompt` + await helper.setToGlobal( + { + stateDefinition: { + name: "userNotificationSettings", + }, + key: "enableChangedPasswordPrompt", + }, + null, + ); + } + } +}