mirror of
https://github.com/bitwarden/clients.git
synced 2026-07-04 21:05:54 +08:00
[PM-29232] Add cookie acquisition (#19392)
* Add cookie acquisition to ServerCommunicationConfigService * Fix DI for ServerCommunicationConfigPlatformApiService * Rename param for acquireCookie from hostname to url --------- Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
This commit is contained in:
parent
e5a6312fee
commit
46bb683d7b
@ -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");
|
||||
|
||||
@ -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,
|
||||
],
|
||||
}),
|
||||
];
|
||||
|
||||
|
||||
@ -29,4 +29,14 @@ export abstract class ServerCommunicationConfigService {
|
||||
* @returns Promise resolving to array of [cookie_name, cookie_value] tuples
|
||||
*/
|
||||
abstract getCookies(hostname: string): Promise<Array<[string, string]>>;
|
||||
|
||||
/**
|
||||
* 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<void>;
|
||||
}
|
||||
|
||||
@ -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", () => ({
|
||||
|
||||
@ -66,4 +66,13 @@ export class DefaultServerCommunicationConfigService implements ServerCommunicat
|
||||
async getCookies(hostname: string): Promise<Array<[string, string]>> {
|
||||
return this.client.cookies(hostname);
|
||||
}
|
||||
|
||||
async acquireCookie(url: string): Promise<void> {
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -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";
|
||||
|
||||
@ -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<AcquiredCookie[] | undefined> {
|
||||
return;
|
||||
}
|
||||
}
|
||||
@ -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<PlatformUtilsService>;
|
||||
let messageListener: MockProxy<MessageListener>;
|
||||
let logService: MockProxy<LogService>;
|
||||
let service: ServerCommunicationConfigPlatformApiService;
|
||||
let callbackSubject: Subject<{ urlString: string }>;
|
||||
|
||||
beforeEach(() => {
|
||||
platformUtilsService = mock<PlatformUtilsService>();
|
||||
messageListener = mock<MessageListener>();
|
||||
logService = mock<LogService>();
|
||||
|
||||
// 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",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -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<AcquiredCookie[] | undefined> {
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user