Skip to content

Commit 95ba149

Browse files
committed
refactor: migrate OAuth callback to WindowBroadcast
1 parent a9e081b commit 95ba149

5 files changed

Lines changed: 29 additions & 62 deletions

File tree

src/core/secretsManager.ts

Lines changed: 13 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { z } from "zod";
22

33
import { DeploymentSchema, type Deployment } from "../deployment/types";
44
import { toSafeHost } from "../util";
5+
import { WindowBroadcast } from "../windowBroadcast";
56

67
import type { OAuth2ClientRegistrationResponse } from "coder/site/src/api/typesGenerated";
78
import type { Memento, SecretStorage, Disposable } from "vscode";
@@ -57,14 +58,24 @@ const OAuthCallbackDataSchema = z.object({
5758
error: z.string().nullable(),
5859
});
5960

60-
type OAuthCallbackData = z.infer<typeof OAuthCallbackDataSchema>;
61+
export type OAuthCallbackData = z.infer<typeof OAuthCallbackDataSchema>;
6162

6263
export class SecretsManager {
64+
public readonly oauthCallback: WindowBroadcast<OAuthCallbackData>;
65+
6366
constructor(
6467
private readonly secrets: SecretStorage,
6568
private readonly memento: Memento,
6669
private readonly logger: Logger,
67-
) {}
70+
) {
71+
this.oauthCallback = new WindowBroadcast(
72+
secrets,
73+
OAUTH_CALLBACK_KEY,
74+
(v: unknown): v is OAuthCallbackData =>
75+
OAuthCallbackDataSchema.safeParse(v).success,
76+
logger,
77+
);
78+
}
6879

