From 6f6d62ce90fe2371f050885032eddddaa9b8d683 Mon Sep 17 00:00:00 2001 From: maxkpower Date: Tue, 18 Nov 2025 01:54:03 +0100 Subject: [PATCH] Add persistent storage for ignored phishing sites --- .../browser/src/background/main.background.ts | 1 + .../phishing-detection.service.spec.ts | 4 ++ .../services/phishing-detection.service.ts | 59 +++++++++++++++++-- .../src/platform/browser/browser-api.ts | 28 +++++++-- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index f59b6648486..0868a0dcfab 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1473,6 +1473,7 @@ export default class MainBackground { this.logService, this.phishingDataService, messageListener, + this.globalStateProvider, ); this.ipcContentScriptManagerService = new IpcContentScriptManagerService(this.configService); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts index e33b4b1b4f1..93085ce2c71 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts @@ -5,6 +5,7 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { FakeGlobalStateProvider } from "@bitwarden/common/spec"; import { MessageListener } from "@bitwarden/messaging"; import { PhishingDataService } from "./phishing-data.service"; @@ -17,6 +18,7 @@ describe("PhishingDetectionService", () => { let logService: LogService; let phishingDataService: MockProxy; let messageListener: MockProxy; + let globalStateProvider: FakeGlobalStateProvider; beforeEach(() => { accountService = { getAccount$: jest.fn(() => of(null)) } as any; @@ -29,6 +31,7 @@ describe("PhishingDetectionService", () => { return new Observable(); }, }); + globalStateProvider = new FakeGlobalStateProvider(); }); it("should initialize without errors", () => { @@ -40,6 +43,7 @@ describe("PhishingDetectionService", () => { logService, phishingDataService, messageListener, + globalStateProvider, ); }).not.toThrow(); }); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts index 4917e740be8..560e95d8864 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts @@ -4,6 +4,7 @@ import { distinctUntilChanged, EMPTY, filter, + firstValueFrom, map, merge, of, @@ -18,6 +19,7 @@ import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { CommandDefinition, MessageListener } from "@bitwarden/messaging"; +import { GlobalStateProvider, KeyDefinition, PHISHING_DETECTION_DISK } from "@bitwarden/state"; import { BrowserApi } from "../../../platform/browser/browser-api"; @@ -44,6 +46,17 @@ export const PHISHING_DETECTION_CANCEL_COMMAND = new CommandDefinition<{ tabId: number; }>("phishing-detection-cancel"); +/** + * Key definition for storing hostnames that the user has chosen to ignore + */ +export const IGNORED_PHISHING_HOSTNAMES_KEY = new KeyDefinition( + PHISHING_DETECTION_DISK, + "ignoredPhishingHostnames", + { + deserializer: (value: string[]) => value ?? [], + }, +); + export class PhishingDetectionService { private static _tabUpdated$ = new Subject(); private static _ignoredHostnames = new Set(); @@ -56,6 +69,7 @@ export class PhishingDetectionService { logService: LogService, phishingDataService: PhishingDataService, messageListener: MessageListener, + globalStateProvider: GlobalStateProvider, ) { if (this._didInit) { logService.debug("[PhishingDetectionService] Initialize already called. Aborting."); @@ -64,6 +78,26 @@ export class PhishingDetectionService { logService.debug("[PhishingDetectionService] Initialize called. Checking prerequisites..."); + // Load previously ignored hostnames from storage (synchronously to avoid race conditions) + const ignoredHostnamesState = globalStateProvider.get(IGNORED_PHISHING_HOSTNAMES_KEY); + firstValueFrom(ignoredHostnamesState.state$.pipe(map((hostnames) => hostnames ?? []))) + .then((initialHostnames) => { + this._ignoredHostnames = new Set(initialHostnames); + logService.debug( + `[PhishingDetectionService] Loaded ${initialHostnames.length} ignored hostnames from storage`, + ); + }) + .catch((error) => { + logService.error("[PhishingDetectionService] Failed to load ignored hostnames", error); + }); + + // Subscribe to future state changes + const ignoredHostnamesSub = ignoredHostnamesState.state$ + .pipe(map((hostnames) => hostnames ?? [])) + .subscribe((hostnames) => { + this._ignoredHostnames = new Set(hostnames); + }); + BrowserApi.addListener(chrome.tabs.onUpdated, this._handleTabUpdated.bind(this)); const onContinueCommand$ = messageListener.messages$(PHISHING_DETECTION_CONTINUE_COMMAND).pipe( @@ -71,9 +105,21 @@ export class PhishingDetectionService { logService.debug(`[PhishingDetectionService] user selected continue for ${message.url}`), ), concatMap(async (message) => { - const url = new URL(message.url); - this._ignoredHostnames.add(url.hostname); - await BrowserApi.navigateTabToUrl(message.tabId, url); + try { + const url = new URL(message.url); + this._ignoredHostnames.add(url.hostname); + // Persist to storage + await ignoredHostnamesState.update(() => Array.from(this._ignoredHostnames)); + logService.debug( + `[PhishingDetectionService] Added ${url.hostname} to ignored hostnames (persisted)`, + ); + await BrowserApi.navigateTabToUrl(message.tabId, url); + } catch (error) { + logService.error( + `[PhishingDetectionService] Failed to process continue command for URL: ${message.url}`, + error, + ); + } }), ); @@ -97,8 +143,10 @@ export class PhishingDetectionService { tap((event) => logService.debug(`[PhishingDetectionService] processing event:`, event)), concatMap(async ({ tabId, url, ignored }) => { if (ignored) { - // The next time this host is visited, block again - this._ignoredHostnames.delete(url.hostname); + // User has previously chosen to continue to this hostname, skip phishing check + logService.debug( + `[PhishingDetectionService] Skipping phishing check for ignored hostname: ${url.hostname}`, + ); return; } const isPhishing = await phishingDataService.isPhishingDomain(url); @@ -158,6 +206,7 @@ export class PhishingDetectionService { this._didInit = true; return () => { initSub.unsubscribe(); + ignoredHostnamesSub.unsubscribe(); this._didInit = false; // Manually type cast to satisfy the listener signature due to the mixture diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts index 723df95ef63..265168322fc 100644 --- a/apps/browser/src/platform/browser/browser-api.ts +++ b/apps/browser/src/platform/browser/browser-api.ts @@ -268,13 +268,31 @@ export class BrowserApi { static async closeTab(tabId: number): Promise { if (tabId) { if (BrowserApi.isWebExtensionsApi) { - browser.tabs.remove(tabId).catch((error) => { + await browser.tabs.remove(tabId).catch((error) => { throw new Error("[BrowserApi] Failed to remove current tab: " + error.message); }); } else if (BrowserApi.isChromeApi) { - chrome.tabs.remove(tabId).catch((error) => { - throw new Error("[BrowserApi] Failed to remove current tab: " + error.message); - }); + if (BrowserApi.isManifestVersion(3)) { + await chrome.tabs.remove(tabId).catch((error) => { + throw new Error("[BrowserApi] Failed to remove current tab: " + error.message); + }); + } else { + // Manifest V2 uses callbacks + return new Promise((resolve, reject) => { + chrome.tabs.remove(tabId, () => { + if (chrome.runtime.lastError) { + reject( + new Error( + "[BrowserApi] Failed to remove current tab: " + + chrome.runtime.lastError.message, + ), + ); + } else { + resolve(); + } + }); + }); + } } } } @@ -288,7 +306,7 @@ export class BrowserApi { static async navigateTabToUrl(tabId: number, url: URL): Promise { if (tabId) { if (BrowserApi.isWebExtensionsApi) { - browser.tabs.update(tabId, { url: url.href }).catch((error) => { + await browser.tabs.update(tabId, { url: url.href }).catch((error) => { throw new Error("Failed to navigate tab to URL: " + error.message); }); } else if (BrowserApi.isChromeApi) {