diff --git a/packages/sim-cli/src/auth/device-flow.test.ts b/packages/sim-cli/src/auth/device-flow.test.ts index 6fa31cc3d80..42a9cf2a724 100644 --- a/packages/sim-cli/src/auth/device-flow.test.ts +++ b/packages/sim-cli/src/auth/device-flow.test.ts @@ -137,6 +137,40 @@ describe('createAuthRequest', () => { } }) + it('keeps a path prefix the endpoint carries, in both login URLs', async () => { + // Both URLs were built with `new URL('/path', endpoint)`. A leading-slash + // path is absolute, so it resolved against the ORIGIN and dropped the + // prefix: a deployment served at https://host/sim sent the browser to + // https://host/cli/auth and polled https://host/api/cli/auth/poll, neither + // of which exists there. Every other command concatenated and worked, so + // the endpoint looked correct and only login failed. + const prefixed = 'https://host.test/sim' + const auth = createAuthRequest() + + expect(buildApprovalUrl(prefixed, auth, 'platform')).toMatch( + /^https:\/\/host\.test\/sim\/cli\/auth\?/ + ) + + // `spyOn`, like the rest of this file: `restoreAllMocks` in teardown undoes + // it, whereas a `stubGlobal` would outlive the test and leak this + // completed-auth response into whatever ran next. + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response(JSON.stringify({ status: 'complete', key: { id: 'k', apiKey: 'sk' } }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + + await pollForKey(prefixed, auth) + expect(fetchSpy.mock.calls[0][0]).toBe('https://host.test/sim/api/cli/auth/poll') + }) + + it('omits an absent workspace rather than sending it blank', () => { + const auth = createAuthRequest() + expect(buildApprovalUrl(ENDPOINT, auth, 'platform')).not.toContain('workspace=') + expect(buildApprovalUrl(ENDPOINT, auth, 'platform', 'ws_1')).toContain('workspace=ws_1') + }) + it('never puts the poll secret in the browser URL', () => { const auth = createAuthRequest() const url = buildApprovalUrl(ENDPOINT, auth, 'platform', 'ws_1') diff --git a/packages/sim-cli/src/auth/device-flow.ts b/packages/sim-cli/src/auth/device-flow.ts index 1c72a2e9f34..da04111a15b 100644 --- a/packages/sim-cli/src/auth/device-flow.ts +++ b/packages/sim-cli/src/auth/device-flow.ts @@ -1,6 +1,6 @@ import { createHash, randomBytes, randomInt } from 'node:crypto' import { sleep } from '../helpers' -import { REDIRECT_STATUSES, redirectEndpoint, SimApiError } from '../http/client' +import { buildUrl, REDIRECT_STATUSES, redirectEndpoint, SimApiError } from '../http/client' import { USER_AGENT } from '../version' /** @@ -20,6 +20,12 @@ const PAIRING_ALPHABET = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789' const POLL_INTERVAL_MS = 2000 const POLL_TIMEOUT_MS = 15 * 60 * 1000 +/** The page the browser is sent to for approval. */ +const APPROVAL_PATH = '/cli/auth' + +/** The route the login poll targets; also the suffix a redirect target is measured against. */ +const POLL_PATH = '/api/cli/auth/poll' + /** * Poll statuses that leave the approval still redeemable, so the login should * keep waiting rather than making the user restart the browser handoff. @@ -89,13 +95,13 @@ export function buildApprovalUrl( scope: CliAuthScope, workspaceId?: string ): string { - const url = new URL('/cli/auth', endpoint) - url.searchParams.set('request', auth.request) - url.searchParams.set('challenge', auth.challenge) - url.searchParams.set('pairing', auth.pairing) - url.searchParams.set('scope', scope) - if (workspaceId) url.searchParams.set('workspace', workspaceId) - return url.toString() + return buildUrl(endpoint, APPROVAL_PATH, { + request: auth.request, + challenge: auth.challenge, + pairing: auth.pairing, + scope, + workspace: workspaceId, + }) } interface PollResponse { @@ -106,9 +112,6 @@ interface PollResponse { workspaceBound?: boolean } -/** The route the login poll targets; also the suffix a redirect target is measured against. */ -const POLL_PATH = '/api/cli/auth/poll' - /** * Explains a redirected poll rather than following it. * @@ -166,7 +169,7 @@ export async function pollForKey( let response: Response | null = null try { - response = await fetch(new URL(POLL_PATH, endpoint), { + response = await fetch(buildUrl(endpoint, POLL_PATH), { method: 'POST', headers: { 'content-type': 'application/json', diff --git a/packages/sim-cli/src/http/client.ts b/packages/sim-cli/src/http/client.ts index d7531971b3a..09cb52ad26e 100644 --- a/packages/sim-cli/src/http/client.ts +++ b/packages/sim-cli/src/http/client.ts @@ -53,7 +53,24 @@ export interface WorkspaceOptions { auth?: AuthRequirement } -function buildUrl(endpoint: string, path: string, query?: Record): string { +/** + * Joins an endpoint and a route into a request URL. + * + * Concatenation rather than `new URL(path, endpoint)`, which is the trap it + * exists to avoid: a leading-slash path is absolute, so `new URL()` resolves it + * against the endpoint's ORIGIN and silently drops any path the endpoint + * carries. A deployment served under a prefix — `https://host/sim` behind a + * proxy that fronts several apps — would have every request rewritten to + * `https://host/...`, losing the prefix that identifies it. + * + * Empty values are skipped rather than sent blank so an omitted optional + * parameter reads as absent, not as the empty string. + */ +export function buildUrl( + endpoint: string, + path: string, + query?: Record +): string { const url = new URL(`${endpoint}${path}`) for (const [key, value] of Object.entries(query ?? {})) { if (value === null || value === undefined || value === '') continue