1
0
mirror of https://github.com/bitwarden/browser synced 2026-01-30 16:23:53 +00:00

[PM-30808] Migrate Phishing Detection storage to PhishingIndexedDbService (#18517) (#18592)

* Initial changes to look at phishing indexeddb service and removal of obsolete compression code

* Convert background update to rxjs format and trigger via subject. Update test cases

* Added addUrls function to use instead of saveUrls so appending daily does not clear all urls

* Added debug logs to phishing-indexeddb service

* Added a fallback url when downloading phishing url list

* Remove obsolete comments

* Fix testUrl default, false scenario and test cases

* Add default return on isPhishingWebAddress

* Added log statement

* Change hostname to href in hasUrl check

* Save fallback response

* Fix matching subpaths in links. Update test cases

* Fix meta data updates storing last checked instead of last updated

* Update QA phishing url to be normalized

* Filter web addresses

* Return previous meta to keep subscription alive

(cherry picked from commit 178fd9a577)

Co-authored-by: Leslie Tilton <23057410+Banrion@users.noreply.github.com>
This commit is contained in:
Alex
2026-01-27 10:30:30 -05:00
committed by GitHub
parent 2c3701358d
commit 15ff818fa8
3 changed files with 124 additions and 3 deletions

View File

@@ -1,6 +1,8 @@
export type PhishingResource = {
name?: string;
remoteUrl: string;
/** Fallback URL to use if remoteUrl fails (e.g., due to SSL interception/cert issues) */
fallbackUrl: string;
checksumUrl: string;
todayUrl: string;
/** Matcher used to decide whether a given URL matches an entry from this resource */
@@ -21,6 +23,8 @@ export const PHISHING_RESOURCES: Record<PhishingResourceType, PhishingResource[]
{
name: "Phishing.Database Domains",
remoteUrl: "https://phish.co.za/latest/phishing-domains-ACTIVE.txt",
fallbackUrl:
"https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-domains-ACTIVE.txt",
checksumUrl:
"https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-domains-ACTIVE.txt.md5",
todayUrl:
@@ -48,6 +52,8 @@ export const PHISHING_RESOURCES: Record<PhishingResourceType, PhishingResource[]
{
name: "Phishing.Database Links",
remoteUrl: "https://phish.co.za/latest/phishing-links-ACTIVE.txt",
fallbackUrl:
"https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-links-ACTIVE.txt",
checksumUrl:
"https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-links-ACTIVE.txt.md5",
todayUrl:
@@ -75,10 +81,10 @@ export const PHISHING_RESOURCES: Record<PhishingResourceType, PhishingResource[]
return true;
}
// Check if URL starts with entry (prefix match for subpaths/query/hash)
// e.g., entry "site.com/phish" matches "site.com/phish/subpage" or "site.com/phish?id=1"
// Check if URL starts with entry (prefix match for query/hash only, NOT subpaths)
// e.g., entry "site.com/phish" matches "site.com/phish?id=1" or "site.com/phish#section"
// but NOT "site.com/phish/subpage" (different endpoint)
if (
urlNoProto.startsWith(entryNoProto + "/") ||
urlNoProto.startsWith(entryNoProto + "?") ||
urlNoProto.startsWith(entryNoProto + "#")
) {

View File

@@ -215,6 +215,86 @@ describe("PhishingIndexedDbService", () => {
});
});
describe("addUrls", () => {
it("appends URLs to IndexedDB without clearing", async () => {
// Pre-populate store with existing data
mockStore.set("https://existing.com", { url: "https://existing.com" });
const urls = ["https://phishing.com", "https://malware.net"];
const result = await service.addUrls(urls);
expect(result).toBe(true);
expect(mockDb.transaction).toHaveBeenCalledWith("phishing-urls", "readwrite");
expect(mockObjectStore.clear).not.toHaveBeenCalled();
expect(mockObjectStore.put).toHaveBeenCalledTimes(2);
// Existing data should still be present
expect(mockStore.has("https://existing.com")).toBe(true);
expect(mockStore.size).toBe(3);
expect(mockDb.close).toHaveBeenCalled();
});
it("handles empty array without clearing", async () => {
mockStore.set("https://existing.com", { url: "https://existing.com" });
const result = await service.addUrls([]);
expect(result).toBe(true);
expect(mockObjectStore.clear).not.toHaveBeenCalled();
expect(mockStore.has("https://existing.com")).toBe(true);
});
it("trims whitespace from URLs", async () => {
const urls = [" https://example.com ", "\nhttps://test.org\n"];
await service.addUrls(urls);
expect(mockObjectStore.put).toHaveBeenCalledWith({ url: "https://example.com" });
expect(mockObjectStore.put).toHaveBeenCalledWith({ url: "https://test.org" });
});
it("skips empty lines", async () => {
const urls = ["https://example.com", "", " ", "https://test.org"];
await service.addUrls(urls);
expect(mockObjectStore.put).toHaveBeenCalledTimes(2);
});
it("handles duplicate URLs via upsert", async () => {
mockStore.set("https://example.com", { url: "https://example.com" });
const urls = [
"https://example.com", // Already exists
"https://test.org",
];
const result = await service.addUrls(urls);
expect(result).toBe(true);
expect(mockObjectStore.put).toHaveBeenCalledTimes(2);
expect(mockStore.size).toBe(2);
});
it("logs error and returns false on failure", async () => {
const error = new Error("IndexedDB error");
mockOpenRequest.error = error;
(global.indexedDB.open as jest.Mock).mockImplementation(() => {
setTimeout(() => {
mockOpenRequest.onerror?.();
}, 0);
return mockOpenRequest;
});
const result = await service.addUrls(["https://test.com"]);
expect(result).toBe(false);
expect(logService.error).toHaveBeenCalledWith(
"[PhishingIndexedDbService] Add failed",
expect.any(Error),
);
});
});
describe("hasUrl", () => {
it("returns true for existing URL", async () => {
mockStore.set("https://example.com", { url: "https://example.com" });

View File

@@ -53,6 +53,9 @@ export class PhishingIndexedDbService {
* @returns `true` if save succeeded, `false` on error
*/
async saveUrls(urls: string[]): Promise<boolean> {
this.logService.debug(
`[PhishingIndexedDbService] Clearing and saving ${urls.length} to the store...`,
);
let db: IDBDatabase | null = null;
try {
db = await this.openDatabase();
@@ -67,6 +70,29 @@ export class PhishingIndexedDbService {
}
}
/**
* Adds an array of phishing URLs to IndexedDB.
* Appends to existing data without clearing.
*
* @param urls - Array of phishing URLs to add
* @returns `true` if add succeeded, `false` on error
*/
async addUrls(urls: string[]): Promise<boolean> {
this.logService.debug(`[PhishingIndexedDbService] Adding ${urls.length} to the store...`);
let db: IDBDatabase | null = null;
try {
db = await this.openDatabase();
await this.saveChunked(db, urls);
return true;
} catch (error) {
this.logService.error("[PhishingIndexedDbService] Add failed", error);
return false;
} finally {
db?.close();
}
}
/**
* Saves URLs in chunks to prevent transaction timeouts and UI freezes.
*/
@@ -100,6 +126,8 @@ export class PhishingIndexedDbService {
* @returns `true` if URL exists, `false` if not found or on error
*/
async hasUrl(url: string): Promise<boolean> {
this.logService.debug(`[PhishingIndexedDbService] Checking if store contains ${url}...`);
let db: IDBDatabase | null = null;
try {
db = await this.openDatabase();
@@ -130,6 +158,8 @@ export class PhishingIndexedDbService {
* @returns Array of all stored URLs, or empty array on error
*/
async loadAllUrls(): Promise<string[]> {
this.logService.debug("[PhishingIndexedDbService] Loading all urls from store...");
let db: IDBDatabase | null = null;
try {
db = await this.openDatabase();
@@ -227,11 +257,16 @@ export class PhishingIndexedDbService {
* @returns `true` if save succeeded, `false` on error
*/
async saveUrlsFromStream(stream: ReadableStream<Uint8Array>): Promise<boolean> {
this.logService.debug("[PhishingIndexedDbService] Saving urls to the store from stream...");
let db: IDBDatabase | null = null;
try {
db = await this.openDatabase();
await this.clearStore(db);
await this.processStream(db, stream);
this.logService.info(
"[PhishingIndexedDbService] Finished saving urls to the store from stream.",
);
return true;
} catch (error) {
this.logService.error("[PhishingIndexedDbService] Stream save failed", error);