From e2fa296b042f3c433786a15a6c4a41909c194fc2 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 26 Jan 2026 17:40:27 -0500 Subject: [PATCH 01/24] chore(deps): Added override for package-lock.json --- .github/CODEOWNERS | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d1266a174e4..3884bfda063 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -84,6 +84,7 @@ apps/web/src/app/billing @bitwarden/team-billing-dev libs/angular/src/billing @bitwarden/team-billing-dev libs/common/src/billing @bitwarden/team-billing-dev libs/billing @bitwarden/team-billing-dev +libs/pricing @bitwarden/team-billing-dev bitwarden_license/bit-web/src/app/billing @bitwarden/team-billing-dev ## Platform team files ## @@ -227,7 +228,9 @@ apps/web/src/locales/en/messages.json **/tsconfig.json @bitwarden/team-platform-dev **/jest.config.js @bitwarden/team-platform-dev **/project.jsons @bitwarden/team-platform-dev -libs/pricing @bitwarden/team-billing-dev +# Platform override specifically for the package-lock.json in +# native-messaging-test-runner so that Platform can manage all lock file updates +apps/desktop/native-messaging-test-runner/package-lock.json @bitwarden/team-platform-dev # Claude related files .claude/ @bitwarden/team-ai-sme From 60c28dd182eb7cbdd73956eb19509976c1c875b5 Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Mon, 26 Jan 2026 17:05:42 -0600 Subject: [PATCH 02/24] [PM-31203] Change Phishing Url Check to use a Cursor Based Search (#18561) * 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 * Change indexeddb lookup from loading all to cursor search * fix(phishing): improve performance and fix URL matching in phishing detection Problem: The cursor-based search takes ~25 seconds to scan the entire phishing database. For non-phishing URLs (99% of cases), this full scan runs to completion every time. Before these fixes, opening a new tab triggered this sequence: 1. chrome://newtab/ fires a phishing check 2. Sequential concatMap blocks while cursor scans all 500k+ URLs (~25 sec) 3. User pastes actual URL and hits enter 4. That URL's check waits in queue behind the chrome:// check 5. Total delay: ~50+ seconds for a simple "open tab, paste link" workflow Even for legitimate phishing checks, the cursor search could take up to 25 seconds per URL when the fast hasUrl lookup misses due to trailing slash mismatches. Changes: phishing-data.service.ts: - Add protocol filter to early-return for non-http(s) URLs, avoiding expensive IndexedDB operations for chrome://, about:, file:// URLs - Add trailing slash normalization for hasUrl lookup - browsers add trailing slashes but DB entries may not have them, causing O(1) lookups to miss and fall back to O(n) cursor search unnecessarily - Add debug logging for hasUrl checks and timing metrics for cursor-based search to aid performance debugging phishing-detection.service.ts: - Replace concatMap with mergeMap for parallel tab processing - each tab check now runs independently instead of sequentially - Add concurrency limit of 5 to prevent overwhelming IndexedDB while still allowing parallel execution Result: - New tabs are instant (no IndexedDB calls for non-web URLs) - One slow phishing check doesn't block other tabs - Common URL patterns hit the fast O(1) path instead of O(n) cursor scan * performance debug logs * disable custom match because too slow * spec fix --------- Co-authored-by: Alex --- .../phishing-detection/phishing-resources.ts | 4 + .../services/phishing-data.service.spec.ts | 42 ++++------ .../services/phishing-data.service.ts | 73 ++++++++++++++-- .../services/phishing-detection.service.ts | 54 ++++++++---- .../phishing-indexeddb.service.spec.ts | 83 +++++++++++++++++++ .../services/phishing-indexeddb.service.ts | 54 ++++++++++++ 6 files changed, 259 insertions(+), 51 deletions(-) diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts index 88068987dd7..6595104207a 100644 --- a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -7,6 +7,8 @@ export type PhishingResource = { todayUrl: string; /** Matcher used to decide whether a given URL matches an entry from this resource */ match: (url: URL, entry: string) => boolean; + /** Whether to use the custom matcher. If false, only exact hasUrl lookups are used. Default: true */ + useCustomMatcher?: boolean; }; export const PhishingResourceType = Object.freeze({ @@ -56,6 +58,8 @@ export const PHISHING_RESOURCES: Record { if (!entry) { return false; 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 d633c0612f5..2d6c7a5a651 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 @@ -40,6 +40,7 @@ describe("PhishingDataService", () => { // Set default mock behaviors mockIndexedDbService.hasUrl.mockResolvedValue(false); mockIndexedDbService.loadAllUrls.mockResolvedValue([]); + mockIndexedDbService.findMatchingUrl.mockResolvedValue(false); mockIndexedDbService.saveUrls.mockResolvedValue(undefined); mockIndexedDbService.addUrls.mockResolvedValue(undefined); mockIndexedDbService.saveUrlsFromStream.mockResolvedValue(undefined); @@ -90,7 +91,7 @@ describe("PhishingDataService", () => { it("should NOT detect QA test addresses - different subpath", async () => { mockIndexedDbService.hasUrl.mockResolvedValue(false); - mockIndexedDbService.loadAllUrls.mockResolvedValue([]); + mockIndexedDbService.findMatchingUrl.mockResolvedValue(false); const url = new URL("https://phishing.testcategory.com/other"); const result = await service.isPhishingWebAddress(url); @@ -120,70 +121,65 @@ describe("PhishingDataService", () => { 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(); + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); - it("should fall back to custom matcher when hasUrl returns false", async () => { + it("should return false when hasUrl returns false (custom matcher disabled)", 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); + // Custom matcher is currently disabled (useCustomMatcher: false), so result is false + expect(result).toBe(false); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/path"); - expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + // Custom matcher should NOT be called since it's disabled + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); it("should not detect a safe web address", async () => { // 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(); + // Custom matcher is disabled, so findMatchingUrl should NOT be called + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); - it("should not match against root web address with subpaths using custom matcher", async () => { + it("should not match against root web address with subpaths (custom matcher disabled)", 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(false); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/login/page"); - expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + // Custom matcher is disabled, so findMatchingUrl should NOT be called + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); - it("should not match against root web address with different subpaths using custom matcher", async () => { + it("should not match against root web address with different subpaths (custom matcher disabled)", 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(); + // Custom matcher is disabled, so findMatchingUrl should NOT be called + expect(mockIndexedDbService.findMatchingUrl).not.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); @@ -193,10 +189,8 @@ describe("PhishingDataService", () => { "[PhishingDataService] IndexedDB lookup via hasUrl failed", expect.any(Error), ); - expect(logService.error).toHaveBeenCalledWith( - "[PhishingDataService] Error running custom matcher", - expect.any(Error), - ); + // Custom matcher is disabled, so no custom matcher error is expected + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); }); 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 10268fa7f93..c34a94ecced 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 @@ -153,8 +153,18 @@ export class PhishingDataService { * @returns True if the URL is a known phishing web address, false otherwise */ async isPhishingWebAddress(url: URL): Promise { + this.logService.debug("[PhishingDataService] isPhishingWebAddress called for: " + url.href); + + // Skip non-http(s) protocols - phishing database only contains web URLs + // This prevents expensive fallback checks for chrome://, about:, file://, etc. + if (url.protocol !== "http:" && url.protocol !== "https:") { + this.logService.debug("[PhishingDataService] Skipping non-http(s) protocol: " + url.protocol); + return false; + } + // Quick check for QA/dev test addresses if (this._testWebAddresses.includes(url.href)) { + this.logService.info("[PhishingDataService] Found test web address: " + url.href); return true; } @@ -162,28 +172,73 @@ export class PhishingDataService { try { // Quick lookup: check direct presence of href in IndexedDB - const hasUrl = await this.indexedDbService.hasUrl(url.href); + // Also check without trailing slash since browsers add it but DB entries may not have it + const urlHref = url.href; + const urlWithoutTrailingSlash = urlHref.endsWith("/") ? urlHref.slice(0, -1) : null; + + this.logService.debug("[PhishingDataService] Checking hasUrl on this string: " + urlHref); + let hasUrl = await this.indexedDbService.hasUrl(urlHref); + + // If not found and URL has trailing slash, try without it + if (!hasUrl && urlWithoutTrailingSlash) { + this.logService.debug( + "[PhishingDataService] Checking hasUrl without trailing slash: " + + urlWithoutTrailingSlash, + ); + hasUrl = await this.indexedDbService.hasUrl(urlWithoutTrailingSlash); + } + if (hasUrl) { + this.logService.info( + "[PhishingDataService] Found phishing web address through direct lookup: " + urlHref, + ); 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) { + // If a custom matcher is provided and enabled, use cursor-based search. + // This avoids loading all URLs into memory and allows early exit on first match. + // Can be disabled via useCustomMatcher: false for performance reasons. + if (resource && resource.match && resource.useCustomMatcher !== false) { try { - const entries = await this.indexedDbService.loadAllUrls(); - for (const entry of entries) { - if (resource.match(url, entry)) { - return true; - } + this.logService.debug( + "[PhishingDataService] Starting cursor-based search for: " + url.href, + ); + const startTime = performance.now(); + + const found = await this.indexedDbService.findMatchingUrl((entry) => + resource.match(url, entry), + ); + + const endTime = performance.now(); + const duration = (endTime - startTime).toFixed(2); + this.logService.debug( + `[PhishingDataService] Cursor-based search completed in ${duration}ms for: ${url.href} (found: ${found})`, + ); + + if (found) { + this.logService.info( + "[PhishingDataService] Found phishing web address through custom matcher: " + url.href, + ); + } else { + this.logService.debug( + "[PhishingDataService] No match found, returning false for: " + url.href, + ); } + return found; } catch (err) { this.logService.error("[PhishingDataService] Error running custom matcher", err); + this.logService.debug( + "[PhishingDataService] Returning false due to error for: " + url.href, + ); + return false; } - return false; } + this.logService.debug( + "[PhishingDataService] No custom matcher, returning false for: " + url.href, + ); return false; } 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 815007e1d4c..6ca5bad8942 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 @@ -1,10 +1,10 @@ import { - concatMap, distinctUntilChanged, EMPTY, filter, map, merge, + mergeMap, Subject, switchMap, tap, @@ -43,6 +43,7 @@ export class PhishingDetectionService { private static _tabUpdated$ = new Subject(); private static _ignoredHostnames = new Set(); private static _didInit = false; + private static _activeSearchCount = 0; static initialize( logService: LogService, @@ -63,7 +64,7 @@ export class PhishingDetectionService { tap((message) => logService.debug(`[PhishingDetectionService] user selected continue for ${message.url}`), ), - concatMap(async (message) => { + mergeMap(async (message) => { const url = new URL(message.url); this._ignoredHostnames.add(url.hostname); await BrowserApi.navigateTabToUrl(message.tabId, url); @@ -88,23 +89,40 @@ export class PhishingDetectionService { prev.ignored === curr.ignored, ), tap((event) => logService.debug(`[PhishingDetectionService] processing event:`, event)), - concatMap(async ({ tabId, url, ignored }) => { - if (ignored) { - // The next time this host is visited, block again - this._ignoredHostnames.delete(url.hostname); - return; - } - const isPhishing = await phishingDataService.isPhishingWebAddress(url); - if (!isPhishing) { - return; - } - - const phishingWarningPage = new URL( - BrowserApi.getRuntimeURL("popup/index.html#/security/phishing-warning") + - `?phishingUrl=${url.toString()}`, + // Use mergeMap for parallel processing - each tab check runs independently + // Concurrency limit of 5 prevents overwhelming IndexedDB + mergeMap(async ({ tabId, url, ignored }) => { + this._activeSearchCount++; + const searchId = `${tabId}-${Date.now()}`; + logService.debug( + `[PhishingDetectionService] Search STARTED [${searchId}] for ${url.href} (active: ${this._activeSearchCount}/5)`, ); - await BrowserApi.navigateTabToUrl(tabId, phishingWarningPage); - }), + const startTime = performance.now(); + + try { + if (ignored) { + // The next time this host is visited, block again + this._ignoredHostnames.delete(url.hostname); + return; + } + const isPhishing = await phishingDataService.isPhishingWebAddress(url); + if (!isPhishing) { + return; + } + + const phishingWarningPage = new URL( + BrowserApi.getRuntimeURL("popup/index.html#/security/phishing-warning") + + `?phishingUrl=${url.toString()}`, + ); + await BrowserApi.navigateTabToUrl(tabId, phishingWarningPage); + } finally { + this._activeSearchCount--; + const duration = (performance.now() - startTime).toFixed(2); + logService.debug( + `[PhishingDetectionService] Search FINISHED [${searchId}] for ${url.href} in ${duration}ms (active: ${this._activeSearchCount}/5)`, + ); + } + }, 5), ); const onCancelCommand$ = messageListener 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 99e101cc199..98835a5b366 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 @@ -435,6 +435,89 @@ describe("PhishingIndexedDbService", () => { }); }); + describe("findMatchingUrl", () => { + it("returns true when matcher finds a match", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + mockStore.set("https://phishing.net", { url: "https://phishing.net" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + + const matcher = (url: string) => url.includes("phishing"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(true); + expect(mockDb.transaction).toHaveBeenCalledWith("phishing-urls", "readonly"); + expect(mockObjectStore.openCursor).toHaveBeenCalled(); + }); + + it("returns false when no URLs match", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + + const matcher = (url: string) => url.includes("notfound"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(false); + }); + + it("returns false when store is empty", async () => { + const matcher = (url: string) => url.includes("anything"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(false); + }); + + it("exits early on first match without iterating all records", async () => { + mockStore.set("https://match1.com", { url: "https://match1.com" }); + mockStore.set("https://match2.com", { url: "https://match2.com" }); + mockStore.set("https://match3.com", { url: "https://match3.com" }); + + const matcherCallCount = jest + .fn() + .mockImplementation((url: string) => url.includes("match2")); + await service.findMatchingUrl(matcherCallCount); + + // Matcher should be called for match1.com and match2.com, but NOT match3.com + // because it exits early on first match + expect(matcherCallCount).toHaveBeenCalledWith("https://match1.com"); + expect(matcherCallCount).toHaveBeenCalledWith("https://match2.com"); + expect(matcherCallCount).not.toHaveBeenCalledWith("https://match3.com"); + expect(matcherCallCount).toHaveBeenCalledTimes(2); + }); + + it("supports complex matcher logic", async () => { + mockStore.set("https://example.com/path", { url: "https://example.com/path" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + mockStore.set("https://phishing.net/login", { url: "https://phishing.net/login" }); + + const matcher = (url: string) => { + return url.includes("phishing") && url.includes("login"); + }; + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(true); + }); + + 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 matcher = (url: string) => url.includes("test"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Cursor search failed", + expect.any(Error), + ); + }); + }); + describe("database initialization", () => { it("creates object store with keyPath on upgrade", async () => { mockDb.objectStoreNames.contains.mockReturnValue(false); 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 fe0f10da221..ea4b7987607 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 @@ -195,6 +195,60 @@ export class PhishingIndexedDbService { }); } + /** + * Checks if any URL in the database matches the given matcher function. + * Uses a cursor to iterate through records without loading all into memory. + * Returns immediately on first match for optimal performance. + * + * @param matcher - Function that tests each URL and returns true if it matches + * @returns `true` if any URL matches, `false` if none match or on error + */ + async findMatchingUrl(matcher: (url: string) => boolean): Promise { + this.logService.debug("[PhishingIndexedDbService] Searching for matching URL with cursor..."); + + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + return await this.cursorSearch(db, matcher); + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Cursor search failed", error); + return false; + } finally { + db?.close(); + } + } + + /** + * Performs cursor-based search through all URLs. + * Tests each URL with the matcher without accumulating records in memory. + */ + private cursorSearch(db: IDBDatabase, matcher: (url: string) => boolean): Promise { + return new Promise((resolve, reject) => { + 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) { + const url = (cursor.value as PhishingUrlRecord).url; + // Test the URL immediately without accumulating in memory + if (matcher(url)) { + // Found a match + resolve(true); + return; + } + // No match, continue to next record + cursor.continue(); + } else { + // Reached end of records without finding a match + resolve(false); + } + }; + }); + } + /** * Saves phishing URLs directly from a stream. * Processes data incrementally to minimize memory usage. From 748c7c544624eb6154c5318c048c1e196b397dc1 Mon Sep 17 00:00:00 2001 From: Nik Gilmore Date: Mon, 26 Jan 2026 15:55:49 -0800 Subject: [PATCH 03/24] [PM-30303] Migrate Cipher Delete Operations to use SDK (#18275) --- .../bulk-delete-dialog.component.ts | 12 +- .../vault/abstractions/cipher-sdk.service.ts | 74 ++++- .../src/vault/abstractions/cipher.service.ts | 26 +- .../vault/services/cipher-sdk.service.spec.ts | 288 ++++++++++++++++++ .../src/vault/services/cipher-sdk.service.ts | 185 ++++++++++- .../src/vault/services/cipher.service.spec.ts | 254 ++++++++++++++- .../src/vault/services/cipher.service.ts | 79 ++++- 7 files changed, 880 insertions(+), 38 deletions(-) diff --git a/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts b/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts index 46f2b5da735..9fcb6f0cec1 100644 --- a/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts +++ b/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts @@ -12,7 +12,6 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CollectionId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; -import { CipherBulkDeleteRequest } from "@bitwarden/common/vault/models/request/cipher-bulk-delete.request"; import { UnionOfValues } from "@bitwarden/common/vault/types/union-of-values"; import { CenterPositionStrategy, @@ -148,11 +147,16 @@ export class BulkDeleteDialogComponent { } private async deleteCiphersAdmin(ciphers: string[]): Promise { - const deleteRequest = new CipherBulkDeleteRequest(ciphers, this.organization.id); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); if (this.permanent) { - return await this.apiService.deleteManyCiphersAdmin(deleteRequest); + await this.cipherService.deleteManyWithServer(ciphers, userId, true, this.organization.id); } else { - return await this.apiService.putDeleteManyCiphersAdmin(deleteRequest); + await this.cipherService.softDeleteManyWithServer( + ciphers, + userId, + true, + this.organization.id, + ); } } diff --git a/libs/common/src/vault/abstractions/cipher-sdk.service.ts b/libs/common/src/vault/abstractions/cipher-sdk.service.ts index 1037bfc2b92..3101531eda6 100644 --- a/libs/common/src/vault/abstractions/cipher-sdk.service.ts +++ b/libs/common/src/vault/abstractions/cipher-sdk.service.ts @@ -1,4 +1,4 @@ -import { UserId } from "@bitwarden/common/types/guid"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; /** @@ -34,4 +34,76 @@ export abstract class CipherSdkService { originalCipherView?: CipherView, orgAdmin?: boolean, ): Promise; + + /** + * Deletes a cipher on the server using the SDK. + * + * @param id The cipher ID to delete + * @param userId The user ID to use for SDK client + * @param asAdmin Whether this is an organization admin operation + * @returns A promise that resolves when the cipher is deleted + */ + abstract deleteWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; + + /** + * Deletes multiple ciphers on the server using the SDK. + * + * @param ids The cipher IDs to delete + * @param userId The user ID to use for SDK client + * @param asAdmin Whether this is an organization admin operation + * @param orgId The organization ID (required when asAdmin is true) + * @returns A promise that resolves when the ciphers are deleted + */ + abstract deleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin?: boolean, + orgId?: OrganizationId, + ): Promise; + + /** + * Soft deletes a cipher on the server using the SDK. + * + * @param id The cipher ID to soft delete + * @param userId The user ID to use for SDK client + * @param asAdmin Whether this is an organization admin operation + * @returns A promise that resolves when the cipher is soft deleted + */ + abstract softDeleteWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; + + /** + * Soft deletes multiple ciphers on the server using the SDK. + * + * @param ids The cipher IDs to soft delete + * @param userId The user ID to use for SDK client + * @param asAdmin Whether this is an organization admin operation + * @param orgId The organization ID (required when asAdmin is true) + * @returns A promise that resolves when the ciphers are soft deleted + */ + abstract softDeleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin?: boolean, + orgId?: OrganizationId, + ): Promise; + + /** + * Restores a soft-deleted cipher on the server using the SDK. + * + * @param id The cipher ID to restore + * @param userId The user ID to use for SDK client + * @param asAdmin Whether this is an organization admin operation + * @returns A promise that resolves when the cipher is restored + */ + abstract restoreWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; + + /** + * Restores multiple soft-deleted ciphers on the server using the SDK. + * + * @param ids The cipher IDs to restore + * @param userId The user ID to use for SDK client + * @param orgId The organization ID (determines whether to use admin API) + * @returns A promise that resolves when the ciphers are restored + */ + abstract restoreManyWithServer(ids: string[], userId: UserId, orgId?: string): Promise; } diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 1db5f8d38a7..4b544b2a34e 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -230,8 +230,13 @@ export abstract class CipherService implements UserKeyRotationDataProvider; abstract moveManyWithServer(ids: string[], folderId: string, userId: UserId): Promise; abstract delete(id: string | string[], userId: UserId): Promise; - abstract deleteWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; - abstract deleteManyWithServer(ids: string[], userId: UserId, asAdmin?: boolean): Promise; + abstract deleteWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; + abstract deleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin?: boolean, + orgId?: OrganizationId, + ): Promise; abstract deleteAttachment( id: string, revisionDate: string, @@ -247,14 +252,19 @@ export abstract class CipherService implements UserKeyRotationDataProvider number; - abstract softDelete(id: string | string[], userId: UserId): Promise; - abstract softDeleteWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; - abstract softDeleteManyWithServer(ids: string[], userId: UserId, asAdmin?: boolean): Promise; + abstract softDelete(id: string | string[], userId: UserId): Promise; + abstract softDeleteWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; + abstract softDeleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin?: boolean, + orgId?: OrganizationId, + ): Promise; abstract restore( cipher: { id: string; revisionDate: string } | { id: string; revisionDate: string }[], userId: UserId, - ): Promise; - abstract restoreWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; + ): Promise; + abstract restoreWithServer(id: string, userId: UserId, asAdmin?: boolean): Promise; abstract restoreManyWithServer(ids: string[], userId: UserId, orgId?: string): Promise; abstract getKeyForCipherKeyDecryption(cipher: Cipher, userId: UserId): Promise; abstract setAddEditCipherInfo(value: AddEditCipherInfo, userId: UserId): Promise; @@ -275,7 +285,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider; /** - * Decrypts a cipher using either the SDK or the legacy method based on the feature flag. + * Decrypts a cipher using either the use-sdk-cipheroperationsSDK or the legacy method based on the feature flag. * @param cipher The cipher to decrypt. * @param userId The user ID to use for decryption. * @returns A promise that resolves to the decrypted cipher view. diff --git a/libs/common/src/vault/services/cipher-sdk.service.spec.ts b/libs/common/src/vault/services/cipher-sdk.service.spec.ts index bd3feb4619e..cb21ff28133 100644 --- a/libs/common/src/vault/services/cipher-sdk.service.spec.ts +++ b/libs/common/src/vault/services/cipher-sdk.service.spec.ts @@ -28,10 +28,22 @@ describe("DefaultCipherSdkService", () => { mockAdminSdk = { create: jest.fn(), edit: jest.fn(), + delete: jest.fn().mockResolvedValue(undefined), + delete_many: jest.fn().mockResolvedValue(undefined), + soft_delete: jest.fn().mockResolvedValue(undefined), + soft_delete_many: jest.fn().mockResolvedValue(undefined), + restore: jest.fn().mockResolvedValue(undefined), + restore_many: jest.fn().mockResolvedValue(undefined), }; mockCiphersSdk = { create: jest.fn(), edit: jest.fn(), + delete: jest.fn().mockResolvedValue(undefined), + delete_many: jest.fn().mockResolvedValue(undefined), + soft_delete: jest.fn().mockResolvedValue(undefined), + soft_delete_many: jest.fn().mockResolvedValue(undefined), + restore: jest.fn().mockResolvedValue(undefined), + restore_many: jest.fn().mockResolvedValue(undefined), admin: jest.fn().mockReturnValue(mockAdminSdk), }; mockVaultSdk = { @@ -243,4 +255,280 @@ describe("DefaultCipherSdkService", () => { ); }); }); + + describe("deleteWithServer()", () => { + const testCipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + + it("should delete cipher using SDK when asAdmin is false", async () => { + await cipherSdkService.deleteWithServer(testCipherId, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.delete).toHaveBeenCalledWith(testCipherId); + expect(mockCiphersSdk.admin).not.toHaveBeenCalled(); + }); + + it("should delete cipher using SDK admin API when asAdmin is true", async () => { + await cipherSdkService.deleteWithServer(testCipherId, userId, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.delete).toHaveBeenCalledWith(testCipherId); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + + await expect(cipherSdkService.deleteWithServer(testCipherId, userId)).rejects.toThrow( + "SDK not available", + ); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to delete cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + mockCiphersSdk.delete.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.deleteWithServer(testCipherId, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to delete cipher"), + ); + }); + }); + + describe("deleteManyWithServer()", () => { + const testCipherIds = [ + "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23" as CipherId, + ]; + + it("should delete multiple ciphers using SDK when asAdmin is false", async () => { + await cipherSdkService.deleteManyWithServer(testCipherIds, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.delete_many).toHaveBeenCalledWith(testCipherIds); + expect(mockCiphersSdk.admin).not.toHaveBeenCalled(); + }); + + it("should delete multiple ciphers using SDK admin API when asAdmin is true", async () => { + await cipherSdkService.deleteManyWithServer(testCipherIds, userId, true, orgId); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.delete_many).toHaveBeenCalledWith(testCipherIds, orgId); + }); + + it("should throw error when asAdmin is true but orgId is missing", async () => { + await expect( + cipherSdkService.deleteManyWithServer(testCipherIds, userId, true, undefined), + ).rejects.toThrow("Organization ID is required for admin delete."); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + + await expect(cipherSdkService.deleteManyWithServer(testCipherIds, userId)).rejects.toThrow( + "SDK not available", + ); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to delete multiple ciphers"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + mockCiphersSdk.delete_many.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.deleteManyWithServer(testCipherIds, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to delete multiple ciphers"), + ); + }); + }); + + describe("softDeleteWithServer()", () => { + const testCipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + + it("should soft delete cipher using SDK when asAdmin is false", async () => { + await cipherSdkService.softDeleteWithServer(testCipherId, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.soft_delete).toHaveBeenCalledWith(testCipherId); + expect(mockCiphersSdk.admin).not.toHaveBeenCalled(); + }); + + it("should soft delete cipher using SDK admin API when asAdmin is true", async () => { + await cipherSdkService.softDeleteWithServer(testCipherId, userId, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.soft_delete).toHaveBeenCalledWith(testCipherId); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + + await expect(cipherSdkService.softDeleteWithServer(testCipherId, userId)).rejects.toThrow( + "SDK not available", + ); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to soft delete cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + mockCiphersSdk.soft_delete.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.softDeleteWithServer(testCipherId, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to soft delete cipher"), + ); + }); + }); + + describe("softDeleteManyWithServer()", () => { + const testCipherIds = [ + "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23" as CipherId, + ]; + + it("should soft delete multiple ciphers using SDK when asAdmin is false", async () => { + await cipherSdkService.softDeleteManyWithServer(testCipherIds, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.soft_delete_many).toHaveBeenCalledWith(testCipherIds); + expect(mockCiphersSdk.admin).not.toHaveBeenCalled(); + }); + + it("should soft delete multiple ciphers using SDK admin API when asAdmin is true", async () => { + await cipherSdkService.softDeleteManyWithServer(testCipherIds, userId, true, orgId); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.soft_delete_many).toHaveBeenCalledWith(testCipherIds, orgId); + }); + + it("should throw error when asAdmin is true but orgId is missing", async () => { + await expect( + cipherSdkService.softDeleteManyWithServer(testCipherIds, userId, true, undefined), + ).rejects.toThrow("Organization ID is required for admin soft delete."); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + + await expect( + cipherSdkService.softDeleteManyWithServer(testCipherIds, userId), + ).rejects.toThrow("SDK not available"); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to soft delete multiple ciphers"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + mockCiphersSdk.soft_delete_many.mockRejectedValue(new Error("SDK error")); + + await expect( + cipherSdkService.softDeleteManyWithServer(testCipherIds, userId), + ).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to soft delete multiple ciphers"), + ); + }); + }); + + describe("restoreWithServer()", () => { + const testCipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + + it("should restore cipher using SDK when asAdmin is false", async () => { + await cipherSdkService.restoreWithServer(testCipherId, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.restore).toHaveBeenCalledWith(testCipherId); + expect(mockCiphersSdk.admin).not.toHaveBeenCalled(); + }); + + it("should restore cipher using SDK admin API when asAdmin is true", async () => { + await cipherSdkService.restoreWithServer(testCipherId, userId, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.restore).toHaveBeenCalledWith(testCipherId); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + + await expect(cipherSdkService.restoreWithServer(testCipherId, userId)).rejects.toThrow( + "SDK not available", + ); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to restore cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + mockCiphersSdk.restore.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.restoreWithServer(testCipherId, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to restore cipher"), + ); + }); + }); + + describe("restoreManyWithServer()", () => { + const testCipherIds = [ + "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23" as CipherId, + ]; + + it("should restore multiple ciphers using SDK when orgId is not provided", async () => { + await cipherSdkService.restoreManyWithServer(testCipherIds, userId); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.restore_many).toHaveBeenCalledWith(testCipherIds); + expect(mockCiphersSdk.admin).not.toHaveBeenCalled(); + }); + + it("should restore multiple ciphers using SDK admin API when orgId is provided", async () => { + const orgIdString = orgId as string; + await cipherSdkService.restoreManyWithServer(testCipherIds, userId, orgIdString); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.restore_many).toHaveBeenCalledWith(testCipherIds, orgIdString); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + + await expect(cipherSdkService.restoreManyWithServer(testCipherIds, userId)).rejects.toThrow( + "SDK not available", + ); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to restore multiple ciphers"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + mockCiphersSdk.restore_many.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.restoreManyWithServer(testCipherIds, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to restore multiple ciphers"), + ); + }); + }); }); diff --git a/libs/common/src/vault/services/cipher-sdk.service.ts b/libs/common/src/vault/services/cipher-sdk.service.ts index 06f5d3eb961..9757b3d2cc7 100644 --- a/libs/common/src/vault/services/cipher-sdk.service.ts +++ b/libs/common/src/vault/services/cipher-sdk.service.ts @@ -1,8 +1,8 @@ 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 { SdkService, asUuid } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherView as SdkCipherView } from "@bitwarden/sdk-internal"; @@ -79,4 +79,185 @@ export class DefaultCipherSdkService implements CipherSdkService { ), ); } + + async deleteWithServer(id: string, userId: UserId, asAdmin = false): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + if (asAdmin) { + await ref.value.vault().ciphers().admin().delete(asUuid(id)); + } else { + await ref.value.vault().ciphers().delete(asUuid(id)); + } + }), + catchError((error: unknown) => { + this.logService.error(`Failed to delete cipher: ${error}`); + throw error; + }), + ), + ); + } + + async deleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin = false, + orgId?: OrganizationId, + ): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + if (asAdmin) { + if (orgId == null) { + throw new Error("Organization ID is required for admin delete."); + } + await ref.value + .vault() + .ciphers() + .admin() + .delete_many( + ids.map((id) => asUuid(id)), + asUuid(orgId), + ); + } else { + await ref.value + .vault() + .ciphers() + .delete_many(ids.map((id) => asUuid(id))); + } + }), + catchError((error: unknown) => { + this.logService.error(`Failed to delete multiple ciphers: ${error}`); + throw error; + }), + ), + ); + } + + async softDeleteWithServer(id: string, userId: UserId, asAdmin = false): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + if (asAdmin) { + await ref.value.vault().ciphers().admin().soft_delete(asUuid(id)); + } else { + await ref.value.vault().ciphers().soft_delete(asUuid(id)); + } + }), + catchError((error: unknown) => { + this.logService.error(`Failed to soft delete cipher: ${error}`); + throw error; + }), + ), + ); + } + + async softDeleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin = false, + orgId?: OrganizationId, + ): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + if (asAdmin) { + if (orgId == null) { + throw new Error("Organization ID is required for admin soft delete."); + } + await ref.value + .vault() + .ciphers() + .admin() + .soft_delete_many( + ids.map((id) => asUuid(id)), + asUuid(orgId), + ); + } else { + await ref.value + .vault() + .ciphers() + .soft_delete_many(ids.map((id) => asUuid(id))); + } + }), + catchError((error: unknown) => { + this.logService.error(`Failed to soft delete multiple ciphers: ${error}`); + throw error; + }), + ), + ); + } + + async restoreWithServer(id: string, userId: UserId, asAdmin = false): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + if (asAdmin) { + await ref.value.vault().ciphers().admin().restore(asUuid(id)); + } else { + await ref.value.vault().ciphers().restore(asUuid(id)); + } + }), + catchError((error: unknown) => { + this.logService.error(`Failed to restore cipher: ${error}`); + throw error; + }), + ), + ); + } + + async restoreManyWithServer(ids: string[], userId: UserId, orgId?: string): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + + // No longer using an asAdmin Param. Org Vault bulkRestore will assess if an item is unassigned or editable + // The Org Vault will pass those ids an array as well as the orgId when calling bulkRestore + if (orgId) { + await ref.value + .vault() + .ciphers() + .admin() + .restore_many( + ids.map((id) => asUuid(id)), + asUuid(orgId), + ); + } else { + await ref.value + .vault() + .ciphers() + .restore_many(ids.map((id) => asUuid(id))); + } + }), + catchError((error: unknown) => { + this.logService.error(`Failed to restore multiple ciphers: ${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 4f98ba62a1c..07444d5d1c6 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -117,6 +117,8 @@ describe("Cipher Service", () => { let cipherService: CipherService; let encryptionContext: EncryptionContext; + // BehaviorSubject for SDK feature flag - allows tests to change the value after service instantiation + let sdkCrudFeatureFlag$: BehaviorSubject; beforeEach(() => { encryptService.encryptFileData.mockReturnValue(Promise.resolve(ENCRYPTED_BYTES)); @@ -132,6 +134,10 @@ describe("Cipher Service", () => { (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); + // Create BehaviorSubject for SDK feature flag - tests can update this to change behavior + sdkCrudFeatureFlag$ = new BehaviorSubject(false); + configService.getFeatureFlag$.mockReturnValue(sdkCrudFeatureFlag$.asObservable()); + cipherService = new CipherService( keyService, domainSettingsService, @@ -280,9 +286,7 @@ describe("Cipher Service", () => { }); it("should delegate to cipherSdkService when feature flag is enabled", async () => { - configService.getFeatureFlag - .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) - .mockResolvedValue(true); + sdkCrudFeatureFlag$.next(true); const cipherView = new CipherView(encryptionContext.cipher); const expectedResult = new CipherView(encryptionContext.cipher); @@ -315,9 +319,9 @@ describe("Cipher Service", () => { }); it("should call apiService.putCipherAdmin when orgAdmin param is true", async () => { - configService.getFeatureFlag + configService.getFeatureFlag$ .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) - .mockResolvedValue(false); + .mockReturnValue(of(false)); const testCipher = new Cipher(cipherData); testCipher.organizationId = orgId; @@ -368,9 +372,7 @@ describe("Cipher Service", () => { }); it("should delegate to cipherSdkService when feature flag is enabled", async () => { - configService.getFeatureFlag - .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) - .mockResolvedValue(true); + sdkCrudFeatureFlag$.next(true); const testCipher = new Cipher(cipherData); const cipherView = new CipherView(testCipher); @@ -392,9 +394,7 @@ describe("Cipher Service", () => { }); it("should delegate to cipherSdkService with orgAdmin when feature flag is enabled", async () => { - configService.getFeatureFlag - .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) - .mockResolvedValue(true); + sdkCrudFeatureFlag$.next(true); const testCipher = new Cipher(cipherData); const cipherView = new CipherView(testCipher); @@ -1009,6 +1009,238 @@ describe("Cipher Service", () => { }); }); + describe("deleteWithServer()", () => { + const testCipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + + it("should call apiService.deleteCipher when feature flag is disabled", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "deleteCipher").mockResolvedValue(undefined); + + await cipherService.deleteWithServer(testCipherId, userId); + + expect(apiSpy).toHaveBeenCalledWith(testCipherId); + }); + + it("should call apiService.deleteCipherAdmin when feature flag is disabled and asAdmin is true", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "deleteCipherAdmin").mockResolvedValue(undefined); + + await cipherService.deleteWithServer(testCipherId, userId, true); + + expect(apiSpy).toHaveBeenCalledWith(testCipherId); + }); + + it("should use SDK to delete cipher when feature flag is enabled", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "deleteWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.deleteWithServer(testCipherId, userId, false); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherId, userId, false); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + + it("should use SDK admin delete when feature flag is enabled and asAdmin is true", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "deleteWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.deleteWithServer(testCipherId, userId, true); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherId, userId, true); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + }); + + describe("deleteManyWithServer()", () => { + const testCipherIds = [ + "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23" as CipherId, + ]; + + it("should call apiService.deleteManyCiphers when feature flag is disabled", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "deleteManyCiphers").mockResolvedValue(undefined); + + await cipherService.deleteManyWithServer(testCipherIds, userId); + + expect(apiSpy).toHaveBeenCalled(); + }); + + it("should call apiService.deleteManyCiphersAdmin when feature flag is disabled and asAdmin is true", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "deleteManyCiphersAdmin").mockResolvedValue(undefined); + + await cipherService.deleteManyWithServer(testCipherIds, userId, true, orgId); + + expect(apiSpy).toHaveBeenCalled(); + }); + + it("should use SDK to delete multiple ciphers when feature flag is enabled", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "deleteManyWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.deleteManyWithServer(testCipherIds, userId, false); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherIds, userId, false, undefined); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + + it("should use SDK admin delete many when feature flag is enabled and asAdmin is true", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "deleteManyWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.deleteManyWithServer(testCipherIds, userId, true, orgId); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherIds, userId, true, orgId); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + }); + + describe("softDeleteWithServer()", () => { + const testCipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + + it("should call apiService.putDeleteCipher when feature flag is disabled", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "putDeleteCipher").mockResolvedValue(undefined); + + await cipherService.softDeleteWithServer(testCipherId, userId); + + expect(apiSpy).toHaveBeenCalledWith(testCipherId); + }); + + it("should call apiService.putDeleteCipherAdmin when feature flag is disabled and asAdmin is true", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "putDeleteCipherAdmin").mockResolvedValue(undefined); + + await cipherService.softDeleteWithServer(testCipherId, userId, true); + + expect(apiSpy).toHaveBeenCalledWith(testCipherId); + }); + + it("should use SDK to soft delete cipher when feature flag is enabled", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "softDeleteWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.softDeleteWithServer(testCipherId, userId, false); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherId, userId, false); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + + it("should use SDK admin soft delete when feature flag is enabled and asAdmin is true", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "softDeleteWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.softDeleteWithServer(testCipherId, userId, true); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherId, userId, true); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + }); + + describe("softDeleteManyWithServer()", () => { + const testCipherIds = [ + "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23" as CipherId, + ]; + + it("should call apiService.putDeleteManyCiphers when feature flag is disabled", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest.spyOn(apiService, "putDeleteManyCiphers").mockResolvedValue(undefined); + + await cipherService.softDeleteManyWithServer(testCipherIds, userId); + + expect(apiSpy).toHaveBeenCalled(); + }); + + it("should call apiService.putDeleteManyCiphersAdmin when feature flag is disabled and asAdmin is true", async () => { + configService.getFeatureFlag$ + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockReturnValue(of(false)); + + const apiSpy = jest + .spyOn(apiService, "putDeleteManyCiphersAdmin") + .mockResolvedValue(undefined); + + await cipherService.softDeleteManyWithServer(testCipherIds, userId, true, orgId); + + expect(apiSpy).toHaveBeenCalled(); + }); + + it("should use SDK to soft delete multiple ciphers when feature flag is enabled", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "softDeleteManyWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.softDeleteManyWithServer(testCipherIds, userId, false); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherIds, userId, false, undefined); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + + it("should use SDK admin soft delete many when feature flag is enabled and asAdmin is true", async () => { + sdkCrudFeatureFlag$.next(true); + + const sdkServiceSpy = jest + .spyOn(cipherSdkService, "softDeleteManyWithServer") + .mockResolvedValue(undefined); + const clearCacheSpy = jest.spyOn(cipherService as any, "clearCache"); + + await cipherService.softDeleteManyWithServer(testCipherIds, userId, true, orgId); + + expect(sdkServiceSpy).toHaveBeenCalledWith(testCipherIds, userId, true, orgId); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + }); + }); + describe("replace (no upsert)", () => { // In order to set up initial state we need to manually update the encrypted state // which will result in an emission. All tests will have this baseline emission. diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 53d7666e304..1fc455a1ae9 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -106,6 +106,13 @@ export class CipherService implements CipherServiceAbstraction { */ private clearCipherViewsForUser$: Subject = new Subject(); + /** + * Observable exposing the feature flag status for using the SDK for cipher CRUD operations. + */ + private readonly sdkCipherCrudEnabled$: Observable = this.configService.getFeatureFlag$( + FeatureFlag.PM27632_SdkCipherCrudOperations, + ); + constructor( private keyService: KeyService, private domainSettingsService: DomainSettingsService, @@ -909,9 +916,7 @@ export class CipherService implements CipherServiceAbstraction { userId: UserId, orgAdmin?: boolean, ): Promise { - const useSdk = await this.configService.getFeatureFlag( - FeatureFlag.PM27632_SdkCipherCrudOperations, - ); + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); if (useSdk) { return ( @@ -970,9 +975,7 @@ export class CipherService implements CipherServiceAbstraction { originalCipherView?: CipherView, orgAdmin?: boolean, ): Promise { - const useSdk = await this.configService.getFeatureFlag( - FeatureFlag.PM27632_SdkCipherCrudOperations, - ); + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); if (useSdk) { return await this.updateWithServerUsingSdk(cipherView, userId, originalCipherView, orgAdmin); @@ -1389,7 +1392,14 @@ export class CipherService implements CipherServiceAbstraction { await this.encryptedCiphersState(userId).update(() => ciphers); } - async deleteWithServer(id: string, userId: UserId, asAdmin = false): Promise { + async deleteWithServer(id: string, userId: UserId, asAdmin = false): Promise { + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); + if (useSdk) { + await this.cipherSdkService.deleteWithServer(id, userId, asAdmin); + await this.clearCache(userId); + return; + } + if (asAdmin) { await this.apiService.deleteCipherAdmin(id); } else { @@ -1399,7 +1409,19 @@ export class CipherService implements CipherServiceAbstraction { await this.delete(id, userId); } - async deleteManyWithServer(ids: string[], userId: UserId, asAdmin = false): Promise { + async deleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin = false, + orgId?: OrganizationId, + ): Promise { + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); + if (useSdk) { + await this.cipherSdkService.deleteManyWithServer(ids, userId, asAdmin, orgId); + await this.clearCache(userId); + return; + } + const request = new CipherBulkDeleteRequest(ids); if (asAdmin) { await this.apiService.deleteManyCiphersAdmin(request); @@ -1539,7 +1561,7 @@ export class CipherService implements CipherServiceAbstraction { }; } - async softDelete(id: string | string[], userId: UserId): Promise { + async softDelete(id: string | string[], userId: UserId): Promise { let ciphers = await firstValueFrom(this.ciphers$(userId)); if (ciphers == null) { return; @@ -1567,7 +1589,14 @@ export class CipherService implements CipherServiceAbstraction { }); } - async softDeleteWithServer(id: string, userId: UserId, asAdmin = false): Promise { + async softDeleteWithServer(id: string, userId: UserId, asAdmin = false): Promise { + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); + if (useSdk) { + await this.cipherSdkService.softDeleteWithServer(id, userId, asAdmin); + await this.clearCache(userId); + return; + } + if (asAdmin) { await this.apiService.putDeleteCipherAdmin(id); } else { @@ -1577,7 +1606,19 @@ export class CipherService implements CipherServiceAbstraction { await this.softDelete(id, userId); } - async softDeleteManyWithServer(ids: string[], userId: UserId, asAdmin = false): Promise { + async softDeleteManyWithServer( + ids: string[], + userId: UserId, + asAdmin = false, + orgId?: OrganizationId, + ): Promise { + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); + if (useSdk) { + await this.cipherSdkService.softDeleteManyWithServer(ids, userId, asAdmin, orgId); + await this.clearCache(userId); + return; + } + const request = new CipherBulkDeleteRequest(ids); if (asAdmin) { await this.apiService.putDeleteManyCiphersAdmin(request); @@ -1621,7 +1662,14 @@ export class CipherService implements CipherServiceAbstraction { }); } - async restoreWithServer(id: string, userId: UserId, asAdmin = false): Promise { + async restoreWithServer(id: string, userId: UserId, asAdmin = false): Promise { + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); + if (useSdk) { + await this.cipherSdkService.restoreWithServer(id, userId, asAdmin); + await this.clearCache(userId); + return; + } + let response; if (asAdmin) { response = await this.apiService.putRestoreCipherAdmin(id); @@ -1637,6 +1685,13 @@ export class CipherService implements CipherServiceAbstraction { * The Org Vault will pass those ids an array as well as the orgId when calling bulkRestore */ async restoreManyWithServer(ids: string[], userId: UserId, orgId?: string): Promise { + const useSdk = await firstValueFrom(this.sdkCipherCrudEnabled$); + if (useSdk) { + await this.cipherSdkService.restoreManyWithServer(ids, userId, orgId); + await this.clearCache(userId); + return; + } + let response; if (orgId) { From ec812a7d77b3650d5ddf23b3ceaf45a08cc5b30c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Tue, 27 Jan 2026 10:46:35 +0100 Subject: [PATCH 04/24] Wire up DI for PRFUnlockService in desktop (#18587) --- .../src/app/services/services.module.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 66613efd115..4fac2555b85 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -33,6 +33,7 @@ import { InternalUserDecryptionOptionsServiceAbstraction, LoginEmailService, SsoUrlService, + UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; @@ -53,6 +54,7 @@ import { import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; +import { WebAuthnLoginPrfKeyServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login-prf-key.service.abstraction"; import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; @@ -123,6 +125,8 @@ import { import { LockComponentService, SessionTimeoutSettingsComponentService, + WebAuthnPrfUnlockService, + DefaultWebAuthnPrfUnlockService, } from "@bitwarden/key-management-ui"; import { SerializedMemoryStorageService } from "@bitwarden/storage-core"; import { @@ -413,6 +417,21 @@ const safeProviders: SafeProvider[] = [ useClass: DesktopLockComponentService, deps: [], }), + safeProvider({ + provide: WebAuthnPrfUnlockService, + useClass: DefaultWebAuthnPrfUnlockService, + deps: [ + WebAuthnLoginPrfKeyServiceAbstraction, + KeyServiceAbstraction, + UserDecryptionOptionsServiceAbstraction, + EncryptService, + EnvironmentService, + PlatformUtilsServiceAbstraction, + WINDOW, + LogServiceAbstraction, + ConfigService, + ], + }), safeProvider({ provide: CLIENT_TYPE, useValue: ClientType.Desktop, From 9454189df59ed39e3d2e9c321cae2684a7b4c066 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Tue, 27 Jan 2026 11:28:13 +0100 Subject: [PATCH 05/24] [PM-27283] [BEEEP] Reactive `availableVaultTimeoutActions$` in vault timeout settings (#17731) * reactive `availableVaultTimeoutActions$` in vault timeout settings * cleanup * deprecation docs * explicitly provided user id * clearer mocking * better docs --- .../settings/account-security.component.ts | 2 +- .../background-browser-biometrics.service.ts | 2 +- .../extension-lock-component.service.spec.ts | 3 +- .../extension-lock-component.service.ts | 2 +- .../src/app/accounts/settings.component.ts | 2 +- .../account-security-nudge.service.ts | 2 +- .../pin/pin-state.service.abstraction.ts | 21 ++- .../pin/pin-state.service.implementation.ts | 68 +++++--- .../pin/pin-state.service.spec.ts | 50 +++++- .../vault-timeout-settings.service.ts | 5 +- .../vault-timeout-settings.service.spec.ts | 160 ++++++++++++------ .../vault-timeout-settings.service.ts | 127 ++++++-------- .../biometric-state.service.spec.ts | 52 +++--- .../src/biometrics/biometric-state.service.ts | 18 +- 14 files changed, 309 insertions(+), 205 deletions(-) diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 6a3378670bf..1789feebe4e 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -257,7 +257,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { pin: await this.pinService.isPinSet(activeAccount.id), pinLockWithMasterPassword: (await this.pinService.getPinLockType(activeAccount.id)) == "EPHEMERAL", - biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(), + biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(activeAccount.id), enableAutoBiometricsPrompt: await firstValueFrom( this.biometricStateService.promptAutomatically$, ), diff --git a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts index c8be58b0bde..d7e755b34ea 100644 --- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts +++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts @@ -35,7 +35,7 @@ export class BackgroundBrowserBiometricsService extends BiometricsService { super(); // Always connect to the native messaging background if biometrics are enabled, not just when it is used // so that there is no wait when used. - const biometricsEnabled = this.biometricStateService.biometricUnlockEnabled$; + const biometricsEnabled = this.biometricStateService.biometricUnlockEnabled$(); combineLatest([timer(0, this.BACKGROUND_POLLING_INTERVAL), biometricsEnabled]) .pipe( diff --git a/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts b/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts index ecdb899b9a7..934fb9307ee 100644 --- a/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts +++ b/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts @@ -375,7 +375,7 @@ describe("ExtensionLockComponentService", () => { platformUtilsService.supportsSecureStorage.mockReturnValue( mockInputs.platformSupportsSecureStorage, ); - biometricStateService.biometricUnlockEnabled$ = of(true); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(true)); // PIN pinService.isPinDecryptionAvailable.mockResolvedValue(mockInputs.pinDecryptionAvailable); @@ -386,6 +386,7 @@ describe("ExtensionLockComponentService", () => { const unlockOptions = await firstValueFrom(service.getAvailableUnlockOptions$(userId)); expect(unlockOptions).toEqual(expectedOutput); + expect(biometricStateService.biometricUnlockEnabled$).toHaveBeenCalledWith(userId); }); }); }); diff --git a/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts b/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts index 5e6e564bbc2..1ed9d1ea967 100644 --- a/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts +++ b/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts @@ -69,7 +69,7 @@ export class ExtensionLockComponentService implements LockComponentService { return combineLatest([ // Note: defer is preferable b/c it delays the execution of the function until the observable is subscribed to defer(async () => { - if (!(await firstValueFrom(this.biometricStateService.biometricUnlockEnabled$))) { + if (!(await firstValueFrom(this.biometricStateService.biometricUnlockEnabled$(userId)))) { return BiometricsStatus.NotEnabledLocally; } else { // TODO remove after 2025.3 diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 3952335af48..f2e828b95ce 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -385,7 +385,7 @@ export class SettingsComponent implements OnInit, OnDestroy { this.vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(activeAccount.id), ), pin: this.userHasPinSet, - biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(), + biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(activeAccount.id), requireMasterPasswordOnAppRestart: !(await this.biometricsService.hasPersistentKey( activeAccount.id, )), diff --git a/libs/angular/src/vault/services/custom-nudges-services/account-security-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/account-security-nudge.service.ts index 835c9e35ac7..ab8a1869266 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/account-security-nudge.service.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/account-security-nudge.service.ts @@ -39,7 +39,7 @@ export class AccountSecurityNudgeService extends DefaultSingleNudgeService { this.getNudgeStatus$(nudgeType, userId), of(Date.now() - THIRTY_DAYS_MS), from(this.pinService.isPinSet(userId)), - this.biometricStateService.biometricUnlockEnabled$, + this.biometricStateService.biometricUnlockEnabled$(userId), this.organizationService.organizations$(userId), this.policyService.policiesByType$(PolicyType.RemoveUnlockWithPin, userId), ]).pipe( diff --git a/libs/common/src/key-management/pin/pin-state.service.abstraction.ts b/libs/common/src/key-management/pin/pin-state.service.abstraction.ts index 4aef268c1c4..d577d75ef6f 100644 --- a/libs/common/src/key-management/pin/pin-state.service.abstraction.ts +++ b/libs/common/src/key-management/pin/pin-state.service.abstraction.ts @@ -11,6 +11,20 @@ import { PinLockType } from "./pin-lock-type"; * The PinStateService manages the storage and retrieval of PIN-related state for user accounts. */ export abstract class PinStateServiceAbstraction { + /** + * Checks if a user is enrolled into PIN unlock + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract pinSet$(userId: UserId): Observable; + + /** + * Gets the user's {@link PinLockType} + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract pinLockType$(userId: UserId): Observable; + /** * Gets the user's UserKey encrypted PIN * @deprecated - This is not a public API. DO NOT USE IT @@ -21,17 +35,12 @@ export abstract class PinStateServiceAbstraction { /** * Gets the user's {@link PinLockType} + * @deprecated Use {@link pinLockType$} instead * @param userId The user's id * @throws If the user id is not provided */ abstract getPinLockType(userId: UserId): Promise; - /** - * Checks if a user is enrolled into PIN unlock - * @param userId The user's id - */ - abstract isPinSet(userId: UserId): Promise; - /** * Gets the user's PIN-protected UserKey envelope, either persistent or ephemeral based on the provided PinLockType * @deprecated - This is not a public API. DO NOT USE IT diff --git a/libs/common/src/key-management/pin/pin-state.service.implementation.ts b/libs/common/src/key-management/pin/pin-state.service.implementation.ts index d5b2608f280..10046191c01 100644 --- a/libs/common/src/key-management/pin/pin-state.service.implementation.ts +++ b/libs/common/src/key-management/pin/pin-state.service.implementation.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, map, Observable } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable } from "rxjs"; import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; import { StateProvider } from "@bitwarden/state"; @@ -26,27 +26,36 @@ export class PinStateService implements PinStateServiceAbstraction { .pipe(map((value) => (value ? new EncString(value) : null))); } - async isPinSet(userId: UserId): Promise { + pinSet$(userId: UserId): Observable { assertNonNullish(userId, "userId"); - return (await this.getPinLockType(userId)) !== "DISABLED"; + return this.pinLockType$(userId).pipe(map((pinLockType) => pinLockType !== "DISABLED")); + } + + pinLockType$(userId: UserId): Observable { + assertNonNullish(userId, "userId"); + + return combineLatest([ + this.pinProtectedUserKeyEnvelope$(userId, "PERSISTENT").pipe(map((key) => key != null)), + this.stateProvider + .getUserState$(USER_KEY_ENCRYPTED_PIN, userId) + .pipe(map((key) => key != null)), + ]).pipe( + map(([isPersistentPinSet, isPinSet]) => { + if (isPersistentPinSet) { + return "PERSISTENT"; + } else if (isPinSet) { + return "EPHEMERAL"; + } else { + return "DISABLED"; + } + }), + ); } async getPinLockType(userId: UserId): Promise { assertNonNullish(userId, "userId"); - const isPersistentPinSet = - (await this.getPinProtectedUserKeyEnvelope(userId, "PERSISTENT")) != null; - const isPinSet = - (await firstValueFrom(this.stateProvider.getUserState$(USER_KEY_ENCRYPTED_PIN, userId))) != - null; - - if (isPersistentPinSet) { - return "PERSISTENT"; - } else if (isPinSet) { - return "EPHEMERAL"; - } else { - return "DISABLED"; - } + return await firstValueFrom(this.pinLockType$(userId)); } async getPinProtectedUserKeyEnvelope( @@ -55,17 +64,7 @@ export class PinStateService implements PinStateServiceAbstraction { ): Promise { assertNonNullish(userId, "userId"); - if (pinLockType === "EPHEMERAL") { - return await firstValueFrom( - this.stateProvider.getUserState$(PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, userId), - ); - } else if (pinLockType === "PERSISTENT") { - return await firstValueFrom( - this.stateProvider.getUserState$(PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, userId), - ); - } else { - throw new Error(`Unsupported PinLockType: ${pinLockType}`); - } + return await firstValueFrom(this.pinProtectedUserKeyEnvelope$(userId, pinLockType)); } async setPinState( @@ -110,4 +109,19 @@ export class PinStateService implements PinStateServiceAbstraction { await this.stateProvider.setUserState(PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, null, userId); } + + private pinProtectedUserKeyEnvelope$( + userId: UserId, + pinLockType: PinLockType, + ): Observable { + assertNonNullish(userId, "userId"); + + if (pinLockType === "EPHEMERAL") { + return this.stateProvider.getUserState$(PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, userId); + } else if (pinLockType === "PERSISTENT") { + return this.stateProvider.getUserState$(PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, userId); + } else { + throw new Error(`Unsupported PinLockType: ${pinLockType}`); + } + } } diff --git a/libs/common/src/key-management/pin/pin-state.service.spec.ts b/libs/common/src/key-management/pin/pin-state.service.spec.ts index 7406701c28d..42dcce9fedc 100644 --- a/libs/common/src/key-management/pin/pin-state.service.spec.ts +++ b/libs/common/src/key-management/pin/pin-state.service.spec.ts @@ -1,4 +1,4 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, of } from "rxjs"; import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; @@ -94,14 +94,50 @@ describe("PinStateService", () => { }); }); - describe("getPinLockType()", () => { + describe("pinSet$", () => { beforeEach(() => { jest.clearAllMocks(); }); it("should throw an error if userId is null", async () => { // Act & Assert - await expect(sut.getPinLockType(null as any)).rejects.toThrow("userId"); + expect(() => sut.pinSet$(null as any)).toThrow("userId"); + }); + + it("should return false when pin lock type is DISABLED", async () => { + // Arrange + jest.spyOn(sut, "pinLockType$").mockReturnValue(of("DISABLED")); + + // Act + const result = await firstValueFrom(sut.pinSet$(mockUserId)); + + // Assert + expect(result).toBe(false); + }); + + it.each([["PERSISTENT" as PinLockType], ["EPHEMERAL" as PinLockType]])( + "should return true when pin lock type is %s", + async (pinLockType) => { + // Arrange + jest.spyOn(sut, "pinLockType$").mockReturnValue(of(pinLockType)); + + // Act + const result = await firstValueFrom(sut.pinSet$(mockUserId)); + + // Assert + expect(result).toBe(true); + }, + ); + }); + + describe("pinLockType$", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should throw an error if userId is null", async () => { + // Act & Assert + expect(() => sut.pinLockType$(null as any)).toThrow("userId"); }); it("should return 'PERSISTENT' if a pin protected user key (persistent) is found", async () => { @@ -114,7 +150,7 @@ describe("PinStateService", () => { ); // Act - const result = await sut.getPinLockType(mockUserId); + const result = await firstValueFrom(sut.pinLockType$(mockUserId)); // Assert expect(result).toBe("PERSISTENT"); @@ -125,7 +161,7 @@ describe("PinStateService", () => { await stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, mockUserKeyEncryptedPin, mockUserId); // Act - const result = await sut.getPinLockType(mockUserId); + const result = await firstValueFrom(sut.pinLockType$(mockUserId)); // Assert expect(result).toBe("EPHEMERAL"); @@ -135,7 +171,7 @@ describe("PinStateService", () => { // Arrange - don't set any PIN-related state // Act - const result = await sut.getPinLockType(mockUserId); + const result = await firstValueFrom(sut.pinLockType$(mockUserId)); // Assert expect(result).toBe("DISABLED"); @@ -151,7 +187,7 @@ describe("PinStateService", () => { await stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, null, mockUserId); // Act - const result = await sut.getPinLockType(mockUserId); + const result = await firstValueFrom(sut.pinLockType$(mockUserId)); // Assert expect(result).toBe("DISABLED"); diff --git a/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts index 697b8a1875c..44108b69513 100644 --- a/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts @@ -20,10 +20,9 @@ export abstract class VaultTimeoutSettingsService { /** * Get the available vault timeout actions for the current user * - * **NOTE:** This observable is not yet connected to the state service, so it will not update when the state changes * @param userId The user id to check. If not provided, the current user is used */ - abstract availableVaultTimeoutActions$(userId?: string): Observable; + abstract availableVaultTimeoutActions$(userId?: UserId): Observable; /** * Evaluates the user's available vault timeout actions and returns a boolean representing @@ -55,5 +54,5 @@ export abstract class VaultTimeoutSettingsService { * @param userId The user id to check. If not provided, the current user is used * @returns boolean true if biometric lock is set */ - abstract isBiometricLockSet(userId?: string): Promise; + abstract isBiometricLockSet(userId?: UserId): Promise; } diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts index 3c391344f04..3fa71598e65 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts @@ -78,7 +78,8 @@ describe("VaultTimeoutSettingsService", () => { vaultTimeoutSettingsService = createVaultTimeoutSettingsService(defaultVaultTimeout); - biometricStateService.biometricUnlockEnabled$ = of(false); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); }); afterEach(() => { @@ -86,72 +87,121 @@ describe("VaultTimeoutSettingsService", () => { }); describe("availableVaultTimeoutActions$", () => { - it("always returns LogOut", async () => { - const result = await firstValueFrom( - vaultTimeoutSettingsService.availableVaultTimeoutActions$(), - ); + describe("when no userId provided (active user)", () => { + it("always returns LogOut", async () => { + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); - expect(result).toContain(VaultTimeoutAction.LogOut); + expect(result).toContain(VaultTimeoutAction.LogOut); + }); + + it("contains Lock when the user has a master password", async () => { + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: true })); + + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + + expect(userDecryptionOptionsService.hasMasterPasswordById$).toHaveBeenCalledWith( + mockUserId, + ); + expect(result).toContain(VaultTimeoutAction.Lock); + }); + + it("contains Lock when the user has either a persistent or ephemeral PIN configured", async () => { + pinStateService.pinSet$.mockReturnValue(of(true)); + + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + + expect(result).toContain(VaultTimeoutAction.Lock); + }); + + it("contains Lock when the user has biometrics configured", async () => { + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(true)); + biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true); + + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + + expect(result).toContain(VaultTimeoutAction.Lock); + }); + + it("not contains Lock when the user does not have a master password, PIN, or biometrics", async () => { + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + + expect(result).not.toContain(VaultTimeoutAction.Lock); + }); + + it("should throw error when activeAccount$ is null", async () => { + accountService.activeAccountSubject.next(null); + + const result$ = vaultTimeoutSettingsService.availableVaultTimeoutActions$(); + + await expect(firstValueFrom(result$)).rejects.toThrow("Null or undefined account"); + }); }); - it("contains Lock when the user has a master password", async () => { - userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: true })); + describe("with explicit userId parameter", () => { + it("should return Lock and LogOut when provided user has master password", async () => { + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); - const result = await firstValueFrom( - vaultTimeoutSettingsService.availableVaultTimeoutActions$(), - ); + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(mockUserId), + ); - expect(result).toContain(VaultTimeoutAction.Lock); - }); + expect(userDecryptionOptionsService.hasMasterPasswordById$).toHaveBeenCalledWith( + mockUserId, + ); + expect(result).toContain(VaultTimeoutAction.Lock); + expect(result).toContain(VaultTimeoutAction.LogOut); + }); - it("contains Lock when the user has either a persistent or ephemeral PIN configured", async () => { - pinStateService.isPinSet.mockResolvedValue(true); + it("should return Lock and LogOut when provided user has PIN configured", async () => { + pinStateService.pinSet$.mockReturnValue(of(true)); - const result = await firstValueFrom( - vaultTimeoutSettingsService.availableVaultTimeoutActions$(), - ); + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(mockUserId), + ); - expect(result).toContain(VaultTimeoutAction.Lock); - }); + expect(pinStateService.pinSet$).toHaveBeenCalledWith(mockUserId); + expect(result).toContain(VaultTimeoutAction.Lock); + expect(result).toContain(VaultTimeoutAction.LogOut); + }); - it("contains Lock when the user has biometrics configured", async () => { - biometricStateService.biometricUnlockEnabled$ = of(true); - biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true); + it("should return Lock and LogOut when provided user has biometrics configured", async () => { + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(true)); - const result = await firstValueFrom( - vaultTimeoutSettingsService.availableVaultTimeoutActions$(), - ); + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(mockUserId), + ); - expect(result).toContain(VaultTimeoutAction.Lock); - }); + expect(biometricStateService.biometricUnlockEnabled$).toHaveBeenCalledWith(mockUserId); + expect(result).toContain(VaultTimeoutAction.Lock); + expect(result).toContain(VaultTimeoutAction.LogOut); + }); - it("not contains Lock when the user does not have a master password, PIN, or biometrics", async () => { - userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); - pinStateService.isPinSet.mockResolvedValue(false); - biometricStateService.biometricUnlockEnabled$ = of(false); + it("should not return Lock when provided user has no unlock methods", async () => { + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); - const result = await firstValueFrom( - vaultTimeoutSettingsService.availableVaultTimeoutActions$(), - ); + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(mockUserId), + ); - expect(result).not.toContain(VaultTimeoutAction.Lock); - }); - - it("should return only LogOut when userId is not provided and there is no active account", async () => { - // Set up accountService to return null for activeAccount - accountService.activeAccount$ = of(null); - pinStateService.isPinSet.mockResolvedValue(false); - biometricStateService.biometricUnlockEnabled$ = of(false); - - // Call availableVaultTimeoutActions$ which internally calls userHasMasterPassword without a userId - const result = await firstValueFrom( - vaultTimeoutSettingsService.availableVaultTimeoutActions$(), - ); - - // Since there's no active account, userHasMasterPassword returns false, - // meaning no master password is available, so Lock should not be available - expect(result).toEqual([VaultTimeoutAction.LogOut]); - expect(result).not.toContain(VaultTimeoutAction.Lock); + expect(result).not.toContain(VaultTimeoutAction.Lock); + expect(result).toContain(VaultTimeoutAction.LogOut); + }); }); }); @@ -237,8 +287,8 @@ describe("VaultTimeoutSettingsService", () => { `( "returns $expected when policy is $policy, has PIN unlock method: $hasPinUnlock or Biometric unlock method: $hasBiometricUnlock, and user preference is $userPreference", async ({ hasPinUnlock, hasBiometricUnlock, policy, userPreference, expected }) => { - biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(hasBiometricUnlock); - pinStateService.isPinSet.mockResolvedValue(hasPinUnlock); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(hasBiometricUnlock)); + pinStateService.pinSet$.mockReturnValue(of(hasPinUnlock)); userDecryptionOptionsSubject.next( new UserDecryptionOptions({ hasMasterPassword: false }), diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts index 57e484fd767..5384d6860b7 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts @@ -3,16 +3,15 @@ import { catchError, combineLatest, - defer, distinctUntilChanged, EMPTY, firstValueFrom, from, map, + of, Observable, shareReplay, switchMap, - tap, concatMap, } from "rxjs"; @@ -28,6 +27,7 @@ import { PolicyType } from "../../../admin-console/enums"; import { getFirstPolicy } from "../../../admin-console/services/policy/default-policy.service"; import { AccountService } from "../../../auth/abstractions/account.service"; import { TokenService } from "../../../auth/abstractions/token.service"; +import { getUserId } from "../../../auth/services/account.service"; import { LogService } from "../../../platform/abstractions/log.service"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; @@ -101,8 +101,29 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA await this.keyService.refreshAdditionalKeys(userId); } - availableVaultTimeoutActions$(userId?: string): Observable { - return defer(() => this.getAvailableVaultTimeoutActions(userId)); + availableVaultTimeoutActions$(userId?: UserId): Observable { + const userId$ = + userId != null + ? of(userId) + : // TODO remove with https://bitwarden.atlassian.net/browse/PM-10647 + getUserId(this.accountService.activeAccount$); + + return userId$.pipe( + switchMap((userId) => + combineLatest([ + this.userDecryptionOptionsService.hasMasterPasswordById$(userId), + this.biometricStateService.biometricUnlockEnabled$(userId), + this.pinStateService.pinSet$(userId), + ]), + ), + map(([haveMasterPassword, biometricUnlockEnabled, isPinSet]) => { + const canLock = haveMasterPassword || biometricUnlockEnabled || isPinSet; + if (canLock) { + return [VaultTimeoutAction.LogOut, VaultTimeoutAction.Lock]; + } + return [VaultTimeoutAction.LogOut]; + }), + ); } async canLock(userId: UserId): Promise { @@ -112,12 +133,8 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return availableVaultTimeoutActions?.includes(VaultTimeoutAction.Lock) || false; } - async isBiometricLockSet(userId?: string): Promise { - const biometricUnlockPromise = - userId == null - ? firstValueFrom(this.biometricStateService.biometricUnlockEnabled$) - : this.biometricStateService.getBiometricUnlockEnabled(userId as UserId); - return await biometricUnlockPromise; + async isBiometricLockSet(userId?: UserId): Promise { + return await firstValueFrom(this.biometricStateService.biometricUnlockEnabled$(userId)); } private async setVaultTimeout(userId: UserId, timeout: VaultTimeout): Promise { @@ -262,45 +279,45 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return combineLatest([ this.stateProvider.getUserState$(VAULT_TIMEOUT_ACTION, userId), this.getMaxSessionTimeoutPolicyDataByUserId$(userId), + this.availableVaultTimeoutActions$(userId), ]).pipe( - switchMap(([currentVaultTimeoutAction, maxSessionTimeoutPolicyData]) => { - return from( - this.determineVaultTimeoutAction( - userId, + concatMap( + async ([ + currentVaultTimeoutAction, + maxSessionTimeoutPolicyData, + availableVaultTimeoutActions, + ]) => { + const vaultTimeoutAction = this.determineVaultTimeoutAction( + availableVaultTimeoutActions, currentVaultTimeoutAction, maxSessionTimeoutPolicyData, - ), - ).pipe( - tap((vaultTimeoutAction: VaultTimeoutAction) => { - // As a side effect, set the new value determined by determineVaultTimeout into state if it's different from the current - // We want to avoid having a null timeout action always so we set it to the default if it is null - // and if the user becomes subject to a policy that requires a specific action, we set it to that - if (vaultTimeoutAction !== currentVaultTimeoutAction) { - return this.stateProvider.setUserState( - VAULT_TIMEOUT_ACTION, - vaultTimeoutAction, - userId, - ); - } - }), - catchError((error: unknown) => { - // Protect outer observable from canceling on error by catching and returning EMPTY - this.logService.error(`Error getting vault timeout: ${error}`); - return EMPTY; - }), - ); + ); + + // As a side effect, set the new value determined by determineVaultTimeout into state if it's different from the current + // We want to avoid having a null timeout action always so we set it to the default if it is null + // and if the user becomes subject to a policy that requires a specific action, we set it to that + if (vaultTimeoutAction !== currentVaultTimeoutAction) { + await this.stateProvider.setUserState(VAULT_TIMEOUT_ACTION, vaultTimeoutAction, userId); + } + + return vaultTimeoutAction; + }, + ), + catchError((error: unknown) => { + // Protect outer observable from canceling on error by catching and returning EMPTY + this.logService.error(`Error getting vault timeout: ${error}`); + return EMPTY; }), distinctUntilChanged(), // Avoid having the set side effect trigger a new emission of the same action shareReplay({ refCount: true, bufferSize: 1 }), ); } - private async determineVaultTimeoutAction( - userId: string, + private determineVaultTimeoutAction( + availableVaultTimeoutActions: VaultTimeoutAction[], currentVaultTimeoutAction: VaultTimeoutAction | null, maxSessionTimeoutPolicyData: MaximumSessionTimeoutPolicyData | null, - ): Promise { - const availableVaultTimeoutActions = await this.getAvailableVaultTimeoutActions(userId); + ): VaultTimeoutAction { if (availableVaultTimeoutActions.length === 1) { return availableVaultTimeoutActions[0]; } @@ -339,38 +356,4 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA map((policy) => (policy?.data ?? null) as MaximumSessionTimeoutPolicyData | null), ); } - - private async getAvailableVaultTimeoutActions(userId?: string): Promise { - userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id; - - const availableActions = [VaultTimeoutAction.LogOut]; - - const canLock = - (await this.userHasMasterPassword(userId)) || - (await this.pinStateService.isPinSet(userId as UserId)) || - (await this.isBiometricLockSet(userId)); - - if (canLock) { - availableActions.push(VaultTimeoutAction.Lock); - } - - return availableActions; - } - - private async userHasMasterPassword(userId: string): Promise { - let resolvedUserId: UserId; - if (userId) { - resolvedUserId = userId as UserId; - } else { - const activeAccount = await firstValueFrom(this.accountService.activeAccount$); - if (!activeAccount) { - return false; // No account, can't have master password - } - resolvedUserId = activeAccount.id; - } - - return await firstValueFrom( - this.userDecryptionOptionsService.hasMasterPasswordById$(resolvedUserId), - ); - } } diff --git a/libs/key-management/src/biometrics/biometric-state.service.spec.ts b/libs/key-management/src/biometrics/biometric-state.service.spec.ts index 32043514ff7..2f1f189a897 100644 --- a/libs/key-management/src/biometrics/biometric-state.service.spec.ts +++ b/libs/key-management/src/biometrics/biometric-state.service.spec.ts @@ -179,18 +179,36 @@ describe("BiometricStateService", () => { }); describe("biometricUnlockEnabled$", () => { - it("emits when biometricUnlockEnabled state is updated", async () => { - const state = stateProvider.activeUser.getFake(BIOMETRIC_UNLOCK_ENABLED); - state.nextState(true); + describe("no user id provided, active user", () => { + it("emits when biometricUnlockEnabled state is updated", async () => { + const state = stateProvider.activeUser.getFake(BIOMETRIC_UNLOCK_ENABLED); + state.nextState(true); - expect(await firstValueFrom(sut.biometricUnlockEnabled$)).toBe(true); + expect(await firstValueFrom(sut.biometricUnlockEnabled$())).toBe(true); + }); + + it("emits false when biometricUnlockEnabled state is undefined", async () => { + const state = stateProvider.activeUser.getFake(BIOMETRIC_UNLOCK_ENABLED); + state.nextState(undefined as unknown as boolean); + + expect(await firstValueFrom(sut.biometricUnlockEnabled$())).toBe(false); + }); }); - it("emits false when biometricUnlockEnabled state is undefined", async () => { - const state = stateProvider.activeUser.getFake(BIOMETRIC_UNLOCK_ENABLED); - state.nextState(undefined as unknown as boolean); + describe("user id provided", () => { + it("returns biometricUnlockEnabled state for the given user", async () => { + stateProvider.singleUser.getFake(userId, BIOMETRIC_UNLOCK_ENABLED).nextState(true); - expect(await firstValueFrom(sut.biometricUnlockEnabled$)).toBe(false); + expect(await firstValueFrom(sut.biometricUnlockEnabled$(userId))).toBe(true); + }); + + it("returns false when the state is not set", async () => { + stateProvider.singleUser + .getFake(userId, BIOMETRIC_UNLOCK_ENABLED) + .nextState(undefined as unknown as boolean); + + expect(await firstValueFrom(sut.biometricUnlockEnabled$(userId))).toBe(false); + }); }); }); @@ -198,7 +216,7 @@ describe("BiometricStateService", () => { it("updates biometricUnlockEnabled$", async () => { await sut.setBiometricUnlockEnabled(true); - expect(await firstValueFrom(sut.biometricUnlockEnabled$)).toBe(true); + expect(await firstValueFrom(sut.biometricUnlockEnabled$())).toBe(true); }); it("updates state", async () => { @@ -210,22 +228,6 @@ describe("BiometricStateService", () => { }); }); - describe("getBiometricUnlockEnabled", () => { - it("returns biometricUnlockEnabled state for the given user", async () => { - stateProvider.singleUser.getFake(userId, BIOMETRIC_UNLOCK_ENABLED).nextState(true); - - expect(await sut.getBiometricUnlockEnabled(userId)).toBe(true); - }); - - it("returns false when the state is not set", async () => { - stateProvider.singleUser - .getFake(userId, BIOMETRIC_UNLOCK_ENABLED) - .nextState(undefined as unknown as boolean); - - expect(await sut.getBiometricUnlockEnabled(userId)).toBe(false); - }); - }); - describe("setFingerprintValidated", () => { it("updates fingerprintValidated$", async () => { await sut.setFingerprintValidated(true); diff --git a/libs/key-management/src/biometrics/biometric-state.service.ts b/libs/key-management/src/biometrics/biometric-state.service.ts index 1488f12b50b..ca1cbcfa871 100644 --- a/libs/key-management/src/biometrics/biometric-state.service.ts +++ b/libs/key-management/src/biometrics/biometric-state.service.ts @@ -18,9 +18,11 @@ import { export abstract class BiometricStateService { /** - * `true` if the currently active user has elected to store a biometric key to unlock their vault. + * Returns whether biometric unlock is enabled for a user. + * @param userId The user id to check. If not provided, returns the state for the currently active user. + * @returns An observable that emits `true` if the user has elected to store a biometric key to unlock their vault. */ - abstract biometricUnlockEnabled$: Observable; // used to be biometricUnlock + abstract biometricUnlockEnabled$(userId?: UserId): Observable; /** * If the user has elected to require a password on first unlock of an application instance, this key will store the * encrypted client key half used to unlock the vault. @@ -53,6 +55,7 @@ export abstract class BiometricStateService { /** * Gets the biometric unlock enabled state for the given user. + * @deprecated Use {@link biometricUnlockEnabled$} instead * @param userId user Id to check */ abstract getBiometricUnlockEnabled(userId: UserId): Promise; @@ -103,7 +106,6 @@ export class DefaultBiometricStateService implements BiometricStateService { private promptAutomaticallyState: ActiveUserState; private fingerprintValidatedState: GlobalState; private lastProcessReloadState: GlobalState; - biometricUnlockEnabled$: Observable; encryptedClientKeyHalf$: Observable; promptCancelled$: Observable; promptAutomatically$: Observable; @@ -112,7 +114,6 @@ export class DefaultBiometricStateService implements BiometricStateService { constructor(private stateProvider: StateProvider) { this.biometricUnlockEnabledState = this.stateProvider.getActive(BIOMETRIC_UNLOCK_ENABLED); - this.biometricUnlockEnabled$ = this.biometricUnlockEnabledState.state$.pipe(map(Boolean)); this.encryptedClientKeyHalfState = this.stateProvider.getActive(ENCRYPTED_CLIENT_KEY_HALF); this.encryptedClientKeyHalf$ = this.encryptedClientKeyHalfState.state$.pipe( @@ -142,6 +143,15 @@ export class DefaultBiometricStateService implements BiometricStateService { await this.biometricUnlockEnabledState.update(() => enabled); } + biometricUnlockEnabled$(userId?: UserId): Observable { + if (userId != null) { + return this.stateProvider.getUser(userId, BIOMETRIC_UNLOCK_ENABLED).state$.pipe(map(Boolean)); + } + // Backwards compatibility for active user state + // TODO remove with https://bitwarden.atlassian.net/browse/PM-12043 + return this.biometricUnlockEnabledState.state$.pipe(map(Boolean)); + } + async getBiometricUnlockEnabled(userId: UserId): Promise { return await firstValueFrom( this.stateProvider.getUser(userId, BIOMETRIC_UNLOCK_ENABLED).state$.pipe(map(Boolean)), From 1008bf5cef0127af21485fdaf3a155a7289e4086 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 27 Jan 2026 14:14:22 +0100 Subject: [PATCH 06/24] [deps] Platform: Update tokio-tracing monorepo (#18238) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- apps/desktop/desktop_native/Cargo.lock | 16 ++++++++-------- apps/desktop/desktop_native/Cargo.toml | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 3e5225d4b5a..35228023224 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -3350,9 +3350,9 @@ checksum = "df8b2b54733674ad286d16267dcfc7a71ed5c776e4ac7aa3c3e2561f7c637bf2" [[package]] name = "tracing" -version = "0.1.41" +version = "0.1.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "784e0ac535deb450455cbfa28a6f0df145ea1bb7ae51b821cf5e7927fdcfbdd0" +checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" dependencies = [ "pin-project-lite", "tracing-attributes", @@ -3361,9 +3361,9 @@ dependencies = [ [[package]] name = "tracing-attributes" -version = "0.1.28" +version = "0.1.31" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "395ae124c09f9e6918a2310af6038fba074bcf474ac352496d5910dd59a2226d" +checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", @@ -3372,9 +3372,9 @@ dependencies = [ [[package]] name = "tracing-core" -version = "0.1.33" +version = "0.1.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e672c95779cf947c5311f83787af4fa8fffd12fb27e4993211a84bdfd9610f9c" +checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" dependencies = [ "once_cell", "valuable", @@ -3405,9 +3405,9 @@ dependencies = [ [[package]] name = "tracing-subscriber" -version = "0.3.20" +version = "0.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2054a14f5307d601f88daf0553e1cbf472acc4f2c51afab632431cdcd72124d5" +checksum = "2f30143827ddab0d256fd843b7a66d164e9f271cfa0dde49142c5ca0ca291f1e" dependencies = [ "matchers", "nu-ansi-term", diff --git a/apps/desktop/desktop_native/Cargo.toml b/apps/desktop/desktop_native/Cargo.toml index facd9554af1..da65db59e8c 100644 --- a/apps/desktop/desktop_native/Cargo.toml +++ b/apps/desktop/desktop_native/Cargo.toml @@ -65,8 +65,8 @@ sysinfo = "=0.37.2" thiserror = "=2.0.17" tokio = "=1.48.0" tokio-util = "=0.7.17" -tracing = "=0.1.41" -tracing-subscriber = { version = "=0.3.20", features = [ +tracing = "=0.1.44" +tracing-subscriber = { version = "=0.3.22", features = [ "fmt", "env-filter", "tracing-log", From 144ddee79200b58e511bca181ba05fad928f1679 Mon Sep 17 00:00:00 2001 From: Bryan Cunningham Date: Tue, 27 Jan 2026 09:15:51 -0500 Subject: [PATCH 07/24] [PM-30640][PM-30641] update angular core and compiler (#18542) Co-authored-by: Will Martin --- package-lock.json | 118 +++++++++++++++++++++++----------------------- package.json | 18 +++---- 2 files changed, 68 insertions(+), 68 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2cd18e11adc..0605c080574 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,15 +14,15 @@ "libs/**/*" ], "dependencies": { - "@angular/animations": "20.3.15", + "@angular/animations": "20.3.16", "@angular/cdk": "20.2.14", - "@angular/common": "20.3.15", - "@angular/compiler": "20.3.15", - "@angular/core": "20.3.15", - "@angular/forms": "20.3.15", - "@angular/platform-browser": "20.3.15", - "@angular/platform-browser-dynamic": "20.3.15", - "@angular/router": "20.3.15", + "@angular/common": "20.3.16", + "@angular/compiler": "20.3.16", + "@angular/core": "20.3.16", + "@angular/forms": "20.3.16", + "@angular/platform-browser": "20.3.16", + "@angular/platform-browser-dynamic": "20.3.16", + "@angular/router": "20.3.16", "@bitwarden/commercial-sdk-internal": "0.2.0-main.470", "@bitwarden/sdk-internal": "0.2.0-main.470", "@electron/fuses": "1.8.0", @@ -74,7 +74,7 @@ "@angular-devkit/build-angular": "20.3.12", "@angular-eslint/schematics": "20.7.0", "@angular/cli": "20.3.12", - "@angular/compiler-cli": "20.3.15", + "@angular/compiler-cli": "20.3.16", "@babel/core": "7.28.5", "@babel/preset-env": "7.28.5", "@compodoc/compodoc": "1.1.32", @@ -2203,9 +2203,9 @@ } }, "node_modules/@angular/animations": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/animations/-/animations-20.3.15.tgz", - "integrity": "sha512-ikyKfhkxoqQA6JcBN0B9RaN6369sM1XYX81Id0lI58dmWCe7gYfrTp8ejqxxKftl514psQO3pkW8Gn1nJ131Gw==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/animations/-/animations-20.3.16.tgz", + "integrity": "sha512-N83/GFY5lKNyWgPV3xHHy2rb3/eP1ZLzSVI+dmMVbf3jbqwY1YPQcMiAG8UDzaILY1Dkus91kWLF8Qdr3nHAzg==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2214,7 +2214,7 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/core": "20.3.15" + "@angular/core": "20.3.16" } }, "node_modules/@angular/build": { @@ -2627,9 +2627,9 @@ } }, "node_modules/@angular/common": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/common/-/common-20.3.15.tgz", - "integrity": "sha512-k4mCXWRFiOHK3bUKfWkRQQ8KBPxW8TAJuKLYCsSHPCpMz6u0eA1F0VlrnOkZVKWPI792fOaEAWH2Y4PTaXlUHw==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/common/-/common-20.3.16.tgz", + "integrity": "sha512-GRAziNlntwdnJy3F+8zCOvDdy7id0gITjDnM6P9+n2lXvtDuBLGJKU3DWBbvxcCjtD6JK/g/rEX5fbCxbUHkQQ==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2638,14 +2638,14 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/core": "20.3.15", + "@angular/core": "20.3.16", "rxjs": "^6.5.3 || ^7.4.0" } }, "node_modules/@angular/compiler": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/compiler/-/compiler-20.3.15.tgz", - "integrity": "sha512-lMicIAFAKZXa+BCZWs3soTjNQPZZXrF/WMVDinm8dQcggNarnDj4UmXgKSyXkkyqK5SLfnLsXVzrX6ndVT6z7A==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/compiler/-/compiler-20.3.16.tgz", + "integrity": "sha512-Pt9Ms9GwTThgzdxWBwMfN8cH1JEtQ2DK5dc2yxYtPSaD+WKmG9AVL1PrzIYQEbaKcWk2jxASUHpEWSlNiwo8uw==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2655,9 +2655,9 @@ } }, "node_modules/@angular/compiler-cli": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/compiler-cli/-/compiler-cli-20.3.15.tgz", - "integrity": "sha512-8sJoxodxsfyZ8eJ5r6Bx7BCbazXYgsZ1+dE8t5u5rTQ6jNggwNtYEzkyReoD5xvP+MMtRkos3xpwq4rtFnpI6A==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/compiler-cli/-/compiler-cli-20.3.16.tgz", + "integrity": "sha512-l3xF/fXfJAl/UrNnH9Ufkr79myjMgXdHq1mmmph2UnpeqilRB1b8lC9sLBV9MipQHVn3dwocxMIvtrcryfOaXw==", "dev": true, "license": "MIT", "dependencies": { @@ -2678,7 +2678,7 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/compiler": "20.3.15", + "@angular/compiler": "20.3.16", "typescript": ">=5.8 <6.0" }, "peerDependenciesMeta": { @@ -2864,9 +2864,9 @@ } }, "node_modules/@angular/core": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/core/-/core-20.3.15.tgz", - "integrity": "sha512-NMbX71SlTZIY9+rh/SPhRYFJU0pMJYW7z/TBD4lqiO+b0DTOIg1k7Pg9ydJGqSjFO1Z4dQaA6TteNuF99TJCNw==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/core/-/core-20.3.16.tgz", + "integrity": "sha512-KSFPKvOmWWLCJBbEO+CuRUXfecX2FRuO0jNi9c54ptXMOPHlK1lIojUnyXmMNzjdHgRug8ci9qDuftvC2B7MKg==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2875,7 +2875,7 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/compiler": "20.3.15", + "@angular/compiler": "20.3.16", "rxjs": "^6.5.3 || ^7.4.0", "zone.js": "~0.15.0" }, @@ -2889,9 +2889,9 @@ } }, "node_modules/@angular/forms": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/forms/-/forms-20.3.15.tgz", - "integrity": "sha512-gS5hQkinq52pm/7mxz4yHPCzEcmRWjtUkOVddPH0V1BW/HMni/p4Y6k2KqKBeGb9p8S5EAp6PDxDVLOPukp3mg==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/forms/-/forms-20.3.16.tgz", + "integrity": "sha512-1yzbXpExTqATpVcqA3wGrq4ACFIP3mRxA4pbso5KoJU+/4JfzNFwLsDaFXKpm5uxwchVnj8KM2vPaDOkvtp7NA==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2900,16 +2900,16 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/common": "20.3.15", - "@angular/core": "20.3.15", - "@angular/platform-browser": "20.3.15", + "@angular/common": "20.3.16", + "@angular/core": "20.3.16", + "@angular/platform-browser": "20.3.16", "rxjs": "^6.5.3 || ^7.4.0" } }, "node_modules/@angular/platform-browser": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/platform-browser/-/platform-browser-20.3.15.tgz", - "integrity": "sha512-TxRM/wTW/oGXv/3/Iohn58yWoiYXOaeEnxSasiGNS1qhbkcKtR70xzxW6NjChBUYAixz2ERkLURkpx3pI8Q6Dw==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/platform-browser/-/platform-browser-20.3.16.tgz", + "integrity": "sha512-YsrLS6vyS77i4pVHg4gdSBW74qvzHjpQRTVQ5Lv/OxIjJdYYYkMmjNalCNgy1ZuyY6CaLIB11ccxhrNnxfKGOQ==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2918,9 +2918,9 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/animations": "20.3.15", - "@angular/common": "20.3.15", - "@angular/core": "20.3.15" + "@angular/animations": "20.3.16", + "@angular/common": "20.3.16", + "@angular/core": "20.3.16" }, "peerDependenciesMeta": { "@angular/animations": { @@ -2929,9 +2929,9 @@ } }, "node_modules/@angular/platform-browser-dynamic": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/platform-browser-dynamic/-/platform-browser-dynamic-20.3.15.tgz", - "integrity": "sha512-RizuRdBt0d6ongQ2y8cr8YsXFyjF8f91vFfpSNw+cFj+oiEmRC1txcWUlH5bPLD9qSDied8qazUi0Tb8VPQDGw==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/platform-browser-dynamic/-/platform-browser-dynamic-20.3.16.tgz", + "integrity": "sha512-5mECCV9YeKH6ue239GXRTGeDSd/eTbM1j8dDejhm5cGnPBhTxRw4o+GgSrWTYtb6VmIYdwUGBTC+wCBphiaQ2A==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2940,16 +2940,16 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/common": "20.3.15", - "@angular/compiler": "20.3.15", - "@angular/core": "20.3.15", - "@angular/platform-browser": "20.3.15" + "@angular/common": "20.3.16", + "@angular/compiler": "20.3.16", + "@angular/core": "20.3.16", + "@angular/platform-browser": "20.3.16" } }, "node_modules/@angular/router": { - "version": "20.3.15", - "resolved": "https://registry.npmjs.org/@angular/router/-/router-20.3.15.tgz", - "integrity": "sha512-6+qgk8swGSoAu7ISSY//GatAyCP36hEvvUgvjbZgkXLLH9yUQxdo77ij05aJ5s0OyB25q/JkqS8VTY0z1yE9NQ==", + "version": "20.3.16", + "resolved": "https://registry.npmjs.org/@angular/router/-/router-20.3.16.tgz", + "integrity": "sha512-e1LiQFZaajKqc00cY5FboIrWJZSMnZ64GDp5R0UejritYrqorQQQNOqP1W85BMuY2owibMmxVfX+dJg/Mc8PuQ==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -2958,9 +2958,9 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/common": "20.3.15", - "@angular/core": "20.3.15", - "@angular/platform-browser": "20.3.15", + "@angular/common": "20.3.16", + "@angular/core": "20.3.16", + "@angular/platform-browser": "20.3.16", "rxjs": "^6.5.3 || ^7.4.0" } }, @@ -32414,9 +32414,9 @@ "license": "MIT" }, "node_modules/msgpackr": { - "version": "1.11.5", - "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-1.11.5.tgz", - "integrity": "sha512-UjkUHN0yqp9RWKy0Lplhh+wlpdt9oQBYgULZOiFhV3VclSF1JnSQWZ5r9gORQlNYaUKQoR8itv7g7z1xDDuACA==", + "version": "1.11.8", + "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-1.11.8.tgz", + "integrity": "sha512-bC4UGzHhVvgDNS7kn9tV8fAucIYUBuGojcaLiz7v+P63Lmtm0Xeji8B/8tYKddALXxJLpwIeBmUN3u64C4YkRA==", "dev": true, "license": "MIT", "optional": true, @@ -34690,9 +34690,9 @@ } }, "node_modules/ordered-binary": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/ordered-binary/-/ordered-binary-1.6.0.tgz", - "integrity": "sha512-IQh2aMfMIDbPjI/8a3Edr+PiOpcsB7yo8NdW7aHWVaoR/pcDldunMvnnwbk/auPGqmKeAdxtZl7MHX/QmPwhvQ==", + "version": "1.6.1", + "resolved": "https://registry.npmjs.org/ordered-binary/-/ordered-binary-1.6.1.tgz", + "integrity": "sha512-QkCdPooczexPLiXIrbVOPYkR3VO3T6v2OyKRkR1Xbhpy7/LAVXwahnRCgRp78Oe/Ehf0C/HATAxfSr6eA1oX+w==", "dev": true, "license": "MIT", "optional": true diff --git a/package.json b/package.json index 8455d97c87c..e2b65ccbef9 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "@angular-devkit/build-angular": "20.3.12", "@angular-eslint/schematics": "20.7.0", "@angular/cli": "20.3.12", - "@angular/compiler-cli": "20.3.15", + "@angular/compiler-cli": "20.3.16", "@babel/core": "7.28.5", "@babel/preset-env": "7.28.5", "@compodoc/compodoc": "1.1.32", @@ -153,15 +153,15 @@ "webpack-node-externals": "3.0.0" }, "dependencies": { - "@angular/animations": "20.3.15", + "@angular/animations": "20.3.16", "@angular/cdk": "20.2.14", - "@angular/common": "20.3.15", - "@angular/compiler": "20.3.15", - "@angular/core": "20.3.15", - "@angular/forms": "20.3.15", - "@angular/platform-browser": "20.3.15", - "@angular/platform-browser-dynamic": "20.3.15", - "@angular/router": "20.3.15", + "@angular/common": "20.3.16", + "@angular/compiler": "20.3.16", + "@angular/core": "20.3.16", + "@angular/forms": "20.3.16", + "@angular/platform-browser": "20.3.16", + "@angular/platform-browser-dynamic": "20.3.16", + "@angular/router": "20.3.16", "@bitwarden/sdk-internal": "0.2.0-main.470", "@bitwarden/commercial-sdk-internal": "0.2.0-main.470", "@electron/fuses": "1.8.0", From 8b9ee0df0684a5bc8a18b4e727c77dc36a083c73 Mon Sep 17 00:00:00 2001 From: lif <1835304752@qq.com> Date: Tue, 27 Jan 2026 22:48:20 +0800 Subject: [PATCH 08/24] fix(importer): preserve protected KeePass custom fields as hidden fields (#18136) Protected fields (ProtectInMemory="True") were being appended to notes when they exceeded 200 characters or contained newlines, instead of being imported as hidden custom fields. Now protected fields are always imported as hidden fields regardless of their length or content, preserving their protected status. Fixes #16897 Signed-off-by: majiayu000 <1835304752@qq.com> Co-authored-by: John Harrington <84741727+harr1424@users.noreply.github.com> --- .../importers/keepass2-xml-importer.spec.ts | 71 +++++++++++++++++++ .../src/importers/keepass2-xml-importer.ts | 23 ++++-- .../keepass2-xml-importer-testdata.ts | 51 +++++++++++++ 3 files changed, 139 insertions(+), 6 deletions(-) diff --git a/libs/importer/src/importers/keepass2-xml-importer.spec.ts b/libs/importer/src/importers/keepass2-xml-importer.spec.ts index 8fbb021883c..c1c0947936b 100644 --- a/libs/importer/src/importers/keepass2-xml-importer.spec.ts +++ b/libs/importer/src/importers/keepass2-xml-importer.spec.ts @@ -1,3 +1,4 @@ +import { FieldType } from "@bitwarden/common/vault/enums"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { KeePass2XmlImporter } from "./keepass2-xml-importer"; @@ -5,6 +6,7 @@ import { TestData, TestData1, TestData2, + TestDataWithProtectedFields, } from "./spec-data/keepass2-xml/keepass2-xml-importer-testdata"; describe("KeePass2 Xml Importer", () => { @@ -43,4 +45,73 @@ describe("KeePass2 Xml Importer", () => { const result = await importer.parse(TestData2); expect(result.success).toBe(false); }); + + describe("protected fields handling", () => { + it("should import protected custom fields as hidden fields", async () => { + const importer = new KeePass2XmlImporter(); + const result = await importer.parse(TestDataWithProtectedFields); + + expect(result.success).toBe(true); + expect(result.ciphers.length).toBe(1); + + const cipher = result.ciphers[0]; + expect(cipher.name).toBe("Test Entry"); + expect(cipher.login.username).toBe("testuser"); + expect(cipher.login.password).toBe("testpass"); + expect(cipher.notes).toContain("Regular notes"); + + // Check that protected custom field is imported as hidden field + const protectedField = cipher.fields.find((f) => f.name === "SAFE UN-LOCKING instructions"); + expect(protectedField).toBeDefined(); + expect(protectedField?.value).toBe("Secret instructions here"); + expect(protectedField?.type).toBe(FieldType.Hidden); + + // Check that regular custom field is imported as text field + const regularField = cipher.fields.find((f) => f.name === "CustomField"); + expect(regularField).toBeDefined(); + expect(regularField?.value).toBe("Custom value"); + expect(regularField?.type).toBe(FieldType.Text); + }); + + it("should import long protected fields as hidden fields (not appended to notes)", async () => { + const importer = new KeePass2XmlImporter(); + const result = await importer.parse(TestDataWithProtectedFields); + + const cipher = result.ciphers[0]; + + // Long protected field should be imported as hidden field + const longField = cipher.fields.find((f) => f.name === "LongProtectedField"); + expect(longField).toBeDefined(); + expect(longField?.type).toBe(FieldType.Hidden); + expect(longField?.value).toContain("This is a very long protected field"); + + // Should not be appended to notes + expect(cipher.notes).not.toContain("LongProtectedField"); + }); + + it("should import multiline protected fields as hidden fields (not appended to notes)", async () => { + const importer = new KeePass2XmlImporter(); + const result = await importer.parse(TestDataWithProtectedFields); + + const cipher = result.ciphers[0]; + + // Multiline protected field should be imported as hidden field + const multilineField = cipher.fields.find((f) => f.name === "MultilineProtectedField"); + expect(multilineField).toBeDefined(); + expect(multilineField?.type).toBe(FieldType.Hidden); + expect(multilineField?.value).toContain("Line 1"); + + // Should not be appended to notes + expect(cipher.notes).not.toContain("MultilineProtectedField"); + }); + + it("should not append protected custom fields to notes", async () => { + const importer = new KeePass2XmlImporter(); + const result = await importer.parse(TestDataWithProtectedFields); + + const cipher = result.ciphers[0]; + expect(cipher.notes).not.toContain("SAFE UN-LOCKING instructions"); + expect(cipher.notes).not.toContain("Secret instructions here"); + }); + }); }); diff --git a/libs/importer/src/importers/keepass2-xml-importer.ts b/libs/importer/src/importers/keepass2-xml-importer.ts index 0af7a6f829c..429ab2aa1b7 100644 --- a/libs/importer/src/importers/keepass2-xml-importer.ts +++ b/libs/importer/src/importers/keepass2-xml-importer.ts @@ -1,6 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { FieldType } from "@bitwarden/common/vault/enums"; +import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { ImportResult } from "../models/import-result"; @@ -92,16 +93,26 @@ export class KeePass2XmlImporter extends BaseImporter implements Importer { } else if (key === "Notes") { cipher.notes += value + "\n"; } else { - let type = FieldType.Text; const attrs = valueEl.attributes as any; - if ( + const isProtected = attrs.length > 0 && attrs.ProtectInMemory != null && - attrs.ProtectInMemory.value === "True" - ) { - type = FieldType.Hidden; + attrs.ProtectInMemory.value === "True"; + + if (isProtected) { + // Protected fields should always be imported as hidden fields, + // regardless of length or newlines (fixes #16897) + if (cipher.fields == null) { + cipher.fields = []; + } + const field = new FieldView(); + field.type = FieldType.Hidden; + field.name = key; + field.value = value; + cipher.fields.push(field); + } else { + this.processKvp(cipher, key, value, FieldType.Text); } - this.processKvp(cipher, key, value, type); } }); diff --git a/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts b/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts index e06ca2cf655..9e1599b7078 100644 --- a/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts +++ b/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts @@ -354,6 +354,57 @@ line2 `; +export const TestDataWithProtectedFields = ` + + + + KvS57lVwl13AfGFLwkvq4Q== + Root + + fAa543oYlgnJKkhKag5HLw== + + Title + Test Entry + + + UserName + testuser + + + Password + testpass + + + URL + https://example.com + + + Notes + Regular notes + + + SAFE UN-LOCKING instructions + Secret instructions here + + + CustomField + Custom value + + + LongProtectedField + This is a very long protected field value that exceeds 200 characters. It contains sensitive information that should be imported as a hidden field and not appended to the notes section. This text is long enough to trigger the old behavior. + + + MultilineProtectedField + Line 1 +Line 2 +Line 3 + + + + +`; + export const TestData2 = ` KeePass From fe1410bed31a3e90c2bbb13963246f966ffab51a Mon Sep 17 00:00:00 2001 From: Mike Amirault Date: Tue, 27 Jan 2026 09:53:03 -0500 Subject: [PATCH 09/24] [PM-30375] Account for differences in RoboForm Windows desktop app CSV export headers (#18403) --- libs/importer/src/importers/roboform-csv-importer.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/importer/src/importers/roboform-csv-importer.ts b/libs/importer/src/importers/roboform-csv-importer.ts index eb8a1ceac6a..6f557bb0db5 100644 --- a/libs/importer/src/importers/roboform-csv-importer.ts +++ b/libs/importer/src/importers/roboform-csv-importer.ts @@ -29,8 +29,9 @@ export class RoboFormCsvImporter extends BaseImporter implements Importer { cipher.notes = this.getValueOrDefault(value.Note); cipher.name = this.getValueOrDefault(value.Name, "--"); cipher.login.username = this.getValueOrDefault(value.Login); - cipher.login.password = this.getValueOrDefault(value.Pwd); - cipher.login.uris = this.makeUriArray(value.Url); + cipher.login.password = + this.getValueOrDefault(value.Pwd) ?? this.getValueOrDefault(value.Password); + cipher.login.uris = this.makeUriArray(value.Url) ?? this.makeUriArray(value.URL); if (!this.isNullOrWhitespace(value.Rf_fields)) { this.parseRfFields(cipher, value); From 00cf24972d944638bbd1adc00a0ae3eeabb6eb9a Mon Sep 17 00:00:00 2001 From: Jeffrey Holland <124393578+jholland-livefront@users.noreply.github.com> Date: Tue, 27 Jan 2026 17:28:02 +0100 Subject: [PATCH 10/24] [PM-28079] Add attributes to filter for the mutationObserver (#17832) * [PM-28079] Add attributes to filter for the mutationObserver * Update attributes based on Claude suggestions * Updated remaining attributes * Adjust placeholder check in `updateAutofillFieldElementData` * Update ordering of constants and add comment * Remove `tagName` and `value` from mutation logic * Add new autocomplete and aria attributes to `updateActions` * Fix autocomplete handlers * Fix broken test for `updateAttributes` * Order attributes for readability in `updateActions` * Fix tests --------- Co-authored-by: Jonathan Prusik --- .../collect-autofill-content.service.spec.ts | 12 +- .../collect-autofill-content.service.ts | 121 +++++++++++------- libs/common/src/autofill/constants/index.ts | 35 +++++ 3 files changed, 116 insertions(+), 52 deletions(-) diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts index 66a692dbe20..58f3ad11166 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts @@ -158,7 +158,7 @@ describe("CollectAutofillContentService", () => { type: "text", value: "", checked: false, - autoCompleteType: "", + autoCompleteType: null, disabled: false, readonly: false, selectInfo: null, @@ -346,7 +346,7 @@ describe("CollectAutofillContentService", () => { type: "text", value: "", checked: false, - autoCompleteType: "", + autoCompleteType: null, disabled: false, readonly: false, selectInfo: null, @@ -379,7 +379,7 @@ describe("CollectAutofillContentService", () => { type: "password", value: "", checked: false, - autoCompleteType: "", + autoCompleteType: null, disabled: false, readonly: false, selectInfo: null, @@ -588,7 +588,7 @@ describe("CollectAutofillContentService", () => { "aria-disabled": false, "aria-haspopup": false, "aria-hidden": false, - autoCompleteType: "", + autoCompleteType: null, checked: false, "data-stripe": null, disabled: false, @@ -621,7 +621,7 @@ describe("CollectAutofillContentService", () => { "aria-disabled": false, "aria-haspopup": false, "aria-hidden": false, - autoCompleteType: "", + autoCompleteType: null, checked: false, "data-stripe": null, disabled: false, @@ -2507,9 +2507,7 @@ describe("CollectAutofillContentService", () => { "class", "tabindex", "title", - "value", "rel", - "tagname", "checked", "disabled", "readonly", 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 117c7c5e2a4..1d464e1313f 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1,5 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { AUTOFILL_ATTRIBUTES } from "@bitwarden/common/autofill/constants"; + import AutofillField from "../models/autofill-field"; import AutofillForm from "../models/autofill-form"; import AutofillPageDetails from "../models/autofill-page-details"; @@ -242,10 +244,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this._autofillFormElements.set(formElement, { opid: formElement.opid, htmlAction: this.getFormActionAttribute(formElement), - htmlName: this.getPropertyOrAttribute(formElement, "name"), - htmlClass: this.getPropertyOrAttribute(formElement, "class"), - htmlID: this.getPropertyOrAttribute(formElement, "id"), - htmlMethod: this.getPropertyOrAttribute(formElement, "method"), + htmlName: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.NAME), + htmlClass: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.CLASS), + htmlID: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.ID), + htmlMethod: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.METHOD), }); } @@ -260,7 +262,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * @private */ private getFormActionAttribute(element: ElementWithOpId): string { - return new URL(this.getPropertyOrAttribute(element, "action"), globalThis.location.href).href; + return new URL( + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ACTION), + globalThis.location.href, + ).href; } /** @@ -335,7 +340,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ return priorityFormFields; } - const fieldType = this.getPropertyOrAttribute(element, "type")?.toLowerCase(); + const fieldType = this.getPropertyOrAttribute( + element, + AUTOFILL_ATTRIBUTES.TYPE, + )?.toLowerCase(); if (unimportantFieldTypesSet.has(fieldType)) { unimportantFormFields.push(element); continue; @@ -384,11 +392,11 @@ export class CollectAutofillContentService implements CollectAutofillContentServ elementNumber: index, maxLength: this.getAutofillFieldMaxLength(element), viewable: await this.domElementVisibilityService.isElementViewable(element), - htmlID: this.getPropertyOrAttribute(element, "id"), - htmlName: this.getPropertyOrAttribute(element, "name"), - htmlClass: this.getPropertyOrAttribute(element, "class"), - tabindex: this.getPropertyOrAttribute(element, "tabindex"), - title: this.getPropertyOrAttribute(element, "title"), + htmlID: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ID), + htmlName: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.NAME), + htmlClass: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.CLASS), + tabindex: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.TABINDEX), + title: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.TITLE), tagName: this.getAttributeLowerCase(element, "tagName"), dataSetValues: this.getDataSetValues(element), }; @@ -404,16 +412,16 @@ export class CollectAutofillContentService implements CollectAutofillContentServ } let autofillFieldLabels = {}; - const elementType = this.getAttributeLowerCase(element, "type"); + const elementType = this.getAttributeLowerCase(element, AUTOFILL_ATTRIBUTES.TYPE); if (elementType !== "hidden") { autofillFieldLabels = { "label-tag": this.createAutofillFieldLabelTag(element as FillableFormFieldElement), - "label-data": this.getPropertyOrAttribute(element, "data-label"), - "label-aria": this.getPropertyOrAttribute(element, "aria-label"), + "label-data": this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.DATA_LABEL), + "label-aria": this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ARIA_LABEL), "label-top": this.createAutofillFieldTopLabel(element), "label-right": this.createAutofillFieldRightLabel(element), "label-left": this.createAutofillFieldLeftLabel(element), - placeholder: this.getPropertyOrAttribute(element, "placeholder"), + placeholder: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.PLACEHOLDER), }; } @@ -421,21 +429,21 @@ export class CollectAutofillContentService implements CollectAutofillContentServ const autofillField = { ...autofillFieldBase, ...autofillFieldLabels, - rel: this.getPropertyOrAttribute(element, "rel"), + rel: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.REL), type: elementType, value: this.getElementValue(element), - checked: this.getAttributeBoolean(element, "checked"), + checked: this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.CHECKED), autoCompleteType: this.getAutoCompleteAttribute(element), - disabled: this.getAttributeBoolean(element, "disabled"), - readonly: this.getAttributeBoolean(element, "readonly"), + disabled: this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.DISABLED), + readonly: this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.READONLY), selectInfo: elementIsSelectElement(element) ? this.getSelectElementOptions(element as HTMLSelectElement) : null, form: fieldFormElement ? this.getPropertyOrAttribute(fieldFormElement, "opid") : null, - "aria-hidden": this.getAttributeBoolean(element, "aria-hidden", true), - "aria-disabled": this.getAttributeBoolean(element, "aria-disabled", true), - "aria-haspopup": this.getAttributeBoolean(element, "aria-haspopup", true), - "data-stripe": this.getPropertyOrAttribute(element, "data-stripe"), + "aria-hidden": this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.ARIA_HIDDEN, true), + "aria-disabled": this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.ARIA_DISABLED, true), + "aria-haspopup": this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.ARIA_HASPOPUP, true), + "data-stripe": this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.DATA_STRIPE), }; this.cacheAutofillFieldElement(index, element, autofillField); @@ -467,9 +475,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ */ private getAutoCompleteAttribute(element: ElementWithOpId): string { return ( - this.getPropertyOrAttribute(element, "x-autocompletetype") || - this.getPropertyOrAttribute(element, "autocompletetype") || - this.getPropertyOrAttribute(element, "autocomplete") + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.AUTOCOMPLETE) || + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.X_AUTOCOMPLETE_TYPE) || + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.AUTOCOMPLETE_TYPE) ); } @@ -957,6 +965,8 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this.mutationObserver = new MutationObserver(this.handleMutationObserverMutation); this.mutationObserver.observe(document.documentElement, { attributes: true, + /** Mutations to node attributes NOT on this list will not be observed! */ + attributeFilter: Object.values(AUTOFILL_ATTRIBUTES), childList: true, subtree: true, }); @@ -1321,6 +1331,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ action: () => (dataTarget.htmlAction = this.getFormActionAttribute(element)), name: () => updateAttribute("htmlName"), id: () => updateAttribute("htmlID"), + class: () => updateAttribute("htmlClass"), method: () => updateAttribute("htmlMethod"), }; @@ -1350,29 +1361,49 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this.updateAutofillDataAttribute({ element, attributeName, dataTarget, dataTargetKey }); }; const updateActions: Record = { - maxlength: () => (dataTarget.maxLength = this.getAutofillFieldMaxLength(element)), - id: () => updateAttribute("htmlID"), - name: () => updateAttribute("htmlName"), - class: () => updateAttribute("htmlClass"), - tabindex: () => updateAttribute("tabindex"), - title: () => updateAttribute("tabindex"), - rel: () => updateAttribute("rel"), - tagname: () => (dataTarget.tagName = this.getAttributeLowerCase(element, "tagName")), - type: () => (dataTarget.type = this.getAttributeLowerCase(element, "type")), - value: () => (dataTarget.value = this.getElementValue(element)), - checked: () => (dataTarget.checked = this.getAttributeBoolean(element, "checked")), - disabled: () => (dataTarget.disabled = this.getAttributeBoolean(element, "disabled")), - readonly: () => (dataTarget.readonly = this.getAttributeBoolean(element, "readonly")), - autocomplete: () => (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), - "data-label": () => updateAttribute("label-data"), + "aria-describedby": () => updateAttribute(AUTOFILL_ATTRIBUTES.ARIA_DESCRIBEDBY), "aria-label": () => updateAttribute("label-aria"), + "aria-labelledby": () => updateAttribute(AUTOFILL_ATTRIBUTES.ARIA_LABELLEDBY), "aria-hidden": () => - (dataTarget["aria-hidden"] = this.getAttributeBoolean(element, "aria-hidden", true)), + (dataTarget["aria-hidden"] = this.getAttributeBoolean( + element, + AUTOFILL_ATTRIBUTES.ARIA_HIDDEN, + true, + )), "aria-disabled": () => - (dataTarget["aria-disabled"] = this.getAttributeBoolean(element, "aria-disabled", true)), + (dataTarget["aria-disabled"] = this.getAttributeBoolean( + element, + AUTOFILL_ATTRIBUTES.ARIA_DISABLED, + true, + )), "aria-haspopup": () => - (dataTarget["aria-haspopup"] = this.getAttributeBoolean(element, "aria-haspopup", true)), - "data-stripe": () => updateAttribute("data-stripe"), + (dataTarget["aria-haspopup"] = this.getAttributeBoolean( + element, + AUTOFILL_ATTRIBUTES.ARIA_HASPOPUP, + true, + )), + autocomplete: () => (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), + autocompletetype: () => + (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), + "x-autocompletetype": () => + (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), + class: () => updateAttribute("htmlClass"), + checked: () => + (dataTarget.checked = this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.CHECKED)), + "data-label": () => updateAttribute("label-data"), + "data-stripe": () => updateAttribute(AUTOFILL_ATTRIBUTES.DATA_STRIPE), + disabled: () => + (dataTarget.disabled = this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.DISABLED)), + id: () => updateAttribute("htmlID"), + maxlength: () => (dataTarget.maxLength = this.getAutofillFieldMaxLength(element)), + name: () => updateAttribute("htmlName"), + placeholder: () => updateAttribute(AUTOFILL_ATTRIBUTES.PLACEHOLDER), + readonly: () => + (dataTarget.readonly = this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.READONLY)), + rel: () => updateAttribute(AUTOFILL_ATTRIBUTES.REL), + tabindex: () => updateAttribute(AUTOFILL_ATTRIBUTES.TABINDEX), + title: () => updateAttribute(AUTOFILL_ATTRIBUTES.TITLE), + type: () => (dataTarget.type = this.getAttributeLowerCase(element, AUTOFILL_ATTRIBUTES.TYPE)), }; if (!updateActions[attributeName]) { diff --git a/libs/common/src/autofill/constants/index.ts b/libs/common/src/autofill/constants/index.ts index dc79e27b6aa..f3f0077a37f 100644 --- a/libs/common/src/autofill/constants/index.ts +++ b/libs/common/src/autofill/constants/index.ts @@ -28,6 +28,41 @@ export const EVENTS = { SUBMIT: "submit", } as const; +/** + * HTML attributes observed by the MutationObserver for autofill form/field tracking. + * If you need to observe a new attribute, add it here. + */ +export const AUTOFILL_ATTRIBUTES = { + ACTION: "action", + ARIA_DESCRIBEDBY: "aria-describedby", + ARIA_DISABLED: "aria-disabled", + ARIA_HASPOPUP: "aria-haspopup", + ARIA_HIDDEN: "aria-hidden", + ARIA_LABEL: "aria-label", + ARIA_LABELLEDBY: "aria-labelledby", + AUTOCOMPLETE: "autocomplete", + AUTOCOMPLETE_TYPE: "autocompletetype", + X_AUTOCOMPLETE_TYPE: "x-autocompletetype", + CHECKED: "checked", + CLASS: "class", + DATA_LABEL: "data-label", + DATA_STRIPE: "data-stripe", + DISABLED: "disabled", + ID: "id", + MAXLENGTH: "maxlength", + METHOD: "method", + NAME: "name", + PLACEHOLDER: "placeholder", + POPOVER: "popover", + POPOVERTARGET: "popovertarget", + POPOVERTARGETACTION: "popovertargetaction", + READONLY: "readonly", + REL: "rel", + TABINDEX: "tabindex", + TITLE: "title", + TYPE: "type", +} as const; + export const ClearClipboardDelay = { Never: null as null, TenSeconds: 10, From 1de2e33bbbf10211f1b3b3354c1ef165b7e92a08 Mon Sep 17 00:00:00 2001 From: Brad <44413459+lastbestdev@users.noreply.github.com> Date: Tue, 27 Jan 2026 09:14:00 -0800 Subject: [PATCH 11/24] [PM-31182] Add HIBP icons URL to dev configuration for allowed Content-Security-Policy domains (#18565) * add url for loading HIBP icons * remove old hibp location --- apps/web/webpack.base.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/webpack.base.js b/apps/web/webpack.base.js index cc17b3b7cfd..016d2b0fe61 100644 --- a/apps/web/webpack.base.js +++ b/apps/web/webpack.base.js @@ -319,7 +319,7 @@ module.exports.buildConfig = function buildConfig(params) { https://*.paypal.com https://www.paypalobjects.com https://q.stripe.com - https://haveibeenpwned.com + https://logos.haveibeenpwned.com ;media-src 'self' https://assets.bitwarden.com From cf6d02fafa5d3a71ef1a78b95603b03cde201128 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Tue, 27 Jan 2026 19:00:13 +0100 Subject: [PATCH 12/24] [PM-31264] Broken vault filters in milestone-1 (#18589) * Fix vault filters Now uses the same `createFilterFunction` as web rather than the custom proxy like approach. * Remove provide --- .../src/vault/app/vault-v3/vault.component.ts | 58 +++++++------------ 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/apps/desktop/src/vault/app/vault-v3/vault.component.ts b/apps/desktop/src/vault/app/vault-v3/vault.component.ts index 9d5fad2fe4c..efb7e4de70f 100644 --- a/apps/desktop/src/vault/app/vault-v3/vault.component.ts +++ b/apps/desktop/src/vault/app/vault-v3/vault.component.ts @@ -4,6 +4,7 @@ import { CommonModule } from "@angular/common"; import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { + combineLatest, firstValueFrom, Subject, takeUntil, @@ -70,6 +71,7 @@ import { CipherFormModule, CipherViewComponent, CollectionAssignmentResult, + createFilterFunction, DecryptionFailureDialogComponent, DefaultChangeLoginPasswordService, DefaultCipherFormConfigService, @@ -79,6 +81,7 @@ import { VaultFilter, VaultFilterServiceAbstraction as VaultFilterService, RoutedVaultFilterBridgeService, + RoutedVaultFilterService, VaultItemsTransferService, DefaultVaultItemsTransferService, } from "@bitwarden/vault"; @@ -216,6 +219,7 @@ export class VaultComponent implements OnInit, OnDestroy, CopyClickListener { private policyService: PolicyService, private archiveCipherUtilitiesService: ArchiveCipherUtilitiesService, private routedVaultFilterBridgeService: RoutedVaultFilterBridgeService, + private routedVaultFilterService: RoutedVaultFilterService, private vaultFilterService: VaultFilterService, private vaultItemTransferService: VaultItemsTransferService, ) {} @@ -234,9 +238,16 @@ export class VaultComponent implements OnInit, OnDestroy, CopyClickListener { }); // Subscribe to filter changes from router params via the bridge service - this.routedVaultFilterBridgeService.activeFilter$ + // Use combineLatest to react to changes in both the filter and archive flag + combineLatest([ + this.routedVaultFilterBridgeService.activeFilter$, + this.routedVaultFilterService.filter$, + this.cipherArchiveService.hasArchiveFlagEnabled$, + ]) .pipe( - switchMap((vaultFilter: VaultFilter) => from(this.applyVaultFilter(vaultFilter))), + switchMap(([vaultFilter, routedFilter, archiveEnabled]) => + from(this.applyVaultFilter(vaultFilter, routedFilter, archiveEnabled)), + ), takeUntil(this.componentIsDestroyed$), ) .subscribe(); @@ -789,48 +800,19 @@ export class VaultComponent implements OnInit, OnDestroy, CopyClickListener { await this.go().catch(() => {}); } - /** - * Wraps a filter function to handle CipherListView objects. - * CipherListView has a different type structure where type can be a string or object. - * This wrapper converts it to CipherView-compatible structure before filtering. - */ - private wrapFilterForCipherListView( - filterFn: (cipher: CipherView) => boolean, - ): (cipher: CipherViewLike) => boolean { - return (cipher: CipherViewLike) => { - // For CipherListView, create a proxy object with the correct type property - if (CipherViewLikeUtils.isCipherListView(cipher)) { - const proxyCipher = { - ...cipher, - type: CipherViewLikeUtils.getType(cipher), - // Normalize undefined organizationId to null for filter compatibility - organizationId: cipher.organizationId ?? null, - // Normalize empty string folderId to null for filter compatibility - folderId: cipher.folderId ? cipher.folderId : null, - // Explicitly include isDeleted and isArchived since they might be getters - isDeleted: CipherViewLikeUtils.isDeleted(cipher), - isArchived: CipherViewLikeUtils.isArchived(cipher), - }; - return filterFn(proxyCipher as any); - } - return filterFn(cipher); - }; - } - - async applyVaultFilter(vaultFilter: VaultFilter) { + async applyVaultFilter( + vaultFilter: VaultFilter, + routedFilter: Parameters[0], + archiveEnabled: boolean, + ) { this.searchBarService.setPlaceholderText( this.i18nService.t(this.calculateSearchBarLocalizationString(vaultFilter)), ); this.activeFilter = vaultFilter; - const originalFilterFn = this.activeFilter.buildFilter(); - const wrappedFilterFn = this.wrapFilterForCipherListView(originalFilterFn); + const filterFn = createFilterFunction(routedFilter, archiveEnabled); - await this.vaultItemsComponent?.reload( - wrappedFilterFn, - vaultFilter.isDeleted, - vaultFilter.isArchived, - ); + await this.vaultItemsComponent?.reload(filterFn, vaultFilter.isDeleted, vaultFilter.isArchived); } private getAvailableCollections(cipher: CipherView): CollectionView[] { From 1b94d16f31347a9c49dc3e459d19a4472c9e6c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Tue, 27 Jan 2026 19:08:07 +0100 Subject: [PATCH 13/24] PM-31294: Unlock Passkey using getWebVaultUrl over getHostname (#18597) --- .../src/lock/services/default-webauthn-prf-unlock.service.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/key-management-ui/src/lock/services/default-webauthn-prf-unlock.service.ts b/libs/key-management-ui/src/lock/services/default-webauthn-prf-unlock.service.ts index 960a663b589..106037bc5f7 100644 --- a/libs/key-management-ui/src/lock/services/default-webauthn-prf-unlock.service.ts +++ b/libs/key-management-ui/src/lock/services/default-webauthn-prf-unlock.service.ts @@ -14,6 +14,7 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { Fido2Utils } from "@bitwarden/common/platform/services/fido2/fido2-utils"; import { UserId } from "@bitwarden/common/types/guid"; import { PrfKey, UserKey } from "@bitwarden/common/types/key"; @@ -267,7 +268,7 @@ export class DefaultWebAuthnPrfUnlockService implements WebAuthnPrfUnlockService private async getRpIdForUser(userId: UserId): Promise { try { const environment = await firstValueFrom(this.environmentService.getEnvironment$(userId)); - const hostname = environment.getHostname(); + const hostname = Utils.getHost(environment.getWebVaultUrl()); // The navigator.credentials.get call will fail if rpId is set but is null/empty. Undefined uses the current host. if (!hostname) { From 3e344212d6860afc94b24cd05e4057d7e8c97116 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Tue, 27 Jan 2026 12:18:37 -0600 Subject: [PATCH 14/24] [PM-29805] - Rollback single org enablement when auto confirm enablement fails. (#18572) --- ...nfirm-edit-policy-dialog.component.spec.ts | 270 ++++++++++++++++++ ...to-confirm-edit-policy-dialog.component.ts | 37 ++- 2 files changed, 292 insertions(+), 15 deletions(-) create mode 100644 apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts new file mode 100644 index 00000000000..09b2f8961f3 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts @@ -0,0 +1,270 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormBuilder } from "@angular/forms"; +import { Router } from "@angular/router"; +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { DIALOG_DATA, DialogRef, ToastService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { + AutoConfirmPolicyDialogComponent, + AutoConfirmPolicyDialogData, +} from "./auto-confirm-edit-policy-dialog.component"; + +describe("AutoConfirmPolicyDialogComponent", () => { + let component: AutoConfirmPolicyDialogComponent; + let fixture: ComponentFixture; + + let mockPolicyApiService: MockProxy; + let mockAccountService: FakeAccountService; + let mockOrganizationService: MockProxy; + let mockPolicyService: MockProxy; + let mockRouter: MockProxy; + let mockAutoConfirmService: MockProxy; + let mockDialogRef: MockProxy; + let mockToastService: MockProxy; + let mockI18nService: MockProxy; + let mockKeyService: MockProxy; + + const mockUserId = newGuid() as UserId; + const mockOrgId = newGuid() as OrganizationId; + + const mockDialogData: AutoConfirmPolicyDialogData = { + organizationId: mockOrgId, + policy: { + name: "autoConfirm", + description: "Auto Confirm Policy", + type: PolicyType.AutoConfirm, + component: {} as any, + showDescription: true, + display$: () => of(true), + }, + firstTimeDialog: false, + }; + + const mockOrg = { + id: mockOrgId, + name: "Test Organization", + enabled: true, + isAdmin: true, + canManagePolicies: true, + } as Organization; + + beforeEach(async () => { + mockPolicyApiService = mock(); + mockAccountService = mockAccountServiceWith(mockUserId); + mockOrganizationService = mock(); + mockPolicyService = mock(); + mockRouter = mock(); + mockAutoConfirmService = mock(); + mockDialogRef = mock(); + mockToastService = mock(); + mockI18nService = mock(); + mockKeyService = mock(); + + mockPolicyService.policies$.mockReturnValue(of([])); + mockOrganizationService.organizations$.mockReturnValue(of([mockOrg])); + + await TestBed.configureTestingModule({ + imports: [AutoConfirmPolicyDialogComponent], + providers: [ + FormBuilder, + { provide: DIALOG_DATA, useValue: mockDialogData }, + { provide: AccountService, useValue: mockAccountService }, + { provide: PolicyApiServiceAbstraction, useValue: mockPolicyApiService }, + { provide: I18nService, useValue: mockI18nService }, + { provide: DialogRef, useValue: mockDialogRef }, + { provide: ToastService, useValue: mockToastService }, + { provide: KeyService, useValue: mockKeyService }, + { provide: OrganizationService, useValue: mockOrganizationService }, + { provide: PolicyService, useValue: mockPolicyService }, + { provide: Router, useValue: mockRouter }, + { provide: AutomaticUserConfirmationService, useValue: mockAutoConfirmService }, + ], + schemas: [NO_ERRORS_SCHEMA], + }) + .overrideComponent(AutoConfirmPolicyDialogComponent, { + set: { template: "
" }, + }) + .compileComponents(); + + fixture = TestBed.createComponent(AutoConfirmPolicyDialogComponent); + component = fixture.componentInstance; + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + describe("handleSubmit", () => { + beforeEach(() => { + // Mock the policyComponent + component.policyComponent = { + buildRequest: jest.fn().mockResolvedValue({ enabled: true, data: null }), + enabled: { value: true }, + setSingleOrgEnabled: jest.fn(), + } as any; + + mockAutoConfirmService.configuration$.mockReturnValue( + of({ enabled: false, showSetupDialog: true, showBrowserNotification: undefined }), + ); + mockAutoConfirmService.upsert.mockResolvedValue(undefined); + mockI18nService.t.mockReturnValue("Policy updated"); + }); + + it("should enable SingleOrg policy when it was not already enabled", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + // Call handleSubmit with singleOrgEnabled = false (meaning it needs to be enabled) + await component["handleSubmit"](false); + + // First call should be SingleOrg enable + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 1, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should not enable SingleOrg policy when it was already enabled", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + // Call handleSubmit with singleOrgEnabled = true (meaning it's already enabled) + await component["handleSubmit"](true); + + // Should only call putPolicyVNext once (for AutoConfirm, not SingleOrg) + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(1); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should rollback SingleOrg policy when AutoConfirm fails and SingleOrg was enabled during action", async () => { + const autoConfirmError = new Error("AutoConfirm failed"); + + // First call (SingleOrg enable) succeeds, second call (AutoConfirm) fails, third call (SingleOrg rollback) succeeds + mockPolicyApiService.putPolicyVNext + .mockResolvedValueOnce({} as any) // SingleOrg enable + .mockRejectedValueOnce(autoConfirmError) // AutoConfirm fails + .mockResolvedValueOnce({} as any); // SingleOrg rollback + + await expect(component["handleSubmit"](false)).rejects.toThrow("AutoConfirm failed"); + + // Verify: SingleOrg enabled, AutoConfirm attempted, SingleOrg rolled back + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(3); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 1, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 2, + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 3, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: false, data: null } }, + ); + }); + + it("should not rollback SingleOrg policy when AutoConfirm fails but SingleOrg was already enabled", async () => { + const autoConfirmError = new Error("AutoConfirm failed"); + + // AutoConfirm call fails (SingleOrg was already enabled, so no SingleOrg calls) + mockPolicyApiService.putPolicyVNext.mockRejectedValue(autoConfirmError); + + await expect(component["handleSubmit"](true)).rejects.toThrow("AutoConfirm failed"); + + // Verify only AutoConfirm was called (no SingleOrg enable/rollback) + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(1); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should keep both policies enabled when both submissions succeed", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + await component["handleSubmit"](false); + + // Verify two calls: SingleOrg enable and AutoConfirm enable + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(2); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 1, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 2, + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should re-throw the error after rollback", async () => { + const autoConfirmError = new Error("Network error"); + + mockPolicyApiService.putPolicyVNext + .mockResolvedValueOnce({} as any) // SingleOrg enable + .mockRejectedValueOnce(autoConfirmError) // AutoConfirm fails + .mockResolvedValueOnce({} as any); // SingleOrg rollback + + await expect(component["handleSubmit"](false)).rejects.toThrow("Network error"); + }); + }); + + describe("setSingleOrgPolicy", () => { + it("should call putPolicyVNext with enabled: true when enabling", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + await component["setSingleOrgPolicy"](true); + + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should call putPolicyVNext with enabled: false when disabling", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + await component["setSingleOrgPolicy"](false); + + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: false, data: null } }, + ); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts index fbdeffc71bb..f0146225b8d 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts @@ -181,10 +181,21 @@ export class AutoConfirmPolicyDialogComponent } private async handleSubmit(singleOrgEnabled: boolean) { - if (!singleOrgEnabled) { - await this.submitSingleOrg(); + const enabledSingleOrgDuringAction = !singleOrgEnabled; + + if (enabledSingleOrgDuringAction) { + await this.setSingleOrgPolicy(true); + } + + try { + await this.submitAutoConfirm(); + } catch (error) { + // Roll back SingleOrg if we enabled it during this action + if (enabledSingleOrgDuringAction) { + await this.setSingleOrgPolicy(false); + } + throw error; } - await this.submitAutoConfirm(); } /** @@ -198,11 +209,9 @@ export class AutoConfirmPolicyDialogComponent const autoConfirmRequest = await this.policyComponent.buildRequest(); - await this.policyApiService.putPolicy( - this.data.organizationId, - this.data.policy.type, - autoConfirmRequest, - ); + await this.policyApiService.putPolicyVNext(this.data.organizationId, this.data.policy.type, { + policy: autoConfirmRequest, + }); const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); @@ -225,17 +234,15 @@ export class AutoConfirmPolicyDialogComponent } } - private async submitSingleOrg(): Promise { + private async setSingleOrgPolicy(enabled: boolean): Promise { const singleOrgRequest: PolicyRequest = { - enabled: true, + enabled, data: null, }; - await this.policyApiService.putPolicyVNext( - this.data.organizationId, - PolicyType.SingleOrg, - singleOrgRequest, - ); + await this.policyApiService.putPolicyVNext(this.data.organizationId, PolicyType.SingleOrg, { + policy: singleOrgRequest, + }); } private async openBrowserExtension() { From 42aec64689f47dd7ef9ea99bec14c74864870c88 Mon Sep 17 00:00:00 2001 From: Jared Date: Tue, 27 Jan 2026 13:31:02 -0500 Subject: [PATCH 15/24] [PM-16863] Update "auto-fill" to "autofill" for org policies (#18483) * Fixes typo in messages.json from auto-fill to autofill to match company preference * Strings have to be immutable as learned from Brandon. Trying to delete old key-value pair to see if that's possible * Fix my typo --- apps/web/src/locales/en/messages.json | 4 ++-- .../policy-edit-definitions/activate-autofill.component.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 5a83bc75810..afb7c223f2c 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -6928,8 +6928,8 @@ "activateAutofill": { "message": "Activate auto-fill" }, - "activateAutofillPolicyDesc": { - "message": "Activate the auto-fill on page load setting on the browser extension for all existing and new members." + "activateAutofillPolicyDescription": { + "message": "Activate the autofill on page load setting on the browser extension for all existing and new members." }, "experimentalFeature": { "message": "Compromised or untrusted websites can exploit auto-fill on page load." diff --git a/bitwarden_license/bit-web/src/app/admin-console/policies/policy-edit-definitions/activate-autofill.component.ts b/bitwarden_license/bit-web/src/app/admin-console/policies/policy-edit-definitions/activate-autofill.component.ts index 08fe807f669..03eb189741c 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/policies/policy-edit-definitions/activate-autofill.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/policies/policy-edit-definitions/activate-autofill.component.ts @@ -12,7 +12,7 @@ import { SharedModule } from "@bitwarden/web-vault/app/shared"; export class ActivateAutofillPolicy extends BasePolicyEditDefinition { name = "activateAutofill"; - description = "activateAutofillPolicyDesc"; + description = "activateAutofillPolicyDescription"; type = PolicyType.ActivateAutofill; component = ActivateAutofillPolicyComponent; From fe753c9c02d28ba104c71cacf175bbf69d666523 Mon Sep 17 00:00:00 2001 From: Jared Date: Tue, 27 Jan 2026 13:34:23 -0500 Subject: [PATCH 16/24] Add support for DuckDuckGo browser in event service (#18576) --- apps/web/src/app/core/event.service.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/web/src/app/core/event.service.ts b/apps/web/src/app/core/event.service.ts index 36afd1850e0..47f4344ec36 100644 --- a/apps/web/src/app/core/event.service.ts +++ b/apps/web/src/app/core/event.service.ts @@ -722,6 +722,8 @@ export class EventService { return ["bwi-browser", this.i18nService.t("webVault") + " - Edge"]; case DeviceType.IEBrowser: return ["bwi-browser", this.i18nService.t("webVault") + " - IE"]; + case DeviceType.DuckDuckGoBrowser: + return ["bwi-browser", this.i18nService.t("webVault") + " - DuckDuckGo"]; case DeviceType.Server: return ["bwi-user-monitor", this.i18nService.t("server")]; case DeviceType.WindowsCLI: From 122bd9864309c7acbafebd5b56bd572493b41972 Mon Sep 17 00:00:00 2001 From: Jared Date: Tue, 27 Jan 2026 13:38:22 -0500 Subject: [PATCH 17/24] Refactor access tab label in collection dialog component to use a getter for improved readability and localization support. (#18537) --- .../collection-dialog/collection-dialog.component.html | 2 +- .../collection-dialog/collection-dialog.component.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html index e509692aba7..431d7711331 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html @@ -63,7 +63,7 @@ - +
{{ "readOnlyCollectionAccess" | i18n }} diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts index 4f40ea701d2..2f9ddddd8cb 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts @@ -361,6 +361,12 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { return this.params.readonly === true; } + protected get accessTabLabel(): string { + return this.dialogReadonly + ? this.i18nService.t("viewAccess") + : this.i18nService.t("editAccess"); + } + protected async cancel() { this.close(CollectionDialogAction.Canceled); } From 4ac38c18c0917e8ddfa8e39c2902add2df91276e Mon Sep 17 00:00:00 2001 From: Jared Date: Tue, 27 Jan 2026 13:39:34 -0500 Subject: [PATCH 18/24] [PM-27909] dialog improvements for claim domain (#18535) * Update domain status message from "Under verification" to "Pending" in localization and adjust corresponding template reference * Update domain status message from "Under verification" to "Pending" in the admin console template * Add domain verification instructions to the admin console dialog Enhanced the domain add/edit dialog by including detailed instructions for the automatic domain claim process when the domain is not verified. Removed the previous callout component for a more streamlined user experience. * Add new localization messages for automatic domain claim process Included detailed instructions for the automatic domain claim process, covering the steps for claiming a domain, account ownership change, and consequences of unclaimed domains. This enhances user guidance during domain management. * Refactor automatic domain claim process localization messages Updated localization keys for the automatic domain claim process to improve clarity and consistency. Removed redundant messages and streamlined the instructions displayed in the admin console dialog for better user experience. --- apps/web/src/locales/en/messages.json | 16 ++++++++-- .../domain-add-edit-dialog.component.html | 32 +++++++++++-------- .../domain-verification.component.html | 2 +- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index afb7c223f2c..9e210b6cb2e 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -11366,6 +11366,18 @@ "automaticDomainClaimProcess": { "message": "Bitwarden will attempt to claim the domain 3 times during the first 72 hours. If the domain can’t be claimed, check the DNS record in your host and manually claim. The domain will be removed from your organization in 7 days if it is not claimed." }, + "automaticDomainClaimProcess1": { + "message": "Bitwarden will attempt to claim the domain within 72 hours. If the domain can't be claimed, verify your DNS record and claim manually. Unclaimed domains are removed after 7 days." + }, + "automaticDomainClaimProcess2": { + "message": "Once claimed, existing members with claimed domains will be emailed about the " + }, + "accountOwnershipChange": { + "message": "account ownership change" + }, + "automaticDomainClaimProcessEnd": { + "message": "." + }, "domainNotClaimed": { "message": "$DOMAIN$ not claimed. Check your DNS records.", "placeholders": { @@ -11378,8 +11390,8 @@ "domainStatusClaimed": { "message": "Claimed" }, - "domainStatusUnderVerification": { - "message": "Under verification" + "domainStatusPending": { + "message": "Pending" }, "claimedDomainsDescription": { "message": "Claim a domain to own member accounts. The SSO identifier page will be skipped during login for members with claimed domains and administrators will be able to delete claimed accounts." diff --git a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-add-edit-dialog/domain-add-edit-dialog.component.html b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-add-edit-dialog/domain-add-edit-dialog.component.html index a2b231ffd48..80e76acac1d 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-add-edit-dialog/domain-add-edit-dialog.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-add-edit-dialog/domain-add-edit-dialog.component.html @@ -10,22 +10,34 @@ {{ "claimDomain" | i18n }} - - {{ data.orgDomain.domainName }} - - - {{ "domainStatusUnderVerification" | i18n }} + {{ "domainStatusPending" | i18n }} {{ "domainStatusClaimed" | i18n }}
+
+

{{ "automaticDomainClaimProcess1" | i18n }}

+

+ {{ "automaticDomainClaimProcess2" | i18n }} + + {{ "accountOwnershipChange" | i18n }} + + + {{ "automaticDomainClaimProcessEnd" | i18n }} +

+
{{ "domainName" | i18n }} - {{ "claimDomainNameInputHint" | i18n }} @@ -40,14 +52,6 @@ (click)="copyDnsTxt()" > - - - {{ "automaticDomainClaimProcess" | i18n }} -
@if (!decryptionFailure) { - + - + 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 d7de51ad20f..7a6c1db8026 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 @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { booleanAttribute, Component, Input } from "@angular/core"; +import { booleanAttribute, Component, input, Input } from "@angular/core"; import { Router, RouterModule } from "@angular/router"; import { BehaviorSubject, combineLatest, firstValueFrom, map, Observable, switchMap } from "rxjs"; import { filter } from "rxjs/operators"; @@ -76,22 +76,10 @@ export class ItemMoreOptionsComponent { } /** - * Flag to show view item menu option. Used when something else is - * assigned as the primary action for the item, such as autofill. - */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ transform: booleanAttribute }) - showViewOption = false; - - /** - * Flag to hide the autofill menu options. Used for items that are + * Flag to show the autofill menu options. Used for items that are * already in the autofill list suggestion. */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ transform: booleanAttribute }) - hideAutofillOptions = false; + readonly showAutofill = input(false, { transform: booleanAttribute }); protected autofillAllowed$ = this.vaultPopupAutofillService.autofillAllowed$; diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html index 3dac158b8e1..d3bc025905e 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html @@ -90,11 +90,11 @@ - + + - +