6980
private buildKey(prefix: SecretKeyPrefix, safeHostname: string): string {
7081
return `${prefix}${safeHostname || "<legacy>"}`;
@@ -306,54 +317,6 @@ export class SecretsManager {
306317
return safeHostname;
307318
}
308319

309-
/**
310-
* Write an OAuth callback result to secrets storage.
311-
* Used for cross-window communication when OAuth callback arrives in a different window.
312-
*/
313-
public async setOAuthCallback(data: OAuthCallbackData): Promise<void> {
314-
const parsed = OAuthCallbackDataSchema.parse(data);
315-
await this.secrets.store(OAUTH_CALLBACK_KEY, JSON.stringify(parsed));
316-
}
317-
318-
/**
319-
* Listen for OAuth callback results from any VS Code window.
320-
* The listener receives the state parameter, code (if success), and error (if failed).
321-
*/
322-
public onDidChangeOAuthCallback(
323-
listener: (data: OAuthCallbackData) => void,
324-
): Disposable {
325-
return this.secrets.onDidChange(async (e) => {
326-
if (e.key !== OAUTH_CALLBACK_KEY) {
327-
return;
328-
}
329-
330-
const raw = await this.secrets.get(OAUTH_CALLBACK_KEY);
331-
if (!raw) {
332-
return;
333-
}
334-
335-
let parsed: unknown;
336-
try {
337-
parsed = JSON.parse(raw);
338-
} catch (err) {
339-
this.logger.error("Failed to parse OAuth callback JSON", err);
340-
return;
341-
}
342-
343-
const result = OAuthCallbackDataSchema.safeParse(parsed);
344-
if (!result.success) {
345-
this.logger.error("Invalid OAuth callback data shape", result.error);
346-
return;
347-
}
348-
349-
try {
350-
listener(result.data);
351-
} catch (err) {
352-
this.logger.error("Error in onDidChangeOAuthCallback listener", err);
353-
}
354-
});
355-
}
356-
357320
public getOAuthClientRegistration(
358321
safeHostname: string,
359322
): Promise<OAuth2ClientRegistrationResponse | undefined> {

src/oauth/authorizer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ export class OAuthAuthorizer implements vscode.Disposable {
247247
timeoutMins * 60 * 1000,
248248
);
249249

250-
const listener = this.secretsManager.onDidChangeOAuthCallback(
250+
const listener = this.secretsManager.oauthCallback.onReceive(
251251
({ state: callbackState, code, error }) => {
252252
if (callbackState !== state) {
253253
this.logger.warn(

src/uri/uriHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ async function handleOAuthCallback(ctx: UriRouteContext): Promise<void> {
211211
}
212212

213213
try {
214-
await secretsManager.setOAuthCallback({ state, code, error });
214+
await secretsManager.oauthCallback.send({ state, code, error });
215215
logger.debug("OAuth callback processed successfully");
216216
} catch (err) {
217217
logger.error("Failed to process OAuth callback:", err);

test/unit/oauth/authorizer.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ function createTestContext() {
8383

8484
/** Completes login by sending successful OAuth callback */
8585
const completeLogin = async (state: string) => {
86-
await base.secretsManager.setOAuthCallback({
86+
await base.secretsManager.oauthCallback.send({
8787
state,
8888
code: "code",
8989
error: null,
@@ -138,7 +138,7 @@ describe("OAuthAuthorizer", () => {
138138
const { state } = await waitForBrowserToOpen();
139139

140140
// Set the callback with the correct state (simulate user clicking authorize)
141-
await secretsManager.setOAuthCallback({
141+
await secretsManager.oauthCallback.send({
142142
state,
143143
code: "auth-code-123",
144144
error: null,
@@ -185,7 +185,7 @@ describe("OAuthAuthorizer", () => {
185185
const { authUrl, state } = await waitForBrowserToOpen();
186186
expect(authUrl.searchParams.get("client_id")).toBe("existing-client-id");
187187

188-
await secretsManager.setOAuthCallback({
188+
await secretsManager.oauthCallback.send({
189189
state,
190190
code: "code",
191191
error: null,
@@ -225,7 +225,7 @@ describe("OAuthAuthorizer", () => {
225225
const { authUrl, state } = await waitForBrowserToOpen();
226226
expect(authUrl.searchParams.get("client_id")).toBe("new-client-id");
227227

228-
await secretsManager.setOAuthCallback({
228+
await secretsManager.oauthCallback.send({
229229
state,
230230
code: "code",
231231
error: null,
@@ -267,7 +267,7 @@ describe("OAuthAuthorizer", () => {
267267
const { loginPromise, state } = await startLogin();
268268

269269
// Send callback with wrong state - should be ignored
270-
await secretsManager.setOAuthCallback({
270+
await secretsManager.oauthCallback.send({
271271
state: "wrong-state",
272272
code: "code",
273273
error: null,
@@ -292,7 +292,7 @@ describe("OAuthAuthorizer", () => {
292292
setupOAuthRoutes();
293293

294294
const { loginPromise, state } = await startLogin();
295-
await secretsManager.setOAuthCallback({
295+
await secretsManager.oauthCallback.send({
296296
state,
297297
code: null,
298298
error: "access_denied",
@@ -307,7 +307,11 @@ describe("OAuthAuthorizer", () => {
307307
setupOAuthRoutes();
308308

309309
const { loginPromise, state } = await startLogin();
310-
await secretsManager.setOAuthCallback({ state, code: null, error: null });
310+
await secretsManager.oauthCallback.send({
311+
state,
312+
code: null,
313+
error: null,
314+
});
311315

312316
await expect(loginPromise).rejects.toThrow(
313317
"No authorization code received",

test/unit/uri/uriHandler.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ describe("uriHandler", () => {
361361
const { handleUri, secretsManager } = createTestContext();
362362

363363
const callbackPromise = new Promise<CallbackData>((resolve) => {
364-
secretsManager.onDidChangeOAuthCallback(resolve);
364+
secretsManager.oauthCallback.onReceive(resolve);
365365
});
366366

367367
await handleUri(
@@ -380,7 +380,7 @@ describe("uriHandler", () => {
380380
const { handleUri, secretsManager } = createTestContext();
381381

382382
const callbackPromise = new Promise<CallbackData>((resolve) => {
383-
secretsManager.onDidChangeOAuthCallback(resolve);
383+
secretsManager.oauthCallback.onReceive(resolve);
384384
});
385385

386386
await handleUri(
@@ -399,7 +399,7 @@ describe("uriHandler", () => {
399399
const { handleUri, secretsManager } = createTestContext();
400400

401401
let callbackReceived = false;
402-
secretsManager.onDidChangeOAuthCallback(() => {
402+
secretsManager.oauthCallback.onReceive(() => {
403403
callbackReceived = true;
404404
});
405405

0 commit comments

Comments
 (0)