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 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/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. 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/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", 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/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, 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/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/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) { 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)),