diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cf7251b259a..4280cabc812 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -111,7 +111,7 @@ jobs: working-directory: ./apps/desktop/desktop_native run: cargo build - - name: Test Ubuntu + - name: Linux unit tests if: ${{ matrix.os=='ubuntu-22.04' }} working-directory: ./apps/desktop/desktop_native run: | @@ -120,17 +120,21 @@ jobs: mkdir -p ~/.local/share/keyrings eval "$(printf '\n' | gnome-keyring-daemon --unlock)" eval "$(printf '\n' | /usr/bin/gnome-keyring-daemon --start)" - cargo test -- --test-threads=1 + cargo test --lib -- --test-threads=1 - - name: Test macOS + - name: MacOS unit tests if: ${{ matrix.os=='macos-14' }} working-directory: ./apps/desktop/desktop_native - run: cargo test -- --test-threads=1 + run: cargo test --lib -- --test-threads=1 - - name: Test Windows + - name: Windows unit tests if: ${{ matrix.os=='windows-2022'}} working-directory: ./apps/desktop/desktop_native - run: cargo test --workspace --exclude=desktop_napi -- --test-threads=1 + run: cargo test --lib --workspace --exclude=desktop_napi -- --test-threads=1 + + - name: Doc tests + working-directory: ./apps/desktop/desktop_native + run: cargo test --doc rust-coverage: name: Rust Coverage diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 90cc4a5c338..dabd238e039 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2473,6 +2473,9 @@ "permanentlyDeletedItem": { "message": "Item permanently deleted" }, + "archivedItemRestored": { + "message": "Archived item restored" + }, "restoreItem": { "message": "Restore item" }, @@ -5670,6 +5673,9 @@ "extraWide": { "message": "Extra wide" }, + "narrow": { + "message": "Narrow" + }, "sshKeyWrongPassword": { "message": "The password you entered is incorrect." }, diff --git a/apps/browser/src/autofill/fido2/content/messaging/messenger.ts b/apps/browser/src/autofill/fido2/content/messaging/messenger.ts index 61ed7a8ed08..257f7e9efd5 100644 --- a/apps/browser/src/autofill/fido2/content/messaging/messenger.ts +++ b/apps/browser/src/autofill/fido2/content/messaging/messenger.ts @@ -1,3 +1,5 @@ +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore import { Message, MessageTypes } from "./message"; const SENDER = "bitwarden-webauthn"; @@ -23,7 +25,7 @@ type Handler = ( * handling aborts and exceptions across separate execution contexts. */ export class Messenger { - private messageEventListener: ((event: MessageEvent) => void) | null = null; + private messageEventListener: (event: MessageEvent) => void | null = null; private onDestroy = new EventTarget(); /** @@ -58,12 +60,6 @@ export class Messenger { this.broadcastChannel.addEventListener(this.messageEventListener); } - private stripMetadata({ SENDER, senderId, ...message }: MessageWithMetadata): Message { - void SENDER; - void senderId; - return message; - } - /** * Sends a request to the content script and returns the response. * AbortController signals will be forwarded to the content script. @@ -78,9 +74,7 @@ export class Messenger { try { const promise = new Promise((resolve) => { - localPort.onmessage = (event: MessageEvent) => { - resolve(this.stripMetadata(event.data)); - }; + localPort.onmessage = (event: MessageEvent) => resolve(event.data); }); const abortListener = () => @@ -135,9 +129,7 @@ export class Messenger { try { const handlerResponse = await this.handler(message, abortController); - if (handlerResponse !== undefined) { - port.postMessage({ ...handlerResponse, SENDER }); - } + port.postMessage({ ...handlerResponse, SENDER }); } catch (error) { port.postMessage({ SENDER, diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 18eb8e2baf8..25fcb9038d8 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1083,6 +1083,8 @@ export class CollectAutofillContentService implements CollectAutofillContentServ setTimeout(this.autofillOverlayContentService.refreshMenuLayerPosition, 100); } }); + + this.autofillOverlayContentService.refreshMenuLayerPosition(); } }; diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts index 746f5a1f8f7..c277b8d33f8 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts @@ -319,7 +319,10 @@ describe("PhishingDataService", () => { jest.spyOn(service as any, "_decompressString").mockResolvedValue("phish.com\nbadguy.net"); - await service["_loadBlobToMemory"](); + // 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); 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 85e91b06a6b..7d5f04cc276 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 @@ -3,11 +3,15 @@ import { EMPTY, first, firstValueFrom, + from, + of, share, + takeUntil, startWith, Subject, switchMap, tap, + map, } from "rxjs"; import { devFlagEnabled, devFlagValue } from "@bitwarden/browser/platform/flags"; @@ -64,12 +68,20 @@ export const PHISHING_DOMAINS_BLOB_KEY = new KeyDefinition( /** Coordinates fetching, caching, and patching of known phishing web addresses */ export class PhishingDataService { + // While background scripts do not necessarily need destroying, + // processes in PhishingDataService are memory intensive. + // 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 _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(); // How often are new web addresses added to the remote? readonly UPDATE_INTERVAL_DURATION = 24 * 60 * 60 * 1000; // 24 hours @@ -81,6 +93,10 @@ 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); }), @@ -90,6 +106,8 @@ export class PhishingDataService { }), ), ), + // Stop emitting when dispose() is called + takeUntil(this._destroy$), share(), ); @@ -109,7 +127,18 @@ export class PhishingDataService { ScheduledTaskNames.phishingDomainUpdate, this.UPDATE_INTERVAL_DURATION, ); - void this._loadBlobToMemory(); + this._setupLoadPipeline(); + } + + 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; + } } /** @@ -269,7 +298,7 @@ export class PhishingDataService { } if (next.blob) { await this._phishingBlobState.update(() => next!.blob!); - await this._loadBlobToMemory(); + this._loadBlobToMemory(); } // Performance logging @@ -293,6 +322,47 @@ export class PhishingDataService { } } + // 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`, + ); + }), + ); + }), + catchError((err: unknown) => { + this.logService.error("[PhishingDataService] Failed to load blob into memory", err); + return of(undefined); + }), + ), + ), + catchError((err: unknown) => { + this.logService.error("[PhishingDataService] Load pipeline failed", err); + return of(undefined); + }), + takeUntil(this._destroy$), + share(), + ) + .subscribe(); + } + // [FIXME] Move compression helpers to a shared utils library // to separate from phishing data service. // ------------------------- Blob and Compression Handling ------------------------- @@ -337,29 +407,9 @@ export class PhishingDataService { } } - // Try to load compressed newline blob into an in-memory Set for fast lookups - private async _loadBlobToMemory(): Promise { - this.logService.debug("[PhishingDataService] Loading data blob into memory..."); - try { - const blobBase64 = await firstValueFrom(this._phishingBlobState.state$); - if (!blobBase64) { - return; - } - - const text = await this._decompressString(blobBase64); - // Split and filter - const lines = text.split(/\r?\n/); - const newWebAddressesSet = new Set(lines); - - // Add test addresses - 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`, - ); - } catch (err) { - this.logService.error("[PhishingDataService] Failed to load blob into memory", err); - } + // 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 diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts index d90e872eef8..815007e1d4c 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts @@ -137,6 +137,9 @@ export class PhishingDetectionService { this._didInit = true; return () => { + // Dispose phishing data service resources + phishingDataService.dispose(); + initSub.unsubscribe(); this._didInit = false; 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 new file mode 100644 index 00000000000..75bd634b1fc --- /dev/null +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts @@ -0,0 +1,375 @@ +import { ReadableStream as NodeReadableStream } from "stream/web"; + +import { mock, MockProxy } from "jest-mock-extended"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; + +import { PhishingIndexedDbService } from "./phishing-indexeddb.service"; + +describe("PhishingIndexedDbService", () => { + let service: PhishingIndexedDbService; + let logService: MockProxy; + + // Mock IndexedDB storage (keyed by URL for row-per-URL storage) + let mockStore: Map; + let mockObjectStore: any; + let mockTransaction: any; + let mockDb: any; + let mockOpenRequest: any; + + beforeEach(() => { + logService = mock(); + mockStore = new Map(); + + // Mock IDBObjectStore + mockObjectStore = { + put: jest.fn().mockImplementation((record: { url: string }) => { + const request = { + error: null as DOMException | null, + result: undefined as undefined, + onsuccess: null as (() => void) | null, + onerror: null as (() => void) | null, + }; + setTimeout(() => { + mockStore.set(record.url, record); + request.onsuccess?.(); + }, 0); + return request; + }), + get: jest.fn().mockImplementation((key: string) => { + const request = { + error: null as DOMException | null, + result: mockStore.get(key), + onsuccess: null as (() => void) | null, + onerror: null as (() => void) | null, + }; + setTimeout(() => { + request.result = mockStore.get(key); + request.onsuccess?.(); + }, 0); + return request; + }), + clear: jest.fn().mockImplementation(() => { + const request = { + error: null as DOMException | null, + result: undefined as undefined, + onsuccess: null as (() => void) | null, + onerror: null as (() => void) | null, + }; + setTimeout(() => { + mockStore.clear(); + request.onsuccess?.(); + }, 0); + return request; + }), + openCursor: jest.fn().mockImplementation(() => { + const entries = Array.from(mockStore.entries()); + let index = 0; + const request = { + error: null as DOMException | null, + result: null as any, + onsuccess: null as ((e: any) => void) | null, + onerror: null as (() => void) | null, + }; + const advanceCursor = () => { + if (index < entries.length) { + const [, value] = entries[index]; + index++; + request.result = { + value, + continue: () => setTimeout(advanceCursor, 0), + }; + } else { + request.result = null; + } + request.onsuccess?.({ target: request }); + }; + setTimeout(advanceCursor, 0); + return request; + }), + }; + + // Mock IDBTransaction + mockTransaction = { + objectStore: jest.fn().mockReturnValue(mockObjectStore), + oncomplete: null as (() => void) | null, + onerror: null as (() => void) | null, + }; + + // Trigger oncomplete after a tick + const originalObjectStore = mockTransaction.objectStore; + mockTransaction.objectStore = jest.fn().mockImplementation((...args: any[]) => { + setTimeout(() => mockTransaction.oncomplete?.(), 0); + return originalObjectStore(...args); + }); + + // Mock IDBDatabase + mockDb = { + transaction: jest.fn().mockReturnValue(mockTransaction), + close: jest.fn(), + objectStoreNames: { + contains: jest.fn().mockReturnValue(true), + }, + createObjectStore: jest.fn(), + }; + + // Mock IDBOpenDBRequest + mockOpenRequest = { + error: null as DOMException | null, + result: mockDb, + onsuccess: null as (() => void) | null, + onerror: null as (() => void) | null, + onupgradeneeded: null as ((event: any) => void) | null, + }; + + // Mock indexedDB.open + const mockIndexedDB = { + open: jest.fn().mockImplementation(() => { + setTimeout(() => { + mockOpenRequest.onsuccess?.(); + }, 0); + return mockOpenRequest; + }), + }; + + global.indexedDB = mockIndexedDB as any; + + service = new PhishingIndexedDbService(logService); + }); + + afterEach(() => { + jest.clearAllMocks(); + delete (global as any).indexedDB; + }); + + describe("saveUrls", () => { + it("stores URLs in IndexedDB and returns true", async () => { + const urls = ["https://phishing.com", "https://malware.net"]; + + const result = await service.saveUrls(urls); + + expect(result).toBe(true); + expect(mockDb.transaction).toHaveBeenCalledWith("phishing-urls", "readwrite"); + expect(mockObjectStore.clear).toHaveBeenCalled(); + expect(mockObjectStore.put).toHaveBeenCalledTimes(2); + expect(mockDb.close).toHaveBeenCalled(); + }); + + it("handles empty array", async () => { + const result = await service.saveUrls([]); + + expect(result).toBe(true); + expect(mockObjectStore.clear).toHaveBeenCalled(); + }); + + it("trims whitespace from URLs", async () => { + const urls = [" https://example.com ", "\nhttps://test.org\n"]; + + await service.saveUrls(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.saveUrls(urls); + + expect(mockObjectStore.put).toHaveBeenCalledTimes(2); + }); + + it("handles duplicate URLs via upsert (keyPath deduplication)", async () => { + const urls = [ + "https://example.com", + "https://example.com", // duplicate + "https://test.org", + ]; + + const result = await service.saveUrls(urls); + + expect(result).toBe(true); + // put() is called 3 times, but mockStore (using Map with URL as key) + // only stores 2 unique entries - demonstrating upsert behavior + expect(mockObjectStore.put).toHaveBeenCalledTimes(3); + 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.saveUrls(["https://test.com"]); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Save failed", + expect.any(Error), + ); + }); + }); + + describe("hasUrl", () => { + it("returns true for existing URL", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + + const result = await service.hasUrl("https://example.com"); + + expect(result).toBe(true); + expect(mockDb.transaction).toHaveBeenCalledWith("phishing-urls", "readonly"); + expect(mockObjectStore.get).toHaveBeenCalledWith("https://example.com"); + }); + + it("returns false for non-existing URL", async () => { + const result = await service.hasUrl("https://notfound.com"); + + expect(result).toBe(false); + }); + + it("returns false on error", 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.hasUrl("https://example.com"); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Check failed", + expect.any(Error), + ); + }); + }); + + describe("loadAllUrls", () => { + it("loads all URLs using cursor", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + + const result = await service.loadAllUrls(); + + expect(result).toContain("https://example.com"); + expect(result).toContain("https://test.org"); + expect(result.length).toBe(2); + }); + + it("returns empty array when no data exists", async () => { + const result = await service.loadAllUrls(); + + expect(result).toEqual([]); + }); + + it("returns empty array on error", 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.loadAllUrls(); + + expect(result).toEqual([]); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Load failed", + expect.any(Error), + ); + }); + }); + + describe("saveUrlsFromStream", () => { + it("saves URLs from stream", async () => { + const content = "https://example.com\nhttps://test.org\nhttps://phishing.net"; + const stream = new NodeReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(content)); + controller.close(); + }, + }) as unknown as ReadableStream; + + const result = await service.saveUrlsFromStream(stream); + + expect(result).toBe(true); + expect(mockObjectStore.clear).toHaveBeenCalled(); + expect(mockObjectStore.put).toHaveBeenCalledTimes(3); + }); + + it("handles chunked stream data", async () => { + const content = "https://url1.com\nhttps://url2.com"; + const encoder = new TextEncoder(); + const encoded = encoder.encode(content); + + // Split into multiple small chunks + const stream = new NodeReadableStream({ + start(controller) { + controller.enqueue(encoded.slice(0, 5)); + controller.enqueue(encoded.slice(5, 10)); + controller.enqueue(encoded.slice(10)); + controller.close(); + }, + }) as unknown as ReadableStream; + + const result = await service.saveUrlsFromStream(stream); + + expect(result).toBe(true); + expect(mockObjectStore.put).toHaveBeenCalledTimes(2); + }); + + it("returns false on error", async () => { + const error = new Error("IndexedDB error"); + mockOpenRequest.error = error; + (global.indexedDB.open as jest.Mock).mockImplementation(() => { + setTimeout(() => { + mockOpenRequest.onerror?.(); + }, 0); + return mockOpenRequest; + }); + + const stream = new NodeReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("https://test.com")); + controller.close(); + }, + }) as unknown as ReadableStream; + + const result = await service.saveUrlsFromStream(stream); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Stream save failed", + expect.any(Error), + ); + }); + }); + + describe("database initialization", () => { + it("creates object store with keyPath on upgrade", async () => { + mockDb.objectStoreNames.contains.mockReturnValue(false); + + (global.indexedDB.open as jest.Mock).mockImplementation(() => { + setTimeout(() => { + mockOpenRequest.onupgradeneeded?.({ target: mockOpenRequest }); + mockOpenRequest.onsuccess?.(); + }, 0); + return mockOpenRequest; + }); + + await service.hasUrl("https://test.com"); + + expect(mockDb.createObjectStore).toHaveBeenCalledWith("phishing-urls", { keyPath: "url" }); + }); + }); +}); 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 new file mode 100644 index 00000000000..099839a38d9 --- /dev/null +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts @@ -0,0 +1,241 @@ +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; + +/** + * Record type for phishing URL storage in IndexedDB. + */ +type PhishingUrlRecord = { url: string }; + +/** + * IndexedDB storage service for phishing URLs. + * Stores URLs as individual rows. + */ +export class PhishingIndexedDbService { + private readonly DB_NAME = "bitwarden-phishing"; + private readonly STORE_NAME = "phishing-urls"; + private readonly DB_VERSION = 1; + private readonly CHUNK_SIZE = 50000; + + constructor(private logService: LogService) {} + + /** + * Opens the IndexedDB database, creating the object store if needed. + */ + private openDatabase(): Promise { + return new Promise((resolve, reject) => { + const req = indexedDB.open(this.DB_NAME, this.DB_VERSION); + req.onerror = () => reject(req.error); + req.onsuccess = () => resolve(req.result); + req.onupgradeneeded = (e) => { + const db = (e.target as IDBOpenDBRequest).result; + if (!db.objectStoreNames.contains(this.STORE_NAME)) { + db.createObjectStore(this.STORE_NAME, { keyPath: "url" }); + } + }; + }); + } + + /** + * Clears all records from the phishing URLs store. + */ + private clearStore(db: IDBDatabase): Promise { + return new Promise((resolve, reject) => { + const req = db.transaction(this.STORE_NAME, "readwrite").objectStore(this.STORE_NAME).clear(); + req.onerror = () => reject(req.error); + req.onsuccess = () => resolve(); + }); + } + + /** + * Saves an array of phishing URLs to IndexedDB. + * Atomically replaces all existing data. + * + * @param urls - Array of phishing URLs to save + * @returns `true` if save succeeded, `false` on error + */ + async saveUrls(urls: string[]): Promise { + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + await this.clearStore(db); + await this.saveChunked(db, urls); + return true; + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Save failed", error); + return false; + } finally { + db?.close(); + } + } + + /** + * Saves URLs in chunks to prevent transaction timeouts and UI freezes. + */ + private async saveChunked(db: IDBDatabase, urls: string[]): Promise { + const cleaned = urls.map((u) => u.trim()).filter(Boolean); + for (let i = 0; i < cleaned.length; i += this.CHUNK_SIZE) { + await this.saveChunk(db, cleaned.slice(i, i + this.CHUNK_SIZE)); + await new Promise((r) => setTimeout(r, 0)); // Yield to event loop + } + } + + /** + * Saves a single chunk of URLs in one transaction. + */ + private saveChunk(db: IDBDatabase, urls: string[]): Promise { + return new Promise((resolve, reject) => { + const tx = db.transaction(this.STORE_NAME, "readwrite"); + const store = tx.objectStore(this.STORE_NAME); + for (const url of urls) { + store.put({ url } as PhishingUrlRecord); + } + tx.oncomplete = () => resolve(); + tx.onerror = () => reject(tx.error); + }); + } + + /** + * Checks if a URL exists in the phishing database. + * + * @param url - The URL to check + * @returns `true` if URL exists, `false` if not found or on error + */ + async hasUrl(url: string): Promise { + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + return await this.checkUrlExists(db, url); + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Check failed", error); + return false; + } finally { + db?.close(); + } + } + + /** + * Performs the actual URL existence check using index lookup. + */ + private checkUrlExists(db: IDBDatabase, url: string): Promise { + return new Promise((resolve, reject) => { + const tx = db.transaction(this.STORE_NAME, "readonly"); + const req = tx.objectStore(this.STORE_NAME).get(url); + req.onerror = () => reject(req.error); + req.onsuccess = () => resolve(req.result !== undefined); + }); + } + + /** + * Loads all phishing URLs from IndexedDB. + * + * @returns Array of all stored URLs, or empty array on error + */ + async loadAllUrls(): Promise { + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + return await this.getAllUrls(db); + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Load failed", error); + return []; + } finally { + db?.close(); + } + } + + /** + * Iterates all records using a cursor. + */ + private getAllUrls(db: IDBDatabase): Promise { + return new Promise((resolve, reject) => { + const urls: string[] = []; + const req = db + .transaction(this.STORE_NAME, "readonly") + .objectStore(this.STORE_NAME) + .openCursor(); + req.onerror = () => reject(req.error); + req.onsuccess = (e) => { + const cursor = (e.target as IDBRequest).result; + if (cursor) { + urls.push((cursor.value as PhishingUrlRecord).url); + cursor.continue(); + } else { + resolve(urls); + } + }; + }); + } + + /** + * Saves phishing URLs directly from a stream. + * Processes data incrementally to minimize memory usage. + * + * @param stream - ReadableStream of newline-delimited URLs + * @returns `true` if save succeeded, `false` on error + */ + async saveUrlsFromStream(stream: ReadableStream): Promise { + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + await this.clearStore(db); + await this.processStream(db, stream); + return true; + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Stream save failed", error); + return false; + } finally { + db?.close(); + } + } + + /** + * Processes a stream of URL data, parsing lines and saving in chunks. + */ + private async processStream(db: IDBDatabase, stream: ReadableStream): Promise { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let buffer = ""; + let urls: string[] = []; + + try { + while (true) { + const { done, value } = await reader.read(); + + // Decode BEFORE done check; stream: !done flushes on final call + buffer += decoder.decode(value, { stream: !done }); + + if (done) { + // Split remaining buffer by newlines in case it contains multiple URLs + const lines = buffer.split("\n"); + for (const line of lines) { + const trimmed = line.trim(); + if (trimmed) { + urls.push(trimmed); + } + } + if (urls.length > 0) { + await this.saveChunk(db, urls); + } + break; + } + + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + + for (const line of lines) { + const trimmed = line.trim(); + if (trimmed) { + urls.push(trimmed); + } + + if (urls.length >= this.CHUNK_SIZE) { + await this.saveChunk(db, urls); + urls = []; + await new Promise((r) => setTimeout(r, 0)); + } + } + } + } finally { + reader.releaseLock(); + } + } +} diff --git a/apps/browser/src/platform/browser/browser-popup-utils.spec.ts b/apps/browser/src/platform/browser/browser-popup-utils.spec.ts index 6e2175e3a79..cb04f30b589 100644 --- a/apps/browser/src/platform/browser/browser-popup-utils.spec.ts +++ b/apps/browser/src/platform/browser/browser-popup-utils.spec.ts @@ -1,7 +1,7 @@ import { createChromeTabMock } from "../../autofill/spec/autofill-mocks"; import { BrowserApi } from "./browser-api"; -import BrowserPopupUtils from "./browser-popup-utils"; +import BrowserPopupUtils, { PopupWidthOptions } from "./browser-popup-utils"; describe("BrowserPopupUtils", () => { afterEach(() => { @@ -152,7 +152,7 @@ describe("BrowserPopupUtils", () => { focused: false, alwaysOnTop: false, incognito: false, - width: 380, + width: PopupWidthOptions.default, }); jest.spyOn(BrowserApi, "createWindow").mockImplementation(); jest.spyOn(BrowserApi, "updateWindowProperties").mockImplementation(); @@ -168,7 +168,7 @@ describe("BrowserPopupUtils", () => { expect(BrowserApi.createWindow).toHaveBeenCalledWith({ type: "popup", focused: true, - width: 380, + width: PopupWidthOptions.default, height: 630, left: 85, top: 190, @@ -197,7 +197,7 @@ describe("BrowserPopupUtils", () => { expect(BrowserApi.createWindow).toHaveBeenCalledWith({ type: "popup", focused: true, - width: 380, + width: PopupWidthOptions.default, height: 630, left: 85, top: 190, @@ -214,7 +214,7 @@ describe("BrowserPopupUtils", () => { expect(BrowserApi.createWindow).toHaveBeenCalledWith({ type: "popup", focused: true, - width: 380, + width: PopupWidthOptions.default, height: 630, left: 85, top: 190, @@ -267,7 +267,7 @@ describe("BrowserPopupUtils", () => { expect(BrowserApi.createWindow).toHaveBeenCalledWith({ type: "popup", focused: true, - width: 380, + width: PopupWidthOptions.default, height: 630, left: 85, top: 190, @@ -290,7 +290,7 @@ describe("BrowserPopupUtils", () => { focused: false, alwaysOnTop: false, incognito: false, - width: 380, + width: PopupWidthOptions.default, state: "fullscreen", }); jest @@ -321,7 +321,7 @@ describe("BrowserPopupUtils", () => { focused: false, alwaysOnTop: false, incognito: false, - width: 380, + width: PopupWidthOptions.default, state: "fullscreen", }); diff --git a/apps/browser/src/platform/browser/browser-popup-utils.ts b/apps/browser/src/platform/browser/browser-popup-utils.ts index 8343799d0eb..c8dba57e708 100644 --- a/apps/browser/src/platform/browser/browser-popup-utils.ts +++ b/apps/browser/src/platform/browser/browser-popup-utils.ts @@ -10,9 +10,9 @@ import { BrowserApi } from "./browser-api"; * Value represents width in pixels */ export const PopupWidthOptions = Object.freeze({ - default: 380, - wide: 480, - "extra-wide": 600, + default: 480, + wide: 600, + narrow: 380, }); type PopupWidthOptions = typeof PopupWidthOptions; diff --git a/apps/browser/src/platform/popup/layout/popup-layout.stories.ts b/apps/browser/src/platform/popup/layout/popup-layout.stories.ts index c6ffe1a6414..2e088b8161e 100644 --- a/apps/browser/src/platform/popup/layout/popup-layout.stories.ts +++ b/apps/browser/src/platform/popup/layout/popup-layout.stories.ts @@ -44,7 +44,7 @@ import { PopupTabNavigationComponent } from "./popup-tab-navigation.component"; @Component({ selector: "extension-container", template: ` -
+
`, @@ -678,7 +678,7 @@ export const WidthOptions: Story = { template: /* HTML */ `
Default:
-
+
Wide:
diff --git a/apps/browser/src/platform/popup/layout/popup-page.component.html b/apps/browser/src/platform/popup/layout/popup-page.component.html index 828d9947373..bb24fb800aa 100644 --- a/apps/browser/src/platform/popup/layout/popup-page.component.html +++ b/apps/browser/src/platform/popup/layout/popup-page.component.html @@ -25,7 +25,6 @@
(false); @@ -33,10 +39,21 @@ export class PopupPageComponent { protected readonly scrolled = signal(false); isScrolled = this.scrolled.asReadonly(); + constructor() { + this.scrollLayout.scrollableRef$ + .pipe( + filter((ref): ref is ElementRef => ref != null), + switchMap((ref) => + fromEvent(ref.nativeElement, "scroll").pipe( + startWith(null), + map(() => ref.nativeElement.scrollTop !== 0), + ), + ), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe((isScrolled) => this.scrolled.set(isScrolled)); + } + /** Accessible loading label for the spinner. Defaults to "loading" */ readonly loadingText = input(this.i18nService.t("loading")); - - handleScroll(event: Event) { - this.scrolled.set((event.currentTarget as HTMLElement).scrollTop !== 0); - } } diff --git a/apps/browser/src/platform/popup/layout/popup-size.service.ts b/apps/browser/src/platform/popup/layout/popup-size.service.ts index 0e4aacb9a97..ff3f09d0d01 100644 --- a/apps/browser/src/platform/popup/layout/popup-size.service.ts +++ b/apps/browser/src/platform/popup/layout/popup-size.service.ts @@ -83,7 +83,7 @@ export class PopupSizeService { } const pxWidth = PopupWidthOptions[width] ?? PopupWidthOptions.default; - document.body.style.minWidth = `${pxWidth}px`; + document.body.style.width = `${pxWidth}px`; } /** diff --git a/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts b/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts index 8fdae06e28a..66c9f655b05 100644 --- a/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts +++ b/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts @@ -10,6 +10,7 @@ import { import { of } from "rxjs"; import { LockIcon, RegistrationCheckEmailIcon } from "@bitwarden/assets/svg"; +import { PopupWidthOptions } from "@bitwarden/browser/platform/browser/browser-popup-utils"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; @@ -243,7 +244,12 @@ export const DefaultContentExample: Story = { }), parameters: { chromatic: { - viewports: [380, 1280], + viewports: [ + PopupWidthOptions.default, + PopupWidthOptions.narrow, + PopupWidthOptions.wide, + 1280, + ], }, }, }; diff --git a/apps/browser/src/popup/scss/tailwind.css b/apps/browser/src/popup/scss/tailwind.css index f58950cc86a..0ef7b82bfed 100644 --- a/apps/browser/src/popup/scss/tailwind.css +++ b/apps/browser/src/popup/scss/tailwind.css @@ -60,7 +60,7 @@ } body { - width: 380px; + width: 480px; height: 100%; position: relative; min-height: inherit; @@ -84,9 +84,9 @@ animation: redraw 1s linear infinite; } - /** + /** * Text selection style: - * suppress user selection for most elements (to make it more app-like) + * suppress user selection for most elements (to make it more app-like) */ h1, h2, @@ -165,7 +165,7 @@ @apply tw-text-muted; } - /** + /** * Text selection style: * Set explicit selection styles (assumes primary accent color has sufficient * contrast against the background, so its inversion is also still readable) diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 06a021085ea..7b207f0fac1 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -27,8 +27,12 @@ import { WINDOW, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; -import { AUTOFILL_NUDGE_SERVICE } from "@bitwarden/angular/vault"; -import { SingleNudgeService } from "@bitwarden/angular/vault/services/default-single-nudge.service"; +import { + AUTOFILL_NUDGE_SERVICE, + AUTO_CONFIRM_NUDGE_SERVICE, + AutoConfirmNudgeService, +} from "@bitwarden/angular/vault"; +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; import { LoginComponentService, TwoFactorAuthComponentService, @@ -786,9 +790,14 @@ const safeProviders: SafeProvider[] = [ ], }), safeProvider({ - provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken, + provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken, useClass: BrowserAutofillNudgeService, - deps: [], + deps: [StateProvider, VaultProfileService, LogService], + }), + safeProvider({ + provide: AUTO_CONFIRM_NUDGE_SERVICE as SafeInjectionToken, + useClass: AutoConfirmNudgeService, + deps: [StateProvider, AutomaticUserConfirmationService], }), ]; diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts index a28b8730109..93cc2cf248a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts @@ -91,10 +91,18 @@ describe("AutofillConfirmationDialogComponent", () => { jest.resetAllMocks(); }); - const findShowAll = (inFx?: ComponentFixture) => - (inFx || fixture).nativeElement.querySelector( - "button.tw-text-sm.tw-font-medium.tw-cursor-pointer", - ) as HTMLButtonElement | null; + const findShowAll = (inFx?: ComponentFixture) => { + // Find the button by its text content (showAll or showLess) + const buttons = Array.from( + (inFx || fixture).nativeElement.querySelectorAll("button"), + ) as HTMLButtonElement[]; + return ( + buttons.find((btn) => { + const text = btn.textContent?.trim() || ""; + return text === "showAll" || text === "showLess"; + }) || null + ); + }; it("normalizes currentUrl and savedUrls via Utils.getHostname", () => { expect(Utils.getHostname).toHaveBeenCalledTimes(1 + (params.savedUrls?.length ?? 0)); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts index e6dffdaff08..2c94d9c226b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts @@ -1,4 +1,3 @@ -import { CdkVirtualScrollableElement } from "@angular/cdk/scrolling"; import { ChangeDetectionStrategy, Component, input, NO_ERRORS_SCHEMA } from "@angular/core"; import { TestBed, fakeAsync, flush, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; @@ -394,21 +393,28 @@ describe("VaultV2Component", () => { expect(values[values.length - 1]).toBe(false); }); - it("ngAfterViewInit waits for allFilters$ then starts scroll position service", fakeAsync(() => { + it("passes popup-page scroll region element to scroll position service", fakeAsync(() => { + const fixture = TestBed.createComponent(VaultV2Component); + const component = fixture.componentInstance; + + const readySubject$ = component["readySubject"] as unknown as BehaviorSubject; + const itemsLoading$ = itemsSvc.loading$ as unknown as BehaviorSubject; const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; - (component as any).virtualScrollElement = {} as CdkVirtualScrollableElement; - - component.ngAfterViewInit(); - expect(scrollSvc.start).not.toHaveBeenCalled(); - - allFilters$.next({ any: true }); + fixture.detectChanges(); tick(); - expect(scrollSvc.start).toHaveBeenCalledTimes(1); - expect(scrollSvc.start).toHaveBeenCalledWith((component as any).virtualScrollElement); + const scrollRegion = fixture.nativeElement.querySelector( + '[data-testid="popup-layout-scroll-region"]', + ) as HTMLElement; - flush(); + // Unblock loading + itemsLoading$.next(false); + readySubject$.next(true); + allFilters$.next({}); + tick(); + + expect(scrollSvc.start).toHaveBeenCalledWith(scrollRegion); })); it("showPremiumDialog opens PremiumUpgradeDialogComponent", () => { diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 761b366bcd2..4678e2733eb 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -1,7 +1,7 @@ import { LiveAnnouncer } from "@angular/cdk/a11y"; -import { CdkVirtualScrollableElement, ScrollingModule } from "@angular/cdk/scrolling"; +import { ScrollingModule } from "@angular/cdk/scrolling"; import { CommonModule } from "@angular/common"; -import { AfterViewInit, Component, DestroyRef, OnDestroy, OnInit, ViewChild } from "@angular/core"; +import { Component, DestroyRef, effect, inject, OnDestroy, OnInit } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { Router, RouterModule } from "@angular/router"; import { @@ -47,6 +47,7 @@ import { ButtonModule, DialogService, NoItemsModule, + ScrollLayoutService, ToastService, TypographyModule, } from "@bitwarden/components"; @@ -119,11 +120,7 @@ type VaultState = UnionOfValues; ], providers: [{ provide: VaultItemsTransferService, useClass: DefaultVaultItemsTransferService }], }) -export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @ViewChild(CdkVirtualScrollableElement) virtualScrollElement?: CdkVirtualScrollableElement; - +export class VaultV2Component implements OnInit, OnDestroy { NudgeType = NudgeType; cipherType = CipherType; private activeUserId$ = this.accountService.activeAccount$.pipe(getUserId); @@ -308,16 +305,21 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { }); } - ngAfterViewInit(): void { - if (this.virtualScrollElement) { - // The filters component can cause the size of the virtual scroll element to change, - // which can cause the scroll position to be land in the wrong spot. To fix this, - // wait until all filters are populated before restoring the scroll position. - this.allFilters$.pipe(take(1), takeUntilDestroyed(this.destroyRef)).subscribe(() => { - this.vaultScrollPositionService.start(this.virtualScrollElement!); + private readonly scrollLayout = inject(ScrollLayoutService); + + private readonly _scrollPositionEffect = effect((onCleanup) => { + const sub = combineLatest([this.scrollLayout.scrollableRef$, this.allFilters$, this.loading$]) + .pipe( + filter(([ref, _filters, loading]) => !!ref && !loading), + take(1), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe(([ref]) => { + this.vaultScrollPositionService.start(ref!.nativeElement); }); - } - } + + onCleanup(() => sub.unsubscribe()); + }); async ngOnInit() { this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); diff --git a/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts index 562375f8f85..af21f664f2d 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts @@ -1,4 +1,3 @@ -import { CdkVirtualScrollableElement } from "@angular/cdk/scrolling"; import { fakeAsync, TestBed, tick } from "@angular/core/testing"; import { NavigationEnd, Router } from "@angular/router"; import { Subject, Subscription } from "rxjs"; @@ -66,21 +65,18 @@ describe("VaultPopupScrollPositionService", () => { }); describe("start", () => { - const elementScrolled$ = new Subject(); - const focus = jest.fn(); - const nativeElement = { - scrollTop: 0, - querySelector: jest.fn(() => ({ focus })), - addEventListener: jest.fn(), - style: { - visibility: "", - }, - }; - const virtualElement = { - elementScrolled: () => elementScrolled$, - getElementRef: () => ({ nativeElement }), - scrollTo: jest.fn(), - } as unknown as CdkVirtualScrollableElement; + let scrollElement: HTMLElement; + + beforeEach(() => { + scrollElement = document.createElement("div"); + + (scrollElement as any).scrollTo = jest.fn(function scrollTo(opts: { top?: number }) { + if (opts?.top != null) { + (scrollElement as any).scrollTop = opts.top; + } + }); + (scrollElement as any).scrollTop = 0; + }); afterEach(() => { // remove the actual subscription created by `.subscribe` @@ -89,47 +85,55 @@ describe("VaultPopupScrollPositionService", () => { describe("initial scroll position", () => { beforeEach(() => { - (virtualElement.scrollTo as jest.Mock).mockClear(); - nativeElement.querySelector.mockClear(); + ((scrollElement as any).scrollTo as jest.Mock).mockClear(); }); it("does not scroll when `scrollPosition` is null", () => { service["scrollPosition"] = null; - service.start(virtualElement); + service.start(scrollElement); - expect(virtualElement.scrollTo).not.toHaveBeenCalled(); + expect((scrollElement as any).scrollTo).not.toHaveBeenCalled(); }); - it("scrolls the virtual element to `scrollPosition`", fakeAsync(() => { + it("scrolls the element to `scrollPosition` (async via setTimeout)", fakeAsync(() => { service["scrollPosition"] = 500; - nativeElement.scrollTop = 500; - service.start(virtualElement); + service.start(scrollElement); tick(); - expect(virtualElement.scrollTo).toHaveBeenCalledWith({ behavior: "instant", top: 500 }); + expect((scrollElement as any).scrollTo).toHaveBeenCalledWith({ + behavior: "instant", + top: 500, + }); + expect((scrollElement as any).scrollTop).toBe(500); })); }); describe("scroll listener", () => { it("unsubscribes from any existing subscription", () => { - service.start(virtualElement); + service.start(scrollElement); expect(unsubscribe).toHaveBeenCalled(); }); - it("subscribes to `elementScrolled`", fakeAsync(() => { - virtualElement.measureScrollOffset = jest.fn(() => 455); + it("stores scrollTop on subsequent scroll events (skips first)", fakeAsync(() => { + service["scrollPosition"] = null; - service.start(virtualElement); + service.start(scrollElement); - elementScrolled$.next(null); // first subscription is skipped by `skip(1)` - elementScrolled$.next(null); + // First scroll event is intentionally ignored (equivalent to old skip(1)). + (scrollElement as any).scrollTop = 111; + scrollElement.dispatchEvent(new Event("scroll")); + tick(); + + expect(service["scrollPosition"]).toBeNull(); + + // Second scroll event should persist. + (scrollElement as any).scrollTop = 455; + scrollElement.dispatchEvent(new Event("scroll")); tick(); - expect(virtualElement.measureScrollOffset).toHaveBeenCalledTimes(1); - expect(virtualElement.measureScrollOffset).toHaveBeenCalledWith("top"); expect(service["scrollPosition"]).toBe(455); })); }); diff --git a/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts index 5bfe0ec9331..7261fdd6633 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts @@ -1,8 +1,7 @@ -import { CdkVirtualScrollableElement } from "@angular/cdk/scrolling"; import { inject, Injectable } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router } from "@angular/router"; -import { filter, skip, Subscription } from "rxjs"; +import { filter, fromEvent, Subscription } from "rxjs"; @Injectable({ providedIn: "root", @@ -31,24 +30,25 @@ export class VaultPopupScrollPositionService { } /** Scrolls the user to the stored scroll position and starts tracking scroll of the page. */ - start(virtualScrollElement: CdkVirtualScrollableElement) { + start(scrollElement: HTMLElement) { if (this.hasScrollPosition()) { // Use `setTimeout` to scroll after rendering is complete setTimeout(() => { - virtualScrollElement.scrollTo({ top: this.scrollPosition!, behavior: "instant" }); + scrollElement.scrollTo({ top: this.scrollPosition!, behavior: "instant" }); }); } this.scrollSubscription?.unsubscribe(); // Skip the first scroll event to avoid settings the scroll from the above `scrollTo` call - this.scrollSubscription = virtualScrollElement - ?.elementScrolled() - .pipe(skip(1)) - .subscribe(() => { - const offset = virtualScrollElement.measureScrollOffset("top"); - this.scrollPosition = offset; - }); + let skipped = false; + this.scrollSubscription = fromEvent(scrollElement, "scroll").subscribe(() => { + if (!skipped) { + skipped = true; + return; + } + this.scrollPosition = scrollElement.scrollTop; + }); } /** Stops the scroll listener from updating the stored location. */ diff --git a/apps/browser/src/vault/popup/settings/appearance-v2.component.ts b/apps/browser/src/vault/popup/settings/appearance-v2.component.ts index e6515ae7461..e02ccf25f3e 100644 --- a/apps/browser/src/vault/popup/settings/appearance-v2.component.ts +++ b/apps/browser/src/vault/popup/settings/appearance-v2.component.ts @@ -79,7 +79,7 @@ export class AppearanceV2Component implements OnInit { protected readonly widthOptions: Option[] = [ { label: this.i18nService.t("default"), value: "default" }, { label: this.i18nService.t("wide"), value: "wide" }, - { label: this.i18nService.t("extraWide"), value: "extra-wide" }, + { label: this.i18nService.t("narrow"), value: "narrow" }, ]; constructor( diff --git a/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts b/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts index bad6011b2d8..edebdab062f 100644 --- a/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts +++ b/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts @@ -115,15 +115,22 @@ export class TrashListItemsContainerComponent { } async restore(cipher: PopupCipherViewLike) { + let toastMessage; try { const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); await this.cipherService.restoreWithServer(cipher.id as string, activeUserId); + if (cipher.archivedDate) { + toastMessage = this.i18nService.t("archivedItemRestored"); + } else { + toastMessage = this.i18nService.t("restoredItem"); + } + await this.router.navigate(["/trash"]); this.toastService.showToast({ variant: "success", title: null, - message: this.i18nService.t("restoredItem"), + message: toastMessage, }); } catch (e) { this.logService.error(e); diff --git a/apps/desktop/desktop_native/autotype/Cargo.toml b/apps/desktop/desktop_native/autotype/Cargo.toml index a9c826af57d..59dd36c6c91 100644 --- a/apps/desktop/desktop_native/autotype/Cargo.toml +++ b/apps/desktop/desktop_native/autotype/Cargo.toml @@ -19,5 +19,14 @@ windows-core = { workspace = true } [dependencies] anyhow = { workspace = true } +[target.'cfg(windows)'.dev-dependencies] +windows = { workspace = true, features = [ + "Win32_UI_Input_KeyboardAndMouse", + "Win32_UI_WindowsAndMessaging", + "Win32_Foundation", + "Win32_System_LibraryLoader", + "Win32_Graphics_Gdi", +] } + [lints] workspace = true diff --git a/apps/desktop/desktop_native/autotype/tests/integration_tests.rs b/apps/desktop/desktop_native/autotype/tests/integration_tests.rs new file mode 100644 index 00000000000..b87219f77fe --- /dev/null +++ b/apps/desktop/desktop_native/autotype/tests/integration_tests.rs @@ -0,0 +1,324 @@ +#![cfg(target_os = "windows")] + +use std::{ + sync::{Arc, Mutex}, + thread, + time::Duration, +}; + +use autotype::{get_foreground_window_title, type_input}; +use serial_test::serial; +use tracing::debug; +use windows::Win32::{ + Foundation::{COLORREF, HINSTANCE, HMODULE, HWND, LPARAM, LRESULT, WPARAM}, + Graphics::Gdi::{CreateSolidBrush, UpdateWindow, ValidateRect, COLOR_WINDOW}, + System::LibraryLoader::{GetModuleHandleA, GetModuleHandleW}, + UI::WindowsAndMessaging::*, +}; +use windows_core::{s, w, Result, PCSTR, PCWSTR}; + +struct TestWindow { + handle: HWND, + capture: Option, +} + +impl Drop for TestWindow { + fn drop(&mut self) { + // Clean up the InputCapture pointer + unsafe { + let capture_ptr = GetWindowLongPtrW(self.handle, GWLP_USERDATA) as *mut InputCapture; + if !capture_ptr.is_null() { + let _ = Box::from_raw(capture_ptr); + } + CloseWindow(self.handle).expect("window handle should be closeable"); + DestroyWindow(self.handle).expect("window handle should be destroyable"); + } + } +} + +// state to capture keyboard input +#[derive(Clone)] +struct InputCapture { + chars: Arc>>, +} + +impl InputCapture { + fn new() -> Self { + Self { + chars: Arc::new(Mutex::new(Vec::new())), + } + } + + fn get_chars(&self) -> Vec { + self.chars + .lock() + .expect("mutex should not be poisoned") + .clone() + } +} + +// Custom window procedure that captures input +unsafe extern "system" fn capture_input_proc( + handle: HWND, + msg: u32, + wparam: WPARAM, + lparam: LPARAM, +) -> LRESULT { + match msg { + WM_CREATE => { + // Store the InputCapture pointer in window data + let create_struct = lparam.0 as *const CREATESTRUCTW; + let capture_ptr = (*create_struct).lpCreateParams as *mut InputCapture; + SetWindowLongPtrW(handle, GWLP_USERDATA, capture_ptr as isize); + LRESULT(0) + } + WM_CHAR => { + // Get the InputCapture from window data + let capture_ptr = GetWindowLongPtrW(handle, GWLP_USERDATA) as *mut InputCapture; + if !capture_ptr.is_null() { + let capture = &*capture_ptr; + if let Some(ch) = char::from_u32(wparam.0 as u32) { + capture + .chars + .lock() + .expect("mutex should not be poisoned") + .push(ch); + } + } + LRESULT(0) + } + WM_DESTROY => { + PostQuitMessage(0); + LRESULT(0) + } + _ => DefWindowProcW(handle, msg, wparam, lparam), + } +} + +// A pointer to the window procedure +type ProcType = unsafe extern "system" fn(HWND, u32, WPARAM, LPARAM) -> LRESULT; + +// +extern "system" fn show_window_proc( + handle: HWND, // the window handle + message: u32, // the system message + wparam: WPARAM, /* additional message information. The contents of the wParam parameter + * depend on the value of the message parameter. */ + lparam: LPARAM, /* additional message information. The contents of the lParam parameter + * depend on the value of the message parameter. */ +) -> LRESULT { + unsafe { + match message { + WM_PAINT => { + debug!("WM_PAINT"); + let res = ValidateRect(Some(handle), None); + debug_assert!(res.ok().is_ok()); + LRESULT(0) + } + WM_DESTROY => { + debug!("WM_DESTROY"); + PostQuitMessage(0); + LRESULT(0) + } + _ => DefWindowProcA(handle, message, wparam, lparam), + } + } +} + +impl TestWindow { + fn set_foreground(&self) -> Result<()> { + unsafe { + let _ = ShowWindow(self.handle, SW_SHOW); + let _ = SetForegroundWindow(self.handle); + let _ = UpdateWindow(self.handle); + let _ = SetForegroundWindow(self.handle); + } + std::thread::sleep(std::time::Duration::from_millis(100)); + Ok(()) + } + + fn wait_for_input(&self, timeout_ms: u64) { + let start = std::time::Instant::now(); + while start.elapsed().as_millis() < timeout_ms as u128 { + process_messages(); + thread::sleep(Duration::from_millis(10)); + } + } +} + +fn process_messages() { + unsafe { + let mut msg = MSG::default(); + while PeekMessageW(&mut msg, None, 0, 0, PM_REMOVE).as_bool() { + let _ = TranslateMessage(&msg); + DispatchMessageW(&msg); + } + } +} + +fn create_input_window(title: PCWSTR, proc_type: ProcType) -> Result { + unsafe { + let instance = GetModuleHandleW(None).unwrap_or(HMODULE(std::ptr::null_mut())); + let instance: HINSTANCE = instance.into(); + debug_assert!(!instance.is_invalid()); + + let window_class = w!("show_window"); + + // Register window class with our custom proc + let wc = WNDCLASSW { + lpfnWndProc: Some(proc_type), + hInstance: instance, + lpszClassName: window_class, + hbrBackground: CreateSolidBrush(COLORREF( + (COLOR_WINDOW.0 + 1).try_into().expect("i32 to fit in u32"), + )), + ..Default::default() + }; + + let _atom = RegisterClassW(&wc); + + let capture = InputCapture::new(); + + // Pass InputCapture as lpParam + let capture_ptr = Box::into_raw(Box::new(capture.clone())); + + // Create window + // + let handle = CreateWindowExW( + WINDOW_EX_STYLE(0), + window_class, + title, + WS_OVERLAPPEDWINDOW | WS_VISIBLE, + CW_USEDEFAULT, + CW_USEDEFAULT, + 400, + 300, + None, + None, + Some(instance), + Some(capture_ptr as *const _), + ) + .expect("window should be created"); + + // Process pending messages + process_messages(); + thread::sleep(Duration::from_millis(100)); + + Ok(TestWindow { + handle, + capture: Some(capture), + }) + } +} + +fn create_title_window(title: PCSTR, proc_type: ProcType) -> Result { + unsafe { + let instance = GetModuleHandleA(None)?; + let instance: HINSTANCE = instance.into(); + debug_assert!(!instance.is_invalid()); + + let window_class = s!("input_window"); + + // Register window class with our custom proc + // + let wc = WNDCLASSA { + hCursor: LoadCursorW(None, IDC_ARROW)?, + hInstance: instance, + lpszClassName: window_class, + style: CS_HREDRAW | CS_VREDRAW, + lpfnWndProc: Some(proc_type), + ..Default::default() + }; + + let _atom = RegisterClassA(&wc); + + // Create window + // + let handle = CreateWindowExA( + WINDOW_EX_STYLE::default(), + window_class, + title, + WS_OVERLAPPEDWINDOW | WS_VISIBLE, + CW_USEDEFAULT, + CW_USEDEFAULT, + 800, + 600, + None, + None, + Some(instance), + None, + ) + .expect("window should be created"); + + Ok(TestWindow { + handle, + capture: None, + }) + } +} + +#[serial] +#[test] +fn test_get_active_window_title_success() { + let title; + { + let window = create_title_window(s!("TITLE_FOOBAR"), show_window_proc).unwrap(); + window.set_foreground().unwrap(); + title = get_foreground_window_title().unwrap(); + } + + assert_eq!(title, "TITLE_FOOBAR\0".to_owned()); + + thread::sleep(Duration::from_millis(100)); +} + +#[serial] +#[test] +fn test_get_active_window_title_doesnt_fail_if_empty_title() { + let title; + { + let window = create_title_window(s!(""), show_window_proc).unwrap(); + window.set_foreground().unwrap(); + title = get_foreground_window_title(); + } + + assert_eq!(title.unwrap(), "".to_owned()); + + thread::sleep(Duration::from_millis(100)); +} + +#[serial] +#[test] +fn test_type_input_success() { + const TAB: u16 = 0x09; + let chars; + { + let window = create_input_window(w!("foo"), capture_input_proc).unwrap(); + window.set_foreground().unwrap(); + + type_input( + &[ + 0x66, 0x6F, 0x6C, 0x6C, 0x6F, 0x77, 0x5F, 0x74, 0x68, 0x65, TAB, 0x77, 0x68, 0x69, + 0x74, 0x65, 0x5F, 0x72, 0x61, 0x62, 0x62, 0x69, 0x74, + ], + &["Control".to_owned(), "Alt".to_owned(), "B".to_owned()], + ) + .unwrap(); + + // Wait for and process input messages + window.wait_for_input(250); + + // Verify captured input + let capture = window.capture.as_ref().unwrap(); + chars = capture.get_chars(); + } + + assert!(!chars.is_empty(), "No input captured"); + + let input_str = String::from_iter(chars.iter()); + let input_str = input_str.replace("\t", "_"); + + assert_eq!(input_str, "follow_the_white_rabbit"); + + thread::sleep(Duration::from_millis(100)); +} diff --git a/apps/desktop/electron-builder.json b/apps/desktop/electron-builder.json index 8a1e914b2fa..93e47225bd1 100644 --- a/apps/desktop/electron-builder.json +++ b/apps/desktop/electron-builder.json @@ -86,7 +86,8 @@ "signIgnore": [ "MacOS/desktop_proxy", "MacOS/desktop_proxy.inherit", - "Contents/Plugins/autofill-extension.appex" + "Contents/Plugins/autofill-extension.appex", + "Frameworks/Electron Framework.framework/(Electron Framework|Libraries|Resources|Versions/Current)/.*" ], "target": ["dmg", "zip"] }, diff --git a/apps/desktop/macos/autofill-extension/autofill_extension_enabled.entitlements b/apps/desktop/macos/autofill-extension/autofill_extension_enabled.entitlements new file mode 100644 index 00000000000..49fda8f8af8 --- /dev/null +++ b/apps/desktop/macos/autofill-extension/autofill_extension_enabled.entitlements @@ -0,0 +1,14 @@ + + + + + com.apple.security.app-sandbox + + com.apple.security.application-groups + + LTZ2PFU5D6.com.bitwarden.desktop + + com.apple.developer.authentication-services.autofill-credential-provider + + + diff --git a/apps/desktop/macos/desktop.xcodeproj/project.pbxproj b/apps/desktop/macos/desktop.xcodeproj/project.pbxproj index ed19fc9ef5d..c3cb34b6bea 100644 --- a/apps/desktop/macos/desktop.xcodeproj/project.pbxproj +++ b/apps/desktop/macos/desktop.xcodeproj/project.pbxproj @@ -256,7 +256,7 @@ isa = XCBuildConfiguration; baseConfigurationReference = D83832AD2D67B9D0003FB9F8 /* ReleaseDeveloper.xcconfig */; buildSettings = { - CODE_SIGN_ENTITLEMENTS = "autofill-extension/autofill_extension.entitlements"; + CODE_SIGN_ENTITLEMENTS = "autofill-extension/autofill_extension_enabled.entitlements"; CODE_SIGN_IDENTITY = "Apple Development"; "CODE_SIGN_IDENTITY[sdk=macosx*]" = "Mac Developer"; CODE_SIGN_STYLE = Manual; @@ -409,7 +409,7 @@ isa = XCBuildConfiguration; baseConfigurationReference = D83832AB2D67B9AE003FB9F8 /* Debug.xcconfig */; buildSettings = { - CODE_SIGN_ENTITLEMENTS = "autofill-extension/autofill_extension.entitlements"; + CODE_SIGN_ENTITLEMENTS = "autofill-extension/autofill_extension_enabled.entitlements"; CODE_SIGN_IDENTITY = "Apple Development"; "CODE_SIGN_IDENTITY[sdk=macosx*]" = "Mac Developer"; CODE_SIGN_STYLE = Manual; diff --git a/apps/desktop/macos/desktop.xcodeproj/xcshareddata/xcschemes/autofill-extension.xcscheme b/apps/desktop/macos/desktop.xcodeproj/xcshareddata/xcschemes/autofill-extension.xcscheme new file mode 100644 index 00000000000..18357be4570 --- /dev/null +++ b/apps/desktop/macos/desktop.xcodeproj/xcshareddata/xcschemes/autofill-extension.xcscheme @@ -0,0 +1,71 @@ + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/apps/desktop/package.json b/apps/desktop/package.json index d3b698cbeb3..b8f28aadfb3 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -46,7 +46,7 @@ "pack:mac:with-extension": "npm run clean:dist && npm run build:macos-extension:mac && electron-builder --mac --universal -p never", "pack:mac:arm64": "npm run clean:dist && electron-builder --mac --arm64 -p never", "pack:mac:mas": "npm run clean:dist && npm run build:macos-extension:mas && electron-builder --mac mas --universal -p never -c.mac.identity=null", - "pack:mac:masdev": "npm run clean:dist && npm run build:macos-extension:masdev && electron-builder --mac mas-dev --universal -p never", + "pack:mac:masdev": "npm run clean:dist && electron-builder --mac mas-dev --universal -p never -c.mac.identity=null -c.mas.identity=$CSC_NAME -c.mas.provisioningProfile=bitwarden_desktop_developer_id.provisionprofile -c.mas.entitlements=resources/entitlements.mas.autofill-enabled.plist", "pack:local:mac": "npm run clean:dist && npm run build:macos-extension:masdev && electron-builder --mac mas-dev --universal -p never -c.mac.provisioningProfile=\"\" -c.mas.provisioningProfile=\"\"", "pack:win": "npm run clean:dist && electron-builder --win --x64 --arm64 --ia32 -p never", "pack:win:beta": "npm run clean:dist && electron-builder --config electron-builder.beta.json --win --x64 --arm64 --ia32 -p never", diff --git a/apps/desktop/resources/entitlements.mas.autofill-enabled.plist b/apps/desktop/resources/entitlements.mas.autofill-enabled.plist new file mode 100644 index 00000000000..f25780e5c12 --- /dev/null +++ b/apps/desktop/resources/entitlements.mas.autofill-enabled.plist @@ -0,0 +1,42 @@ + + + + + com.apple.application-identifier + LTZ2PFU5D6.com.bitwarden.desktop + com.apple.developer.team-identifier + LTZ2PFU5D6 + com.apple.security.app-sandbox + + com.apple.security.application-groups + + LTZ2PFU5D6.com.bitwarden.desktop + + com.apple.security.cs.allow-jit + + com.apple.security.device.usb + + com.apple.security.files.user-selected.read-write + + com.apple.security.network.client + + com.apple.security.temporary-exception.files.home-relative-path.read-write + + /Library/Application Support/Mozilla/NativeMessagingHosts/ + /Library/Application Support/Google/Chrome/NativeMessagingHosts/ + /Library/Application Support/Google/Chrome Beta/NativeMessagingHosts/ + /Library/Application Support/Google/Chrome Dev/NativeMessagingHosts/ + /Library/Application Support/Google/Chrome Canary/NativeMessagingHosts/ + /Library/Application Support/Chromium/NativeMessagingHosts/ + /Library/Application Support/Microsoft Edge/NativeMessagingHosts/ + /Library/Application Support/Microsoft Edge Beta/NativeMessagingHosts/ + /Library/Application Support/Microsoft Edge Dev/NativeMessagingHosts/ + /Library/Application Support/Microsoft Edge Canary/NativeMessagingHosts/ + /Library/Application Support/Vivaldi/NativeMessagingHosts/ + /Library/Application Support/Zen/NativeMessagingHosts/ + /Library/Application Support/net.imput.helium + + com.apple.developer.authentication-services.autofill-credential-provider + + + diff --git a/apps/desktop/resources/entitlements.mas.plist b/apps/desktop/resources/entitlements.mas.plist index 226e9827e37..9760af69e8b 100644 --- a/apps/desktop/resources/entitlements.mas.plist +++ b/apps/desktop/resources/entitlements.mas.plist @@ -34,7 +34,7 @@ /Library/Application Support/Microsoft Edge Canary/NativeMessagingHosts/ /Library/Application Support/Vivaldi/NativeMessagingHosts/ /Library/Application Support/Zen/NativeMessagingHosts/ - /Library/Application Support/net.imput.helium + /Library/Application Support/net.imput.helium/NativeMessagingHosts/ diff --git a/apps/desktop/scripts/after-sign.js b/apps/desktop/scripts/after-sign.js index 4275ec7d051..0e0e22fc24a 100644 --- a/apps/desktop/scripts/after-sign.js +++ b/apps/desktop/scripts/after-sign.js @@ -16,7 +16,9 @@ async function run(context) { const appPath = `${context.appOutDir}/${appName}.app`; const macBuild = context.electronPlatformName === "darwin"; const copySafariExtension = ["darwin", "mas"].includes(context.electronPlatformName); - const copyAutofillExtension = ["darwin"].includes(context.electronPlatformName); // Disabled for mas builds + const isMasDevBuild = + context.electronPlatformName === "mas" && context.targets.at(0)?.name === "mas-dev"; + const copyAutofillExtension = ["darwin"].includes(context.electronPlatformName) || isMasDevBuild; let shouldResign = false; @@ -31,7 +33,6 @@ async function run(context) { fse.mkdirSync(path.join(appPath, "Contents/PlugIns")); } fse.copySync(extensionPath, path.join(appPath, "Contents/PlugIns/autofill-extension.appex")); - shouldResign = true; } } diff --git a/apps/desktop/src/app/layout/desktop-layout.component.html b/apps/desktop/src/app/layout/desktop-layout.component.html index cb969f573fc..f9921ac11ef 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.html +++ b/apps/desktop/src/app/layout/desktop-layout.component.html @@ -1,4 +1,4 @@ - + diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index a5a91c52e7e..66613efd115 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -89,6 +89,7 @@ import { PlatformUtilsService, PlatformUtilsService as PlatformUtilsServiceAbstraction, } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { RegisterSdkService } from "@bitwarden/common/platform/abstractions/sdk/register-sdk.service"; import { SdkClientFactory } from "@bitwarden/common/platform/abstractions/sdk/sdk-client-factory"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; @@ -432,6 +433,7 @@ const safeProviders: SafeProvider[] = [ InternalUserDecryptionOptionsServiceAbstraction, MessagingServiceAbstraction, AccountCryptographicStateService, + RegisterSdkService, ], }), safeProvider({ diff --git a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts index 6b29a464e2c..9bb7d5077cf 100644 --- a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts +++ b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts @@ -1,8 +1,10 @@ -import { MockProxy, mock } from "jest-mock-extended"; +import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, of } from "rxjs"; import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; +import { DefaultSetInitialPasswordService } from "@bitwarden/angular/auth/password-management/set-initial-password/default-set-initial-password.service.implementation"; import { + InitializeJitPasswordCredentials, SetInitialPasswordCredentials, SetInitialPasswordService, SetInitialPasswordUserType, @@ -19,12 +21,14 @@ import { AccountCryptographicStateService } from "@bitwarden/common/key-manageme import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { MasterPasswordSalt } from "@bitwarden/common/key-management/master-password/types/master-password.types"; import { KeysRequest } from "@bitwarden/common/models/request/keys.request"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { RegisterSdkService } from "@bitwarden/common/platform/abstractions/sdk/register-sdk.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CsprngArray } from "@bitwarden/common/types/csprng"; -import { UserId } from "@bitwarden/common/types/guid"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { DEFAULT_KDF_CONFIG, KdfConfigService, KeyService } from "@bitwarden/key-management"; @@ -45,6 +49,7 @@ describe("DesktopSetInitialPasswordService", () => { let userDecryptionOptionsService: MockProxy; let messagingService: MockProxy; let accountCryptographicStateService: MockProxy; + let registerSdkService: MockProxy; beforeEach(() => { apiService = mock(); @@ -59,6 +64,7 @@ describe("DesktopSetInitialPasswordService", () => { userDecryptionOptionsService = mock(); messagingService = mock(); accountCryptographicStateService = mock(); + registerSdkService = mock(); sut = new DesktopSetInitialPasswordService( apiService, @@ -73,6 +79,7 @@ describe("DesktopSetInitialPasswordService", () => { userDecryptionOptionsService, messagingService, accountCryptographicStateService, + registerSdkService, ); }); @@ -179,4 +186,36 @@ describe("DesktopSetInitialPasswordService", () => { }); }); }); + + describe("initializePasswordJitPasswordUserV2Encryption(...)", () => { + it("should send a 'redrawMenu' message", async () => { + // Arrange + const credentials: InitializeJitPasswordCredentials = { + newPasswordHint: "newPasswordHint", + orgSsoIdentifier: "orgSsoIdentifier", + orgId: "orgId" as OrganizationId, + resetPasswordAutoEnroll: false, + newPassword: "newPassword123!", + salt: "user@example.com" as MasterPasswordSalt, + }; + const userId = "userId" as UserId; + + const superSpy = jest + .spyOn( + DefaultSetInitialPasswordService.prototype, + "initializePasswordJitPasswordUserV2Encryption", + ) + .mockResolvedValue(undefined); + + // Act + await sut.initializePasswordJitPasswordUserV2Encryption(credentials, userId); + + // Assert + expect(superSpy).toHaveBeenCalledWith(credentials, userId); + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + + superSpy.mockRestore(); + }); + }); }); diff --git a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.ts b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.ts index cedfa3fe589..f9fb8361056 100644 --- a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.ts +++ b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.ts @@ -1,6 +1,7 @@ import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; import { DefaultSetInitialPasswordService } from "@bitwarden/angular/auth/password-management/set-initial-password/default-set-initial-password.service.implementation"; import { + InitializeJitPasswordCredentials, SetInitialPasswordCredentials, SetInitialPasswordService, SetInitialPasswordUserType, @@ -14,6 +15,7 @@ import { EncryptService } from "@bitwarden/common/key-management/crypto/abstract import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { RegisterSdkService } from "@bitwarden/common/platform/abstractions/sdk/register-sdk.service"; import { UserId } from "@bitwarden/common/types/guid"; import { KdfConfigService, KeyService } from "@bitwarden/key-management"; @@ -34,6 +36,7 @@ export class DesktopSetInitialPasswordService protected userDecryptionOptionsService: InternalUserDecryptionOptionsServiceAbstraction, private messagingService: MessagingService, protected accountCryptographicStateService: AccountCryptographicStateService, + protected registerSdkService: RegisterSdkService, ) { super( apiService, @@ -47,6 +50,7 @@ export class DesktopSetInitialPasswordService organizationUserApiService, userDecryptionOptionsService, accountCryptographicStateService, + registerSdkService, ); } @@ -59,4 +63,13 @@ export class DesktopSetInitialPasswordService this.messagingService.send("redrawMenu"); } + + override async initializePasswordJitPasswordUserV2Encryption( + credentials: InitializeJitPasswordCredentials, + userId: UserId, + ): Promise { + await super.initializePasswordJitPasswordUserV2Encryption(credentials, userId); + + this.messagingService.send("redrawMenu"); + } } diff --git a/apps/desktop/src/app/tools/send-v2/send-v2.component.html b/apps/desktop/src/app/tools/send-v2/send-v2.component.html index eda740fa721..dad0e541a4d 100644 --- a/apps/desktop/src/app/tools/send-v2/send-v2.component.html +++ b/apps/desktop/src/app/tools/send-v2/send-v2.component.html @@ -1,25 +1,93 @@ - - - @if (!disableSend()) { - - } - - - - - +@if (useDrawerEditMode()) { +
+ + + + @if (!disableSend()) { + + } + + + + + +
+} @else { + +
+
+ + + @if (!disableSend()) { + + } + +
+ + + + +
+
+ + + @if (action() == "add" || action() == "edit") { + + } + + + @if (!action()) { + + } +
+} diff --git a/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts b/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts index a73a0534ff9..3670713f8f3 100644 --- a/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts +++ b/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts @@ -49,6 +49,7 @@ describe("SendV2Component", () => { let sendApiService: MockProxy; let toastService: MockProxy; let i18nService: MockProxy; + let configService: MockProxy; beforeEach(async () => { sendService = mock(); @@ -62,6 +63,10 @@ describe("SendV2Component", () => { sendApiService = mock(); toastService = mock(); i18nService = mock(); + configService = mock(); + + // Setup configService mock - feature flag returns true to test the new drawer mode + configService.getFeatureFlag$.mockReturnValue(of(true)); // Setup environmentService mock environmentService.getEnvironment.mockResolvedValue({ @@ -117,7 +122,7 @@ describe("SendV2Component", () => { useValue: mock(), }, { provide: MessagingService, useValue: mock() }, - { provide: ConfigService, useValue: mock() }, + { provide: ConfigService, useValue: configService }, { provide: ActivatedRoute, useValue: { diff --git a/apps/desktop/src/app/tools/send-v2/send-v2.component.ts b/apps/desktop/src/app/tools/send-v2/send-v2.component.ts index 7fab0cb6702..95c0c971d2c 100644 --- a/apps/desktop/src/app/tools/send-v2/send-v2.component.ts +++ b/apps/desktop/src/app/tools/send-v2/send-v2.component.ts @@ -1,11 +1,13 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { - ChangeDetectionStrategy, ChangeDetectorRef, Component, + computed, effect, inject, + signal, + viewChild, } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; import { combineLatest, map, switchMap, lastValueFrom } from "rxjs"; @@ -15,6 +17,8 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -36,12 +40,27 @@ import { import { DesktopPremiumUpgradePromptService } from "../../../services/desktop-premium-upgrade-prompt.service"; import { DesktopHeaderComponent } from "../../layout/header"; +import { AddEditComponent } from "../send/add-edit.component"; +const Action = Object.freeze({ + /** No action is currently active. */ + None: "", + /** The user is adding a new Send. */ + Add: "add", + /** The user is editing an existing Send. */ + Edit: "edit", +} as const); + +type Action = (typeof Action)[keyof typeof Action]; + +// 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: "app-send-v2", imports: [ JslibModule, ButtonModule, + AddEditComponent, SendListComponent, NewSendDropdownV2Component, DesktopHeaderComponent, @@ -54,13 +73,19 @@ import { DesktopHeaderComponent } from "../../layout/header"; }, ], templateUrl: "./send-v2.component.html", - changeDetection: ChangeDetectionStrategy.OnPush, }) export class SendV2Component { + protected readonly addEditComponent = viewChild(AddEditComponent); + + protected readonly sendId = signal(null); + protected readonly action = signal(Action.None); + private readonly selectedSendTypeOverride = signal(undefined); + private sendFormConfigService = inject(DefaultSendFormConfigService); private sendItemsService = inject(SendItemsService); private policyService = inject(PolicyService); private accountService = inject(AccountService); + private configService = inject(ConfigService); private i18nService = inject(I18nService); private platformUtilsService = inject(PlatformUtilsService); private environmentService = inject(EnvironmentService); @@ -70,6 +95,11 @@ export class SendV2Component { private logService = inject(LogService); private cdr = inject(ChangeDetectorRef); + protected readonly useDrawerEditMode = toSignal( + this.configService.getFeatureFlag$(FeatureFlag.DesktopUiMigrationMilestone2), + { initialValue: false }, + ); + protected readonly filteredSends = toSignal(this.sendItemsService.filteredAndSortedSends$, { initialValue: [], }); @@ -119,28 +149,79 @@ export class SendV2Component { }); } + protected readonly selectedSendType = computed(() => { + const action = this.action(); + const typeOverride = this.selectedSendTypeOverride(); + + if (action === Action.Add && typeOverride !== undefined) { + return typeOverride; + } + + const sendId = this.sendId(); + return this.filteredSends().find((s) => s.id === sendId)?.type; + }); + protected async addSend(type: SendType): Promise { - const formConfig = await this.sendFormConfigService.buildConfig("add", undefined, type); + if (this.useDrawerEditMode()) { + const formConfig = await this.sendFormConfigService.buildConfig("add", undefined, type); - const dialogRef = SendAddEditDialogComponent.openDrawer(this.dialogService, { - formConfig, - }); + const dialogRef = SendAddEditDialogComponent.openDrawer(this.dialogService, { + formConfig, + }); - await lastValueFrom(dialogRef.closed); + await lastValueFrom(dialogRef.closed); + } else { + this.action.set(Action.Add); + this.sendId.set(null); + this.selectedSendTypeOverride.set(type); + + const component = this.addEditComponent(); + if (component) { + await component.resetAndLoad(); + } + } } - protected async selectSend(sendId: SendId): Promise { - const formConfig = await this.sendFormConfigService.buildConfig("edit", sendId); + /** Used by old UI to add a send without specifying type (defaults to Text) */ + protected async addSendWithoutType(): Promise { + await this.addSend(SendType.Text); + } - const dialogRef = SendAddEditDialogComponent.openDrawer(this.dialogService, { - formConfig, - }); + protected closeEditPanel(): void { + this.action.set(Action.None); + this.sendId.set(null); + this.selectedSendTypeOverride.set(undefined); + } - await lastValueFrom(dialogRef.closed); + protected async savedSend(send: SendView): Promise { + await this.selectSend(send.id); + } + + protected async selectSend(sendId: string): Promise { + if (this.useDrawerEditMode()) { + const formConfig = await this.sendFormConfigService.buildConfig("edit", sendId as SendId); + + const dialogRef = SendAddEditDialogComponent.openDrawer(this.dialogService, { + formConfig, + }); + + await lastValueFrom(dialogRef.closed); + } else { + if (sendId === this.sendId() && this.action() === Action.Edit) { + return; + } + this.action.set(Action.Edit); + this.sendId.set(sendId); + const component = this.addEditComponent(); + if (component) { + component.sendId = sendId; + await component.refresh(); + } + } } protected async onEditSend(send: SendView): Promise { - await this.selectSend(send.id as SendId); + await this.selectSend(send.id); } protected async onCopySend(send: SendView): Promise { @@ -176,6 +257,11 @@ export class SendV2Component { title: null, message: this.i18nService.t("removedPassword"), }); + + if (!this.useDrawerEditMode() && this.sendId() === send.id) { + this.sendId.set(null); + await this.selectSend(send.id); + } } catch (e) { this.logService.error(e); } @@ -199,5 +285,9 @@ export class SendV2Component { title: null, message: this.i18nService.t("deletedSend"), }); + + if (!this.useDrawerEditMode()) { + this.closeEditPanel(); + } } } diff --git a/apps/desktop/src/autofill/services/desktop-autofill.service.ts b/apps/desktop/src/autofill/services/desktop-autofill.service.ts index c50964e31e3..e5cd85aa7a3 100644 --- a/apps/desktop/src/autofill/services/desktop-autofill.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autofill.service.ts @@ -10,6 +10,7 @@ import { mergeMap, switchMap, takeUntil, + tap, } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -52,6 +53,8 @@ import type { NativeWindowObject } from "./desktop-fido2-user-interface.service" export class DesktopAutofillService implements OnDestroy { private destroy$ = new Subject(); private registrationRequest: autofill.PasskeyRegistrationRequest; + private featureFlag?: FeatureFlag; + private isEnabled: boolean = false; constructor( private logService: LogService, @@ -60,19 +63,26 @@ export class DesktopAutofillService implements OnDestroy { private fido2AuthenticatorService: Fido2AuthenticatorServiceAbstraction, private accountService: AccountService, private authService: AuthService, - private platformUtilsService: PlatformUtilsService, - ) {} + platformUtilsService: PlatformUtilsService, + ) { + const deviceType = platformUtilsService.getDevice(); + if (deviceType === DeviceType.MacOsDesktop) { + this.featureFlag = FeatureFlag.MacOsNativeCredentialSync; + } + } async init() { - // Currently only supported for MacOS - if (this.platformUtilsService.getDevice() !== DeviceType.MacOsDesktop) { + this.isEnabled = + this.featureFlag && (await this.configService.getFeatureFlag(this.featureFlag)); + if (!this.isEnabled) { return; } this.configService - .getFeatureFlag$(FeatureFlag.MacOsNativeCredentialSync) + .getFeatureFlag$(this.featureFlag) .pipe( distinctUntilChanged(), + tap((enabled) => (this.isEnabled = enabled)), filter((enabled) => enabled === true), // Only proceed if feature is enabled switchMap(() => { return combineLatest([ @@ -199,11 +209,11 @@ export class DesktopAutofillService implements OnDestroy { listenIpc() { ipc.autofill.listenPasskeyRegistration(async (clientId, sequenceNumber, request, callback) => { - if (!(await this.configService.getFeatureFlag(FeatureFlag.MacOsNativeCredentialSync))) { + if (!this.isEnabled) { this.logService.debug( - "listenPasskeyRegistration: MacOsNativeCredentialSync feature flag is disabled", + `listenPasskeyRegistration: Native credential sync feature flag (${this.featureFlag}) is disabled`, ); - callback(new Error("MacOsNativeCredentialSync feature flag is disabled"), null); + callback(new Error("Native credential sync feature flag is disabled"), null); return; } @@ -230,11 +240,11 @@ export class DesktopAutofillService implements OnDestroy { ipc.autofill.listenPasskeyAssertionWithoutUserInterface( async (clientId, sequenceNumber, request, callback) => { - if (!(await this.configService.getFeatureFlag(FeatureFlag.MacOsNativeCredentialSync))) { + if (!this.isEnabled) { this.logService.debug( - "listenPasskeyAssertionWithoutUserInterface: MacOsNativeCredentialSync feature flag is disabled", + `listenPasskeyAssertionWithoutUserInterface: Native credential sync feature flag (${this.featureFlag}) is disabled`, ); - callback(new Error("MacOsNativeCredentialSync feature flag is disabled"), null); + callback(new Error("Native credential sync feature flag is disabled"), null); return; } @@ -297,11 +307,11 @@ export class DesktopAutofillService implements OnDestroy { ); ipc.autofill.listenPasskeyAssertion(async (clientId, sequenceNumber, request, callback) => { - if (!(await this.configService.getFeatureFlag(FeatureFlag.MacOsNativeCredentialSync))) { + if (!this.isEnabled) { this.logService.debug( - "listenPasskeyAssertion: MacOsNativeCredentialSync feature flag is disabled", + `listenPasskeyAssertion: Native credential sync feature flag (${this.featureFlag}) is disabled`, ); - callback(new Error("MacOsNativeCredentialSync feature flag is disabled"), null); + callback(new Error("Native credential sync feature flag is disabled"), null); return; } @@ -324,9 +334,9 @@ export class DesktopAutofillService implements OnDestroy { // Listen for native status messages ipc.autofill.listenNativeStatus(async (clientId, sequenceNumber, status) => { - if (!(await this.configService.getFeatureFlag(FeatureFlag.MacOsNativeCredentialSync))) { + if (!this.isEnabled) { this.logService.debug( - "listenNativeStatus: MacOsNativeCredentialSync feature flag is disabled", + `listenNativeStatus: Native credential sync feature flag (${this.featureFlag}) is disabled`, ); return; } diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index f9947e16692..0ce98b8c62b 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -2092,6 +2092,9 @@ "permanentlyDeletedItem": { "message": "Item permanently deleted" }, + "archivedItemRestored": { + "message": "Archived item restored" + }, "restoredItem": { "message": "Item restored" }, diff --git a/apps/desktop/src/scss/migration.scss b/apps/desktop/src/scss/migration.scss new file mode 100644 index 00000000000..ba70d4fa009 --- /dev/null +++ b/apps/desktop/src/scss/migration.scss @@ -0,0 +1,29 @@ +/** + * Desktop UI Migration + * + * These are temporary styles during the desktop ui migration. + **/ + +/** + * This removes any padding applied by the bit-layout to content. + * This should be revisited once the table is migrated, and again once drawers are migrated. + **/ +bit-layout { + #main-content { + padding: 0 0 0 0; + } +} +/** + * Send list panel styling for send-v2 component + * Temporary during migration - width handled by tw-w-2/5 + **/ +.vault > .send-items-panel { + order: 2; + min-width: 200px; + border-right: 1px solid; + + @include themify($themes) { + background-color: themed("backgroundColor"); + border-right-color: themed("borderColor"); + } +} diff --git a/apps/desktop/src/scss/styles.scss b/apps/desktop/src/scss/styles.scss index c579e6acdc0..b4082afd38c 100644 --- a/apps/desktop/src/scss/styles.scss +++ b/apps/desktop/src/scss/styles.scss @@ -15,5 +15,6 @@ @import "left-nav.scss"; @import "loading.scss"; @import "plugins.scss"; +@import "migration.scss"; @import "../../../../libs/angular/src/scss/icons.scss"; @import "../../../../libs/components/src/multi-select/scss/bw.theme"; diff --git a/apps/desktop/src/vault/app/vault/item-footer.component.html b/apps/desktop/src/vault/app/vault/item-footer.component.html index a03f3e96b06..0af73bf7d8a 100644 --- a/apps/desktop/src/vault/app/vault/item-footer.component.html +++ b/apps/desktop/src/vault/app/vault/item-footer.component.html @@ -36,15 +36,11 @@ > - + @if (showCloneOption) { + + }
- - + + + + + } +
-
-