From 3a208e648efc3f88a35f0fb3067fefb07d1d3f30 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 17 Jul 2026 23:34:58 +0200 Subject: [PATCH] fix(auth): preserve OAuth interaction semantics --- src/client/src/api/parsers.test.ts | 44 ++++- src/client/src/api/parsers.ts | 48 +++++- src/client/src/components/AuthDialog.test.ts | 11 ++ src/client/src/components/AuthDialog.ts | 28 +++- .../src/controllers/authController.test.ts | 20 +++ .../sessions/oauthLoginFlowService.test.ts | 133 +++++++++++++-- src/server/sessions/oauthLoginFlowService.ts | 152 ++++++++++++------ src/shared/apiTypes.ts | 17 +- 8 files changed, 387 insertions(+), 66 deletions(-) create mode 100644 src/client/src/components/AuthDialog.test.ts diff --git a/src/client/src/api/parsers.test.ts b/src/client/src/api/parsers.test.ts index e661852..6020eb5 100644 --- a/src/client/src/api/parsers.test.ts +++ b/src/client/src/api/parsers.test.ts @@ -1,8 +1,50 @@ import { describe, expect, it } from "vitest"; import { PI_WEB_CAPABILITIES } from "../../../shared/capabilities"; -import { parseCommandResult, parseFileContentResponse, parseFileSuggestion, parseGitStatusResponse, parseMachineRuntime, parseMessagePage, parsePiPackageMutationResponse, parsePiPackagesResponse, parsePiWebConfigResponse, parsePiWebPluginsResponse, parsePiWebRuntimeResponse, parsePiWebStatusResponse, parseSessionBulkArchiveResponse, parseSessionBulkDeleteArchivedResponse, parseSessionCleanupExecuteResponse, parseSessionCleanupPreviewResponse, parseSessionInfo, parseSessionStatus, parseSessionStreamSnapshot, parseSlashCommand, parseTerminalCommandRun, parseTerminalInfo, parseWorkspace, parseWorkspaceActivityResponse } from "./parsers"; +import { parseCommandResult, parseFileContentResponse, parseFileSuggestion, parseGitStatusResponse, parseMachineRuntime, parseMessagePage, parseOAuthFlowState, parsePiPackageMutationResponse, parsePiPackagesResponse, parsePiWebConfigResponse, parsePiWebPluginsResponse, parsePiWebRuntimeResponse, parsePiWebStatusResponse, parseSessionBulkArchiveResponse, parseSessionBulkDeleteArchivedResponse, parseSessionCleanupExecuteResponse, parseSessionCleanupPreviewResponse, parseSessionInfo, parseSessionStatus, parseSessionStreamSnapshot, parseSlashCommand, parseTerminalCommandRun, parseTerminalInfo, parseWorkspace, parseWorkspaceActivityResponse } from "./parsers"; describe("API parsers", () => { + it("preserves additive OAuth interaction semantics", () => { + expect(parseOAuthFlowState({ + flowId: "flow-1", + providerId: "provider", + providerName: "Provider", + status: "running", + auth: { + url: "https://example.test/device", + instructions: "Enter code", + deviceCode: { userCode: "ABCD", intervalSeconds: 5, expiresInSeconds: 900 }, + }, + prompt: { requestId: "prompt-1", message: "Secret", kind: "prompt", promptType: "secret", allowEmpty: false, placeholder: "token" }, + select: { requestId: "select-1", message: "Choose", options: [{ value: "work", label: "Work", description: "Company account" }] }, + progress: ["Read the guide"], + info: [{ message: "Read the guide", links: [{ url: "https://example.test/docs", label: "Guide" }] }], + })).toMatchObject({ + auth: { deviceCode: { userCode: "ABCD", intervalSeconds: 5, expiresInSeconds: 900 } }, + prompt: { kind: "prompt", promptType: "secret", allowEmpty: false }, + select: { options: [{ value: "work", description: "Company account" }] }, + info: [{ links: [{ url: "https://example.test/docs", label: "Guide" }] }], + }); + }); + + it("defaults semantic prompt types from legacy OAuth wire kinds", () => { + const flow = { + flowId: "flow-1", + providerId: "provider", + providerName: "Provider", + status: "running", + progress: [], + }; + + expect(parseOAuthFlowState({ ...flow, prompt: { requestId: "text", message: "Value", kind: "prompt" } }).prompt).toMatchObject({ + kind: "prompt", + promptType: "text", + }); + expect(parseOAuthFlowState({ ...flow, prompt: { requestId: "manual", message: "Code", kind: "manual" } }).prompt).toMatchObject({ + kind: "manual", + promptType: "manual_code", + }); + }); + it("parses PI WEB config responses", () => { expect(parsePiWebConfigResponse({ path: "/tmp/config.json", diff --git a/src/client/src/api/parsers.ts b/src/client/src/api/parsers.ts index 2a22399..d97aa17 100644 --- a/src/client/src/api/parsers.ts +++ b/src/client/src/api/parsers.ts @@ -392,6 +392,7 @@ export function parseOAuthFlowState(value: unknown): OAuthFlowState { ...optionalField("auth", optionalOAuthAuth(record["auth"])), ...optionalField("prompt", optionalOAuthPrompt(record["prompt"])), ...optionalField("select", optionalOAuthSelect(record["select"])), + ...optionalField("info", optionalOAuthInfo(record["info"])), }; return flow; } @@ -404,7 +405,21 @@ function parseOAuthFlowStatus(value: unknown): OAuthFlowState["status"] { function optionalOAuthAuth(value: unknown): OAuthFlowState["auth"] | undefined { if (value === undefined) return undefined; const record = requireRecord(value); - return { url: requireString(record, "url"), ...optionalField("instructions", optionalString(record, "instructions")) }; + return { + url: requireString(record, "url"), + ...optionalField("instructions", optionalString(record, "instructions")), + ...optionalField("deviceCode", optionalOAuthDeviceCode(record["deviceCode"])), + }; +} + +function optionalOAuthDeviceCode(value: unknown): NonNullable["deviceCode"] | undefined { + if (value === undefined) return undefined; + const record = requireRecord(value); + return { + userCode: requireString(record, "userCode"), + ...optionalField("intervalSeconds", optionalNumber(record, "intervalSeconds")), + ...optionalField("expiresInSeconds", optionalNumber(record, "expiresInSeconds")), + }; } function optionalOAuthPrompt(value: unknown): OAuthFlowState["prompt"] | undefined { @@ -412,7 +427,20 @@ function optionalOAuthPrompt(value: unknown): OAuthFlowState["prompt"] | undefin const record = requireRecord(value); const kind = requireString(record, "kind"); if (kind !== "prompt" && kind !== "manual") throw new Error("Invalid OAuth prompt kind"); - return { requestId: requireString(record, "requestId"), message: requireString(record, "message"), kind, ...optionalField("placeholder", optionalString(record, "placeholder")), ...(record["allowEmpty"] === true ? { allowEmpty: true } : {}) }; + const promptType = record["promptType"] === undefined ? (kind === "manual" ? "manual_code" : "text") : parseOAuthPromptType(record["promptType"]); + return { + requestId: requireString(record, "requestId"), + message: requireString(record, "message"), + kind, + promptType, + ...optionalField("placeholder", optionalString(record, "placeholder")), + ...optionalField("allowEmpty", optionalBoolean(record, "allowEmpty")), + }; +} + +function parseOAuthPromptType(value: unknown): "text" | "secret" | "manual_code" { + if (value !== "text" && value !== "secret" && value !== "manual_code") throw new Error("Invalid OAuth prompt type"); + return value; } function optionalOAuthSelect(value: unknown): OAuthFlowState["select"] | undefined { @@ -421,6 +449,22 @@ function optionalOAuthSelect(value: unknown): OAuthFlowState["select"] | undefin return { requestId: requireString(record, "requestId"), message: requireString(record, "message"), options: arrayOf(parseCommandOption)(record["options"]) }; } +function optionalOAuthInfo(value: unknown): OAuthFlowState["info"] | undefined { + if (value === undefined) return undefined; + return arrayOf((item) => { + const record = requireRecord(item); + return { + message: requireString(record, "message"), + ...optionalField("links", record["links"] === undefined ? undefined : arrayOf(parseOAuthInfoLink)(record["links"])), + }; + })(value); +} + +function parseOAuthInfoLink(value: unknown): NonNullable[number]["links"]>[number] { + const record = requireRecord(value); + return { url: requireString(record, "url"), ...optionalField("label", optionalString(record, "label")) }; +} + function optionalContextUsage(value: unknown): Pick | object { if (value === undefined) return {}; const record = requireRecord(value); diff --git a/src/client/src/components/AuthDialog.test.ts b/src/client/src/components/AuthDialog.test.ts new file mode 100644 index 0000000..788ccc9 --- /dev/null +++ b/src/client/src/components/AuthDialog.test.ts @@ -0,0 +1,11 @@ +import { describe, expect, it } from "vitest"; +import { oauthPromptInputType } from "./AuthDialog"; + +describe("oauthPromptInputType", () => { + it("renders additive secret prompts as password inputs and defaults legacy prompts to text", () => { + expect(oauthPromptInputType("secret")).toBe("password"); + expect(oauthPromptInputType("text")).toBe("text"); + expect(oauthPromptInputType("manual_code")).toBe("text"); + expect(oauthPromptInputType(undefined)).toBe("text"); + }); +}); diff --git a/src/client/src/components/AuthDialog.ts b/src/client/src/components/AuthDialog.ts index 7225ee2..7663a55 100644 --- a/src/client/src/components/AuthDialog.ts +++ b/src/client/src/components/AuthDialog.ts @@ -1,7 +1,7 @@ import { LitElement, css, html } from "lit"; import { customElement, property, query } from "lit/decorators.js"; import type { AuthDialogState } from "../appState"; -import type { AuthProviderOption } from "../api"; +import type { AuthProviderOption, OAuthFlowState } from "../api"; import { commandPickerStyles } from "./shared"; @customElement("auth-dialog") @@ -86,22 +86,35 @@ export class AuthDialog extends LitElement { const flow = state.flow; const prompt = flow.prompt; const select = flow.select; + const promptInputType = oauthPromptInputType(prompt?.promptType); return html`
${flow.auth !== undefined ? html`

Open this authorization link:

${flow.auth.url}

- ${flow.auth.instructions !== undefined ? html`

${flow.auth.instructions}

` : null} + ${flow.auth.deviceCode !== undefined ? html` +

Enter code: ${flow.auth.deviceCode.userCode}

+ ` : flow.auth.instructions !== undefined ? html`

${flow.auth.instructions}

` : null} ` : html`

Starting login flow…

`} ${flow.progress.length > 0 ? html`
    ${flow.progress.map((line) => html`
  • ${line}
    • `)}
` : null} + ${flow.info?.map((item) => item.links === undefined || item.links.length === 0 ? null : html` +
+ ${item.links.map((link) => html`${link.label ?? link.url}`)} +
+ `) ?? null} ${prompt !== undefined ? html` - { if (event.target instanceof HTMLInputElement) this.onOAuthInput?.(event.target.value); }}> + { if (event.target instanceof HTMLInputElement) this.onOAuthInput?.(event.target.value); }}>
` : null} ${select !== undefined ? html`

${select.message}

-
${select.options.map((option) => html``)}
+
${select.options.map((option) => html` + + `)}
` : null} ${state.error !== undefined && state.error !== "" ? html`
${state.error}
` : null} ${flow.status === "error" || flow.status === "cancelled" ? html`
${flow.error ?? flow.status}
` : null} @@ -157,11 +170,18 @@ export class AuthDialog extends LitElement { .warning { color: var(--pi-warning); } .error-text { color: var(--pi-danger); } .progress { margin: 0; padding-left: 18px; color: var(--pi-muted); } + .info-links { display: flex; flex-wrap: wrap; gap: 8px 12px; } .inline-options { display: grid; gap: 8px; } + .inline-options button { display: grid; gap: 2px; text-align: left; } + .inline-options small { color: var(--pi-muted); } em { color: var(--pi-success); font-style: normal; font-size: 12px; } `]; } +export function oauthPromptInputType(promptType: NonNullable["promptType"]): "text" | "password" { + return promptType === "secret" ? "password" : "text"; +} + function authTypeLabel(authType: "oauth" | "api_key"): string { return authType === "oauth" ? "subscription" : "API key"; } diff --git a/src/client/src/controllers/authController.test.ts b/src/client/src/controllers/authController.test.ts index e344174..1e9481b 100644 --- a/src/client/src/controllers/authController.test.ts +++ b/src/client/src/controllers/authController.test.ts @@ -43,6 +43,26 @@ describe("AuthController", () => { expect(getState().authDialog).toMatchObject({ step: "oauth", inputValue: "https://callback", responding: true }); }); + it("submits an allowed blank OAuth text response without client-side rejection", async () => { + const flow = oauthFlow({ + prompt: { requestId: "request-1", message: "GitHub Enterprise URL/domain (blank for github.com)", kind: "prompt", promptType: "text", allowEmpty: true }, + }); + const respondCalls: string[] = []; + const { controller } = createController( + { authDialog: { step: "oauth", flow, inputValue: "" } }, + { + respondOAuthFlow: (_flowId, _requestId, value) => { + respondCalls.push(value); + return Promise.resolve(oauthFlow({ status: "complete" })); + }, + }, + ); + + await controller.respondOAuth(); + + expect(respondCalls).toEqual([""]); + }); + it("resets OAuth prompt input and submit state when the request id changes", async () => { const flow = oauthFlow({ prompt: { requestId: "request-1", message: "Paste callback", kind: "manual" } }); const { controller, getState } = createController( diff --git a/src/server/sessions/oauthLoginFlowService.test.ts b/src/server/sessions/oauthLoginFlowService.test.ts index 6bc75bb..6349b38 100644 --- a/src/server/sessions/oauthLoginFlowService.test.ts +++ b/src/server/sessions/oauthLoginFlowService.test.ts @@ -29,7 +29,7 @@ describe("OAuthLoginFlowService", () => { const prompt = state.prompt; if (prompt === undefined) throw new Error("Expected prompt"); expect(state).toMatchObject({ auth: { url: "https://example.test/auth", instructions: "Open it" }, progress: ["Waiting for code"] }); - expect(prompt).toMatchObject({ message: "Paste code", placeholder: "code", kind: "prompt" }); + expect(prompt).toMatchObject({ message: "Paste code", placeholder: "code", kind: "prompt", promptType: "text", allowEmpty: true }); const afterRespond = service.respond(state.flowId, prompt.requestId, "abc123"); expect(afterRespond.prompt).toBeUndefined(); @@ -41,18 +41,103 @@ describe("OAuthLoginFlowService", () => { service.dispose(); }); + it("allows blank text responses for providers that use blank as a default", async () => { + let domain: string | undefined; + const service = new OAuthLoginFlowService(); + const state = service.start({ + providerId: "github-copilot", + providerName: "GitHub Copilot", + runtime: fakeRuntime(async (_providerId, interaction) => { + domain = await interaction.prompt({ + type: "text", + message: "GitHub Enterprise URL/domain (blank for github.com)", + }); + }), + }); + + const prompt = state.prompt; + if (prompt === undefined) throw new Error("Expected text prompt"); + expect(prompt).toMatchObject({ kind: "prompt", promptType: "text", allowEmpty: true }); + + service.respond(state.flowId, prompt.requestId, ""); + await flushAsyncLogin(); + + expect(domain).toBe(""); + expect(service.get(state.flowId).status).toBe("complete"); + service.dispose(); + }); + + it("preserves secret prompt semantics behind the legacy prompt kind", () => { + const service = new OAuthLoginFlowService(); + const state = service.start({ + providerId: "test-provider", + providerName: "Test Provider", + runtime: fakeRuntime(async (_providerId, interaction) => { + await interaction.prompt({ type: "secret", message: "Enter secret", placeholder: "token" }); + }), + }); + + const prompt = state.prompt; + if (prompt === undefined) throw new Error("Expected secret prompt"); + expect(prompt).toMatchObject({ + kind: "prompt", + promptType: "secret", + message: "Enter secret", + placeholder: "token", + }); + expect(prompt).not.toHaveProperty("allowEmpty"); + expect(() => { service.respond(state.flowId, prompt.requestId, ""); }).toThrow("A value is required"); + service.dispose(); + }); + + it("preserves info-event links without replacing the authorization URL", () => { + const service = new OAuthLoginFlowService(); + const state = service.start({ + providerId: "test-provider", + providerName: "Test Provider", + runtime: fakeRuntime(async (_providerId, interaction) => { + interaction.notify({ type: "auth_url", url: "https://example.test/login" }); + interaction.notify({ + type: "info", + message: "Review the provider setup guide", + links: [{ url: "https://example.test/docs", label: "Setup guide" }], + }); + await interaction.prompt({ type: "text", message: "Continue" }); + }), + }); + + expect(state).toMatchObject({ + auth: { url: "https://example.test/login" }, + progress: ["Review the provider setup guide"], + info: [{ message: "Review the provider setup guide", links: [{ url: "https://example.test/docs", label: "Setup guide" }] }], + }); + service.dispose(); + }); + it("surfaces device-code events through the auth field", () => { const service = new OAuthLoginFlowService(); const state = service.start({ providerId: "test-provider", providerName: "Test Provider", runtime: fakeRuntime(async (_providerId, interaction) => { - interaction.notify({ type: "device_code", userCode: "WXYZ-1234", verificationUri: "https://example.test/device" }); + interaction.notify({ + type: "device_code", + userCode: "WXYZ-1234", + verificationUri: "https://example.test/device", + intervalSeconds: 5, + expiresInSeconds: 900, + }); await interaction.prompt({ type: "text", message: "Waiting" }); }), }); - expect(service.get(state.flowId)).toMatchObject({ auth: { url: "https://example.test/device", instructions: "Enter code: WXYZ-1234" } }); + expect(service.get(state.flowId)).toMatchObject({ + auth: { + url: "https://example.test/device", + instructions: "Enter code: WXYZ-1234", + deviceCode: { userCode: "WXYZ-1234", intervalSeconds: 5, expiresInSeconds: 900 }, + }, + }); service.dispose(); }); @@ -66,14 +151,14 @@ describe("OAuthLoginFlowService", () => { selectedValue = await interaction.prompt({ type: "select", message: "Choose account", - options: [{ id: "work", label: "Work" }, { id: "personal", label: "Personal" }], + options: [{ id: "work", label: "Work", description: "Company account" }, { id: "personal", label: "Personal" }], }); }), }); const select = state.select; if (select === undefined) throw new Error("Expected select prompt"); - expect(select).toMatchObject({ message: "Choose account", options: [{ value: "work", label: "Work" }, { value: "personal", label: "Personal" }] }); + expect(select).toMatchObject({ message: "Choose account", options: [{ value: "work", label: "Work", description: "Company account" }, { value: "personal", label: "Personal" }] }); service.respond(state.flowId, select.requestId, "personal"); await flushAsyncLogin(); @@ -83,25 +168,53 @@ describe("OAuthLoginFlowService", () => { service.dispose(); }); - it("uses a manual-code prompt for callback-server flows", async () => { - let manualValue: string | undefined; + it("rejects responses outside the pending select options", () => { const service = new OAuthLoginFlowService(); const state = service.start({ providerId: "test-provider", providerName: "Test Provider", runtime: fakeRuntime(async (_providerId, interaction) => { - manualValue = await interaction.prompt({ type: "manual_code", message: "Paste the callback URL or authorization code" }); + await interaction.prompt({ + type: "select", + message: "Choose account", + options: [{ id: "work", label: "Work" }], + }); + }), + }); + + const select = state.select; + if (select === undefined) throw new Error("Expected select prompt"); + expect(() => { service.respond(state.flowId, select.requestId, "personal"); }).toThrow("Invalid OAuth selection"); + expect(service.get(state.flowId).select).toEqual(select); + service.dispose(); + }); + + it("uses a manual-code prompt for callback-server flows and cleans up its abort listener", async () => { + let manualValue: string | undefined; + const service = new OAuthLoginFlowService(); + const controller = new AbortController(); + const removeAbortListener = vi.spyOn(controller.signal, "removeEventListener"); + const state = service.start({ + providerId: "test-provider", + providerName: "Test Provider", + runtime: fakeRuntime(async (_providerId, interaction) => { + manualValue = await interaction.prompt({ + type: "manual_code", + message: "Paste the callback URL or authorization code", + signal: controller.signal, + }); }), }); const prompt = state.prompt; if (prompt === undefined) throw new Error("Expected manual prompt"); - expect(prompt).toMatchObject({ kind: "manual", message: "Paste the callback URL or authorization code" }); + expect(prompt).toMatchObject({ kind: "manual", promptType: "manual_code", message: "Paste the callback URL or authorization code" }); service.respond(state.flowId, prompt.requestId, "https://localhost/callback?code=abc"); await flushAsyncLogin(); expect(manualValue).toBe("https://localhost/callback?code=abc"); + expect(removeAbortListener).toHaveBeenCalledWith("abort", expect.any(Function)); expect(service.get(state.flowId).status).toBe("complete"); service.dispose(); }); @@ -110,6 +223,7 @@ describe("OAuthLoginFlowService", () => { const promptRejected = deferred(); const service = new OAuthLoginFlowService(); const controller = new AbortController(); + const removeAbortListener = vi.spyOn(controller.signal, "removeEventListener"); const state = service.start({ providerId: "test-provider", providerName: "Test Provider", @@ -128,6 +242,7 @@ describe("OAuthLoginFlowService", () => { expect(state.prompt).toMatchObject({ kind: "manual" }); controller.abort(); await expect(promptRejected.promise).resolves.toMatchObject({ message: "Prompt cancelled" }); + expect(removeAbortListener).toHaveBeenCalledWith("abort", expect.any(Function)); const afterAbort = service.get(state.flowId); expect(afterAbort.status).toBe("running"); diff --git a/src/server/sessions/oauthLoginFlowService.ts b/src/server/sessions/oauthLoginFlowService.ts index cf150bf..e5d9786 100644 --- a/src/server/sessions/oauthLoginFlowService.ts +++ b/src/server/sessions/oauthLoginFlowService.ts @@ -6,12 +6,16 @@ import type { CommandOption, OAuthFlowState } from "../../shared/apiTypes.js"; /** The single runtime capability this service drives — narrowed for testable DI. */ type OAuthLoginRuntime = Pick; type TimerHandle = ReturnType; +type SelectPrompt = Extract; +type ValuePrompt = Exclude; interface PendingOAuthRequest { requestId: string; allowEmpty: boolean; resolve: (value: string) => void; reject: (error: Error) => void; + allowedValues?: ReadonlySet; + cleanup?: () => void; } interface OAuthFlowRecord { @@ -79,13 +83,13 @@ export class OAuthLoginFlowService { void options.runtime.login(options.providerId, "oauth", interaction) .then(() => { if (!this.isCurrentRunning(record)) return; - record.pending = undefined; + this.clearPending(record); this.markTerminal(record, { ...withoutInteraction(record.state), status: "complete", progress: [...record.state.progress, "Login complete"] }); options.onComplete?.(); }) .catch((error: unknown) => { if (this.flows.get(record.flowId) !== record) return; - record.pending = undefined; + this.clearPending(record); if (record.state.status !== "running") return; this.markTerminal(record, { ...withoutInteraction(record.state), status: "error", error: error instanceof Error ? error.message : String(error) }); }); @@ -106,7 +110,8 @@ export class OAuthLoginFlowService { const pending = record.pending; if (pending?.requestId !== requestId) throw new Error("OAuth login request expired"); if (!pending.allowEmpty && value.trim() === "") throw new Error("A value is required"); - record.pending = undefined; + if (pending.allowedValues !== undefined && !pending.allowedValues.has(value)) throw new Error("Invalid OAuth selection"); + this.clearPending(record); this.updateState(record, withoutInteraction(record.state)); pending.resolve(value); return cloneState(record.state); @@ -117,8 +122,7 @@ export class OAuthLoginFlowService { if (record === undefined) throw new Error("OAuth login flow not found"); if (record.state.status === "running") { record.abort.abort(); - const pending = record.pending; - record.pending = undefined; + const pending = this.clearPending(record); this.markTerminal(record, { ...withoutInteraction(record.state), status: "cancelled", error: "Login cancelled" }); pending?.reject(new Error("Login cancelled")); } @@ -129,25 +133,15 @@ export class OAuthLoginFlowService { for (const record of this.flows.values()) { this.clearTimer(record); record.abort.abort(); - const pending = record.pending; - record.pending = undefined; + const pending = this.clearPending(record); pending?.reject(new Error("Login cancelled")); } this.flows.clear(); } private handlePrompt(record: OAuthFlowRecord, prompt: AuthPrompt): Promise { - if (prompt.type === "select") { - return this.waitForSelect(record, prompt.message, prompt.options, prompt.signal); - } - // `manual_code` is the paste-back path for callback-server flows; text/secret - // are ordinary interactive entry. Both map to the single web-UI prompt shape. - const kind = prompt.type === "manual_code" ? "manual" : "prompt"; - return this.waitForPrompt(record, { - message: prompt.message, - ...(prompt.placeholder === undefined ? {} : { placeholder: prompt.placeholder }), - ...(prompt.signal === undefined ? {} : { signal: prompt.signal }), - }, kind); + if (prompt.type === "select") return this.waitForSelect(record, prompt); + return this.waitForPrompt(record, prompt); } private handleEvent(record: OAuthFlowRecord, event: AuthEvent): void { @@ -156,75 +150,127 @@ export class OAuthLoginFlowService { case "auth_url": this.updateState(record, { ...record.state, auth: { url: event.url, ...(event.instructions === undefined ? {} : { instructions: event.instructions }) } }); return; - // Device-code flows have no redirect URL; reuse the auth field so the web UI - // shows the verification link and user code without a dedicated API shape. + // Keep the legacy auth URL/instructions while adding structured metadata + // that newer browsers can use during rolling sessiond upgrades. case "device_code": - this.updateState(record, { ...record.state, auth: { url: event.verificationUri, instructions: `Enter code: ${event.userCode}` } }); + this.updateState(record, { + ...record.state, + auth: { + url: event.verificationUri, + instructions: `Enter code: ${event.userCode}`, + deviceCode: { + userCode: event.userCode, + ...(event.intervalSeconds === undefined ? {} : { intervalSeconds: event.intervalSeconds }), + ...(event.expiresInSeconds === undefined ? {} : { expiresInSeconds: event.expiresInSeconds }), + }, + }, + }); return; case "info": + this.updateState(record, { + ...record.state, + progress: [...record.state.progress, event.message], + info: [ + ...(record.state.info ?? []), + { + message: event.message, + ...(event.links === undefined ? {} : { + links: event.links.map((link) => ({ + url: link.url, + ...(link.label === undefined ? {} : { label: link.label }), + })), + }), + }, + ], + }); + return; case "progress": this.updateState(record, { ...record.state, progress: [...record.state.progress, event.message] }); return; } } - private waitForPrompt(record: OAuthFlowRecord, prompt: { message: string; placeholder?: string; signal?: AbortSignal }, kind: "prompt" | "manual"): Promise { + private waitForPrompt(record: OAuthFlowRecord, prompt: ValuePrompt): Promise { return new Promise((resolve, reject) => { if (!this.isCurrentRunning(record)) { reject(new Error("Login cancelled")); return; } - if (prompt.signal?.aborted === true) { - reject(new Error("Prompt cancelled")); - return; - } const requestId = crypto.randomUUID(); - record.pending = { requestId, allowEmpty: false, resolve, reject }; - this.bindPromptSignal(record, requestId, prompt.signal); + const pending: PendingOAuthRequest = { + requestId, + allowEmpty: prompt.type === "text", + resolve, + reject, + }; + record.pending = pending; + if (!this.bindPromptSignal(record, pending, prompt.signal)) return; const base = withoutInteraction(record.state); this.updateState(record, { ...base, prompt: { requestId, message: prompt.message, - kind, + kind: prompt.type === "manual_code" ? "manual" : "prompt", + promptType: prompt.type, + ...(prompt.type === "text" ? { allowEmpty: true } : {}), ...(prompt.placeholder === undefined ? {} : { placeholder: prompt.placeholder }), }, }); }); } - private waitForSelect(record: OAuthFlowRecord, message: string, promptOptions: readonly { id: string; label: string; description?: string }[], signal?: AbortSignal): Promise { + private waitForSelect(record: OAuthFlowRecord, prompt: SelectPrompt): Promise { return new Promise((resolve, reject) => { if (!this.isCurrentRunning(record)) { reject(new Error("Login cancelled")); return; } - if (signal?.aborted === true) { - reject(new Error("Prompt cancelled")); - return; - } const requestId = crypto.randomUUID(); - const options: CommandOption[] = promptOptions.map((option) => ({ value: option.id, label: option.label })); - record.pending = { requestId, allowEmpty: true, resolve, reject }; - this.bindPromptSignal(record, requestId, signal); + const options: CommandOption[] = prompt.options.map((option) => ({ + value: option.id, + label: option.label, + ...(option.description === undefined ? {} : { description: option.description }), + })); + const pending: PendingOAuthRequest = { + requestId, + allowEmpty: false, + resolve, + reject, + allowedValues: new Set(options.map((option) => option.value)), + }; + record.pending = pending; + if (!this.bindPromptSignal(record, pending, prompt.signal)) return; const base = withoutInteraction(record.state); - this.updateState(record, { ...base, select: { requestId, message, options } }); + this.updateState(record, { ...base, select: { requestId, message: prompt.message, options } }); }); } // A prompt may carry its own AbortSignal (e.g. a manual_code prompt raced // against a callback server). When it fires, drop just that pending request // and clear the interaction from state — the overall login keeps running. - private bindPromptSignal(record: OAuthFlowRecord, requestId: string, signal?: AbortSignal): void { - if (signal === undefined) return; - signal.addEventListener("abort", () => { - const pending = record.pending; - if (pending?.requestId !== requestId) return; - record.pending = undefined; + private bindPromptSignal(record: OAuthFlowRecord, pending: PendingOAuthRequest, signal?: AbortSignal): boolean { + if (signal === undefined) return true; + const onAbort = () => { + if (record.pending !== pending) return; + this.clearPending(record); if (this.isCurrentRunning(record)) this.updateState(record, withoutInteraction(record.state)); pending.reject(new Error("Prompt cancelled")); - }, { once: true }); + }; + pending.cleanup = () => { signal.removeEventListener("abort", onAbort); }; + signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) { + onAbort(); + return false; + } + return true; + } + + private clearPending(record: OAuthFlowRecord): PendingOAuthRequest | undefined { + const pending = record.pending; + record.pending = undefined; + pending?.cleanup?.(); + return pending; } private isCurrentRunning(record: OAuthFlowRecord): boolean { @@ -270,8 +316,7 @@ export class OAuthLoginFlowService { private expireRunningFlow(record: OAuthFlowRecord): void { if (!this.isCurrentRunning(record)) return; record.abort.abort(); - const pending = record.pending; - record.pending = undefined; + const pending = this.clearPending(record); this.markTerminal(record, { ...withoutInteraction(record.state), status: "error", error: "OAuth login flow expired" }); pending?.reject(new Error("OAuth login flow expired")); } @@ -300,9 +345,20 @@ function cloneState(state: OAuthFlowState): OAuthFlowState { return { ...state, progress: [...state.progress], - ...(state.auth === undefined ? {} : { auth: { ...state.auth } }), + ...(state.auth === undefined ? {} : { + auth: { + ...state.auth, + ...(state.auth.deviceCode === undefined ? {} : { deviceCode: { ...state.auth.deviceCode } }), + }, + }), ...(state.prompt === undefined ? {} : { prompt: { ...state.prompt } }), ...(state.select === undefined ? {} : { select: { ...state.select, options: state.select.options.map((option) => ({ ...option })) } }), + ...(state.info === undefined ? {} : { + info: state.info.map((item) => ({ + ...item, + ...(item.links === undefined ? {} : { links: item.links.map((link) => ({ ...link })) }), + })), + }), }; } diff --git a/src/shared/apiTypes.ts b/src/shared/apiTypes.ts index 24ffc97..78666d4 100644 --- a/src/shared/apiTypes.ts +++ b/src/shared/apiTypes.ts @@ -384,10 +384,23 @@ export interface OAuthFlowState { providerId: string; providerName: string; status: "running" | "complete" | "error" | "cancelled"; - auth?: { url: string; instructions?: string }; - prompt?: { requestId: string; message: string; placeholder?: string; allowEmpty?: boolean; kind: "prompt" | "manual" }; + auth?: { + url: string; + instructions?: string; + deviceCode?: { userCode: string; intervalSeconds?: number; expiresInSeconds?: number }; + }; + prompt?: { + requestId: string; + message: string; + placeholder?: string; + allowEmpty?: boolean; + /** Additive semantic detail; legacy peers continue to use `kind`. */ + promptType?: "text" | "secret" | "manual_code"; + kind: "prompt" | "manual"; + }; select?: { requestId: string; message: string; options: CommandOption[] }; progress: string[]; + info?: { message: string; links?: { url: string; label?: string }[] }[]; error?: string; }