From 178fd9a5776f120516c9bf51d7234b9cf8bc4381 Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Mon, 26 Jan 2026 10:16:40 -0600 Subject: [PATCH 1/6] [PM-30808] Migrate Phishing Detection storage to PhishingIndexedDbService (#18517) * 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 --- .../phishing-detection/phishing-resources.ts | 12 +- .../services/phishing-data.service.spec.ts | 512 ++++++++++-------- .../services/phishing-data.service.ts | 488 ++++++++--------- .../phishing-indexeddb.service.spec.ts | 80 +++ .../services/phishing-indexeddb.service.ts | 35 ++ 5 files changed, 638 insertions(+), 489 deletions(-) diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts index 4cd155c8ae3..88068987dd7 100644 --- a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -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 */ @@ -19,6 +21,8 @@ export const PHISHING_RESOURCES: Record - new Promise((resolve) => jest.requireActual("timers").setImmediate(resolve)); - -// [FIXME] Move mocking and compression helpers to a shared test utils library -// to separate from phishing data service tests. -export const setupPhishingMocks = (mockedResult: string | ArrayBuffer = "mocked-data") => { - // Store original globals - const originals = { - Response: global.Response, - CompressionStream: global.CompressionStream, - DecompressionStream: global.DecompressionStream, - Blob: global.Blob, - atob: global.atob, - btoa: global.btoa, - }; - - // Mock missing or browser-only globals - global.atob = (str) => Buffer.from(str, "base64").toString("binary"); - global.btoa = (str) => Buffer.from(str, "binary").toString("base64"); - - (global as any).CompressionStream = class {}; - (global as any).DecompressionStream = class {}; - - global.Blob = class { - constructor(public parts: any[]) {} - stream() { - return { pipeThrough: () => ({}) }; - } - } as any; - - global.Response = class { - body = { pipeThrough: () => ({}) }; - // Return string for decompression - text() { - return Promise.resolve(typeof mockedResult === "string" ? mockedResult : ""); - } - // Return ArrayBuffer for compression - arrayBuffer() { - if (typeof mockedResult === "string") { - const bytes = new TextEncoder().encode(mockedResult); - return Promise.resolve(bytes.buffer); - } - - return Promise.resolve(mockedResult); - } - } as any; - - // Cleanup function - return () => { - Object.assign(global, originals); - }; -}; +import { PHISHING_DOMAINS_META_KEY, PhishingDataService } from "./phishing-data.service"; +import type { PhishingIndexedDbService } from "./phishing-indexeddb.service"; describe("PhishingDataService", () => { let service: PhishingDataService; @@ -76,33 +19,30 @@ describe("PhishingDataService", () => { let taskSchedulerService: TaskSchedulerService; let logService: MockProxy; let platformUtilsService: MockProxy; + let mockIndexedDbService: MockProxy; const fakeGlobalStateProvider: FakeGlobalStateProvider = new FakeGlobalStateProvider(); - - const setMockMeta = (state: PhishingDataMeta) => { - fakeGlobalStateProvider.getFake(PHISHING_DOMAINS_META_KEY).stateSubject.next(state); - return state; - }; - const setMockBlob = (state: PhishingDataBlob) => { - fakeGlobalStateProvider.getFake(PHISHING_DOMAINS_BLOB_KEY).stateSubject.next(state); - return state; - }; - let fetchChecksumSpy: jest.SpyInstance; - let fetchAndCompressSpy: jest.SpyInstance; - - const mockMeta: PhishingDataMeta = { - checksum: "abc", - timestamp: Date.now(), - applicationVersion: "1.0.0", - }; - const mockBlob = "http://phish.com\nhttps://badguy.net"; - const mockCompressedBlob = - "H4sIAAAAAAAA/8vMTSzJzM9TSE7MLchJLElVyE9TyC9KSS1S0FFIz8hLz0ksSQUAtK7XMSYAAAA="; beforeEach(async () => { - jest.useFakeTimers(); + jest.clearAllMocks(); + + // Mock Request global if not available + if (typeof Request === "undefined") { + (global as any).Request = class { + constructor(public url: string) {} + }; + } + apiService = mock(); logService = mock(); + mockIndexedDbService = mock(); + + // Set default mock behaviors + mockIndexedDbService.hasUrl.mockResolvedValue(false); + mockIndexedDbService.loadAllUrls.mockResolvedValue([]); + mockIndexedDbService.saveUrls.mockResolvedValue(undefined); + mockIndexedDbService.addUrls.mockResolvedValue(undefined); + mockIndexedDbService.saveUrlsFromStream.mockResolvedValue(undefined); platformUtilsService = mock(); platformUtilsService.getApplicationVersion.mockResolvedValue("1.0.0"); @@ -116,217 +56,315 @@ describe("PhishingDataService", () => { logService, platformUtilsService, ); - fetchChecksumSpy = jest.spyOn(service as any, "fetchPhishingChecksum"); - fetchAndCompressSpy = jest.spyOn(service as any, "fetchAndCompress"); + // Replace the IndexedDB service with our mock + service["indexedDbService"] = mockIndexedDbService; + + fetchChecksumSpy = jest.spyOn(service as any, "fetchPhishingChecksum"); fetchChecksumSpy.mockResolvedValue("new-checksum"); - fetchAndCompressSpy.mockResolvedValue("compressed-blob"); }); describe("initialization", () => { - beforeEach(() => { - jest.spyOn(service as any, "_compressString").mockResolvedValue(mockCompressedBlob); - jest.spyOn(service as any, "_decompressString").mockResolvedValue(mockBlob); + it("should initialize with IndexedDB service", () => { + expect(service["indexedDbService"]).toBeDefined(); }); - it("should perform background update", async () => { - platformUtilsService.getApplicationVersion.mockResolvedValue("1.0.x"); - jest - .spyOn(service as any, "getNextWebAddresses") - .mockResolvedValue({ meta: mockMeta, blob: mockBlob }); - - setMockBlob(mockBlob); - setMockMeta(mockMeta); - - const sub = service.update$.subscribe(); - await flushPromises(); - - const url = new URL("http://phish.com"); - const QAurl = new URL("http://phishing.testcategory.com"); + it("should detect QA test addresses - http protocol", async () => { + const url = new URL("http://phishing.testcategory.com"); expect(await service.isPhishingWebAddress(url)).toBe(true); - expect(await service.isPhishingWebAddress(QAurl)).toBe(true); + // IndexedDB should not be called for test addresses + expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled(); + }); - sub.unsubscribe(); + it("should detect QA test addresses - https protocol", async () => { + const url = new URL("https://phishing.testcategory.com"); + expect(await service.isPhishingWebAddress(url)).toBe(true); + expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled(); + }); + + it("should detect QA test addresses - specific subpath /block", async () => { + const url = new URL("https://phishing.testcategory.com/block"); + expect(await service.isPhishingWebAddress(url)).toBe(true); + expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled(); + }); + + it("should NOT detect QA test addresses - different subpath", async () => { + mockIndexedDbService.hasUrl.mockResolvedValue(false); + mockIndexedDbService.loadAllUrls.mockResolvedValue([]); + + const url = new URL("https://phishing.testcategory.com/other"); + const result = await service.isPhishingWebAddress(url); + + // This should NOT be detected as a test address since only /block subpath is hardcoded + expect(result).toBe(false); + }); + + it("should detect QA test addresses - root path with trailing slash", async () => { + const url = new URL("https://phishing.testcategory.com/"); + const result = await service.isPhishingWebAddress(url); + + // This SHOULD be detected since URLs are normalized (trailing slash added to root URLs) + expect(result).toBe(true); + expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled(); }); }); describe("isPhishingWebAddress", () => { - beforeEach(() => { - jest.spyOn(service as any, "_compressString").mockResolvedValue(mockCompressedBlob); - jest.spyOn(service as any, "_decompressString").mockResolvedValue(mockBlob); - }); + it("should detect a phishing web address using quick hasUrl lookup", async () => { + // Mock hasUrl to return true for direct hostname match + mockIndexedDbService.hasUrl.mockResolvedValue(true); - it("should detect a phishing web address", async () => { - service["_webAddressesSet"] = new Set(["phish.com", "badguy.net"]); - - const url = new URL("http://phish.com"); + const url = new URL("http://phish.com/testing-param"); const result = await service.isPhishingWebAddress(url); expect(result).toBe(true); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/testing-param"); + // Should not fall back to custom matcher when hasUrl returns true + expect(mockIndexedDbService.loadAllUrls).not.toHaveBeenCalled(); + }); + + it("should fall back to custom matcher when hasUrl returns false", async () => { + // Mock hasUrl to return false (no direct href match) + mockIndexedDbService.hasUrl.mockResolvedValue(false); + // Mock loadAllUrls to return phishing URLs for custom matcher + mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com/path"]); + + const url = new URL("http://phish.com/path"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(true); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/path"); + expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); }); it("should not detect a safe web address", async () => { - service["_webAddressesSet"] = new Set(["phish.com", "badguy.net"]); + // Mock hasUrl to return false + mockIndexedDbService.hasUrl.mockResolvedValue(false); + // Mock loadAllUrls to return phishing URLs that don't match + mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com", "http://badguy.net"]); + const url = new URL("http://safe.com"); const result = await service.isPhishingWebAddress(url); + expect(result).toBe(false); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://safe.com/"); + expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); }); - it("should match against root web address", async () => { - service["_webAddressesSet"] = new Set(["phish.com", "badguy.net"]); - const url = new URL("http://phish.com/about"); + it("should not match against root web address with subpaths using custom matcher", async () => { + // Mock hasUrl to return false (no direct href match) + mockIndexedDbService.hasUrl.mockResolvedValue(false); + // Mock loadAllUrls to return entry that matches with subpath + mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com/login"]); + + const url = new URL("http://phish.com/login/page"); const result = await service.isPhishingWebAddress(url); - expect(result).toBe(true); + + expect(result).toBe(false); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/login/page"); + expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); }); - it("should not error on empty state", async () => { - service["_webAddressesSet"] = null; + it("should not match against root web address with different subpaths using custom matcher", async () => { + // Mock hasUrl to return false (no direct hostname match) + mockIndexedDbService.hasUrl.mockResolvedValue(false); + // Mock loadAllUrls to return entry that matches with subpath + mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com/login/page1"]); + + const url = new URL("http://phish.com/login/page2"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(false); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/login/page2"); + expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + }); + + it("should handle IndexedDB errors gracefully", async () => { + // Mock hasUrl to throw error + mockIndexedDbService.hasUrl.mockRejectedValue(new Error("hasUrl error")); + // Mock loadAllUrls to also throw error + mockIndexedDbService.loadAllUrls.mockRejectedValue(new Error("IndexedDB error")); + const url = new URL("http://phish.com/about"); const result = await service.isPhishingWebAddress(url); + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingDataService] IndexedDB lookup via hasUrl failed", + expect.any(Error), + ); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingDataService] Error running custom matcher", + expect.any(Error), + ); }); }); - describe("getNextWebAddresses", () => { - beforeEach(() => { - jest.spyOn(service as any, "_compressString").mockResolvedValue(mockCompressedBlob); - jest.spyOn(service as any, "_decompressString").mockResolvedValue(mockBlob); + describe("data updates", () => { + it("should update full dataset via stream", async () => { + // Mock full dataset update + const mockResponse = { + ok: true, + body: {} as ReadableStream, + } as Response; + apiService.nativeFetch.mockResolvedValue(mockResponse); + + await firstValueFrom(service["_updateFullDataSet"]()); + + expect(mockIndexedDbService.saveUrlsFromStream).toHaveBeenCalled(); }); - it("refetches all web addresses if applicationVersion has changed", async () => { - const prev: PhishingDataMeta = { - timestamp: Date.now() - 60000, - checksum: "old", - applicationVersion: "1.0.0", - }; - fetchChecksumSpy.mockResolvedValue("new"); + it("should update daily dataset via addUrls", async () => { + // Mock daily update + const mockResponse = { + ok: true, + text: jest.fn().mockResolvedValue("newphish.com\nanotherbad.net"), + } as unknown as Response; + apiService.nativeFetch.mockResolvedValue(mockResponse); + + await firstValueFrom(service["_updateDailyDataSet"]()); + + expect(mockIndexedDbService.addUrls).toHaveBeenCalledWith(["newphish.com", "anotherbad.net"]); + }); + + it("should get updated meta information", async () => { + fetchChecksumSpy.mockResolvedValue("new-checksum"); platformUtilsService.getApplicationVersion.mockResolvedValue("2.0.0"); - const result = await service.getNextWebAddresses(prev); + const meta = await firstValueFrom(service["_getUpdatedMeta"]()); - expect(result!.blob).toBe("compressed-blob"); - expect(result!.meta!.checksum).toBe("new"); - expect(result!.meta!.applicationVersion).toBe("2.0.0"); - }); - - it("returns null when checksum matches and cache not expired", async () => { - const prev: PhishingDataMeta = { - timestamp: Date.now(), - checksum: "abc", - applicationVersion: "1.0.0", - }; - fetchChecksumSpy.mockResolvedValue("abc"); - const result = await service.getNextWebAddresses(prev); - expect(result).toBeNull(); - }); - - it("patches daily domains when cache is expired and checksum unchanged", async () => { - const prev: PhishingDataMeta = { - timestamp: 0, - checksum: "old", - applicationVersion: "1.0.0", - }; - const dailyLines = ["b.com", "c.com"]; - fetchChecksumSpy.mockResolvedValue("old"); - jest.spyOn(service as any, "fetchText").mockResolvedValue(dailyLines); - - setMockBlob(mockBlob); - - const expectedBlob = - "H4sIAAAAAAAA/8vMTSzJzM9TSE7MLchJLElVyE9TyC9KSS1S0FFIz8hLz0ksSQUAtK7XMSYAAAA="; - const result = await service.getNextWebAddresses(prev); - - expect(result!.blob).toBe(expectedBlob); - expect(result!.meta!.checksum).toBe("old"); - }); - - it("fetches all domains when checksum has changed", async () => { - const prev: PhishingDataMeta = { - timestamp: 0, - checksum: "old", - applicationVersion: "1.0.0", - }; - fetchChecksumSpy.mockResolvedValue("new"); - fetchAndCompressSpy.mockResolvedValue("new-blob"); - const result = await service.getNextWebAddresses(prev); - expect(result!.blob).toBe("new-blob"); - expect(result!.meta!.checksum).toBe("new"); + expect(meta).toBeDefined(); + expect(meta.checksum).toBe("new-checksum"); + expect(meta.applicationVersion).toBe("2.0.0"); + expect(meta.timestamp).toBeDefined(); }); }); - describe("compression helpers", () => { - let restore: () => void; + describe("phishing meta data updates", () => { + it("should not update metadata when no data updates occur", async () => { + // Set up existing metadata + const existingMeta = { + checksum: "existing-checksum", + timestamp: Date.now() - 1000, // 1 second ago (not expired) + applicationVersion: "1.0.0", + }; + await fakeGlobalStateProvider.get(PHISHING_DOMAINS_META_KEY).update(() => existingMeta); - beforeEach(async () => { - restore = setupPhishingMocks("abc"); + // Mock conditions where no update is needed + fetchChecksumSpy.mockResolvedValue("existing-checksum"); // Same checksum + platformUtilsService.getApplicationVersion.mockResolvedValue("1.0.0"); // Same version + const mockResponse = { + ok: true, + body: {} as ReadableStream, + } as Response; + apiService.nativeFetch.mockResolvedValue(mockResponse); + + // Trigger background update + const result = await firstValueFrom(service["_backgroundUpdate"](existingMeta)); + + // Verify metadata was NOT updated (same reference returned) + expect(result).toEqual(existingMeta); + expect(result?.timestamp).toBe(existingMeta.timestamp); + + // Verify no data updates were performed + expect(mockIndexedDbService.saveUrlsFromStream).not.toHaveBeenCalled(); + expect(mockIndexedDbService.addUrls).not.toHaveBeenCalled(); }); - afterEach(() => { - if (restore) { - restore(); - } - delete (Uint8Array as any).fromBase64; - jest.restoreAllMocks(); + it("should update metadata when full dataset update occurs due to checksum change", async () => { + // Set up existing metadata + const existingMeta = { + checksum: "old-checksum", + timestamp: Date.now() - 1000, + applicationVersion: "1.0.0", + }; + await fakeGlobalStateProvider.get(PHISHING_DOMAINS_META_KEY).update(() => existingMeta); + + // Mock conditions for full update + fetchChecksumSpy.mockResolvedValue("new-checksum"); // Different checksum + platformUtilsService.getApplicationVersion.mockResolvedValue("1.0.0"); + const mockResponse = { + ok: true, + body: {} as ReadableStream, + } as Response; + apiService.nativeFetch.mockResolvedValue(mockResponse); + + // Trigger background update + const result = await firstValueFrom(service["_backgroundUpdate"](existingMeta)); + + // Verify metadata WAS updated with new values + expect(result?.checksum).toBe("new-checksum"); + expect(result?.timestamp).toBeGreaterThan(existingMeta.timestamp); + + // Verify full update was performed + expect(mockIndexedDbService.saveUrlsFromStream).toHaveBeenCalled(); + expect(mockIndexedDbService.addUrls).not.toHaveBeenCalled(); // Daily should not run }); - describe("_compressString", () => { - it("compresses a string to base64", async () => { - const out = await service["_compressString"]("abc"); - expect(out).toBe("YWJj"); // base64 for 'abc' - }); + it("should update metadata when full dataset update occurs due to version change", async () => { + // Set up existing metadata + const existingMeta = { + checksum: "same-checksum", + timestamp: Date.now() - 1000, + applicationVersion: "1.0.0", + }; + await fakeGlobalStateProvider.get(PHISHING_DOMAINS_META_KEY).update(() => existingMeta); - it("compresses using fallback on older browsers", async () => { - const input = "abc"; - const expected = btoa(encodeURIComponent(input)); - const out = await service["_compressString"](input); - expect(out).toBe(expected); - }); + // Mock conditions for full update + fetchChecksumSpy.mockResolvedValue("same-checksum"); + platformUtilsService.getApplicationVersion.mockResolvedValue("2.0.0"); // Different version + const mockResponse = { + ok: true, + body: {} as ReadableStream, + } as Response; + apiService.nativeFetch.mockResolvedValue(mockResponse); - it("compresses using btoa on error", async () => { - const input = "abc"; - const expected = btoa(encodeURIComponent(input)); - const out = await service["_compressString"](input); - expect(out).toBe(expected); - }); + // Trigger background update + const result = await firstValueFrom(service["_backgroundUpdate"](existingMeta)); + + // Verify metadata WAS updated + expect(result?.applicationVersion).toBe("2.0.0"); + expect(result?.timestamp).toBeGreaterThan(existingMeta.timestamp); + + // Verify full update was performed + expect(mockIndexedDbService.saveUrlsFromStream).toHaveBeenCalled(); + expect(mockIndexedDbService.addUrls).not.toHaveBeenCalled(); }); - describe("_decompressString", () => { - it("decompresses a string from base64", async () => { - const base64 = btoa("ignored"); - const out = await service["_decompressString"](base64); - expect(out).toBe("abc"); - }); - it("decompresses using fallback on older browsers", async () => { - // Provide a fromBase64 implementation - (Uint8Array as any).fromBase64 = (b64: string) => new Uint8Array([100, 101, 102]); + it("should update metadata when daily update occurs due to cache expiration", async () => { + // Set up existing metadata (expired cache) + const existingMeta = { + checksum: "same-checksum", + timestamp: Date.now() - 25 * 60 * 60 * 1000, // 25 hours ago (expired) + applicationVersion: "1.0.0", + }; + await fakeGlobalStateProvider.get(PHISHING_DOMAINS_META_KEY).update(() => existingMeta); - const out = await service["_decompressString"]("ignored"); - expect(out).toBe("abc"); - }); + // Mock conditions for daily update only + fetchChecksumSpy.mockResolvedValue("same-checksum"); // Same checksum (no full update) + platformUtilsService.getApplicationVersion.mockResolvedValue("1.0.0"); // Same version + const mockFullResponse = { + ok: true, + body: {} as ReadableStream, + } as Response; + const mockDailyResponse = { + ok: true, + text: jest.fn().mockResolvedValue("newdomain.com"), + } as unknown as Response; + apiService.nativeFetch + .mockResolvedValueOnce(mockFullResponse) + .mockResolvedValueOnce(mockDailyResponse); - it("decompresses using atob on error", async () => { - const base64 = btoa(encodeURIComponent("abc")); - const out = await service["_decompressString"](base64); - expect(out).toBe("abc"); - }); - }); - }); + // Trigger background update + const result = await firstValueFrom(service["_backgroundUpdate"](existingMeta)); - describe("_loadBlobToMemory", () => { - it("loads blob into memory set", async () => { - const prevBlob = "ignored-base64"; - fakeGlobalStateProvider.getFake(PHISHING_DOMAINS_BLOB_KEY).stateSubject.next(prevBlob); + // Verify metadata WAS updated + expect(result?.timestamp).toBeGreaterThan(existingMeta.timestamp); + expect(result?.checksum).toBe("same-checksum"); - jest.spyOn(service as any, "_decompressString").mockResolvedValue("phish.com\nbadguy.net"); - - // Trigger the load pipeline and allow async RxJS processing to complete - service["_loadBlobToMemory"](); - await flushPromises(); - - const set = service["_webAddressesSet"] as Set; - expect(set).toBeDefined(); - expect(set.has("phish.com")).toBe(true); - expect(set.has("badguy.net")).toBe(true); + // Verify only daily update was performed + expect(mockIndexedDbService.saveUrlsFromStream).not.toHaveBeenCalled(); + expect(mockIndexedDbService.addUrls).toHaveBeenCalledWith(["newdomain.com"]); }); }); }); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts index 7d5f04cc276..10268fa7f93 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts @@ -1,17 +1,25 @@ import { catchError, + concatMap, + defer, EMPTY, + exhaustMap, first, - firstValueFrom, + forkJoin, from, + iif, + map, + Observable, of, + retry, share, takeUntil, startWith, Subject, switchMap, tap, - map, + throwError, + timer, } from "rxjs"; import { devFlagEnabled, devFlagValue } from "@bitwarden/browser/platform/flags"; @@ -23,6 +31,8 @@ import { GlobalStateProvider, KeyDefinition, PHISHING_DETECTION_DISK } from "@bi import { getPhishingResources, PhishingResourceType } from "../phishing-resources"; +import { PhishingIndexedDbService } from "./phishing-indexeddb.service"; + /** * Metadata about the phishing data set */ @@ -73,19 +83,16 @@ export class PhishingDataService { // We are adding the destroy to guard against accidental leaks. private _destroy$ = new Subject(); - private _testWebAddresses = this.getTestWebAddresses().concat("phishing.testcategory.com"); // Included for QA to test in prod + private _testWebAddresses = this.getTestWebAddresses(); private _phishingMetaState = this.globalStateProvider.get(PHISHING_DOMAINS_META_KEY); - private _phishingBlobState = this.globalStateProvider.get(PHISHING_DOMAINS_BLOB_KEY); - // In-memory set loaded from blob for fast lookups without reading large storage repeatedly - private _webAddressesSet: Set | null = null; - // Loading variables for web addresses set - // Triggers a load for _webAddressesSet - private _loadTrigger$ = new Subject(); + private indexedDbService: PhishingIndexedDbService; // How often are new web addresses added to the remote? readonly UPDATE_INTERVAL_DURATION = 24 * 60 * 60 * 1000; // 24 hours + private _backgroundUpdateTrigger$ = new Subject(); + private _triggerUpdate$ = new Subject(); update$ = this._triggerUpdate$.pipe( startWith(undefined), // Always emit once @@ -93,12 +100,8 @@ export class PhishingDataService { this._phishingMetaState.state$.pipe( first(), // Only take the first value to avoid an infinite loop when updating the cache below tap((metaState) => { - // Initial loading of web addresses set if not already loaded - if (!this._webAddressesSet) { - this._loadBlobToMemory(); - } - // Perform any updates in the background if needed - void this._backgroundUpdate(metaState); + // Perform any updates in the background + this._backgroundUpdateTrigger$.next(metaState); }), catchError((err: unknown) => { this.logService.error("[PhishingDataService] Background update failed to start.", err); @@ -106,7 +109,6 @@ export class PhishingDataService { }), ), ), - // Stop emitting when dispose() is called takeUntil(this._destroy$), share(), ); @@ -120,6 +122,7 @@ export class PhishingDataService { private resourceType: PhishingResourceType = PhishingResourceType.Links, ) { this.logService.debug("[PhishingDataService] Initializing service..."); + this.indexedDbService = new PhishingIndexedDbService(this.logService); this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.phishingDomainUpdate, () => { this._triggerUpdate$.next(); }); @@ -127,18 +130,20 @@ export class PhishingDataService { ScheduledTaskNames.phishingDomainUpdate, this.UPDATE_INTERVAL_DURATION, ); - this._setupLoadPipeline(); + this._backgroundUpdateTrigger$ + .pipe( + exhaustMap((currentMeta) => { + return this._backgroundUpdate(currentMeta); + }), + takeUntil(this._destroy$), + ) + .subscribe(); } dispose(): void { // Signal all pipelines to stop and unsubscribe stored subscriptions this._destroy$.next(); this._destroy$.complete(); - - // Clear web addresses set from memory - if (this._webAddressesSet !== null) { - this._webAddressesSet = null; - } } /** @@ -148,105 +153,65 @@ export class PhishingDataService { * @returns True if the URL is a known phishing web address, false otherwise */ async isPhishingWebAddress(url: URL): Promise { - if (!this._webAddressesSet) { - this.logService.debug("[PhishingDataService] Set not loaded; skipping check"); - return false; + // Quick check for QA/dev test addresses + if (this._testWebAddresses.includes(url.href)) { + return true; } - const set = this._webAddressesSet!; const resource = getPhishingResources(this.resourceType); - // Custom matcher per resource - if (resource && resource?.match) { - for (const entry of set) { - if (resource.match(url, entry)) { - return true; + try { + // Quick lookup: check direct presence of href in IndexedDB + const hasUrl = await this.indexedDbService.hasUrl(url.href); + if (hasUrl) { + return true; + } + } catch (err) { + this.logService.error("[PhishingDataService] IndexedDB lookup via hasUrl failed", err); + } + + // If a custom matcher is provided, iterate stored entries and apply the matcher. + if (resource && resource.match) { + try { + const entries = await this.indexedDbService.loadAllUrls(); + for (const entry of entries) { + if (resource.match(url, entry)) { + return true; + } } + } catch (err) { + this.logService.error("[PhishingDataService] Error running custom matcher", err); } return false; } - - // Default set-based lookup - return set.has(url.hostname); - } - - async getNextWebAddresses( - previous: PhishingDataMeta | null, - ): Promise | null> { - const prevMeta = previous ?? { timestamp: 0, checksum: "", applicationVersion: "" }; - const now = Date.now(); - - // Updates to check - const applicationVersion = await this.platformUtilsService.getApplicationVersion(); - const remoteChecksum = await this.fetchPhishingChecksum(this.resourceType); - - // Logic checks - const appVersionChanged = applicationVersion !== prevMeta.applicationVersion; - const masterChecksumChanged = remoteChecksum !== prevMeta.checksum; - - // Check for full updated - if (masterChecksumChanged || appVersionChanged) { - this.logService.info("[PhishingDataService] Checksum or version changed; Fetching ALL."); - const remoteUrl = getPhishingResources(this.resourceType)!.remoteUrl; - const blob = await this.fetchAndCompress(remoteUrl); - return { - blob, - meta: { checksum: remoteChecksum, timestamp: now, applicationVersion }, - }; - } - - // Check for daily file - const isCacheExpired = now - prevMeta.timestamp > this.UPDATE_INTERVAL_DURATION; - - if (isCacheExpired) { - this.logService.info("[PhishingDataService] Daily cache expired; Fetching TODAY's"); - const url = getPhishingResources(this.resourceType)!.todayUrl; - const newLines = await this.fetchText(url); - const prevBlob = (await firstValueFrom(this._phishingBlobState.state$)) ?? ""; - const oldText = prevBlob ? await this._decompressString(prevBlob) : ""; - - // Join the new lines to the existing list - const combined = (oldText ? oldText + "\n" : "") + newLines.join("\n"); - - return { - blob: await this._compressString(combined), - meta: { - checksum: remoteChecksum, - timestamp: now, // Reset the timestamp - applicationVersion, - }, - }; - } - - return null; + return false; } + // [FIXME] Pull fetches into api service private async fetchPhishingChecksum(type: PhishingResourceType = PhishingResourceType.Domains) { const checksumUrl = getPhishingResources(type)!.checksumUrl; - const response = await this.apiService.nativeFetch(new Request(checksumUrl)); - if (!response.ok) { - throw new Error(`[PhishingDataService] Failed to fetch checksum: ${response.status}`); - } - return response.text(); - } - private async fetchAndCompress(url: string): Promise { - const response = await this.apiService.nativeFetch(new Request(url)); - if (!response.ok) { - throw new Error("Fetch failed"); - } + this.logService.debug(`[PhishingDataService] Fetching checksum from: ${checksumUrl}`); - const downloadStream = response.body!; - // Pipe through CompressionStream while it's downloading - const compressedStream = downloadStream.pipeThrough(new CompressionStream("gzip")); - // Convert to ArrayBuffer - const buffer = await new Response(compressedStream).arrayBuffer(); - const bytes = new Uint8Array(buffer); + try { + const response = await this.apiService.nativeFetch(new Request(checksumUrl)); + if (!response.ok) { + throw new Error( + `[PhishingDataService] Failed to fetch checksum: ${response.status} ${response.statusText}`, + ); + } - // Return as Base64 for storage - return (bytes as any).toBase64 ? (bytes as any).toBase64() : this._uint8ToBase64Fallback(bytes); + return await response.text(); + } catch (error) { + this.logService.error( + `[PhishingDataService] Checksum fetch failed from ${checksumUrl}`, + error, + ); + throw error; + } } - private async fetchText(url: string) { + // [FIXME] Pull fetches into api service + private async fetchToday(url: string) { const response = await this.apiService.nativeFetch(new Request(url)); if (!response.ok) { @@ -258,171 +223,196 @@ export class PhishingDataService { private getTestWebAddresses() { const flag = devFlagEnabled("testPhishingUrls"); + // Normalize URLs by converting to URL object and back to ensure consistent format (e.g., trailing slashes) + const testWebAddresses: string[] = [ + new URL("http://phishing.testcategory.com").href, + new URL("https://phishing.testcategory.com").href, + new URL("https://phishing.testcategory.com/block").href, + ]; if (!flag) { - return []; + return testWebAddresses; } const webAddresses = devFlagValue("testPhishingUrls") as unknown[]; if (webAddresses && webAddresses instanceof Array) { this.logService.debug( - "[PhishingDetectionService] Dev flag enabled for testing phishing detection. Adding test phishing web addresses:", + "[PhishingDataService] Dev flag enabled for testing phishing detection. Adding test phishing web addresses:", webAddresses, ); - return webAddresses as string[]; + // Normalize dev flag URLs as well, filtering out invalid ones + const normalizedDevAddresses = (webAddresses as string[]) + .filter((addr) => { + try { + new URL(addr); + return true; + } catch { + this.logService.warning( + `[PhishingDataService] Invalid test URL in dev flag, skipping: ${addr}`, + ); + return false; + } + }) + .map((addr) => new URL(addr).href); + return testWebAddresses.concat(normalizedDevAddresses); } - return []; + return testWebAddresses; } - // Runs the update flow in the background and retries up to 3 times on failure - private async _backgroundUpdate(previous: PhishingDataMeta | null): Promise { - this.logService.info(`[PhishingDataService] Update web addresses triggered...`); - const phishingMeta: PhishingDataMeta = previous ?? { - timestamp: 0, - checksum: "", - applicationVersion: "", - }; - // Start time for logging performance of update - const startTime = Date.now(); - const maxAttempts = 3; - const delayMs = 5 * 60 * 1000; // 5 minutes + private _getUpdatedMeta(): Observable { + return defer(() => { + const now = Date.now(); - for (let attempt = 1; attempt <= maxAttempts; attempt++) { - try { - const next = await this.getNextWebAddresses(phishingMeta); - if (!next) { - return; // No update needed - } + return forkJoin({ + applicationVersion: from(this.platformUtilsService.getApplicationVersion()), + remoteChecksum: from(this.fetchPhishingChecksum(this.resourceType)), + }).pipe( + map(({ applicationVersion, remoteChecksum }) => { + return { + checksum: remoteChecksum, + timestamp: now, + applicationVersion, + }; + }), + ); + }); + } - if (next.meta) { - await this._phishingMetaState.update(() => next!.meta!); - } - if (next.blob) { - await this._phishingBlobState.update(() => next!.blob!); - this._loadBlobToMemory(); - } + // Streams the full phishing data set and saves it to IndexedDB + private _updateFullDataSet() { + const resource = getPhishingResources(this.resourceType); - // Performance logging - const elapsed = Date.now() - startTime; - this.logService.info(`[PhishingDataService] Phishing data cache updated in ${elapsed}ms`); - } catch (err) { - this.logService.error( - `[PhishingDataService] Unable to update web addresses. Attempt ${attempt}.`, - err, - ); - if (attempt < maxAttempts) { - await new Promise((res) => setTimeout(res, delayMs)); - } else { - const elapsed = Date.now() - startTime; - this.logService.error( - `[PhishingDataService] Retries unsuccessful after ${elapsed}ms. Unable to update web addresses.`, - err, + if (!resource?.remoteUrl) { + return throwError(() => new Error("Invalid resource URL")); + } + + this.logService.info(`[PhishingDataService] Starting FULL update using ${resource.remoteUrl}`); + return from(this.apiService.nativeFetch(new Request(resource.remoteUrl))).pipe( + switchMap((response) => { + if (!response.ok || !response.body) { + return throwError( + () => + new Error( + `[PhishingDataService] Full fetch failed: ${response.status}, ${response.statusText}`, + ), ); } - } - } + + return from(this.indexedDbService.saveUrlsFromStream(response.body)); + }), + catchError((err: unknown) => { + this.logService.error( + `[PhishingDataService] Full dataset update failed using primary source ${err}`, + ); + this.logService.warning( + `[PhishingDataService] Falling back to: ${resource.fallbackUrl} (Note: Fallback data may be less up-to-date)`, + ); + // Try fallback URL + return from(this.apiService.nativeFetch(new Request(resource.fallbackUrl))).pipe( + switchMap((fallbackResponse) => { + if (!fallbackResponse.ok || !fallbackResponse.body) { + return throwError( + () => + new Error( + `[PhishingDataService] Fallback fetch failed: ${fallbackResponse.status}, ${fallbackResponse.statusText}`, + ), + ); + } + + return from(this.indexedDbService.saveUrlsFromStream(fallbackResponse.body)); + }), + catchError((fallbackError: unknown) => { + this.logService.error(`[PhishingDataService] Fallback source failed`); + return throwError(() => fallbackError); + }), + ); + }), + ); } - // Sets up the load pipeline to load the blob into memory when triggered - private _setupLoadPipeline(): void { - this._loadTrigger$ - .pipe( - switchMap(() => - this._phishingBlobState.state$.pipe( - first(), - switchMap((blobBase64) => { - if (!blobBase64) { - return of(undefined); - } - // Note: _decompressString wraps a promise that cannot be aborted - // If performance improvements are needed, consider migrating to a cancellable approach - return from(this._decompressString(blobBase64)).pipe( - map((text) => { - const lines = text.split(/\r?\n/); - const newWebAddressesSet = new Set(lines); - this._testWebAddresses.forEach((a) => newWebAddressesSet.add(a)); - this._webAddressesSet = new Set(newWebAddressesSet); - this.logService.info( - `[PhishingDataService] loaded ${this._webAddressesSet.size} addresses into memory from blob`, - ); - }), + private _updateDailyDataSet() { + this.logService.info("[PhishingDataService] Starting DAILY update..."); + + const todayUrl = getPhishingResources(this.resourceType)?.todayUrl; + if (!todayUrl) { + return throwError(() => new Error("Today URL missing")); + } + + return from(this.fetchToday(todayUrl)).pipe( + switchMap((lines) => from(this.indexedDbService.addUrls(lines))), + ); + } + + private _backgroundUpdate( + previous: PhishingDataMeta | null, + ): Observable { + // Use defer to restart timer if retry is activated + return defer(() => { + const startTime = Date.now(); + this.logService.info(`[PhishingDataService] Update triggered...`); + + // Get updated meta info + return this._getUpdatedMeta().pipe( + // Update full data set if application version or checksum changed + concatMap((newMeta) => + iif( + () => { + const appVersionChanged = newMeta.applicationVersion !== previous?.applicationVersion; + const checksumChanged = newMeta.checksum !== previous?.checksum; + + this.logService.info( + `[PhishingDataService] Checking if full update is needed: appVersionChanged=${appVersionChanged}, checksumChanged=${checksumChanged}`, ); - }), - catchError((err: unknown) => { - this.logService.error("[PhishingDataService] Failed to load blob into memory", err); - return of(undefined); - }), + return appVersionChanged || checksumChanged; + }, + this._updateFullDataSet().pipe(map(() => ({ meta: newMeta, updated: true }))), + of({ meta: newMeta, updated: false }), ), ), - catchError((err: unknown) => { - this.logService.error("[PhishingDataService] Load pipeline failed", err); - return of(undefined); + // Update daily data set if last update was more than UPDATE_INTERVAL_DURATION ago + concatMap((result) => + iif( + () => { + const isCacheExpired = + Date.now() - (previous?.timestamp ?? 0) > this.UPDATE_INTERVAL_DURATION; + return isCacheExpired; + }, + this._updateDailyDataSet().pipe(map(() => ({ meta: result.meta, updated: true }))), + of(result), + ), + ), + concatMap((result) => { + if (!result.updated) { + this.logService.debug(`[PhishingDataService] No update needed, metadata unchanged`); + return of(previous); + } + + this.logService.debug(`[PhishingDataService] Updated phishing meta data:`, result.meta); + return from(this._phishingMetaState.update(() => result.meta)).pipe( + tap(() => { + const elapsed = Date.now() - startTime; + this.logService.info(`[PhishingDataService] Updated data set in ${elapsed}ms`); + }), + ); }), - takeUntil(this._destroy$), - share(), - ) - .subscribe(); - } - - // [FIXME] Move compression helpers to a shared utils library - // to separate from phishing data service. - // ------------------------- Blob and Compression Handling ------------------------- - private async _compressString(input: string): Promise { - try { - const stream = new Blob([input]).stream().pipeThrough(new CompressionStream("gzip")); - - const compressedBuffer = await new Response(stream).arrayBuffer(); - const bytes = new Uint8Array(compressedBuffer); - - // Modern browsers support direct toBase64 conversion - // For older support, use fallback - return (bytes as any).toBase64 - ? (bytes as any).toBase64() - : this._uint8ToBase64Fallback(bytes); - } catch (err) { - this.logService.error("[PhishingDataService] Compression failed", err); - return btoa(encodeURIComponent(input)); - } - } - - private async _decompressString(base64: string): Promise { - try { - // Modern browsers support direct toBase64 conversion - // For older support, use fallback - const bytes = (Uint8Array as any).fromBase64 - ? (Uint8Array as any).fromBase64(base64) - : this._base64ToUint8Fallback(base64); - if (bytes == null) { - throw new Error("Base64 decoding resulted in null"); - } - const byteResponse = new Response(bytes); - if (!byteResponse.body) { - throw new Error("Response body is null"); - } - const stream = byteResponse.body.pipeThrough(new DecompressionStream("gzip")); - const streamResponse = new Response(stream); - return await streamResponse.text(); - } catch (err) { - this.logService.error("[PhishingDataService] Decompression failed", err); - return decodeURIComponent(atob(base64)); - } - } - - // Trigger a load of the blob into memory - private _loadBlobToMemory(): void { - this._loadTrigger$.next(); - } - private _uint8ToBase64Fallback(bytes: Uint8Array): string { - const CHUNK_SIZE = 0x8000; // 32KB chunks - let binary = ""; - for (let i = 0; i < bytes.length; i += CHUNK_SIZE) { - const chunk = bytes.subarray(i, i + CHUNK_SIZE); - binary += String.fromCharCode.apply(null, chunk as any); - } - return btoa(binary); - } - - private _base64ToUint8Fallback(base64: string): Uint8Array { - const binary = atob(base64); - return Uint8Array.from(binary, (c) => c.charCodeAt(0)); + retry({ + count: 2, // Total 3 attempts (initial + 2 retries) + delay: (error, retryCount) => { + this.logService.error( + `[PhishingDataService] Attempt ${retryCount} failed. Retrying in 5m...`, + error, + ); + return timer(5 * 60 * 1000); // Wait 5 mins before next attempt + }, + }), + catchError((err: unknown) => { + const elapsed = Date.now() - startTime; + this.logService.error( + `[PhishingDataService] Retries unsuccessful after ${elapsed}ms.`, + err, + ); + return of(previous); + }), + ); + }); } } diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts index 75bd634b1fc..99e101cc199 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts @@ -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" }); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts index 099839a38d9..fe0f10da221 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts @@ -53,6 +53,9 @@ export class PhishingIndexedDbService { * @returns `true` if save succeeded, `false` on error */ async saveUrls(urls: string[]): Promise { + 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 { + 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 { + 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 { + this.logService.debug("[PhishingIndexedDbService] Loading all urls from store..."); + let db: IDBDatabase | null = null; try { db = await this.openDatabase(); @@ -173,11 +203,16 @@ export class PhishingIndexedDbService { * @returns `true` if save succeeded, `false` on error */ async saveUrlsFromStream(stream: ReadableStream): Promise { + 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); From d64db8fbf58a90769683c2b00c7cc0e4ac0d6180 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Mon, 26 Jan 2026 17:44:16 +0100 Subject: [PATCH 2/6] [CL-904] Migrate CL/Navigation to use OnPush (#16958) * Migrate CL/Navigation to use OnPush * Modernize the code * Swap to signals and class * Further tweaks * Remove this. * Replace setOpen and setClose with a public signal * fix merge issues and signal-ifying service * fix class and style bindings * fix accidental behavior change from merge conflicts * fix redundant check * fix missed ngClass * fix comment * Re-add share ng-template --------- Co-authored-by: Vicki League Co-authored-by: Will Martin Co-authored-by: Claude Sonnet 4.5 --- .../src/layout/layout.component.html | 23 ++-- .../src/navigation/nav-base.component.ts | 20 +-- .../src/navigation/nav-divider.component.html | 2 +- .../src/navigation/nav-divider.component.ts | 12 +- .../src/navigation/nav-group.component.html | 4 +- .../src/navigation/nav-group.component.ts | 42 +++--- .../src/navigation/nav-group.stories.ts | 5 +- .../src/navigation/nav-item.component.html | 69 +++++----- .../src/navigation/nav-item.component.ts | 62 +++++---- .../src/navigation/nav-logo.component.html | 15 +-- .../src/navigation/nav-logo.component.ts | 26 ++-- .../src/navigation/side-nav.component.html | 126 ++++++++---------- .../src/navigation/side-nav.component.ts | 30 +++-- .../src/navigation/side-nav.service.ts | 69 +++------- 14 files changed, 240 insertions(+), 265 deletions(-) diff --git a/libs/components/src/layout/layout.component.html b/libs/components/src/layout/layout.component.html index 66bfcafafe9..f0e2b601e38 100644 --- a/libs/components/src/layout/layout.component.html +++ b/libs/components/src/layout/layout.component.html @@ -30,21 +30,14 @@ - @if ( - { - open: sideNavService.open$ | async, - }; - as data - ) { -
- @if (data.open) { -
- } -
- } +
+ @if (sideNavService.open()) { +
+ } +
diff --git a/libs/components/src/navigation/nav-base.component.ts b/libs/components/src/navigation/nav-base.component.ts index 706df2b25ad..e20edf5a0f9 100644 --- a/libs/components/src/navigation/nav-base.component.ts +++ b/libs/components/src/navigation/nav-base.component.ts @@ -1,8 +1,11 @@ -import { Directive, EventEmitter, Output, input, model } from "@angular/core"; +import { Directive, output, input, model } from "@angular/core"; import { RouterLink, RouterLinkActive } from "@angular/router"; /** - * `NavGroupComponent` builds upon `NavItemComponent`. This class represents the properties that are passed down to `NavItemComponent`. + * Base class for navigation components in the side navigation. + * + * `NavGroupComponent` builds upon `NavItemComponent`. This class represents the properties + * that are passed down to `NavItemComponent`. */ @Directive() export abstract class NavBaseComponent { @@ -38,23 +41,26 @@ export abstract class NavBaseComponent { * * --- * + * @remarks * We can't name this "routerLink" because Angular will mount the `RouterLink` directive. * - * See: {@link https://github.com/angular/angular/issues/24482} + * @see {@link RouterLink.routerLink} + * @see {@link https://github.com/angular/angular/issues/24482} */ readonly route = input(); /** * Passed to internal `routerLink` * - * See {@link RouterLink.relativeTo} + * @see {@link RouterLink.relativeTo} */ readonly relativeTo = input(); /** * Passed to internal `routerLink` * - * See {@link RouterLinkActive.routerLinkActiveOptions} + * @default { paths: "subset", queryParams: "ignored", fragment: "ignored", matrixParams: "ignored" } + * @see {@link RouterLinkActive.routerLinkActiveOptions} */ readonly routerLinkActiveOptions = input({ paths: "subset", @@ -71,7 +77,5 @@ export abstract class NavBaseComponent { /** * Fires when main content is clicked */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-output-emitter-ref - @Output() mainContentClicked: EventEmitter = new EventEmitter(); + readonly mainContentClicked = output(); } diff --git a/libs/components/src/navigation/nav-divider.component.html b/libs/components/src/navigation/nav-divider.component.html index 2d8e1dfa24b..7af7de2a28a 100644 --- a/libs/components/src/navigation/nav-divider.component.html +++ b/libs/components/src/navigation/nav-divider.component.html @@ -1,3 +1,3 @@ -@if (sideNavService.open$ | async) { +@if (sideNavService.open()) {
} diff --git a/libs/components/src/navigation/nav-divider.component.ts b/libs/components/src/navigation/nav-divider.component.ts index 2f33883fd58..05a69563312 100644 --- a/libs/components/src/navigation/nav-divider.component.ts +++ b/libs/components/src/navigation/nav-divider.component.ts @@ -1,15 +1,15 @@ -import { CommonModule } from "@angular/common"; -import { Component } from "@angular/core"; +import { ChangeDetectionStrategy, Component, inject } from "@angular/core"; import { SideNavService } from "./side-nav.service"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection +/** + * A visual divider for separating navigation items in the side navigation. + */ @Component({ selector: "bit-nav-divider", templateUrl: "./nav-divider.component.html", - imports: [CommonModule], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class NavDividerComponent { - constructor(protected sideNavService: SideNavService) {} + protected readonly sideNavService = inject(SideNavService); } diff --git a/libs/components/src/navigation/nav-group.component.html b/libs/components/src/navigation/nav-group.component.html index d305f89063e..26d1c68da43 100644 --- a/libs/components/src/navigation/nav-group.component.html +++ b/libs/components/src/navigation/nav-group.component.html @@ -20,9 +20,7 @@ - + } @if (open) {
}
+ + + +
+ @if (icon()) { + + } + @if (open) { + {{ text() }} + } +
+ diff --git a/libs/components/src/navigation/nav-item.component.ts b/libs/components/src/navigation/nav-item.component.ts index e57413d9980..53b181ec083 100644 --- a/libs/components/src/navigation/nav-item.component.ts +++ b/libs/components/src/navigation/nav-item.component.ts @@ -1,7 +1,14 @@ -import { CommonModule } from "@angular/common"; -import { Component, HostListener, Optional, computed, input, model } from "@angular/core"; -import { RouterLinkActive, RouterModule } from "@angular/router"; -import { BehaviorSubject, map } from "rxjs"; +import { NgTemplateOutlet } from "@angular/common"; +import { + ChangeDetectionStrategy, + Component, + input, + inject, + signal, + computed, + model, +} from "@angular/core"; +import { RouterModule, RouterLinkActive } from "@angular/router"; import { IconButtonModule } from "../icon-button"; @@ -14,13 +21,16 @@ export abstract class NavGroupAbstraction { abstract treeDepth: ReturnType>; } -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "bit-nav-item", templateUrl: "./nav-item.component.html", providers: [{ provide: NavBaseComponent, useExisting: NavItemComponent }], - imports: [CommonModule, IconButtonModule, RouterModule], + imports: [NgTemplateOutlet, IconButtonModule, RouterModule], + changeDetection: ChangeDetectionStrategy.OnPush, + host: { + "(focusin)": "onFocusIn($event.target)", + "(focusout)": "onFocusOut()", + }, }) export class NavItemComponent extends NavBaseComponent { /** @@ -35,9 +45,14 @@ export class NavItemComponent extends NavBaseComponent { */ protected readonly TREE_DEPTH_PADDING = 1.75; - /** Forces active styles to be shown, regardless of the `routerLinkActiveOptions` */ + /** + * Forces active styles to be shown, regardless of the `routerLinkActiveOptions` + */ readonly forceActiveStyles = input(false); + protected readonly sideNavService = inject(SideNavService); + private readonly parentNavGroup = inject(NavGroupAbstraction, { optional: true }); + /** * Is `true` if `to` matches the current route */ @@ -56,7 +71,7 @@ export class NavItemComponent extends NavBaseComponent { * adding calculation for tree variant due to needing visual alignment on different indentation levels needed between the first level and subsequent levels */ protected readonly navItemIndentationPadding = computed(() => { - const open = this.sideNavService.open; + const open = this.sideNavService.open(); const depth = this.treeDepth() ?? 0; if (open && this.variant() === "tree") { @@ -87,25 +102,22 @@ export class NavItemComponent extends NavBaseComponent { * (denoted with the data-fvw attribute) matches :focus-visible. We then map that state to some * styles, so the entire component can have an outline. */ - protected focusVisibleWithin$ = new BehaviorSubject(false); - protected fvwStyles$ = this.focusVisibleWithin$.pipe( - map((value) => - value ? "tw-z-10 tw-rounded tw-outline-none tw-ring tw-ring-inset tw-ring-border-focus" : "", - ), + protected readonly focusVisibleWithin = signal(false); + protected readonly fvwStyles = computed(() => + this.focusVisibleWithin() + ? "tw-z-10 tw-rounded tw-outline-none tw-ring tw-ring-inset tw-ring-border-focus" + : "", ); - @HostListener("focusin", ["$event.target"]) - onFocusIn(target: HTMLElement) { - this.focusVisibleWithin$.next(target.matches("[data-fvw]:focus-visible")); - } - @HostListener("focusout") - onFocusOut() { - this.focusVisibleWithin$.next(false); + + protected onFocusIn(target: HTMLElement) { + this.focusVisibleWithin.set(target.matches("[data-fvw]:focus-visible")); } - constructor( - protected sideNavService: SideNavService, - @Optional() private parentNavGroup: NavGroupAbstraction, - ) { + protected onFocusOut() { + this.focusVisibleWithin.set(false); + } + + constructor() { super(); // Set tree depth based on parent's depth diff --git a/libs/components/src/navigation/nav-logo.component.html b/libs/components/src/navigation/nav-logo.component.html index 1d9961554c2..9f18855ae13 100644 --- a/libs/components/src/navigation/nav-logo.component.html +++ b/libs/components/src/navigation/nav-logo.component.html @@ -1,22 +1,21 @@
- +
diff --git a/libs/components/src/navigation/nav-logo.component.ts b/libs/components/src/navigation/nav-logo.component.ts index 0602e8b753c..fec50ee8902 100644 --- a/libs/components/src/navigation/nav-logo.component.ts +++ b/libs/components/src/navigation/nav-logo.component.ts @@ -1,5 +1,4 @@ -import { CommonModule } from "@angular/common"; -import { Component, input } from "@angular/core"; +import { ChangeDetectionStrategy, Component, input, inject } from "@angular/core"; import { RouterLinkActive, RouterLink } from "@angular/router"; import { BitwardenShield, Icon } from "@bitwarden/assets/svg"; @@ -8,18 +7,25 @@ import { BitIconComponent } from "../icon/icon.component"; import { SideNavService } from "./side-nav.service"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "bit-nav-logo", templateUrl: "./nav-logo.component.html", - imports: [CommonModule, RouterLinkActive, RouterLink, BitIconComponent], + imports: [RouterLinkActive, RouterLink, BitIconComponent], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class NavLogoComponent { - /** Icon that is displayed when the side nav is closed */ + protected readonly sideNavService = inject(SideNavService); + + /** + * Icon that is displayed when the side nav is closed + * + * @default BitwardenShield + */ readonly closedIcon = input(BitwardenShield); - /** Icon that is displayed when the side nav is open */ + /** + * Icon that is displayed when the side nav is open + */ readonly openIcon = input.required(); /** @@ -27,8 +33,8 @@ export class NavLogoComponent { */ readonly route = input.required(); - /** Passed to `attr.aria-label` and `attr.title` */ + /** + * Passed to `attr.aria-label` and `attr.title` + */ readonly label = input.required(); - - constructor(protected sideNavService: SideNavService) {} } diff --git a/libs/components/src/navigation/side-nav.component.html b/libs/components/src/navigation/side-nav.component.html index b70d650622a..78fed07011d 100644 --- a/libs/components/src/navigation/side-nav.component.html +++ b/libs/components/src/navigation/side-nav.component.html @@ -1,68 +1,60 @@ -@if ( - { - open: sideNavService.open$ | async, - isOverlay: sideNavService.isOverlay$ | async, - }; - as data -) { -
- +@let open = sideNavService.open(); +@let isOverlay = sideNavService.isOverlay(); + +
+
-} + class="[@media(min-height:53rem)]:tw-sticky tw-bottom-0 tw-left-0 tw-z-20 tw-mt-auto tw-w-full tw-bg-bg-sidenav" + > + + @if (open) { + + } +
+ +
+
+ + +
diff --git a/libs/components/src/navigation/side-nav.component.ts b/libs/components/src/navigation/side-nav.component.ts index b13920d9749..35835f1be96 100644 --- a/libs/components/src/navigation/side-nav.component.ts +++ b/libs/components/src/navigation/side-nav.component.ts @@ -1,7 +1,14 @@ import { CdkTrapFocus } from "@angular/cdk/a11y"; import { DragDropModule, CdkDragMove } from "@angular/cdk/drag-drop"; -import { CommonModule } from "@angular/common"; -import { Component, ElementRef, inject, input, viewChild } from "@angular/core"; +import { AsyncPipe } from "@angular/common"; +import { + ChangeDetectionStrategy, + Component, + ElementRef, + input, + viewChild, + inject, +} from "@angular/core"; import { I18nPipe } from "@bitwarden/ui-common"; @@ -12,35 +19,42 @@ import { SideNavService } from "./side-nav.service"; export type SideNavVariant = "primary" | "secondary"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection +/** + * Side navigation component that provides a collapsible navigation menu. + */ @Component({ selector: "bit-side-nav", templateUrl: "side-nav.component.html", imports: [ - CommonModule, CdkTrapFocus, NavDividerComponent, BitIconButtonComponent, I18nPipe, DragDropModule, + AsyncPipe, ], host: { class: "tw-block tw-h-full", }, + changeDetection: ChangeDetectionStrategy.OnPush, }) export class SideNavComponent { - protected sideNavService = inject(SideNavService); + protected readonly sideNavService = inject(SideNavService); + /** + * Visual variant of the side navigation + * + * @default "primary" + */ readonly variant = input("primary"); private readonly toggleButton = viewChild("toggleButton", { read: ElementRef }); private elementRef = inject>(ElementRef); - protected handleKeyDown = (event: KeyboardEvent) => { + protected readonly handleKeyDown = (event: KeyboardEvent) => { if (event.key === "Escape") { - this.sideNavService.setClose(); + this.sideNavService.open.set(false); this.toggleButton()?.nativeElement.focus(); return false; } diff --git a/libs/components/src/navigation/side-nav.service.ts b/libs/components/src/navigation/side-nav.service.ts index 63e54c81fe5..05713006a43 100644 --- a/libs/components/src/navigation/side-nav.service.ts +++ b/libs/components/src/navigation/side-nav.service.ts @@ -1,15 +1,6 @@ -import { inject, Injectable } from "@angular/core"; -import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; -import { - BehaviorSubject, - Observable, - combineLatest, - fromEvent, - map, - startWith, - debounceTime, - first, -} from "rxjs"; +import { computed, effect, inject, Injectable, signal } from "@angular/core"; +import { takeUntilDestroyed, toSignal } from "@angular/core/rxjs-interop"; +import { BehaviorSubject, Observable, fromEvent, map, startWith, debounceTime, first } from "rxjs"; import { BIT_SIDE_NAV_DISK, GlobalStateProvider, KeyDefinition } from "@bitwarden/state"; @@ -32,16 +23,17 @@ export class SideNavService { private rootFontSizePx: number; - private _open$ = new BehaviorSubject(isAtOrLargerThanBreakpoint("md")); - open$ = this._open$.asObservable(); + /** + * Whether the side navigation is open or closed. + */ + readonly open = signal(isAtOrLargerThanBreakpoint("md")); private isLargeScreen$ = media(`(min-width: ${BREAKPOINTS.md}px)`); - private _userCollapsePreference$ = new BehaviorSubject(null); - userCollapsePreference$ = this._userCollapsePreference$.asObservable(); + readonly isLargeScreen = toSignal(this.isLargeScreen$, { requireSync: true }); - isOverlay$ = combineLatest([this.open$, this.isLargeScreen$]).pipe( - map(([open, isLargeScreen]) => open && !isLargeScreen), - ); + readonly userCollapsePreference = signal(null); + + readonly isOverlay = computed(() => this.open() && !this.isLargeScreen()); /** * Local component state width @@ -67,16 +59,14 @@ export class SideNavService { this.rootFontSizePx = parseFloat(getComputedStyle(document.documentElement).fontSize || "16"); // Handle open/close state - combineLatest([this.isLargeScreen$, this.userCollapsePreference$]) - .pipe(takeUntilDestroyed()) - .subscribe(([isLargeScreen, userCollapsePreference]) => { - if (!isLargeScreen) { - this.setClose(); - } else if (userCollapsePreference !== "closed") { - // Auto-open when user hasn't set preference (null) or prefers open - this.setOpen(); - } - }); + effect(() => { + if (!this.isLargeScreen()) { + this.open.set(false); + } else if (this.userCollapsePreference() !== "closed") { + // Auto-open when user hasn't set preference (null) or prefers open + this.open.set(true); + } + }); // Initialize the resizable width from state provider this.widthState$.pipe(first()).subscribe((width: number) => { @@ -89,31 +79,14 @@ export class SideNavService { }); } - get open() { - return this._open$.getValue(); - } - - setOpen() { - this._open$.next(true); - } - - setClose() { - this._open$.next(false); - } - /** * Toggle the open/close state of the side nav */ toggle() { - const curr = this._open$.getValue(); // Store user's preference based on what state they're toggling TO - this._userCollapsePreference$.next(curr ? "closed" : "open"); + this.userCollapsePreference.set(this.open() ? "closed" : "open"); - if (curr) { - this.setClose(); - } else { - this.setOpen(); - } + this.open.set(!this.open()); } /** From d459e81319f9670d78156f707295641c7b853ccf Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Mon, 26 Jan 2026 11:22:07 -0600 Subject: [PATCH 3/6] upgrade node-fetch (#18482) --- apps/cli/package.json | 2 +- package-lock.json | 59 ++++++++----------------------------------- package.json | 4 +-- 3 files changed, 13 insertions(+), 52 deletions(-) diff --git a/apps/cli/package.json b/apps/cli/package.json index a19c811b4bf..c80f79feff8 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -81,7 +81,7 @@ "lowdb": "1.0.0", "lunr": "2.3.9", "multer": "2.0.2", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "open": "11.0.0", "papaparse": "5.5.3", diff --git a/package-lock.json b/package-lock.json index 42206a1b46c..2cd18e11adc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52,7 +52,7 @@ "lunr": "2.3.9", "multer": "2.0.2", "ngx-toastr": "19.1.0", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "oidc-client-ts": "2.4.1", "open": "11.0.0", @@ -110,7 +110,7 @@ "@types/lowdb": "1.0.15", "@types/lunr": "2.3.7", "@types/node": "22.19.3", - "@types/node-fetch": "2.6.4", + "@types/node-fetch": "2.6.13", "@types/node-forge": "1.3.14", "@types/papaparse": "5.5.0", "@types/proper-lockfile": "4.1.4", @@ -217,7 +217,7 @@ "lowdb": "1.0.0", "lunr": "2.3.9", "multer": "2.0.2", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "open": "11.0.0", "papaparse": "5.5.3", @@ -15842,53 +15842,14 @@ } }, "node_modules/@types/node-fetch": { - "version": "2.6.4", - "resolved": "https://registry.npmjs.org/@types/node-fetch/-/node-fetch-2.6.4.tgz", - "integrity": "sha512-1ZX9fcN4Rvkvgv4E6PAY5WXUFWFcRWxZa3EW83UjycOB9ljJCedb2CupIP4RZMEwF/M3eTcCihbBRgwtGbg5Rg==", + "version": "2.6.13", + "resolved": "https://registry.npmjs.org/@types/node-fetch/-/node-fetch-2.6.13.tgz", + "integrity": "sha512-QGpRVpzSaUs30JBSGPjOg4Uveu384erbHBoT1zeONvyCfwQxIkUshLAOqN/k9EjGviPRmWTTe6aH2qySWKTVSw==", "dev": true, "license": "MIT", "dependencies": { "@types/node": "*", - "form-data": "^3.0.0" - } - }, - "node_modules/@types/node-fetch/node_modules/form-data": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.3.tgz", - "integrity": "sha512-q5YBMeWy6E2Un0nMGWMgI65MAKtaylxfNJGJxpGh45YDciZB4epbWpaAfImil6CPAPTYB4sh0URQNDRIZG5F2w==", - "dev": true, - "license": "MIT", - "dependencies": { - "asynckit": "^0.4.0", - "combined-stream": "^1.0.8", - "es-set-tostringtag": "^2.1.0", - "mime-types": "^2.1.35" - }, - "engines": { - "node": ">= 6" - } - }, - "node_modules/@types/node-fetch/node_modules/mime-db": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", - "integrity": "sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/@types/node-fetch/node_modules/mime-types": { - "version": "2.1.35", - "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.35.tgz", - "integrity": "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw==", - "dev": true, - "license": "MIT", - "dependencies": { - "mime-db": "1.52.0" - }, - "engines": { - "node": ">= 0.6" + "form-data": "^4.0.4" } }, "node_modules/@types/node-forge": { @@ -32816,9 +32777,9 @@ } }, "node_modules/node-fetch": { - "version": "2.6.12", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.12.tgz", - "integrity": "sha512-C/fGU2E8ToujUivIO0H+tpQ6HWo4eEmchoPIoXtxCrVghxdKq+QOHqEZW7tuP3KlV3bC8FRMO5nMCC7Zm1VP6g==", + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", + "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", "license": "MIT", "dependencies": { "whatwg-url": "^5.0.0" diff --git a/package.json b/package.json index 829dc91370a..8455d97c87c 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "@types/lowdb": "1.0.15", "@types/lunr": "2.3.7", "@types/node": "22.19.3", - "@types/node-fetch": "2.6.4", + "@types/node-fetch": "2.6.13", "@types/node-forge": "1.3.14", "@types/papaparse": "5.5.0", "@types/proper-lockfile": "4.1.4", @@ -191,7 +191,7 @@ "lunr": "2.3.9", "multer": "2.0.2", "ngx-toastr": "19.1.0", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "oidc-client-ts": "2.4.1", "open": "11.0.0", From 87555eaabdb0703010e478f28283479f2611d01f Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Mon, 26 Jan 2026 12:07:31 -0600 Subject: [PATCH 4/6] remove risk insights for premium feature flag (#18446) --- libs/common/src/enums/feature-flag.enum.ts | 2 -- .../cipher-view/cipher-view.component.spec.ts | 21 ------------------- .../src/cipher-view/cipher-view.component.ts | 19 +++-------------- 3 files changed, 3 insertions(+), 39 deletions(-) diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index f761aea1b08..77df258ad3a 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -65,7 +65,6 @@ export enum FeatureFlag { PM22134SdkCipherListView = "pm-22134-sdk-cipher-list-view", PM22136_SdkCipherEncryption = "pm-22136-sdk-cipher-encryption", CipherKeyEncryption = "cipher-key-encryption", - RiskInsightsForPremium = "pm-23904-risk-insights-for-premium", VaultLoadingSkeletons = "pm-25081-vault-skeleton-loaders", BrowserPremiumSpotlight = "pm-23384-browser-premium-spotlight", MigrateMyVaultToMyItems = "pm-20558-migrate-myvault-to-myitems", @@ -129,7 +128,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM19941MigrateCipherDomainToSdk]: FALSE, [FeatureFlag.PM22134SdkCipherListView]: FALSE, [FeatureFlag.PM22136_SdkCipherEncryption]: FALSE, - [FeatureFlag.RiskInsightsForPremium]: FALSE, [FeatureFlag.VaultLoadingSkeletons]: FALSE, [FeatureFlag.BrowserPremiumSpotlight]: FALSE, [FeatureFlag.MigrateMyVaultToMyItems]: FALSE, diff --git a/libs/vault/src/cipher-view/cipher-view.component.spec.ts b/libs/vault/src/cipher-view/cipher-view.component.spec.ts index 18a5132781b..2300565035e 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.spec.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.spec.ts @@ -8,7 +8,6 @@ import { CollectionService } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService, Account } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -42,11 +41,9 @@ describe("CipherViewComponent", () => { let mockLogService: LogService; let mockCipherRiskService: CipherRiskService; let mockBillingAccountProfileStateService: BillingAccountProfileStateService; - let mockConfigService: ConfigService; // Mock data let mockCipherView: CipherView; - let featureFlagEnabled$: BehaviorSubject; let hasPremiumFromAnySource$: BehaviorSubject; let activeAccount$: BehaviorSubject; @@ -57,7 +54,6 @@ describe("CipherViewComponent", () => { email: "test@example.com", } as Account); - featureFlagEnabled$ = new BehaviorSubject(false); hasPremiumFromAnySource$ = new BehaviorSubject(true); // Create service mocks @@ -83,9 +79,6 @@ describe("CipherViewComponent", () => { .fn() .mockReturnValue(hasPremiumFromAnySource$); - mockConfigService = mock(); - mockConfigService.getFeatureFlag$ = jest.fn().mockReturnValue(featureFlagEnabled$); - // Setup mock cipher view mockCipherView = new CipherView(); mockCipherView.id = "cipher-id"; @@ -110,7 +103,6 @@ describe("CipherViewComponent", () => { provide: BillingAccountProfileStateService, useValue: mockBillingAccountProfileStateService, }, - { provide: ConfigService, useValue: mockConfigService }, ], schemas: [NO_ERRORS_SCHEMA], }) @@ -145,7 +137,6 @@ describe("CipherViewComponent", () => { beforeEach(() => { // Reset observables to default values for this test suite - featureFlagEnabled$.next(true); hasPremiumFromAnySource$.next(true); // Setup default mock for computeCipherRiskForUser (individual tests can override) @@ -162,18 +153,6 @@ describe("CipherViewComponent", () => { component = fixture.componentInstance; }); - it("returns false when feature flag is disabled", fakeAsync(() => { - featureFlagEnabled$.next(false); - - const cipher = createLoginCipherView(); - fixture.componentRef.setInput("cipher", cipher); - fixture.detectChanges(); - tick(); - - expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); - expect(component.passwordIsAtRisk()).toBe(false); - })); - it("returns false when cipher has no login password", fakeAsync(() => { const cipher = createLoginCipherView(); cipher.login = {} as any; // No password diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index b5c063df51e..26e3f18b542 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -13,8 +13,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { isCardExpired } from "@bitwarden/common/autofill/utils"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { getByIds } from "@bitwarden/common/platform/misc"; @@ -113,7 +111,6 @@ export class CipherViewComponent { private logService: LogService, private cipherRiskService: CipherRiskService, private billingAccountService: BillingAccountProfileStateService, - private configService: ConfigService, ) {} readonly resolvedCollections = toSignal( @@ -248,19 +245,9 @@ export class CipherViewComponent { * The password is only evaluated when the user is premium and has edit access to the cipher. */ readonly passwordIsAtRisk = toSignal( - combineLatest([ - this.activeUserId$, - this.cipher$, - this.configService.getFeatureFlag$(FeatureFlag.RiskInsightsForPremium), - ]).pipe( - switchMap(([userId, cipher, featureEnabled]) => { - if ( - !featureEnabled || - !cipher.hasLoginPassword || - !cipher.edit || - cipher.organizationId || - cipher.isDeleted - ) { + combineLatest([this.activeUserId$, this.cipher$]).pipe( + switchMap(([userId, cipher]) => { + if (!cipher.hasLoginPassword || !cipher.edit || cipher.organizationId || cipher.isDeleted) { return of(false); } return this.switchPremium$( From 06c8c7316d71b1d3a799a29dde55e88ea9ad2d1b Mon Sep 17 00:00:00 2001 From: Nik Gilmore Date: Mon, 26 Jan 2026 11:43:35 -0800 Subject: [PATCH 5/6] [PM-30301][PM-30302] Use SDK for Create and Update cipher operations (#18149) * Migrate create and edit operations to use SDK for ciphers * WIP: Adds admin call to edit ciphers with SDK * Add client version to SDK intialization settings * Remove console.log statements * Adds originalCipherId and collectionIds to updateCipher * Update tests for new cipehrService interfaces * Rename SdkCipherOperations feature flag * Add call to Admin edit SDK if flag is passed * Add tests for SDK path * Revert changes to .npmrc * Remove outdated comments * Fix feature flag name * Fix UUID format in cipher.service.spec.ts * Update calls to cipherService.updateWithServer and .createWithServer to new interface * Update CLI and Desktop to use new cipherSErvice interfaces * Fix tests for new cipherService interface change * Bump sdk-internal and commercial-sdk-internal versions to 0.2.0-main.439 * Fix linting errors * Fix typescript errors impacted by this chnage * Fix caching issue on browser extension when using SDK cipher ops. * Remove commented code * Fix bug causing race condition due to not consuming / awaiting observable. * Add missing 'await' to decrypt call * Clean up unnecessary else statements and fix function naming * Add comments for this.clearCache * Add tests for SDK CipherView conversion functions * Replace sdkservice with cipher-sdk.service * Fix import issues in browser * Fix import issues in cli * Fix type issues * Fix type issues * Fix type issues * Fix test that fails sporadically due to timing issue --- .../notification.background.spec.ts | 19 +- .../background/notification.background.ts | 9 +- .../autofill/popup/fido2/fido2.component.ts | 5 +- .../browser/src/background/main.background.ts | 6 + .../item-more-options.component.ts | 3 +- .../vault-popup-autofill.service.spec.ts | 3 +- .../services/vault-popup-autofill.service.ts | 3 +- apps/cli/src/commands/edit.command.ts | 4 +- .../service-container/service-container.ts | 6 + apps/cli/src/vault/create.command.ts | 9 +- .../desktop-fido2-user-interface.service.ts | 10 +- .../encrypted-message-handler.service.ts | 6 +- .../vault/individual-vault/vault.component.ts | 3 +- .../src/services/jslib-services.module.ts | 10 + libs/common/src/enums/feature-flag.enum.ts | 2 + .../fido2/fido2-authenticator.service.spec.ts | 31 +- .../fido2/fido2-authenticator.service.ts | 6 +- .../services/sdk/default-sdk.service.ts | 7 +- .../services/sdk/register-sdk.service.ts | 7 +- .../vault/abstractions/cipher-sdk.service.ts | 37 ++ .../src/vault/abstractions/cipher.service.ts | 13 +- .../src/vault/models/view/cipher.view.spec.ts | 362 ++++++++++++++++++ .../src/vault/models/view/cipher.view.ts | 86 ++++- .../vault/services/cipher-sdk.service.spec.ts | 246 ++++++++++++ .../src/vault/services/cipher-sdk.service.ts | 82 ++++ .../src/vault/services/cipher.service.spec.ts | 164 +++++++- .../src/vault/services/cipher.service.ts | 75 +++- .../services/default-cipher-form.service.ts | 37 +- 28 files changed, 1126 insertions(+), 125 deletions(-) create mode 100644 libs/common/src/vault/abstractions/cipher-sdk.service.ts create mode 100644 libs/common/src/vault/services/cipher-sdk.service.spec.ts create mode 100644 libs/common/src/vault/services/cipher-sdk.service.ts diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index ab16788ea6f..a927c75dba0 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -767,7 +767,6 @@ describe("NotificationBackground", () => { let createWithServerSpy: jest.SpyInstance; let updateWithServerSpy: jest.SpyInstance; let folderExistsSpy: jest.SpyInstance; - let cipherEncryptSpy: jest.SpyInstance; beforeEach(() => { activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); @@ -791,7 +790,6 @@ describe("NotificationBackground", () => { createWithServerSpy = jest.spyOn(cipherService, "createWithServer"); updateWithServerSpy = jest.spyOn(cipherService, "updateWithServer"); folderExistsSpy = jest.spyOn(notificationBackground as any, "folderExists"); - cipherEncryptSpy = jest.spyOn(cipherService, "encrypt"); accountService.activeAccount$ = activeAccountSubject; }); @@ -1190,13 +1188,7 @@ describe("NotificationBackground", () => { folderExistsSpy.mockResolvedValueOnce(false); convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView); editItemSpy.mockResolvedValueOnce(undefined); - cipherEncryptSpy.mockResolvedValueOnce({ - cipher: { - ...cipherView, - id: "testId", - }, - encryptedFor: userId, - }); + createWithServerSpy.mockResolvedValueOnce(cipherView); sendMockExtensionMessage(message, sender); await flushPromises(); @@ -1205,7 +1197,6 @@ describe("NotificationBackground", () => { queueMessage, null, ); - expect(cipherEncryptSpy).toHaveBeenCalledWith(cipherView, "testId"); expect(createWithServerSpy).toHaveBeenCalled(); expect(tabSendMessageDataSpy).toHaveBeenCalledWith( sender.tab, @@ -1241,13 +1232,6 @@ describe("NotificationBackground", () => { folderExistsSpy.mockResolvedValueOnce(true); convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView); editItemSpy.mockResolvedValueOnce(undefined); - cipherEncryptSpy.mockResolvedValueOnce({ - cipher: { - ...cipherView, - id: "testId", - }, - encryptedFor: userId, - }); const errorMessage = "fetch error"; createWithServerSpy.mockImplementation(() => { throw new Error(errorMessage); @@ -1256,7 +1240,6 @@ describe("NotificationBackground", () => { sendMockExtensionMessage(message, sender); await flushPromises(); - expect(cipherEncryptSpy).toHaveBeenCalledWith(cipherView, "testId"); expect(createWithServerSpy).toThrow(errorMessage); expect(tabSendMessageSpy).not.toHaveBeenCalledWith(sender.tab, { command: "addedCipher", diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 1cbf915b06a..f8459cf8f23 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -866,13 +866,11 @@ export default class NotificationBackground { return; } - const encrypted = await this.cipherService.encrypt(newCipher, activeUserId); - const { cipher } = encrypted; try { - await this.cipherService.createWithServer(encrypted); + const resultCipher = await this.cipherService.createWithServer(newCipher, activeUserId); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { itemName: newCipher?.name && String(newCipher?.name), - cipherId: cipher?.id && String(cipher?.id), + cipherId: resultCipher?.id && String(resultCipher?.id), }); await BrowserApi.tabSendMessage(tab, { command: "addedCipher" }); } catch (error) { @@ -910,7 +908,6 @@ export default class NotificationBackground { await BrowserApi.tabSendMessage(tab, { command: "editedCipher" }); return; } - const cipher = await this.cipherService.encrypt(cipherView, userId); try { if (!cipherView.edit) { @@ -939,7 +936,7 @@ export default class NotificationBackground { return; } - await this.cipherService.updateWithServer(cipher); + await this.cipherService.updateWithServer(cipherView, userId); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { itemName: cipherView?.name && String(cipherView?.name), diff --git a/apps/browser/src/autofill/popup/fido2/fido2.component.ts b/apps/browser/src/autofill/popup/fido2/fido2.component.ts index c1982d27d24..5720419f909 100644 --- a/apps/browser/src/autofill/popup/fido2/fido2.component.ts +++ b/apps/browser/src/autofill/popup/fido2/fido2.component.ts @@ -444,10 +444,9 @@ export class Fido2Component implements OnInit, OnDestroy { ); this.buildCipher(name, username); - const encrypted = await this.cipherService.encrypt(this.cipher, activeUserId); try { - await this.cipherService.createWithServer(encrypted); - this.cipher.id = encrypted.cipher.id; + const result = await this.cipherService.createWithServer(this.cipher, activeUserId); + this.cipher.id = result.id; } catch (e) { this.logService.error(e); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 58a7eb99ec6..660fcb97bcf 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -194,6 +194,7 @@ import { SendService } from "@bitwarden/common/tools/send/services/send.service" import { InternalSendService as InternalSendServiceAbstraction } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service"; +import { CipherSdkService } from "@bitwarden/common/vault/abstractions/cipher-sdk.service"; import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherFileUploadService as CipherFileUploadServiceAbstraction } from "@bitwarden/common/vault/abstractions/file-upload/cipher-file-upload.service"; import { FolderApiServiceAbstraction } from "@bitwarden/common/vault/abstractions/folder/folder-api.service.abstraction"; @@ -211,6 +212,7 @@ import { CipherAuthorizationService, DefaultCipherAuthorizationService, } from "@bitwarden/common/vault/services/cipher-authorization.service"; +import { DefaultCipherSdkService } from "@bitwarden/common/vault/services/cipher-sdk.service"; import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; import { DefaultCipherEncryptionService } from "@bitwarden/common/vault/services/default-cipher-encryption.service"; import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-upload/cipher-file-upload.service"; @@ -367,6 +369,7 @@ export default class MainBackground { apiService: ApiServiceAbstraction; hibpApiService: HibpApiService; environmentService: BrowserEnvironmentService; + cipherSdkService: CipherSdkService; cipherService: CipherServiceAbstraction; folderService: InternalFolderServiceAbstraction; userDecryptionOptionsService: InternalUserDecryptionOptionsServiceAbstraction; @@ -973,6 +976,8 @@ export default class MainBackground { this.logService, ); + this.cipherSdkService = new DefaultCipherSdkService(this.sdkService, this.logService); + this.cipherService = new CipherService( this.keyService, this.domainSettingsService, @@ -988,6 +993,7 @@ export default class MainBackground { this.logService, this.cipherEncryptionService, this.messagingService, + this.cipherSdkService, ); this.folderService = new FolderService( this.keyService, diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts index f881b07282b..d7de51ad20f 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -277,8 +277,7 @@ export class ItemMoreOptionsComponent { this.accountService.activeAccount$.pipe(map((a) => a?.id)), )) as UserId; - const encryptedCipher = await this.cipherService.encrypt(cipher, activeUserId); - await this.cipherService.updateWithServer(encryptedCipher); + await this.cipherService.updateWithServer(cipher, activeUserId); this.toastService.showToast({ variant: "success", message: this.i18nService.t( diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts index 5818c6e32ff..94542009a89 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts @@ -378,8 +378,7 @@ describe("VaultPopupAutofillService", () => { expect(result).toBe(true); expect(mockCipher.login.uris).toHaveLength(1); expect(mockCipher.login.uris[0].uri).toBe(mockCurrentTab.url); - expect(mockCipherService.encrypt).toHaveBeenCalledWith(mockCipher, mockUserId); - expect(mockCipherService.updateWithServer).toHaveBeenCalledWith(mockEncryptedCipher); + expect(mockCipherService.updateWithServer).toHaveBeenCalledWith(mockCipher, mockUserId); }); it("should add a URI to the cipher when there are no existing URIs", async () => { diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts index 6feeec29efc..025088e029e 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts @@ -426,8 +426,7 @@ export class VaultPopupAutofillService { const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); - const encCipher = await this.cipherService.encrypt(cipher, activeUserId); - await this.cipherService.updateWithServer(encCipher); + await this.cipherService.updateWithServer(cipher, activeUserId); this.messagingService.send("editedCipher"); return true; } catch { diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index d95e8333dca..dbcb0489187 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -138,10 +138,8 @@ export class EditCommand { ); } - const encCipher = await this.cipherService.encrypt(cipherView, activeUserId); try { - const updatedCipher = await this.cipherService.updateWithServer(encCipher); - const decCipher = await this.cipherService.decrypt(updatedCipher, activeUserId); + const decCipher = await this.cipherService.updateWithServer(cipherView, activeUserId); const res = new CipherResponse(decCipher); return Response.success(res); } catch (e) { diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index bc3d3153b13..7bb8da27040 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -147,11 +147,13 @@ import { SendService } from "@bitwarden/common/tools/send/services/send.service" import { UserId } from "@bitwarden/common/types/guid"; import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service"; +import { CipherSdkService } from "@bitwarden/common/vault/abstractions/cipher-sdk.service"; import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { CipherAuthorizationService, DefaultCipherAuthorizationService, } from "@bitwarden/common/vault/services/cipher-authorization.service"; +import { DefaultCipherSdkService } from "@bitwarden/common/vault/services/cipher-sdk.service"; import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; import { DefaultCipherArchiveService } from "@bitwarden/common/vault/services/default-cipher-archive.service"; import { DefaultCipherEncryptionService } from "@bitwarden/common/vault/services/default-cipher-encryption.service"; @@ -254,6 +256,7 @@ export class ServiceContainer { twoFactorApiService: TwoFactorApiService; hibpApiService: HibpApiService; environmentService: EnvironmentService; + cipherSdkService: CipherSdkService; cipherService: CipherService; folderService: InternalFolderService; organizationUserApiService: OrganizationUserApiService; @@ -794,6 +797,8 @@ export class ServiceContainer { this.logService, ); + this.cipherSdkService = new DefaultCipherSdkService(this.sdkService, this.logService); + this.cipherService = new CipherService( this.keyService, this.domainSettingsService, @@ -809,6 +814,7 @@ export class ServiceContainer { this.logService, this.cipherEncryptionService, this.messagingService, + this.cipherSdkService, ); this.cipherArchiveService = new DefaultCipherArchiveService( diff --git a/apps/cli/src/vault/create.command.ts b/apps/cli/src/vault/create.command.ts index d826766dc65..e1a91966afd 100644 --- a/apps/cli/src/vault/create.command.ts +++ b/apps/cli/src/vault/create.command.ts @@ -103,10 +103,11 @@ export class CreateCommand { return Response.error("Creating this item type is restricted by organizational policy."); } - const cipher = await this.cipherService.encrypt(CipherExport.toView(req), activeUserId); - const newCipher = await this.cipherService.createWithServer(cipher); - const decCipher = await this.cipherService.decrypt(newCipher, activeUserId); - const res = new CipherResponse(decCipher); + const newCipher = await this.cipherService.createWithServer( + CipherExport.toView(req), + activeUserId, + ); + const res = new CipherResponse(newCipher); return Response.success(res); } catch (e) { return Response.error(e); diff --git a/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts b/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts index cf29370840d..432448faba3 100644 --- a/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts +++ b/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts @@ -299,12 +299,11 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi throw new Error("No active user ID found!"); } - const encCipher = await this.cipherService.encrypt(cipher, activeUserId); - try { - const createdCipher = await this.cipherService.createWithServer(encCipher); + const createdCipher = await this.cipherService.createWithServer(cipher, activeUserId); + const encryptedCreatedCipher = await this.cipherService.encrypt(createdCipher, activeUserId); - return createdCipher; + return encryptedCreatedCipher.cipher; } catch { throw new Error("Unable to create cipher"); } @@ -316,8 +315,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi this.accountService.activeAccount$.pipe( map(async (a) => { if (a) { - const encCipher = await this.cipherService.encrypt(cipher, a.id); - await this.cipherService.updateWithServer(encCipher); + await this.cipherService.updateWithServer(cipher, a.id); } }), ), diff --git a/apps/desktop/src/services/encrypted-message-handler.service.ts b/apps/desktop/src/services/encrypted-message-handler.service.ts index 366a144c021..ccbc7c539d0 100644 --- a/apps/desktop/src/services/encrypted-message-handler.service.ts +++ b/apps/desktop/src/services/encrypted-message-handler.service.ts @@ -166,8 +166,7 @@ export class EncryptedMessageHandlerService { try { const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - const encrypted = await this.cipherService.encrypt(cipherView, activeUserId); - await this.cipherService.createWithServer(encrypted); + await this.cipherService.createWithServer(cipherView, activeUserId); // Notify other clients of new login await this.messagingService.send("addedCipher"); @@ -212,9 +211,8 @@ export class EncryptedMessageHandlerService { cipherView.login.password = credentialUpdatePayload.password; cipherView.login.username = credentialUpdatePayload.userName; cipherView.login.uris[0].uri = credentialUpdatePayload.uri; - const encrypted = await this.cipherService.encrypt(cipherView, activeUserId); - await this.cipherService.updateWithServer(encrypted); + await this.cipherService.updateWithServer(cipherView, activeUserId); // Notify other clients of update await this.messagingService.send("editedCipher"); diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index 5ca3a11d5ab..532757852a3 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -1536,8 +1536,7 @@ export class VaultComponent implements OnInit, OnDestr const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const cipherFullView = await this.cipherService.getFullCipherView(cipher); cipherFullView.favorite = !cipherFullView.favorite; - const encryptedCipher = await this.cipherService.encrypt(cipherFullView, activeUserId); - await this.cipherService.updateWithServer(encryptedCipher); + await this.cipherService.updateWithServer(cipherFullView, activeUserId); this.toastService.showToast({ variant: "success", diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 7b504548ff5..1ecf7fe3e3d 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -303,6 +303,7 @@ import { import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service"; import { CipherRiskService } from "@bitwarden/common/vault/abstractions/cipher-risk.service"; +import { CipherSdkService } from "@bitwarden/common/vault/abstractions/cipher-sdk.service"; import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherFileUploadService as CipherFileUploadServiceAbstraction } from "@bitwarden/common/vault/abstractions/file-upload/cipher-file-upload.service"; import { FolderApiServiceAbstraction } from "@bitwarden/common/vault/abstractions/folder/folder-api.service.abstraction"; @@ -321,6 +322,7 @@ import { CipherAuthorizationService, DefaultCipherAuthorizationService, } from "@bitwarden/common/vault/services/cipher-authorization.service"; +import { DefaultCipherSdkService } from "@bitwarden/common/vault/services/cipher-sdk.service"; import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; import { DefaultCipherArchiveService } from "@bitwarden/common/vault/services/default-cipher-archive.service"; import { DefaultCipherEncryptionService } from "@bitwarden/common/vault/services/default-cipher-encryption.service"; @@ -590,6 +592,11 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultDomainSettingsService, deps: [StateProvider, PolicyServiceAbstraction, AccountService], }), + safeProvider({ + provide: CipherSdkService, + useClass: DefaultCipherSdkService, + deps: [SdkService, LogService], + }), safeProvider({ provide: CipherServiceAbstraction, useFactory: ( @@ -607,6 +614,7 @@ const safeProviders: SafeProvider[] = [ logService: LogService, cipherEncryptionService: CipherEncryptionService, messagingService: MessagingServiceAbstraction, + cipherSdkService: CipherSdkService, ) => new CipherService( keyService, @@ -623,6 +631,7 @@ const safeProviders: SafeProvider[] = [ logService, cipherEncryptionService, messagingService, + cipherSdkService, ), deps: [ KeyService, @@ -639,6 +648,7 @@ const safeProviders: SafeProvider[] = [ LogService, CipherEncryptionService, MessagingServiceAbstraction, + CipherSdkService, ], }), safeProvider({ diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 77df258ad3a..94656d48826 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -68,6 +68,7 @@ export enum FeatureFlag { VaultLoadingSkeletons = "pm-25081-vault-skeleton-loaders", BrowserPremiumSpotlight = "pm-23384-browser-premium-spotlight", MigrateMyVaultToMyItems = "pm-20558-migrate-myvault-to-myitems", + PM27632_SdkCipherCrudOperations = "pm-27632-cipher-crud-operations-to-sdk", /* Platform */ IpcChannelFramework = "ipc-channel-framework", @@ -130,6 +131,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM22136_SdkCipherEncryption]: FALSE, [FeatureFlag.VaultLoadingSkeletons]: FALSE, [FeatureFlag.BrowserPremiumSpotlight]: FALSE, + [FeatureFlag.PM27632_SdkCipherCrudOperations]: FALSE, [FeatureFlag.MigrateMyVaultToMyItems]: FALSE, /* Auth */ diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts index 9c50bd1ab65..6223e4274bf 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts @@ -254,17 +254,17 @@ describe("FidoAuthenticatorService", () => { } it("should save credential to vault if request confirmed by user", async () => { - const encryptedCipher = Symbol(); userInterfaceSession.confirmNewCredential.mockResolvedValue({ cipherId: existingCipher.id, userVerified: false, }); - cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as EncryptionContext); await authenticator.makeCredential(params, windowReference); - const saved = cipherService.encrypt.mock.lastCall?.[0]; - expect(saved).toEqual( + const savedCipher = cipherService.updateWithServer.mock.lastCall?.[0]; + const actualUserId = cipherService.updateWithServer.mock.lastCall?.[1]; + expect(actualUserId).toEqual(userId); + expect(savedCipher).toEqual( expect.objectContaining({ type: CipherType.Login, name: existingCipher.name, @@ -288,7 +288,6 @@ describe("FidoAuthenticatorService", () => { }), }), ); - expect(cipherService.updateWithServer).toHaveBeenCalledWith(encryptedCipher); }); /** Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. */ @@ -361,17 +360,14 @@ describe("FidoAuthenticatorService", () => { cipherService.getAllDecrypted.mockResolvedValue([await cipher]); cipherService.decrypt.mockResolvedValue(cipher); - cipherService.encrypt.mockImplementation(async (cipher) => { - cipher.login.fido2Credentials[0].credentialId = credentialId; // Replace id for testability - return { cipher: {} as any as Cipher, encryptedFor: userId }; - }); - cipherService.createWithServer.mockImplementation(async ({ cipher }) => { - cipher.id = cipherId; + cipherService.createWithServer.mockImplementation(async (cipherView, _userId) => { + cipherView.id = cipherId; return cipher; }); - cipherService.updateWithServer.mockImplementation(async ({ cipher }) => { - cipher.id = cipherId; - return cipher; + cipherService.updateWithServer.mockImplementation(async (cipherView, _userId) => { + cipherView.id = cipherId; + cipherView.login.fido2Credentials[0].credentialId = credentialId; // Replace id for testability + return cipherView; }); }); @@ -701,14 +697,11 @@ describe("FidoAuthenticatorService", () => { /** Spec: Increment the credential associated signature counter */ it("should increment counter and save to server when stored counter is larger than zero", async () => { - const encrypted = Symbol(); - cipherService.encrypt.mockResolvedValue(encrypted as any); ciphers[0].login.fido2Credentials[0].counter = 9000; await authenticator.getAssertion(params, windowReference); - expect(cipherService.updateWithServer).toHaveBeenCalledWith(encrypted); - expect(cipherService.encrypt).toHaveBeenCalledWith( + expect(cipherService.updateWithServer).toHaveBeenCalledWith( expect.objectContaining({ id: ciphers[0].id, login: expect.objectContaining({ @@ -725,8 +718,6 @@ describe("FidoAuthenticatorService", () => { /** Spec: Authenticators that do not implement a signature counter leave the signCount in the authenticator data constant at zero. */ it("should not save to server when stored counter is zero", async () => { - const encrypted = Symbol(); - cipherService.encrypt.mockResolvedValue(encrypted as any); ciphers[0].login.fido2Credentials[0].counter = 0; await authenticator.getAssertion(params, windowReference); diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts index d1081e9f7b2..1b150207290 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts @@ -187,8 +187,7 @@ export class Fido2AuthenticatorService< if (Utils.isNullOrEmpty(cipher.login.username)) { cipher.login.username = fido2Credential.userName; } - const reencrypted = await this.cipherService.encrypt(cipher, activeUserId); - await this.cipherService.updateWithServer(reencrypted); + await this.cipherService.updateWithServer(cipher, activeUserId); await this.cipherService.clearCache(activeUserId); credentialId = fido2Credential.credentialId; } catch (error) { @@ -328,8 +327,7 @@ export class Fido2AuthenticatorService< const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getUserId), ); - const encrypted = await this.cipherService.encrypt(selectedCipher, activeUserId); - await this.cipherService.updateWithServer(encrypted); + await this.cipherService.updateWithServer(selectedCipher, activeUserId); await this.cipherService.clearCache(activeUserId); } diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.ts b/libs/common/src/platform/services/sdk/default-sdk.service.ts index 5084f5f5f18..e2c9c77e204 100644 --- a/libs/common/src/platform/services/sdk/default-sdk.service.ts +++ b/libs/common/src/platform/services/sdk/default-sdk.service.ts @@ -80,7 +80,7 @@ export class DefaultSdkService implements SdkService { client$ = this.environmentService.environment$.pipe( concatMap(async (env) => { await SdkLoadService.Ready; - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService), settings, @@ -210,7 +210,7 @@ export class DefaultSdkService implements SdkService { return undefined; } - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService, userId), settings, @@ -322,11 +322,12 @@ export class DefaultSdkService implements SdkService { client.platform().load_flags(featureFlagMap); } - private toSettings(env: Environment): ClientSettings { + private async toSettings(env: Environment): Promise { return { apiUrl: env.getApiUrl(), identityUrl: env.getIdentityUrl(), deviceType: toSdkDevice(this.platformUtilsService.getDevice()), + bitwardenClientVersion: await this.platformUtilsService.getApplicationVersionNumber(), userAgent: this.userAgent ?? navigator.userAgent, }; } diff --git a/libs/common/src/platform/services/sdk/register-sdk.service.ts b/libs/common/src/platform/services/sdk/register-sdk.service.ts index a222807640f..073c5c0560c 100644 --- a/libs/common/src/platform/services/sdk/register-sdk.service.ts +++ b/libs/common/src/platform/services/sdk/register-sdk.service.ts @@ -62,7 +62,7 @@ export class DefaultRegisterSdkService implements RegisterSdkService { client$ = this.environmentService.environment$.pipe( concatMap(async (env) => { await SdkLoadService.Ready; - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService), settings, @@ -137,7 +137,7 @@ export class DefaultRegisterSdkService implements RegisterSdkService { return undefined; } - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService, userId), settings, @@ -185,12 +185,13 @@ export class DefaultRegisterSdkService implements RegisterSdkService { client.platform().load_flags(featureFlagMap); } - private toSettings(env: Environment): ClientSettings { + private async toSettings(env: Environment): Promise { return { apiUrl: env.getApiUrl(), identityUrl: env.getIdentityUrl(), deviceType: toSdkDevice(this.platformUtilsService.getDevice()), userAgent: this.userAgent ?? navigator.userAgent, + bitwardenClientVersion: await this.platformUtilsService.getApplicationVersionNumber(), }; } } diff --git a/libs/common/src/vault/abstractions/cipher-sdk.service.ts b/libs/common/src/vault/abstractions/cipher-sdk.service.ts new file mode 100644 index 00000000000..1037bfc2b92 --- /dev/null +++ b/libs/common/src/vault/abstractions/cipher-sdk.service.ts @@ -0,0 +1,37 @@ +import { UserId } from "@bitwarden/common/types/guid"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +/** + * Service responsible for cipher operations using the SDK. + */ +export abstract class CipherSdkService { + /** + * Creates a new cipher on the server using the SDK. + * + * @param cipherView The cipher view to create + * @param userId The user ID to use for SDK client + * @param orgAdmin Whether this is an organization admin operation + * @returns A promise that resolves to the created cipher view + */ + abstract createWithServer( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise; + + /** + * Updates a cipher on the server using the SDK. + * + * @param cipher The cipher view to update + * @param userId The user ID to use for SDK client + * @param originalCipherView The original cipher view before changes (optional, used for admin operations) + * @param orgAdmin Whether this is an organization admin operation + * @returns A promise that resolves to the updated cipher view + */ + abstract updateWithServer( + cipher: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise; +} diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 203984075f7..1db5f8d38a7 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -119,9 +119,11 @@ export abstract class CipherService implements UserKeyRotationDataProvider; + ): Promise; + /** * Update a cipher with the server * @param cipher The cipher to update @@ -131,10 +133,11 @@ export abstract class CipherService implements UserKeyRotationDataProvider; + ): Promise; /** * Move a cipher to an organization by re-encrypting its keys with the organization's key. diff --git a/libs/common/src/vault/models/view/cipher.view.spec.ts b/libs/common/src/vault/models/view/cipher.view.spec.ts index 475fe9e23f3..1c7017d5d89 100644 --- a/libs/common/src/vault/models/view/cipher.view.spec.ts +++ b/libs/common/src/vault/models/view/cipher.view.spec.ts @@ -353,4 +353,366 @@ describe("CipherView", () => { }); }); }); + + // Note: These tests use jest.requireActual() because the file has jest.mock() calls + // at the top that mock LoginView, FieldView, etc. Those mocks are needed for other tests + // but interfere with these tests which need the real implementations. + describe("toSdkCreateCipherRequest", () => { + it("maps all properties correctly for a login cipher", () => { + const { FieldView: RealFieldView } = jest.requireActual("./field.view"); + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.organizationId = "000f2a6e-da5e-4726-87ed-1c5c77322c3c"; + cipherView.folderId = "41b22db4-8e2a-4ed2-b568-f1186c72922f"; + cipherView.collectionIds = ["b0473506-3c3c-4260-a734-dfaaf833ab6f"]; + cipherView.name = "Test Login"; + cipherView.notes = "Test notes"; + cipherView.type = CipherType.Login; + cipherView.favorite = true; + cipherView.reprompt = CipherRepromptType.Password; + + const field = new RealFieldView(); + field.name = "testField"; + field.value = "testValue"; + field.type = SdkFieldType.Text; + cipherView.fields = [field]; + + cipherView.login = new RealLoginView(); + cipherView.login.username = "testuser"; + cipherView.login.password = "testpass"; + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.organizationId).toEqual(asUuid("000f2a6e-da5e-4726-87ed-1c5c77322c3c")); + expect(result.folderId).toEqual(asUuid("41b22db4-8e2a-4ed2-b568-f1186c72922f")); + expect(result.collectionIds).toEqual([asUuid("b0473506-3c3c-4260-a734-dfaaf833ab6f")]); + expect(result.name).toBe("Test Login"); + expect(result.notes).toBe("Test notes"); + expect(result.favorite).toBe(true); + expect(result.reprompt).toBe(CipherRepromptType.Password); + expect(result.fields).toHaveLength(1); + expect(result.fields![0]).toMatchObject({ + name: "testField", + value: "testValue", + type: SdkFieldType.Text, + }); + expect(result.type).toHaveProperty("login"); + expect((result.type as any).login).toMatchObject({ + username: "testuser", + password: "testpass", + }); + }); + + it("handles undefined organizationId and folderId", () => { + const { SecureNoteView: RealSecureNoteView } = jest.requireActual("./secure-note.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.SecureNote; + cipherView.secureNote = new RealSecureNoteView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.organizationId).toBeUndefined(); + expect(result.folderId).toBeUndefined(); + expect(result.name).toBe("Test Cipher"); + }); + + it("handles empty collectionIds array", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.collectionIds = []; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.collectionIds).toEqual([]); + }); + + it("defaults favorite to false when undefined", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.favorite = undefined as any; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.favorite).toBe(false); + }); + + it("defaults reprompt to None when undefined", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.reprompt = undefined as any; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.reprompt).toBe(CipherRepromptType.None); + }); + + test.each([ + ["Login", CipherType.Login, "login.view", "LoginView"], + ["Card", CipherType.Card, "card.view", "CardView"], + ["Identity", CipherType.Identity, "identity.view", "IdentityView"], + ["SecureNote", CipherType.SecureNote, "secure-note.view", "SecureNoteView"], + ["SshKey", CipherType.SshKey, "ssh-key.view", "SshKeyView"], + ])( + "creates correct type property for %s cipher", + (typeName: string, cipherType: CipherType, moduleName: string, className: string) => { + const module = jest.requireActual(`./${moduleName}`); + const ViewClass = module[className]; + + const cipherView = new CipherView(); + cipherView.name = `Test ${typeName}`; + cipherView.type = cipherType; + + // Set the appropriate view property + const viewPropertyName = typeName.charAt(0).toLowerCase() + typeName.slice(1); + (cipherView as any)[viewPropertyName] = new ViewClass(); + + const result = cipherView.toSdkCreateCipherRequest(); + + const typeKey = typeName.charAt(0).toLowerCase() + typeName.slice(1); + expect(result.type).toHaveProperty(typeKey); + }, + ); + }); + + describe("toSdkUpdateCipherRequest", () => { + it("maps all properties correctly for an update request", () => { + const { FieldView: RealFieldView } = jest.requireActual("./field.view"); + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.organizationId = "000f2a6e-da5e-4726-87ed-1c5c77322c3c"; + cipherView.folderId = "41b22db4-8e2a-4ed2-b568-f1186c72922f"; + cipherView.name = "Updated Login"; + cipherView.notes = "Updated notes"; + cipherView.type = CipherType.Login; + cipherView.favorite = true; + cipherView.reprompt = CipherRepromptType.Password; + cipherView.revisionDate = new Date("2022-01-02T12:00:00.000Z"); + cipherView.archivedDate = new Date("2022-01-03T12:00:00.000Z"); + cipherView.key = new EncString("cipher-key"); + + const mockField = new RealFieldView(); + mockField.name = "testField"; + mockField.value = "testValue"; + cipherView.fields = [mockField]; + + cipherView.login = new RealLoginView(); + cipherView.login.username = "testuser"; + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.id).toEqual(asUuid("0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602")); + expect(result.organizationId).toEqual(asUuid("000f2a6e-da5e-4726-87ed-1c5c77322c3c")); + expect(result.folderId).toEqual(asUuid("41b22db4-8e2a-4ed2-b568-f1186c72922f")); + expect(result.name).toBe("Updated Login"); + expect(result.notes).toBe("Updated notes"); + expect(result.favorite).toBe(true); + expect(result.reprompt).toBe(CipherRepromptType.Password); + expect(result.revisionDate).toBe("2022-01-02T12:00:00.000Z"); + expect(result.archivedDate).toBe("2022-01-03T12:00:00.000Z"); + expect(result.fields).toHaveLength(1); + expect(result.fields![0]).toMatchObject({ + name: "testField", + value: "testValue", + }); + expect(result.type).toHaveProperty("login"); + expect((result.type as any).login).toMatchObject({ + username: "testuser", + }); + expect(result.key).toBeDefined(); + }); + + it("handles undefined optional properties", () => { + const { SecureNoteView: RealSecureNoteView } = jest.requireActual("./secure-note.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.SecureNote; + cipherView.secureNote = new RealSecureNoteView(); + cipherView.revisionDate = new Date("2022-01-02T12:00:00.000Z"); + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.organizationId).toBeUndefined(); + expect(result.folderId).toBeUndefined(); + expect(result.archivedDate).toBeUndefined(); + expect(result.key).toBeUndefined(); + }); + + it("converts dates to ISO strings", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + cipherView.revisionDate = new Date("2022-05-15T10:30:00.000Z"); + cipherView.archivedDate = new Date("2022-06-20T14:45:00.000Z"); + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.revisionDate).toBe("2022-05-15T10:30:00.000Z"); + expect(result.archivedDate).toBe("2022-06-20T14:45:00.000Z"); + }); + + it("includes attachments when present", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + const { AttachmentView: RealAttachmentView } = jest.requireActual("./attachment.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const attachment1 = new RealAttachmentView(); + attachment1.id = "attachment-id-1"; + attachment1.fileName = "file1.txt"; + + const attachment2 = new RealAttachmentView(); + attachment2.id = "attachment-id-2"; + attachment2.fileName = "file2.pdf"; + + cipherView.attachments = [attachment1, attachment2]; + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.attachments).toHaveLength(2); + }); + + test.each([ + ["Login", CipherType.Login, "login.view", "LoginView"], + ["Card", CipherType.Card, "card.view", "CardView"], + ["Identity", CipherType.Identity, "identity.view", "IdentityView"], + ["SecureNote", CipherType.SecureNote, "secure-note.view", "SecureNoteView"], + ["SshKey", CipherType.SshKey, "ssh-key.view", "SshKeyView"], + ])( + "creates correct type property for %s cipher", + (typeName: string, cipherType: CipherType, moduleName: string, className: string) => { + const module = jest.requireActual(`./${moduleName}`); + const ViewClass = module[className]; + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = `Test ${typeName}`; + cipherView.type = cipherType; + + // Set the appropriate view property + const viewPropertyName = typeName.charAt(0).toLowerCase() + typeName.slice(1); + (cipherView as any)[viewPropertyName] = new ViewClass(); + + const result = cipherView.toSdkUpdateCipherRequest(); + + const typeKey = typeName.charAt(0).toLowerCase() + typeName.slice(1); + expect(result.type).toHaveProperty(typeKey); + }, + ); + }); + + describe("getSdkCipherViewType", () => { + it("returns login type for Login cipher", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + cipherView.login.username = "testuser"; + cipherView.login.password = "testpass"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("login"); + expect((result as any).login).toMatchObject({ + username: "testuser", + password: "testpass", + }); + }); + + it("returns card type for Card cipher", () => { + const { CardView: RealCardView } = jest.requireActual("./card.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.Card; + cipherView.card = new RealCardView(); + cipherView.card.cardholderName = "John Doe"; + cipherView.card.number = "4111111111111111"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("card"); + expect((result as any).card.cardholderName).toBe("John Doe"); + expect((result as any).card.number).toBe("4111111111111111"); + }); + + it("returns identity type for Identity cipher", () => { + const { IdentityView: RealIdentityView } = jest.requireActual("./identity.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.Identity; + cipherView.identity = new RealIdentityView(); + cipherView.identity.firstName = "John"; + cipherView.identity.lastName = "Doe"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("identity"); + expect((result as any).identity.firstName).toBe("John"); + expect((result as any).identity.lastName).toBe("Doe"); + }); + + it("returns secureNote type for SecureNote cipher", () => { + const { SecureNoteView: RealSecureNoteView } = jest.requireActual("./secure-note.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.SecureNote; + cipherView.secureNote = new RealSecureNoteView(); + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("secureNote"); + }); + + it("returns sshKey type for SshKey cipher", () => { + const { SshKeyView: RealSshKeyView } = jest.requireActual("./ssh-key.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.SshKey; + cipherView.sshKey = new RealSshKeyView(); + cipherView.sshKey.privateKey = "privateKeyData"; + cipherView.sshKey.publicKey = "publicKeyData"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("sshKey"); + expect((result as any).sshKey.privateKey).toBe("privateKeyData"); + expect((result as any).sshKey.publicKey).toBe("publicKeyData"); + }); + + it("defaults to empty login for unknown cipher type", () => { + const cipherView = new CipherView(); + cipherView.type = 999 as CipherType; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("login"); + }); + }); }); diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 89f59665681..0909d0bda80 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -1,7 +1,12 @@ import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { asUuid, uuidAsString } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { ItemView } from "@bitwarden/common/vault/models/view/item.view"; -import { CipherView as SdkCipherView } from "@bitwarden/sdk-internal"; +import { + CipherCreateRequest, + CipherEditRequest, + CipherViewType, + CipherView as SdkCipherView, +} from "@bitwarden/sdk-internal"; import { View } from "../../../models/view/view"; import { InitializerMetadata } from "../../../platform/interfaces/initializer-metadata.interface"; @@ -332,6 +337,85 @@ export class CipherView implements View, InitializerMetadata { return cipherView; } + /** + * Maps CipherView to an SDK CipherCreateRequest + * + * @returns {CipherCreateRequest} The SDK cipher create request object + */ + toSdkCreateCipherRequest(): CipherCreateRequest { + const sdkCipherCreateRequest: CipherCreateRequest = { + organizationId: this.organizationId ? asUuid(this.organizationId) : undefined, + collectionIds: this.collectionIds ? this.collectionIds.map((i) => asUuid(i)) : [], + folderId: this.folderId ? asUuid(this.folderId) : undefined, + name: this.name ?? "", + notes: this.notes, + favorite: this.favorite ?? false, + reprompt: this.reprompt ?? CipherRepromptType.None, + fields: this.fields?.map((f) => f.toSdkFieldView()), + type: this.getSdkCipherViewType(), + }; + + return sdkCipherCreateRequest; + } + + /** + * Maps CipherView to an SDK CipherEditRequest + * + * @returns {CipherEditRequest} The SDK cipher edit request object + */ + toSdkUpdateCipherRequest(): CipherEditRequest { + const sdkCipherEditRequest: CipherEditRequest = { + id: asUuid(this.id), + organizationId: this.organizationId ? asUuid(this.organizationId) : undefined, + folderId: this.folderId ? asUuid(this.folderId) : undefined, + name: this.name ?? "", + notes: this.notes, + favorite: this.favorite ?? false, + reprompt: this.reprompt ?? CipherRepromptType.None, + fields: this.fields?.map((f) => f.toSdkFieldView()), + type: this.getSdkCipherViewType(), + revisionDate: this.revisionDate?.toISOString(), + archivedDate: this.archivedDate?.toISOString(), + attachments: this.attachments?.map((a) => a.toSdkAttachmentView()), + key: this.key?.toSdk(), + }; + + return sdkCipherEditRequest; + } + + /** + * Returns the SDK CipherViewType object for the cipher. + * + * @returns {CipherViewType} The SDK CipherViewType for the cipher.t + */ + getSdkCipherViewType(): CipherViewType { + let viewType: CipherViewType; + switch (this.type) { + case CipherType.Card: + viewType = { card: this.card?.toSdkCardView() }; + break; + case CipherType.Identity: + viewType = { identity: this.identity?.toSdkIdentityView() }; + break; + case CipherType.Login: + viewType = { login: this.login?.toSdkLoginView() }; + break; + case CipherType.SecureNote: + viewType = { secureNote: this.secureNote?.toSdkSecureNoteView() }; + break; + case CipherType.SshKey: + viewType = { sshKey: this.sshKey?.toSdkSshKeyView() }; + break; + default: + viewType = { + // Default to empty login - should not be valid code path. + login: new LoginView().toSdkLoginView(), + }; + break; + } + return viewType; + } + /** * Maps CipherView to SdkCipherView * diff --git a/libs/common/src/vault/services/cipher-sdk.service.spec.ts b/libs/common/src/vault/services/cipher-sdk.service.spec.ts new file mode 100644 index 00000000000..bd3feb4619e --- /dev/null +++ b/libs/common/src/vault/services/cipher-sdk.service.spec.ts @@ -0,0 +1,246 @@ +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { UserId, CipherId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +import { CipherType } from "../enums/cipher-type"; + +import { DefaultCipherSdkService } from "./cipher-sdk.service"; + +describe("DefaultCipherSdkService", () => { + const sdkService = mock(); + const logService = mock(); + const userId = "test-user-id" as UserId; + const cipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId; + + let cipherSdkService: DefaultCipherSdkService; + let mockSdkClient: any; + let mockCiphersSdk: any; + let mockAdminSdk: any; + let mockVaultSdk: any; + + beforeEach(() => { + // Mock the SDK client chain for admin operations + mockAdminSdk = { + create: jest.fn(), + edit: jest.fn(), + }; + mockCiphersSdk = { + create: jest.fn(), + edit: jest.fn(), + admin: jest.fn().mockReturnValue(mockAdminSdk), + }; + mockVaultSdk = { + ciphers: jest.fn().mockReturnValue(mockCiphersSdk), + }; + const mockSdkValue = { + vault: jest.fn().mockReturnValue(mockVaultSdk), + }; + mockSdkClient = { + take: jest.fn().mockReturnValue({ + value: mockSdkValue, + [Symbol.dispose]: jest.fn(), + }), + }; + + // Mock sdkService to return the mock client + sdkService.userClient$.mockReturnValue(of(mockSdkClient)); + + cipherSdkService = new DefaultCipherSdkService(sdkService, logService); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("createWithServer()", () => { + it("should create cipher using SDK when orgAdmin is false", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Test Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockCiphersSdk.create.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.createWithServer(cipherView, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.create).toHaveBeenCalledWith( + expect.objectContaining({ + name: cipherView.name, + organizationId: expect.anything(), + }), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result?.name).toBe(cipherView.name); + }); + + it("should create cipher using SDK admin API when orgAdmin is true", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Test Admin Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockAdminSdk.create.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.createWithServer(cipherView, userId, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.create).toHaveBeenCalledWith( + expect.objectContaining({ + name: cipherView.name, + }), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result?.name).toBe(cipherView.name); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + await expect(cipherSdkService.createWithServer(cipherView, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to create cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + mockCiphersSdk.create.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.createWithServer(cipherView, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to create cipher"), + ); + }); + }); + + describe("updateWithServer()", () => { + it("should update cipher using SDK when orgAdmin is false", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Updated Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockCiphersSdk.edit.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.updateWithServer(cipherView, userId, undefined, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.edit).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.anything(), + name: cipherView.name, + }), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result.name).toBe(cipherView.name); + }); + + it("should update cipher using SDK admin API when orgAdmin is true", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Updated Admin Cipher"; + cipherView.organizationId = orgId; + + const originalCipherView = new CipherView(); + originalCipherView.id = cipherId; + originalCipherView.name = "Original Cipher"; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockAdminSdk.edit.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.updateWithServer( + cipherView, + userId, + originalCipherView, + true, + ); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.edit).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.anything(), + name: cipherView.name, + }), + originalCipherView.toSdkCipherView(), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result.name).toBe(cipherView.name); + }); + + it("should update cipher using SDK admin API without originalCipherView", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Updated Admin Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockAdminSdk.edit.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.updateWithServer(cipherView, userId, undefined, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.edit).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.anything(), + name: cipherView.name, + }), + expect.anything(), // Empty CipherView - timestamps vary so we just verify it was called + ); + expect(result).toBeInstanceOf(CipherView); + expect(result.name).toBe(cipherView.name); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + await expect( + cipherSdkService.updateWithServer(cipherView, userId, undefined, false), + ).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to update cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + mockCiphersSdk.edit.mockRejectedValue(new Error("SDK error")); + + await expect( + cipherSdkService.updateWithServer(cipherView, userId, undefined, false), + ).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to update cipher"), + ); + }); + }); +}); diff --git a/libs/common/src/vault/services/cipher-sdk.service.ts b/libs/common/src/vault/services/cipher-sdk.service.ts new file mode 100644 index 00000000000..06f5d3eb961 --- /dev/null +++ b/libs/common/src/vault/services/cipher-sdk.service.ts @@ -0,0 +1,82 @@ +import { firstValueFrom, switchMap, catchError } from "rxjs"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { CipherView as SdkCipherView } from "@bitwarden/sdk-internal"; + +import { CipherSdkService } from "../abstractions/cipher-sdk.service"; + +export class DefaultCipherSdkService implements CipherSdkService { + constructor( + private sdkService: SdkService, + private logService: LogService, + ) {} + + async createWithServer( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + const sdkCreateRequest = cipherView.toSdkCreateCipherRequest(); + let result: SdkCipherView; + if (orgAdmin) { + result = await ref.value.vault().ciphers().admin().create(sdkCreateRequest); + } else { + result = await ref.value.vault().ciphers().create(sdkCreateRequest); + } + return CipherView.fromSdkCipherView(result); + }), + catchError((error: unknown) => { + this.logService.error(`Failed to create cipher: ${error}`); + throw error; + }), + ), + ); + } + + async updateWithServer( + cipher: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + const sdkUpdateRequest = cipher.toSdkUpdateCipherRequest(); + let result: SdkCipherView; + if (orgAdmin) { + result = await ref.value + .vault() + .ciphers() + .admin() + .edit( + sdkUpdateRequest, + originalCipherView?.toSdkCipherView() || new CipherView().toSdkCipherView(), + ); + } else { + result = await ref.value.vault().ciphers().edit(sdkUpdateRequest); + } + return CipherView.fromSdkCipherView(result); + }), + catchError((error: unknown) => { + this.logService.error(`Failed to update cipher: ${error}`); + throw error; + }), + ), + ); + } +} diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index 153bb01403c..4f98ba62a1c 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -28,6 +28,7 @@ import { ContainerService } from "../../platform/services/container.service"; import { CipherId, UserId, OrganizationId, CollectionId } from "../../types/guid"; import { CipherKey, OrgKey, UserKey } from "../../types/key"; import { CipherEncryptionService } from "../abstractions/cipher-encryption.service"; +import { CipherSdkService } from "../abstractions/cipher-sdk.service"; import { EncryptionContext } from "../abstractions/cipher.service"; import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service"; import { SearchService } from "../abstractions/search.service"; @@ -54,9 +55,9 @@ function encryptText(clearText: string | Uint8Array) { const ENCRYPTED_BYTES = mock(); const cipherData: CipherData = { - id: "id", - organizationId: "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b2" as OrganizationId, - folderId: "folderId", + id: "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + organizationId: "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId, + folderId: "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23", edit: true, viewPassword: true, organizationUseTotp: true, @@ -109,9 +110,10 @@ describe("Cipher Service", () => { const stateProvider = new FakeStateProvider(accountService); const cipherEncryptionService = mock(); const messageSender = mock(); + const cipherSdkService = mock(); const userId = "TestUserId" as UserId; - const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b2" as OrganizationId; + const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId; let cipherService: CipherService; let encryptionContext: EncryptionContext; @@ -145,6 +147,7 @@ describe("Cipher Service", () => { logService, cipherEncryptionService, messageSender, + cipherSdkService, ); encryptionContext = { cipher: new Cipher(cipherData), encryptedFor: userId }; @@ -207,11 +210,22 @@ describe("Cipher Service", () => { }); describe("createWithServer()", () => { + beforeEach(() => { + jest.spyOn(cipherService, "encrypt").mockResolvedValue(encryptionContext); + jest.spyOn(cipherService, "decrypt").mockImplementation(async (cipher) => { + return new CipherView(cipher); + }); + }); + it("should call apiService.postCipherAdmin when orgAdmin param is true and the cipher orgId != null", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); const spy = jest .spyOn(apiService, "postCipherAdmin") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext, true); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId, true); const expectedObj = new CipherCreateRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -219,11 +233,15 @@ describe("Cipher Service", () => { }); it("should call apiService.postCipher when orgAdmin param is true and the cipher orgId is null", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.organizationId = null!; const spy = jest .spyOn(apiService, "postCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext, true); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId, true); const expectedObj = new CipherRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -231,11 +249,15 @@ describe("Cipher Service", () => { }); it("should call apiService.postCipherCreate if collectionsIds != null", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.collectionIds = ["123"]; const spy = jest .spyOn(apiService, "postCipherCreate") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId); const expectedObj = new CipherCreateRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -243,35 +265,86 @@ describe("Cipher Service", () => { }); it("should call apiService.postCipher when orgAdmin and collectionIds logic is false", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); const spy = jest .spyOn(apiService, "postCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId); const expectedObj = new CipherRequest(encryptionContext); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(expectedObj); }); + + it("should delegate to cipherSdkService when feature flag is enabled", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(true); + + const cipherView = new CipherView(encryptionContext.cipher); + const expectedResult = new CipherView(encryptionContext.cipher); + + const cipherSdkServiceSpy = jest + .spyOn(cipherSdkService, "createWithServer") + .mockResolvedValue(expectedResult); + + const clearCacheSpy = jest.spyOn(cipherService, "clearCache"); + const apiSpy = jest.spyOn(apiService, "postCipher"); + + const result = await cipherService.createWithServer(cipherView, userId); + + expect(cipherSdkServiceSpy).toHaveBeenCalledWith(cipherView, userId, undefined); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + expect(result).toBeInstanceOf(CipherView); + }); }); describe("updateWithServer()", () => { + beforeEach(() => { + jest.spyOn(cipherService, "encrypt").mockResolvedValue(encryptionContext); + jest.spyOn(cipherService, "decrypt").mockImplementation(async (cipher) => { + return new CipherView(cipher); + }); + jest.spyOn(cipherService, "upsert").mockResolvedValue({ + [cipherData.id as CipherId]: cipherData, + }); + }); + it("should call apiService.putCipherAdmin when orgAdmin param is true", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); + + const testCipher = new Cipher(cipherData); + testCipher.organizationId = orgId; + const testContext = { cipher: testCipher, encryptedFor: userId }; + jest.spyOn(cipherService, "encrypt").mockResolvedValue(testContext); + const spy = jest .spyOn(apiService, "putCipherAdmin") - .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.updateWithServer(encryptionContext, true); - const expectedObj = new CipherRequest(encryptionContext); + .mockImplementation(() => Promise.resolve(testCipher.toCipherData())); + const cipherView = new CipherView(testCipher); + await cipherService.updateWithServer(cipherView, userId, undefined, true); + const expectedObj = new CipherRequest(testContext); expect(spy).toHaveBeenCalled(); - expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj); + expect(spy).toHaveBeenCalledWith(testCipher.id, expectedObj); }); it("should call apiService.putCipher if cipher.edit is true", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.edit = true; const spy = jest .spyOn(apiService, "putCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.updateWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.updateWithServer(cipherView, userId); const expectedObj = new CipherRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -279,16 +352,79 @@ describe("Cipher Service", () => { }); it("should call apiService.putPartialCipher when orgAdmin, and edit are false", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.edit = false; const spy = jest .spyOn(apiService, "putPartialCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.updateWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.updateWithServer(cipherView, userId); const expectedObj = new CipherPartialRequest(encryptionContext.cipher); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj); }); + + it("should delegate to cipherSdkService when feature flag is enabled", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(true); + + const testCipher = new Cipher(cipherData); + const cipherView = new CipherView(testCipher); + const expectedResult = new CipherView(testCipher); + + const cipherSdkServiceSpy = jest + .spyOn(cipherSdkService, "updateWithServer") + .mockResolvedValue(expectedResult); + + const clearCacheSpy = jest.spyOn(cipherService, "clearCache"); + const apiSpy = jest.spyOn(apiService, "putCipher"); + + const result = await cipherService.updateWithServer(cipherView, userId); + + expect(cipherSdkServiceSpy).toHaveBeenCalledWith(cipherView, userId, undefined, undefined); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + expect(result).toBeInstanceOf(CipherView); + }); + + it("should delegate to cipherSdkService with orgAdmin when feature flag is enabled", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(true); + + const testCipher = new Cipher(cipherData); + const cipherView = new CipherView(testCipher); + const originalCipherView = new CipherView(testCipher); + const expectedResult = new CipherView(testCipher); + + const cipherSdkServiceSpy = jest + .spyOn(cipherSdkService, "updateWithServer") + .mockResolvedValue(expectedResult); + + const clearCacheSpy = jest.spyOn(cipherService, "clearCache"); + const apiSpy = jest.spyOn(apiService, "putCipherAdmin"); + + const result = await cipherService.updateWithServer( + cipherView, + userId, + originalCipherView, + true, + ); + + expect(cipherSdkServiceSpy).toHaveBeenCalledWith( + cipherView, + userId, + originalCipherView, + true, + ); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + expect(result).toBeInstanceOf(CipherView); + }); }); describe("encrypt", () => { diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 2e0adc892e3..53d7666e304 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -42,6 +42,7 @@ import { CipherId, CollectionId, OrganizationId, UserId } from "../../types/guid import { OrgKey, UserKey } from "../../types/key"; import { filterOutNullish, perUserCache$ } from "../../vault/utils/observable-utilities"; import { CipherEncryptionService } from "../abstractions/cipher-encryption.service"; +import { CipherSdkService } from "../abstractions/cipher-sdk.service"; import { CipherService as CipherServiceAbstraction, EncryptionContext, @@ -120,6 +121,7 @@ export class CipherService implements CipherServiceAbstraction { private logService: LogService, private cipherEncryptionService: CipherEncryptionService, private messageSender: MessageSender, + private cipherSdkService: CipherSdkService, ) {} localData$(userId: UserId): Observable> { @@ -903,6 +905,40 @@ export class CipherService implements CipherServiceAbstraction { } async createWithServer( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise { + const useSdk = await this.configService.getFeatureFlag( + FeatureFlag.PM27632_SdkCipherCrudOperations, + ); + + if (useSdk) { + return ( + (await this.createWithServerUsingSdk(cipherView, userId, orgAdmin)) || new CipherView() + ); + } + + const encrypted = await this.encrypt(cipherView, userId); + const result = await this.createWithServer_legacy(encrypted, orgAdmin); + return await this.decrypt(result, userId); + } + + private async createWithServerUsingSdk( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise { + const resultCipherView = await this.cipherSdkService.createWithServer( + cipherView, + userId, + orgAdmin, + ); + await this.clearCache(userId); + return resultCipherView; + } + + private async createWithServer_legacy( { cipher, encryptedFor }: EncryptionContext, orgAdmin?: boolean, ): Promise { @@ -929,6 +965,42 @@ export class CipherService implements CipherServiceAbstraction { } async updateWithServer( + cipherView: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise { + const useSdk = await this.configService.getFeatureFlag( + FeatureFlag.PM27632_SdkCipherCrudOperations, + ); + + if (useSdk) { + return await this.updateWithServerUsingSdk(cipherView, userId, originalCipherView, orgAdmin); + } + + const encrypted = await this.encrypt(cipherView, userId); + const updatedCipher = await this.updateWithServer_legacy(encrypted, orgAdmin); + const updatedCipherView = await this.decrypt(updatedCipher, userId); + return updatedCipherView; + } + + async updateWithServerUsingSdk( + cipher: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise { + const resultCipherView = await this.cipherSdkService.updateWithServer( + cipher, + userId, + originalCipherView, + orgAdmin, + ); + await this.clearCache(userId); + return resultCipherView; + } + + async updateWithServer_legacy( { cipher, encryptedFor }: EncryptionContext, orgAdmin?: boolean, ): Promise { @@ -1119,8 +1191,7 @@ export class CipherService implements CipherServiceAbstraction { //in order to keep item and it's attachments with the same encryption level if (cipher.key != null && !cipherKeyEncryptionEnabled) { const model = await this.decrypt(cipher, userId); - const reEncrypted = await this.encrypt(model, userId); - await this.updateWithServer(reEncrypted); + await this.updateWithServer(model, userId); } const encFileName = await this.encryptService.encryptString(filename, cipherEncKey); diff --git a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts index 59c583f980b..8566e51d74f 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts @@ -37,14 +37,13 @@ export class DefaultCipherFormService implements CipherFormService { // Creating a new cipher if (cipher.id == null || cipher.id === "") { - const encrypted = await this.cipherService.encrypt(cipher, activeUserId); - savedCipher = await this.cipherService.createWithServer(encrypted, config.admin); - return await this.cipherService.decrypt(savedCipher, activeUserId); + return await this.cipherService.createWithServer(cipher, activeUserId, config.admin); } if (config.originalCipher == null) { throw new Error("Original cipher is required for updating an existing cipher"); } + const originalCipherView = await this.decryptCipher(config.originalCipher); // Updating an existing cipher @@ -66,35 +65,31 @@ export class DefaultCipherFormService implements CipherFormService { ); // If the collectionIds are the same, update the cipher normally } else if (isSetEqual(originalCollectionIds, newCollectionIds)) { - const encrypted = await this.cipherService.encrypt( + const savedCipherView = await this.cipherService.updateWithServer( cipher, activeUserId, - null, - null, - config.originalCipher, + originalCipherView, + config.admin, ); - savedCipher = await this.cipherService.updateWithServer(encrypted, config.admin); + savedCipher = await this.cipherService + .encrypt(savedCipherView, activeUserId) + .then((res) => res.cipher); } else { - const encrypted = await this.cipherService.encrypt( - cipher, - activeUserId, - null, - null, - config.originalCipher, - ); - const encryptedCipher = encrypted.cipher; - // Updating a cipher with collection changes is not supported with a single request currently // First update the cipher with the original collectionIds - encryptedCipher.collectionIds = config.originalCipher.collectionIds; - await this.cipherService.updateWithServer( - encrypted, + cipher.collectionIds = config.originalCipher.collectionIds; + const newCipher = await this.cipherService.updateWithServer( + cipher, + activeUserId, + originalCipherView, config.admin || originalCollectionIds.size === 0, ); // Then save the new collection changes separately - encryptedCipher.collectionIds = cipher.collectionIds; + newCipher.collectionIds = cipher.collectionIds; + // TODO: Remove after migrating all SDK ops + const { cipher: encryptedCipher } = await this.cipherService.encrypt(newCipher, activeUserId); if (config.admin || originalCollectionIds.size === 0) { // When using an admin config or the cipher was unassigned, update collections as an admin savedCipher = await this.cipherService.saveCollectionsWithServerAdmin(encryptedCipher); From 8b9211ea620f5cfcbe908fed31e390ae06268d1e Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 26 Jan 2026 11:52:30 -0800 Subject: [PATCH 6/6] do not show badge/button in AC (#18489) --- .../reports/pages/cipher-report.component.ts | 1 + .../vault-item-dialog.component.html | 2 +- .../vault-item-dialog.component.spec.ts | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts b/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts index d8519b86094..f775ed84ede 100644 --- a/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts @@ -193,6 +193,7 @@ export abstract class CipherReportComponent implements OnDestroy { formConfig, activeCollectionId, disableForm, + isAdminConsoleAction: true, }); const result = await lastValueFrom(this.vaultItemDialogRef.closed); diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html index 059347709f0..ec06c740f24 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html @@ -3,7 +3,7 @@ {{ title }} - @if (isCipherArchived) { + @if (isCipherArchived && !params.isAdminConsoleAction) { {{ "archived" | i18n }} } diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts index 63b5071d1f5..9a048b7a8b3 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts @@ -303,6 +303,25 @@ describe("VaultItemDialogComponent", () => { }); }); + describe("archive badge", () => { + it('should show "archived" badge when the item is archived and not an admin console action', () => { + component.setTestCipher({ isArchived: true }); + component.setTestParams({ mode: "view" }); + fixture.detectChanges(); + const archivedBadge = fixture.debugElement.query(By.css("span[bitBadge]")); + expect(archivedBadge).toBeTruthy(); + expect(archivedBadge.nativeElement.textContent.trim()).toBe("archived"); + }); + + it('should not show "archived" badge when the item is archived and is an admin console action', () => { + component.setTestCipher({ isArchived: true }); + component.setTestParams({ mode: "view", isAdminConsoleAction: true }); + fixture.detectChanges(); + const archivedBadge = fixture.debugElement.query(By.css("span[bitBadge]")); + expect(archivedBadge).toBeFalsy(); + }); + }); + describe("submitButtonText$", () => { it("should return 'unArchiveAndSave' when premium is false and cipher is archived", (done) => { jest.spyOn(component as any, "userHasPremium$", "get").mockReturnValue(of(false));