diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 6445789ede0..08ebab94637 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -63,6 +63,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications"; +import { SSO_COOKIE_VENDOR_CALLBACK_COMMAND } from "@bitwarden/common/platform/services/server-communication-config/server-communication-config-platform-api.service"; import { StateEventRunnerService } from "@bitwarden/common/platform/state"; import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; @@ -839,6 +840,12 @@ export class AppComponent implements OnInit, OnDestroy { // Process the sso callback links private processDeepLink(urlString: string) { + // Handle SSO cookie vendor callback + if (urlString.indexOf("bitwarden://sso-cookie-vendor") === 0) { + this.messagingService.send(SSO_COOKIE_VENDOR_CALLBACK_COMMAND, { urlString }); + return; + } + const url = new URL(urlString); const code = url.searchParams.get("code"); const receivedState = url.searchParams.get("state"); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index d585e8a595f..a82c42a8519 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -111,7 +111,7 @@ import { NoopSdkLoadService } from "@bitwarden/common/platform/services/sdk/noop import { DefaultServerCommunicationConfigService, ServerCommunicationConfigRepository, - NoopServerCommunicationConfigPlatformApiService, + ServerCommunicationConfigPlatformApiService, } from "@bitwarden/common/platform/services/server-communication-config"; import { SystemService } from "@bitwarden/common/platform/services/system.service"; import { GlobalStateProvider, StateProvider } from "@bitwarden/common/platform/state"; @@ -589,13 +589,29 @@ const safeProviders: SafeProvider[] = [ }), safeProvider({ provide: ServerCommunicationConfigService, - useFactory: (stateProvider: StateProvider, configService: ConfigService) => + useFactory: ( + stateProvider: StateProvider, + platformUtilsService: PlatformUtilsServiceAbstraction, + messageListener: MessageListener, + logService: LogService, + configService: ConfigService, + ) => new DefaultServerCommunicationConfigService( new ServerCommunicationConfigRepository(stateProvider), - new NoopServerCommunicationConfigPlatformApiService(), + new ServerCommunicationConfigPlatformApiService( + platformUtilsService, + messageListener, + logService, + ), configService, ), - deps: [StateProvider, ConfigService], + deps: [ + StateProvider, + PlatformUtilsServiceAbstraction, + MessageListener, + LogService, + ConfigService, + ], }), ]; diff --git a/libs/common/src/platform/abstractions/server-communication-config/server-communication-config.service.ts b/libs/common/src/platform/abstractions/server-communication-config/server-communication-config.service.ts index 63a60a57d70..674ed724616 100644 --- a/libs/common/src/platform/abstractions/server-communication-config/server-communication-config.service.ts +++ b/libs/common/src/platform/abstractions/server-communication-config/server-communication-config.service.ts @@ -29,4 +29,14 @@ export abstract class ServerCommunicationConfigService { * @returns Promise resolving to array of [cookie_name, cookie_value] tuples */ abstract getCookies(hostname: string): Promise>; + + /** + * Initiates cookie acquisition flow for the specified hostname. + * Opens browser for user authentication, then captures and validates cookies. + * + * @param url - The server url requiring cookies + * @returns Promise that resolves when cookies acquired and saved, or rejects on error/cancellation + * @throws AcquireCookieError on validation failure or cancellation + */ + abstract acquireCookie(url: string): Promise; } diff --git a/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.spec.ts b/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.spec.ts index f2b61d8cab7..4ba258b296f 100644 --- a/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.spec.ts +++ b/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.spec.ts @@ -21,8 +21,10 @@ jest.mock("@bitwarden/common/platform/abstractions/sdk/sdk-load.service", () => // Mock SDK client const mockClientInstance = { - setCommunicationType: jest.fn(), + needsBootstrap: jest.fn(), cookies: jest.fn(), + setCommunicationType: jest.fn(), + acquireCookie: jest.fn(), }; jest.mock("@bitwarden/sdk-internal", () => ({ diff --git a/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.ts b/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.ts index 8348cc756ca..41ce4e8a87f 100644 --- a/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.ts +++ b/libs/common/src/platform/services/server-communication-config/default-server-communication-config.service.ts @@ -66,4 +66,13 @@ export class DefaultServerCommunicationConfigService implements ServerCommunicat async getCookies(hostname: string): Promise> { return this.client.cookies(hostname); } + + async acquireCookie(url: string): Promise { + // SDK client handles: + // 1. Calling platform API (this.platformApi.acquireCookies(url)) + // 2. Validating cookie names match config + // 3. Saving validated cookies to repository + // 4. Throwing appropriate AcquireCookieError on failure + await this.client.acquireCookie(url); + } } diff --git a/libs/common/src/platform/services/server-communication-config/index.ts b/libs/common/src/platform/services/server-communication-config/index.ts index e918d4e6301..776f312b35f 100644 --- a/libs/common/src/platform/services/server-communication-config/index.ts +++ b/libs/common/src/platform/services/server-communication-config/index.ts @@ -1,4 +1,4 @@ export { ServerCommunicationConfigRepository } from "./server-communication-config.repository"; -export { NoopServerCommunicationConfigPlatformApiService } from "./noop-server-communication-config-platform-api.service"; +export { ServerCommunicationConfigPlatformApiService } from "./server-communication-config-platform-api.service"; export { DefaultServerCommunicationConfigService } from "./default-server-communication-config.service"; export { SERVER_COMMUNICATION_CONFIGS } from "./server-communication-config.state"; diff --git a/libs/common/src/platform/services/server-communication-config/noop-server-communication-config-platform-api.service.ts b/libs/common/src/platform/services/server-communication-config/noop-server-communication-config-platform-api.service.ts deleted file mode 100644 index a30e1198710..00000000000 --- a/libs/common/src/platform/services/server-communication-config/noop-server-communication-config-platform-api.service.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { AcquiredCookie, ServerCommunicationConfigPlatformApi } from "@bitwarden/sdk-internal"; - -/** - * Noop implementation for SSO cookie acquisition. - * - * Temporary implementation, will be replaced after https://github.com/bitwarden/clients/pull/18837 merges - */ -export class NoopServerCommunicationConfigPlatformApiService implements ServerCommunicationConfigPlatformApi { - constructor() {} - - async acquireCookies(hostname: string): Promise { - return; - } -} diff --git a/libs/common/src/platform/services/server-communication-config/server-communication-config-platform-api.service.spec.ts b/libs/common/src/platform/services/server-communication-config/server-communication-config-platform-api.service.spec.ts new file mode 100644 index 00000000000..ffb28491a0f --- /dev/null +++ b/libs/common/src/platform/services/server-communication-config/server-communication-config-platform-api.service.spec.ts @@ -0,0 +1,212 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { Subject } from "rxjs"; + +import { LogService } from "@bitwarden/logging"; +import { MessageListener } from "@bitwarden/messaging"; +import { AcquiredCookie } from "@bitwarden/sdk-internal"; + +import { PlatformUtilsService } from "../../abstractions/platform-utils.service"; + +import { ServerCommunicationConfigPlatformApiService } from "./server-communication-config-platform-api.service"; + +describe("ServerCommunicationConfigPlatformApiService", () => { + let platformUtilsService: MockProxy; + let messageListener: MockProxy; + let logService: MockProxy; + let service: ServerCommunicationConfigPlatformApiService; + let callbackSubject: Subject<{ urlString: string }>; + + beforeEach(() => { + platformUtilsService = mock(); + messageListener = mock(); + logService = mock(); + + // Create a Subject to simulate message stream + callbackSubject = new Subject<{ urlString: string }>(); + messageListener.messages$.mockReturnValue(callbackSubject.asObservable()); + + service = new ServerCommunicationConfigPlatformApiService( + platformUtilsService, + messageListener, + logService, + ); + }); + + describe("acquireCookies", () => { + it("opens browser to correct URL", async () => { + const promise = service.acquireCookies("vault.acme.com"); + + expect(platformUtilsService.launchUri).toHaveBeenCalledWith( + "https://vault.acme.com/proxy-cookie-redirect-connector.html", + ); + + // Simulate callback to resolve promise + callbackSubject.next({ + urlString: "bitwarden://sso-cookie-vendor?testCookie=value123", + }); + + await promise; + }); + + it("parses single cookie from callback URL", async () => { + const promise = service.acquireCookies("vault.acme.com"); + + // Simulate callback with single cookie + callbackSubject.next({ + urlString: "bitwarden://sso-cookie-vendor?AWSELBAuthSessionCookie=abc123", + }); + + const result = await promise; + + expect(result).toEqual([{ name: "AWSELBAuthSessionCookie", value: "abc123" }]); + }); + + it("parses sharded cookies from callback URL", async () => { + const promise = service.acquireCookies("vault.acme.com"); + + // Simulate callback with sharded cookies + callbackSubject.next({ + urlString: + "bitwarden://sso-cookie-vendor?AWSELBAuthSessionCookie-0=part1&AWSELBAuthSessionCookie-1=part2&AWSELBAuthSessionCookie-2=part3", + }); + + const result = await promise; + + expect(result).toEqual([ + { name: "AWSELBAuthSessionCookie-0", value: "part1" }, + { name: "AWSELBAuthSessionCookie-1", value: "part2" }, + { name: "AWSELBAuthSessionCookie-2", value: "part3" }, + ]); + }); + + it("excludes 'd' parameter from cookies", async () => { + const promise = service.acquireCookies("vault.acme.com"); + + // Simulate callback with 'd' integrity marker + callbackSubject.next({ + urlString: + "bitwarden://sso-cookie-vendor?AWSELBAuthSessionCookie=abc123&d=integrity_marker_value", + }); + + const result = await promise; + + // Should only have the cookie, not the 'd' parameter + expect(result).toEqual([{ name: "AWSELBAuthSessionCookie", value: "abc123" }]); + expect(result?.find((c: AcquiredCookie) => c.name === "d")).toBeUndefined(); + }); + + it("returns undefined when no cookies in callback", async () => { + const promise = service.acquireCookies("vault.acme.com"); + + // Simulate callback with only 'd' parameter (which is excluded) + callbackSubject.next({ + urlString: "bitwarden://sso-cookie-vendor?d=integrity", + }); + + const result = await promise; + + expect(result).toBeUndefined(); + }); + + it("returns undefined on timeout after 5 minutes", async () => { + jest.useFakeTimers(); + + const promise = service.acquireCookies("vault.acme.com"); + + // Fast-forward time by 5 minutes + jest.advanceTimersByTime(5 * 60 * 1000); + + const result = await promise; + + expect(result).toBeUndefined(); + expect(logService.warning).toHaveBeenCalledWith( + "Cookie acquisition timeout for vault.acme.com", + ); + + jest.useRealTimers(); + }); + + it("deduplicates concurrent calls for same hostname", async () => { + const promise1 = service.acquireCookies("vault.acme.com"); + const promise2 = service.acquireCookies("vault.acme.com"); + + // Should only launch browser once + expect(platformUtilsService.launchUri).toHaveBeenCalledTimes(1); + + // Simulate callback + callbackSubject.next({ + urlString: "bitwarden://sso-cookie-vendor?cookie=value", + }); + + const [result1, result2] = await Promise.all([promise1, promise2]); + + // Both promises should resolve with same result + expect(result1).toEqual([{ name: "cookie", value: "value" }]); + expect(result2).toEqual([{ name: "cookie", value: "value" }]); + }); + + it("cancels previous acquisition when different hostname requested", async () => { + jest.useFakeTimers(); + + // The esline rule below is disabled, as the promise purposefully is not awaited/resolved, to test the behaviour. + // eslint-disable-next-line @typescript-eslint/no-floating-promises + service.acquireCookies("vault1.acme.com"); + + // Request acquisition for different hostname + const promise2 = service.acquireCookies("vault2.acme.com"); + + // Should launch browser twice (once for each hostname) + expect(platformUtilsService.launchUri).toHaveBeenCalledTimes(2); + expect(platformUtilsService.launchUri).toHaveBeenCalledWith( + "https://vault1.acme.com/proxy-cookie-redirect-connector.html", + ); + expect(platformUtilsService.launchUri).toHaveBeenCalledWith( + "https://vault2.acme.com/proxy-cookie-redirect-connector.html", + ); + + // Simulate callback for second hostname + callbackSubject.next({ + urlString: "bitwarden://sso-cookie-vendor?cookie=value2", + }); + + // First promise should still be pending (not resolved) + // Second promise should resolve + const result2 = await promise2; + expect(result2).toEqual([{ name: "cookie", value: "value2" }]); + + expect(logService.warning).toHaveBeenCalledWith( + "Cancelling previous cookie acquisition for different hostname", + ); + + jest.useRealTimers(); + }); + + it("handles invalid callback URL gracefully", async () => { + const promise = service.acquireCookies("vault.acme.com"); + + // Simulate callback with invalid URL + callbackSubject.next({ + urlString: "not-a-valid-url", + }); + + const result = await promise; + + expect(result).toBeUndefined(); + expect(logService.error).toHaveBeenCalledWith( + "Failed to parse cookie callback URL", + expect.anything(), + ); + }); + + it("ignores callback when no acquisition pending", () => { + // Simulate callback without any pending acquisition + callbackSubject.next({ + urlString: "bitwarden://sso-cookie-vendor?cookie=value", + }); + + expect(logService.warning).toHaveBeenCalledWith( + "Received cookie callback but no acquisition pending", + ); + }); + }); +}); diff --git a/libs/common/src/platform/services/server-communication-config/server-communication-config-platform-api.service.ts b/libs/common/src/platform/services/server-communication-config/server-communication-config-platform-api.service.ts new file mode 100644 index 00000000000..61ae21d064c --- /dev/null +++ b/libs/common/src/platform/services/server-communication-config/server-communication-config-platform-api.service.ts @@ -0,0 +1,128 @@ +import { Subscription } from "rxjs"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CommandDefinition, MessageListener } from "@bitwarden/common/platform/messaging"; +import { AcquiredCookie, ServerCommunicationConfigPlatformApi } from "@bitwarden/sdk-internal"; + +export const SSO_COOKIE_VENDOR_CALLBACK_COMMAND = new CommandDefinition<{ urlString: string }>( + "ssoCookieVendorCallback", +); + +/** + * API implementation for SSO cookie acquisition. + * + * Handles browser launch and deep link callback processing for SSO cookie vendor flows. + * Uses the MessageListener to listen for deep link callbacks from app.component.ts. + * + * @remarks + * - Opens browser via platformUtilsService.launchUri() + * - Listens for callbacks via MessageListener subscribing to SSO_COOKIE_VENDOR_CALLBACK_COMMAND + * - Deduplicates concurrent calls (single in-flight promise) + * - 5-minute timeout for safety + * - Returns undefined on timeout/cancellation (not error) + * + */ +export class ServerCommunicationConfigPlatformApiService implements ServerCommunicationConfigPlatformApi { + private pendingAcquisition: { + hostname: string; + resolve: (cookies: AcquiredCookie[] | undefined) => void; + timeoutId: NodeJS.Timeout; + } | null = null; + + // Listen for callback messages from app.component.ts + private callbackSubscription: Subscription | null = this.messageListener + .messages$(SSO_COOKIE_VENDOR_CALLBACK_COMMAND) + .subscribe((msg) => { + this.handleCallback(msg.urlString); + }); + + private static readonly TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes + + constructor( + private platformUtilsService: PlatformUtilsService, + private messageListener: MessageListener, + private logService: LogService, + ) {} + + async acquireCookies(hostname: string): Promise { + // Deduplicate concurrent calls - return existing promise + if (this.pendingAcquisition) { + if (this.pendingAcquisition.hostname === hostname) { + this.logService.info( + "Cookie acquisition already in progress for hostname, returning existing promise", + ); + return new Promise((resolve) => { + const existing = this.pendingAcquisition!.resolve; + this.pendingAcquisition!.resolve = (cookies) => { + existing(cookies); + resolve(cookies); + }; + }); + } else { + // Different hostname - cancel previous and start new + this.logService.warning("Cancelling previous cookie acquisition for different hostname"); + this.cleanup(); + } + } + + return new Promise((resolve) => { + // Set 5-minute timeout + const timeoutId = setTimeout(() => { + this.logService.warning(`Cookie acquisition timeout for ${hostname}`); + this.cleanup(); + resolve(undefined); + }, ServerCommunicationConfigPlatformApiService.TIMEOUT_MS); + + this.pendingAcquisition = { hostname, resolve, timeoutId }; + + // Open browser to cookie redirect page + // FIXME: Ensure that hostname either includes the schema and remove the httpsL//-prefiox or strip it beforehand + const url = `https://${hostname}/proxy-cookie-redirect-connector.html`; + this.logService.info(`Opening browser for cookie acquisition: ${url}`); + this.platformUtilsService.launchUri(url); + }); + } + + /** + * Handles deep link callback from browser. + * Called via MessageListener subscription when app.component.ts sends callback message. + * + * @param urlString - Full callback URL with query parameters + */ + private handleCallback(urlString: string): void { + if (!this.pendingAcquisition) { + this.logService.warning("Received cookie callback but no acquisition pending"); + return; + } + + try { + const url = new URL(urlString); + const cookies: AcquiredCookie[] = []; + + // Parse all query params except 'd' (integrity marker) + for (const [name, value] of url.searchParams.entries()) { + if (name !== "d") { + cookies.push({ name, value }); + } + } + + this.logService.info(`Acquired ${cookies.length} cookie(s)`); + clearTimeout(this.pendingAcquisition.timeoutId); + this.pendingAcquisition.resolve(cookies.length > 0 ? cookies : undefined); + this.cleanup(); + } catch (error) { + this.logService.error("Failed to parse cookie callback URL", error); + clearTimeout(this.pendingAcquisition.timeoutId); + this.pendingAcquisition.resolve(undefined); + this.cleanup(); + } + } + + /** + * Cleans up pending acquisition state. + */ + private cleanup(): void { + this.pendingAcquisition = null; + } +}