From 1e32f8a8028c2e7c0bbee15072c0a871c20f4d15 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Mon, 28 Sep 2026 14:38:19 -0500 Subject: [PATCH 1/3] fix(installer): bound unauthorized recovery to verified same-target credentials Adapt the bounded REST recovery and credential forwarding from Nick Nisi's PR #219 (e7303234b460d65b26f14cc1767371768d47135e, latest head 0ce2d030f6f7c84092b786edfc57da16be0d6dbd) to current main. This is an adaptation, not a cherry-pick: reject pasted or foreign-target keys, keep sandbox safeguards, and never claim authentication refreshed when the session was reused. Offer one retry through existing auth facilities; wait for concurrent writes, preserve typed 401s, keep manual fallback, and redact credential endpoint failures. Cover real legacy Ruby/Go/.NET orchestration with offline auth/network/agent mocks. Refs: AUTH-6735, https://github.com/workos/cli/pull/219 Co-authored-by: Nick Nisi --- src/integrations/dotnet/index.ts | 10 +- src/integrations/go/index.ts | 10 +- src/integrations/ruby/index.ts | 15 +- src/lib/agent-runner.spec.ts | 27 ++++ src/lib/agent-runner.ts | 10 +- src/lib/configuration-recovery.spec.ts | 28 ++++ src/lib/configuration-recovery.ts | 107 +++++++++++++ src/lib/dashboard-graphql.spec.ts | 25 ++- src/lib/dashboard-graphql.ts | 5 +- src/lib/legacy-configuration-recovery.spec.ts | 100 ++++++++++++ src/lib/staging-api.spec.ts | 15 +- src/lib/staging-api.ts | 9 +- src/lib/workos-management.spec.ts | 142 +++++++++++++++++- src/lib/workos-management.ts | 90 ++++++++--- 14 files changed, 548 insertions(+), 45 deletions(-) create mode 100644 src/lib/configuration-recovery.spec.ts create mode 100644 src/lib/configuration-recovery.ts create mode 100644 src/lib/legacy-configuration-recovery.spec.ts diff --git a/src/integrations/dotnet/index.ts b/src/integrations/dotnet/index.ts index 8faeb7fa..beec2733 100644 --- a/src/integrations/dotnet/index.ts +++ b/src/integrations/dotnet/index.ts @@ -92,16 +92,22 @@ export async function run(options: InstallerOptions): Promise { integration: config.metadata.integration, }); - const { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); + let { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); // Auto-configure WorkOS environment (redirect URI, CORS, homepage) const callerHandledConfig = Boolean(options.apiKey || options.clientId); if (!callerHandledConfig && apiKey) { const port = 5000; // ASP.NET Core default HTTP port - await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, port, { + const result = await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, port, { + clientId, + interactive: !options.ci, homepageUrl: options.homepageUrl, redirectUri: options.redirectUri, }); + if (result?.recoveredCredentials) { + ({ apiKey, clientId } = result.recoveredCredentials); + Object.assign(options, result.recoveredCredentials); + } } // Build prompt — credentials are passed via prompt context since .NET doesn't use .env.local diff --git a/src/integrations/go/index.ts b/src/integrations/go/index.ts index d821a894..cedc032a 100644 --- a/src/integrations/go/index.ts +++ b/src/integrations/go/index.ts @@ -127,16 +127,22 @@ export async function run(options: InstallerOptions): Promise { }); // Get WorkOS credentials - const { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); + let { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); // Auto-configure WorkOS environment (redirect URI, CORS) const callerHandledConfig = Boolean(options.apiKey || options.clientId); if (!callerHandledConfig && apiKey) { const redirectUri = options.redirectUri || `http://localhost:${GO_DEFAULT_PORT}${GO_CALLBACK_PATH}`; - await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, GO_DEFAULT_PORT, { + const result = await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, GO_DEFAULT_PORT, { + clientId, + interactive: !options.ci, homepageUrl: options.homepageUrl, redirectUri, }); + if (result?.recoveredCredentials) { + ({ apiKey, clientId } = result.recoveredCredentials); + Object.assign(options, result.recoveredCredentials); + } } // Gather Go-specific context diff --git a/src/integrations/ruby/index.ts b/src/integrations/ruby/index.ts index 30c1919e..adb2ae67 100644 --- a/src/integrations/ruby/index.ts +++ b/src/integrations/ruby/index.ts @@ -79,19 +79,22 @@ export async function run(options: InstallerOptions): Promise { }); // Get WorkOS credentials - const { apiKey, clientId: _clientId } = await getOrAskForWorkOSCredentials( - options, - config.environment.requiresApiKey, - ); + let { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); // Auto-configure WorkOS environment (redirect URI, CORS, homepage) if not already done const callerHandledConfig = Boolean(options.apiKey || options.clientId); if (!callerHandledConfig && apiKey) { const port = 3000; // Rails default - await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, port, { + const result = await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, port, { + clientId, + interactive: !options.ci, homepageUrl: options.homepageUrl, redirectUri: options.redirectUri, }); + if (result?.recoveredCredentials) { + ({ apiKey, clientId } = result.recoveredCredentials); + Object.assign(options, result.recoveredCredentials); + } } // Build prompt for the agent @@ -108,7 +111,7 @@ export async function run(options: InstallerOptions): Promise { The following environment variables are needed (create a .env file if one does not exist): - WORKOS_API_KEY -- WORKOS_CLIENT_ID +- WORKOS_CLIENT_ID=${clientId} - WORKOS_REDIRECT_URI=${redirectUri} ## Integration Instructions diff --git a/src/lib/agent-runner.spec.ts b/src/lib/agent-runner.spec.ts index 2c6725c6..b7ace415 100644 --- a/src/lib/agent-runner.spec.ts +++ b/src/lib/agent-runner.spec.ts @@ -168,6 +168,33 @@ describe('installer prompt', () => { expect(initializeAgent).not.toHaveBeenCalled(); }); + it('forwards an accepted recovered pair to files, agent initialization and downstream options', async () => { + const pair = { apiKey: 'sk_test_fake_recovered', clientId: 'client_test' }; + vi.mocked(autoConfigureWorkOSEnvironment).mockResolvedValueOnce({ + redirectUri: { success: true, alreadyExists: false }, + corsOrigin: { success: true, alreadyExists: false }, + recoveredCredentials: pair, + }); + const direct = { ...options, clientId: undefined }; + await runAgentInstaller({ ...config, metadata: { ...config.metadata, integration: 'sveltekit' } }, direct); + expect(autoConfigureWorkOSEnvironment).toHaveBeenCalledWith( + 'test-key', + 'sveltekit', + expect.any(Number), + expect.objectContaining({ clientId: pair.clientId }), + ); + expect(writeEnvLocal).toHaveBeenCalledWith( + direct.installDir, + expect.objectContaining({ WORKOS_API_KEY: pair.apiKey, WORKOS_CLIENT_ID: pair.clientId }), + ); + expect(initializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ workOSApiKey: pair.apiKey }), + expect.objectContaining(pair), + ); + expect(direct).toMatchObject(pair); + expect(vi.mocked(runAgent).mock.calls[0][1]).not.toContain(pair.apiKey); + }); + it('does not register a callback in the API-key environment when run directly', async () => { await runAgentInstaller(config, { ...options, clientId: undefined }); expect(autoConfigureWorkOSEnvironment).not.toHaveBeenCalled(); diff --git a/src/lib/agent-runner.ts b/src/lib/agent-runner.ts index 6e29d2f6..0dd2d443 100644 --- a/src/lib/agent-runner.ts +++ b/src/lib/agent-runner.ts @@ -65,7 +65,7 @@ export async function runAgentInstaller(config: FrameworkConfig, options: Instal } // Get WorkOS credentials (API key optional for client-only SDKs) - const { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); + let { apiKey, clientId } = await getOrAskForWorkOSCredentials(options, config.environment.requiresApiKey); // Check if caller (state machine) already configured WorkOS environment // If credentials were passed via options, the caller handled config+env writing @@ -77,10 +77,16 @@ export async function runAgentInstaller(config: FrameworkConfig, options: Instal // dashboard targeting or the API-only callback path, never both. if (!callerHandledConfig && apiKey && config.environment.requiresApiKey && config.metadata.integration !== 'nextjs') { const port = detectPort(config.metadata.integration, options.installDir); - await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, port, { + const result = await autoConfigureWorkOSEnvironment(apiKey, config.metadata.integration, port, { + clientId, + interactive: !options.ci, homepageUrl: options.homepageUrl, redirectUri: options.redirectUri, }); + if (result?.recoveredCredentials) { + ({ apiKey, clientId } = result.recoveredCredentials); + Object.assign(options, result.recoveredCredentials); + } } // Write environment variables to .env.local BEFORE agent runs diff --git a/src/lib/configuration-recovery.spec.ts b/src/lib/configuration-recovery.spec.ts new file mode 100644 index 00000000..fc7e8aae --- /dev/null +++ b/src/lib/configuration-recovery.spec.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; +import { UnauthorizedException } from '@workos-inc/node'; +import { DashboardGraphqlError } from './dashboard-graphql.js'; +import { WorkOSApiError } from './workos-api.js'; +import { DashboardConfigError, isConfigurationUnauthorized } from './configuration-recovery.js'; + +describe('configuration Unauthorized classification', () => { + it.each([ + new UnauthorizedException('fake-request-id'), + new WorkOSApiError('Unauthorized', 401), + new DashboardConfigError(401), + new DashboardGraphqlError('Session rejected', 'forbidden', 401), + ])('accepts typed 401 from each supported transport: %s', (error) => { + expect(isConfigurationUnauthorized(error)).toBe(true); + }); + + it.each([ + new Error('Unauthorized HTTP 401'), + { status: 401, message: 'Unauthorized' }, + new WorkOSApiError('Unauthorized', 403), + new DashboardConfigError(422), + new DashboardGraphqlError('Unauthorized', 'forbidden', 403), + new DashboardGraphqlError('Unauthorized', 'network_error'), + new DashboardGraphqlError('Unauthorized', 'graphql_error'), + ])('does not infer authentication failure from an untyped message: %s', (error) => { + expect(isConfigurationUnauthorized(error)).toBe(false); + }); +}); diff --git a/src/lib/configuration-recovery.ts b/src/lib/configuration-recovery.ts new file mode 100644 index 00000000..3d43cb25 --- /dev/null +++ b/src/lib/configuration-recovery.ts @@ -0,0 +1,107 @@ +import { UnauthorizedException } from '@workos-inc/node'; +import { DashboardGraphqlError } from './dashboard-graphql.js'; +import { WorkOSApiError } from './workos-api.js'; +import ui from '../utils/ui.js'; +import { isPromptAllowed } from '../utils/interaction-mode.js'; +import { isJsonMode } from '../utils/output.js'; +import { CliExit } from '../utils/cli-exit.js'; +import { ExitCode } from '../utils/exit-codes.js'; +import { formatWorkOSCommand } from '../utils/command-invocation.js'; + +/** Preserve HTTP status without exposing backend response bodies (which may contain secrets). */ +export class DashboardConfigError extends Error { + constructor(readonly status: number) { + super(`WorkOS configuration request failed (HTTP ${status}).`); + this.name = 'DashboardConfigError'; + } +} + +export function isConfigurationUnauthorized(error: unknown): boolean { + return ( + ((error instanceof DashboardConfigError || error instanceof DashboardGraphqlError) && error.status === 401) || + (error instanceof WorkOSApiError && error.statusCode === 401) || + error instanceof UnauthorizedException + ); +} + +export interface ConfigurationCredentials { + apiKey: string; + clientId: string; +} + +export function configurationRecoveryHint(): string { + return `Run \`${formatWorkOSCommand('auth login')}\` to check dashboard access, then retry setup for the same application, or configure its URLs manually in the WorkOS dashboard. A dashboard login does not itself replace a rejected API key.`; +} + +type RecoveryResult = + | { token: string; credentials?: ConfigurationCredentials } + | { reason: string; code?: 'cancelled' }; + +/** One offer, not a retry loop. Callers own the single retry and target/read-back checks. */ +export async function recoverConfigurationAccess( + rejected: { token: string } | { apiKey: string; clientId?: string }, + interactive = true, +): Promise { + const hint = configurationRecoveryHint(); + if (!interactive || !isPromptAllowed() || isJsonMode() || !process.stdin.isTTY) { + return { reason: `Unauthorized. Interactive recovery is unavailable. ${hint}` }; + } + const choice = await ui.select({ + message: 'WorkOS returned Unauthorized. How would you like to proceed?', + options: [ + { + value: 'retry', + label: 'Check authentication and retry once', + hint: 'Keep the same application; sign in only if needed', + }, + { value: 'manual', label: 'Configure manually', hint: 'Leave credentials unchanged' }, + ], + }); + if (ui.isCancel(choice)) return { reason: `Unauthorized recovery cancelled. ${hint}`, code: 'cancelled' }; + if (choice !== 'retry') return { reason: `Unauthorized recovery declined; manual configuration selected. ${hint}` }; + + try { + const { ensureAuthenticated } = await import('./ensure-auth.js'); + const auth = await ensureAuthenticated(); + if (!auth.authenticated) return { reason: `Authentication check failed. ${hint}` }; + ui.log.info( + auth.loginTriggered + ? 'Signed in to WorkOS.' + : auth.tokenRefreshed + ? 'Dashboard session refreshed.' + : 'Using the existing dashboard session; no new login was needed.', + ); + const { getAccessToken } = await import('./credentials.js'); + const token = getAccessToken(); + if (!token) return { reason: `No usable dashboard session. ${hint}` }; + if ('token' in rejected) { + return token === rejected.token + ? { reason: `The dashboard session is unchanged; the rejected session was not retried. ${hint}` } + : { token }; + } + if (!rejected.clientId) + return { reason: `Cannot verify replacement credentials without the intended client ID. ${hint}` }; + const { fetchStagingCredentials } = await import('./staging-api.js'); + const credentials = await fetchStagingCredentials(token); + // This endpoint returns an authoritative pair. Never adopt a different target, + // a pasted key, a production key, or the same rejected key. Do not save a profile. + if ( + credentials.clientId !== rejected.clientId || + !credentials.apiKey.startsWith('sk_test_') || + /[\r\n]/.test(credentials.apiKey) + ) { + return { + reason: `No replacement sandbox credentials verified for this application. Application credentials were left unchanged. ${hint}`, + }; + } + if (credentials.apiKey === rejected.apiKey) { + return { reason: `The API key is unchanged; the rejected key was not retried. ${hint}` }; + } + return { token, credentials }; + } catch (error) { + if (error instanceof CliExit && error.exitCode === ExitCode.CANCELLED) { + return { reason: `Unauthorized recovery cancelled. ${hint}`, code: 'cancelled' }; + } + return { reason: `Authentication recovery failed. Application credentials were left unchanged. ${hint}` }; + } +} diff --git a/src/lib/dashboard-graphql.spec.ts b/src/lib/dashboard-graphql.spec.ts index 29f3218d..9507d674 100644 --- a/src/lib/dashboard-graphql.spec.ts +++ b/src/lib/dashboard-graphql.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { dashboardGraphqlUpload, DashboardGraphqlError } from './dashboard-graphql.js'; +import { dashboardGraphqlRequest, dashboardGraphqlUpload, DashboardGraphqlError } from './dashboard-graphql.js'; /** * Covers the multipart (`Upload`) transport. The plain JSON path is exercised @@ -31,6 +31,29 @@ describe('dashboardGraphqlUpload', () => { vi.unstubAllGlobals(); }); + it.each([401, 403])('retains HTTP %s on dashboard-session failures', async (status) => { + fetchMock.mockResolvedValue(new Response('private body', { status })); + await expect(dashboardGraphqlRequest('query {}', { token: 'fake-token' })).rejects.toMatchObject({ + status, + message: expect.not.stringContaining('private body'), + }); + }); + + it('recognizes explicit UNAUTHENTICATED errors, not arbitrary Unauthorized messages', async () => { + fetchMock.mockResolvedValue( + Response.json({ errors: [{ message: 'private details', extensions: { code: 'UNAUTHENTICATED' } }] }), + ); + await expect(dashboardGraphqlRequest('query {}', { token: 'fake-token' })).rejects.toMatchObject({ + status: 401, + message: expect.not.stringContaining('private details'), + }); + fetchMock.mockResolvedValue(Response.json({ errors: [{ message: 'Unauthorized' }] })); + await expect(dashboardGraphqlRequest('query {}', { token: 'fake-token' })).rejects.toMatchObject({ + code: 'graphql_error', + status: undefined, + }); + }); + /** The FormData handed to fetch on the most recent call. */ function sentForm(): FormData { const init = fetchMock.mock.calls[0]?.[1] as RequestInit; diff --git a/src/lib/dashboard-graphql.ts b/src/lib/dashboard-graphql.ts index 655ea9fd..06a31253 100644 --- a/src/lib/dashboard-graphql.ts +++ b/src/lib/dashboard-graphql.ts @@ -48,7 +48,7 @@ export class DashboardGraphqlError extends Error { interface GraphqlResponseBody { data?: T | null; - errors?: Array<{ message: string }>; + errors?: Array<{ message: string; extensions?: { code?: string } }>; } export interface DashboardGraphqlOptions { @@ -151,6 +151,9 @@ async function sendDashboardRequest( } if (body.errors?.length) { + if (body.errors.every((error) => error.extensions?.code === 'UNAUTHENTICATED')) { + throw new DashboardGraphqlError('The dashboard session was rejected.', 'forbidden', 401); + } throw new DashboardGraphqlError(body.errors.map((e) => e.message).join('; '), 'graphql_error'); } diff --git a/src/lib/legacy-configuration-recovery.spec.ts b/src/lib/legacy-configuration-recovery.spec.ts new file mode 100644 index 00000000..46855c63 --- /dev/null +++ b/src/lib/legacy-configuration-recovery.spec.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { InstallerOptions } from '../utils/types.js'; + +vi.mock('./skills-assets.js', () => ({ getReference: vi.fn(async () => 'Offline instructions') })); +vi.mock('./agent-interface.js', () => ({ initializeAgent: vi.fn(), runAgent: vi.fn(async () => ({})) })); +vi.mock('./ensure-auth.js', () => ({ + ensureAuthenticated: vi.fn(async () => ({ authenticated: true, loginTriggered: false, tokenRefreshed: false })), +})); +vi.mock('./credentials.js', () => ({ getAccessToken: vi.fn(() => 'fake-token') })); +vi.mock('./staging-api.js', () => ({ + fetchStagingCredentials: vi.fn(async () => ({ apiKey: 'sk_test_fake_recovered', clientId: 'client_fake' })), +})); +vi.mock('./config-store.js', () => ({ + getActiveEnvironment: vi.fn(() => null), + isUnclaimedEnvironment: vi.fn(() => false), +})); +vi.mock('../utils/ui-utils.js', () => ({ + getOrAskForWorkOSCredentials: vi.fn(async () => ({ apiKey: 'sk_test_fake_rejected', clientId: 'client_fake' })), +})); +vi.mock('../utils/analytics.js', () => ({ analytics: { capture: vi.fn(), setTag: vi.fn(), shutdown: vi.fn() } })); + +import ui from '../utils/ui.js'; +import { initializeAgent, runAgent } from './agent-interface.js'; +import { run as ruby } from '../integrations/ruby/index.js'; +import { run as go } from '../integrations/go/index.js'; +import { run as dotnet } from '../integrations/dotnet/index.js'; +import { setInteractionMode, resetInteractionModeForTests } from '../utils/interaction-mode.js'; +import { setOutputMode } from '../utils/output.js'; + +const tty = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); +let directory: string; +beforeEach(async () => { + vi.clearAllMocks(); + directory = await mkdtemp(join(tmpdir(), 'legacy-recovery-')); + Object.defineProperty(process.stdin, 'isTTY', { configurable: true, value: true }); + setInteractionMode({ mode: 'human', source: 'flag' }); + setOutputMode('human'); + vi.spyOn(ui, 'select').mockResolvedValue('retry'); + vi.spyOn(ui, 'rows').mockImplementation(() => {}); + for (const method of ['info', 'warn', 'step', 'success'] as const) + vi.spyOn(ui.log, method).mockImplementation(() => {}); + vi.stubGlobal( + 'fetch', + vi.fn(async (_url: string, init: RequestInit) => + Response.json( + {}, + { + status: (init.headers as Record).Authorization.includes('rejected') ? 401 : 201, + }, + ), + ), + ); +}); +afterEach(async () => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + if (tty) Object.defineProperty(process.stdin, 'isTTY', tty); + else Reflect.deleteProperty(process.stdin, 'isTTY'); + resetInteractionModeForTests(); + await rm(directory, { recursive: true, force: true }); +}); + +describe('legacy pre-agent callers', () => { + it.each([ + ['ruby', ruby], + ['go', go], + ['dotnet', dotnet], + ] as const)('%s forwards the accepted same-target pair after real REST recovery', async (name, run) => { + const options: InstallerOptions = { + installDir: directory, + debug: false, + forceInstall: false, + local: false, + ci: false, + noValidate: true, + }; + const summary = await run(options); + const pair = { apiKey: 'sk_test_fake_recovered', clientId: 'client_fake' }; + expect(options).toMatchObject(pair); + expect(initializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ workOSApiKey: pair.apiKey }), + expect.objectContaining(pair), + ); + expect(vi.mocked(runAgent).mock.calls[0][2]).toMatchObject(pair); + expect(vi.mocked(runAgent).mock.calls[0][1]).not.toContain(pair.apiKey); + if (name === 'ruby') expect(vi.mocked(runAgent).mock.calls[0][1]).toContain(pair.clientId); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(globalThis.fetch).toHaveBeenCalledTimes(4); + expect(summary).not.toContain(pair.apiKey); + if (name === 'go') { + const env = await readFile(join(directory, '.env'), 'utf8'); + expect(env).toContain(`WORKOS_API_KEY=${pair.apiKey}`); + expect(env).toContain(`WORKOS_CLIENT_ID=${pair.clientId}`); + expect(env).not.toContain('fake_rejected'); + } + }); +}); diff --git a/src/lib/staging-api.spec.ts b/src/lib/staging-api.spec.ts index 917ac562..3489d1c5 100644 --- a/src/lib/staging-api.spec.ts +++ b/src/lib/staging-api.spec.ts @@ -1,5 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { fetchStagingCredentials, StagingApiError } from './staging-api.js'; +import { logError } from '../utils/debug.js'; +vi.mock('../utils/debug.js', () => ({ logInfo: vi.fn(), logError: vi.fn() })); describe('staging-api', () => { const mockFetch = vi.fn(); @@ -8,6 +10,7 @@ describe('staging-api', () => { beforeEach(() => { globalThis.fetch = mockFetch; mockFetch.mockReset(); + vi.mocked(logError).mockClear(); }); afterEach(() => { @@ -59,6 +62,14 @@ describe('staging-api', () => { expect(result).toEqual({ clientId: 'camel_client', apiKey: 'camel_key' }); }); + it('does not expose credential response bodies or transport messages in logs or errors', async () => { + mockFetch.mockResolvedValueOnce(new Response('sk_test_fake_private', { status: 500 })); + await expect(fetchStagingCredentials('fake-token')).rejects.toThrow('HTTP 500'); + mockFetch.mockRejectedValueOnce(new Error('sk_test_fake_private')); + await expect(fetchStagingCredentials('fake-token')).rejects.toThrow('Network error while fetching credentials.'); + expect(JSON.stringify(vi.mocked(logError).mock.calls)).not.toContain('sk_test_fake_private'); + }); + it('throws StagingApiError on 401', async () => { mockFetch.mockResolvedValueOnce({ ok: false, @@ -106,7 +117,7 @@ describe('staging-api', () => { text: async () => 'Internal Server Error', }); - await expect(fetchStagingCredentials('token')).rejects.toThrow('Failed to fetch credentials: 500'); + await expect(fetchStagingCredentials('token')).rejects.toThrow('Failed to fetch credentials: HTTP 500'); }); it('throws StagingApiError when response missing clientId', async () => { @@ -130,7 +141,7 @@ describe('staging-api', () => { it('throws StagingApiError on network error', async () => { mockFetch.mockRejectedValueOnce(new Error('Network failed')); - await expect(fetchStagingCredentials('token')).rejects.toThrow('Network error: Network failed'); + await expect(fetchStagingCredentials('token')).rejects.toThrow('Network error while fetching credentials.'); }); it('throws StagingApiError on timeout (AbortError)', async () => { diff --git a/src/lib/staging-api.ts b/src/lib/staging-api.ts index 5a1403ab..ecee5e01 100644 --- a/src/lib/staging-api.ts +++ b/src/lib/staging-api.ts @@ -45,8 +45,7 @@ export async function fetchStagingCredentials(accessToken: string): Promise ''); - logError('[staging-api] Error response:', res.status, text); + logError('[staging-api] Error response:', res.status); if (res.status === 401) { throw new StagingApiError('Authentication expired. Please log in again.', 401); @@ -58,7 +57,7 @@ export async function fetchStagingCredentials(accessToken: string): Promise ({ isUnclaimedEnvironment: (env: EnvironmentConfig) => env.type === 'unclaimed', })); +vi.mock('./ensure-auth.js', () => ({ ensureAuthenticated: vi.fn() })); +vi.mock('./credentials.js', () => ({ getAccessToken: vi.fn() })); +vi.mock('./staging-api.js', () => ({ fetchStagingCredentials: vi.fn() })); + vi.mock('../utils/analytics.js', () => ({ analytics: { capture: vi.fn(), captureException: vi.fn() }, })); const { analytics } = await import('../utils/analytics.js'); -const ui = (await import('../utils/ui.js')).default; +const { default: ui, CANCEL } = await import('../utils/ui.js'); const { autoConfigureWorkOSEnvironment, SANDBOX_ONLY_REASON } = await import('./workos-management.js'); const API_KEY = 'sk_test_123'; @@ -116,6 +125,137 @@ describe('workos-management', () => { vi.restoreAllMocks(); }); + describe('bounded Unauthorized recovery', () => { + const pair = { apiKey: 'sk_test_fake_recovered', clientId: 'client_fake' }; + const tty = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + beforeEach(() => { + Object.defineProperty(process.stdin, 'isTTY', { configurable: true, value: true }); + setInteractionMode({ mode: 'human', source: 'flag' }); + setOutputMode('human'); + vi.spyOn(ui, 'select').mockResolvedValue('retry'); + vi.mocked(ensureAuthenticated) + .mockReset() + .mockResolvedValue({ authenticated: true, loginTriggered: false, tokenRefreshed: false }); + vi.mocked(getAccessToken).mockReset().mockReturnValue('fake-token'); + vi.mocked(fetchStagingCredentials).mockReset().mockResolvedValue(pair); + }); + afterEach(() => { + if (tty) Object.defineProperty(process.stdin, 'isTTY', tty); + else Reflect.deleteProperty(process.stdin, 'isTTY'); + resetInteractionModeForTests(); + setOutputMode('human'); + }); + + it.each([ + [ + { authenticated: true, loginTriggered: false, tokenRefreshed: false }, + 'Using the existing dashboard session; no new login was needed.', + ], + [{ authenticated: true, loginTriggered: false, tokenRefreshed: true }, 'Dashboard session refreshed.'], + [{ authenticated: true, loginTriggered: true, tokenRefreshed: false }, 'Signed in to WorkOS.'], + ] as const)('recovers with a same-target pair and truthful auth wording: %s', async (auth, message) => { + vi.mocked(ensureAuthenticated).mockResolvedValue(auth); + const fetch = vi.fn(async (_url: string, init: RequestInit) => + Response.json( + {}, + { + status: (init.headers as Record).Authorization === `Bearer ${API_KEY}` ? 401 : 201, + }, + ), + ); + vi.stubGlobal('fetch', fetch); + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { clientId: pair.clientId }); + expect(result?.recoveredCredentials).toEqual(pair); + expect(fetch).toHaveBeenCalledTimes(4); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(ui.log.info).toHaveBeenCalledWith(message); + expect(JSON.stringify(vi.mocked(analytics.capture).mock.calls)).not.toContain(pair.apiKey); + }); + + it.each(['same', 'mismatch', 'production', 'noClient', 'manual', 'cancel', 'exhausted'])( + 'stops without adopting a replacement: %s', + async (failure) => { + const fetch = vi.fn(async () => Response.json({ message: 'fake_secret_backend' }, { status: 401 })); + vi.stubGlobal('fetch', fetch); + if (failure === 'same') vi.mocked(fetchStagingCredentials).mockResolvedValue({ ...pair, apiKey: API_KEY }); + if (failure === 'mismatch') + vi.mocked(fetchStagingCredentials).mockResolvedValue({ ...pair, clientId: 'client_other' }); + if (failure === 'production') + vi.mocked(fetchStagingCredentials).mockResolvedValue({ ...pair, apiKey: 'sk_live_fake' }); + if (failure === 'manual') vi.mocked(ui.select).mockResolvedValue('manual'); + if (failure === 'cancel') vi.mocked(ui.select).mockResolvedValue(CANCEL); + expect( + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { + clientId: failure === 'noClient' ? undefined : pair.clientId, + }), + ).toBeNull(); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledTimes(failure === 'exhausted' ? 4 : 2); + expect(ui.log.success).not.toHaveBeenCalled(); + const output = JSON.stringify([ + vi.mocked(ui.log.warn).mock.calls, + vi.mocked(ui.log.info).mock.calls, + vi.mocked(analytics.capture).mock.calls, + ]); + expect(output).not.toContain('fake_secret_backend'); + expect(output).not.toContain(pair.apiKey); + expect(ui.rows).toHaveBeenCalledWith( + expect.arrayContaining([{ key: 'Redirect URI', value: `${BASE_URL}/auth/callback` }]), + ); + }, + ); + + it.each([403, 422, 500])( + 'does not offer auth recovery for HTTP %s even with Unauthorized in the body', + async (status) => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => Response.json({ message: 'Unauthorized 401 fake_secret_backend' }, { status })), + ); + expect( + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { clientId: pair.clientId }), + ).toBeNull(); + expect(ui.select).not.toHaveBeenCalled(); + expect(ensureAuthenticated).not.toHaveBeenCalled(); + expect(JSON.stringify(vi.mocked(analytics.capture).mock.calls)).not.toContain('fake_secret_backend'); + }, + ); + + it('waits for in-flight writes and does not disguise concurrent non-auth failure as a 401', async () => { + let settle!: (response: Response) => void; + const slow = new Promise((resolve) => { + settle = resolve; + }); + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValueOnce(Response.json({}, { status: 401 })) + .mockReturnValueOnce(slow), + ); + const result = autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { clientId: pair.clientId }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(ui.select).not.toHaveBeenCalled(); + settle(Response.json({}, { status: 403 })); + expect(await result).toBeNull(); + expect(ui.select).not.toHaveBeenCalled(); + }); + + it('never writes the homepage after its GET rejects authorization', async () => { + const request = vi.fn(async (url: string) => + Response.json({}, { status: url === HOMEPAGE_ENDPOINT ? 401 : 201 }), + ); + vi.stubGlobal('fetch', request); + vi.mocked(ui.select).mockResolvedValue('manual'); + await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { + homepageUrl: BASE_URL, + clientId: pair.clientId, + }); + expect(request.mock.calls).toHaveLength(3); + expect(ui.select).toHaveBeenCalledTimes(1); + }); + }); + describe('setHomepageUrl read-then-write', () => { // The homepage is only written where nothing can be overwritten; an // unclaimed environment is one (see 'homepage without a user choice'). diff --git a/src/lib/workos-management.ts b/src/lib/workos-management.ts index acb985e0..58a75d7e 100644 --- a/src/lib/workos-management.ts +++ b/src/lib/workos-management.ts @@ -1,3 +1,10 @@ +import { + DashboardConfigError, + isConfigurationUnauthorized, + recoverConfigurationAccess, + configurationRecoveryHint, + type ConfigurationCredentials, +} from './configuration-recovery.js'; import type { Integration } from './constants.js'; import type { SetupItemId, SetupItemStatus } from './events.js'; import type { EnvironmentConfig } from './config-store.js'; @@ -17,6 +24,8 @@ const HOMEPAGE_URL_ENDPOINT = '/user_management/app_homepage_url'; const SUPPLIED_KEY_PROVENANCE = 'the API key supplied to this run'; export interface AutoConfigResult { + /** Accepted only after the complete retry succeeds; callers must adopt both values. */ + recoveredCredentials?: ConfigurationCredentials; redirectUri: { success: boolean; alreadyExists: boolean }; corsOrigin: { success: boolean; alreadyExists: boolean }; /** Absent when the homepage was left alone (see `autoConfigureWorkOSEnvironment`). */ @@ -78,7 +87,7 @@ async function createRedirectUri(apiKey: string, uri: string): Promise<{ success return { success: true, alreadyExists: true }; } - throw new Error(error.message || `HTTP ${error.status}`); + throw new DashboardConfigError(error.status); } /** @@ -101,7 +110,7 @@ export async function createCorsOrigin( return { success: true, alreadyExists: true }; } - throw new Error(error.message || `HTTP ${error.status}`); + throw new DashboardConfigError(error.status); } /** @@ -130,11 +139,11 @@ export async function setHomepageUrl( if (data?.url === url || (preserveExisting && data?.url)) { return { success: true, alreadyExists: true }; } - } else if (preserveExisting) { - throw new Error('Could not read the current homepage URL.'); + } else if (current.status === 401 || preserveExisting) { + throw new DashboardConfigError(current.status); } } catch (error) { - if (preserveExisting) throw error; + if (preserveExisting || isConfigurationUnauthorized(error)) throw error; // Legacy callers fall through to the write on read failures. } @@ -142,7 +151,7 @@ export async function setHomepageUrl( if (!response.ok) { const error = await parseFetchError(response); - throw new Error(error.message || `HTTP ${error.status}`); + throw new DashboardConfigError(error.status); } return { success: true, alreadyExists: false }; @@ -200,6 +209,10 @@ function describeCredentialProvenance(apiKey: string): string { } export interface AutoConfigOptions { + /** Required to verify a replacement key against this application. */ + clientId?: string; + /** Explicit CI callers may disable recovery even in a human terminal. */ + interactive?: boolean; /** Custom homepage URL (defaults to http://localhost:{port}) */ homepageUrl?: string; /** Custom redirect URI (defaults to framework convention) */ @@ -230,6 +243,40 @@ export async function autoConfigureWorkOSEnvironment( integration: Integration, port: number, options: AutoConfigOptions = {}, +): Promise { + try { + return await autoConfigureOnce(apiKey, integration, port, options); + } catch (error) { + if (!isConfigurationUnauthorized(error)) throw error; + const recovered = await recoverConfigurationAccess({ apiKey, clientId: options.clientId }, options.interactive); + let reason = + 'reason' in recovered ? recovered.reason : `Unauthorized recovery exhausted. ${configurationRecoveryHint()}`; + if ('credentials' in recovered && recovered.credentials) { + try { + const result = await autoConfigureOnce(recovered.credentials.apiKey, integration, port, options); + if (result) return { ...result, recoveredCredentials: recovered.credentials }; + reason = 'Configuration retry failed. Credentials were left unchanged.'; + } catch { + // The single retry is exhausted. No staged credentials escape on failure. + } + } + ui.log.warn(reason); + ui.log.info('Configure these settings manually in the WorkOS dashboard:'); + const baseUrl = `http://localhost:${port}`; + ui.rows([ + { key: 'Redirect URI', value: options.redirectUri || `${baseUrl}${getCallbackPath(integration)}` }, + { key: 'CORS origin', value: baseUrl }, + ...(options.homepageUrl ? [{ key: 'Homepage URL', value: options.homepageUrl }] : []), + ]); + return null; + } +} + +async function autoConfigureOnce( + apiKey: string, + integration: Integration, + port: number, + options: AutoConfigOptions, ): Promise { const baseUrl = `http://localhost:${port}`; const callbackPath = getCallbackPath(integration); @@ -271,19 +318,26 @@ export async function autoConfigureWorkOSEnvironment( return result; }, (error: unknown) => { - onStep(step, 'failed', error instanceof Error ? error.message : String(error)); + onStep(step, 'failed', isConfigurationUnauthorized(error) ? 'Unauthorized' : 'Configuration request failed'); throw error; }, ); }; try { - const [redirectUri, corsOrigin, homepageUrl] = await Promise.all([ + // Settle every in-flight write before offering recovery. These are additive + // upserts (or an explicit homepage override), so the same-target retry is safe. + const requests = [ track('redirect-uri', createRedirectUri(apiKey, callbackUrl)), track('cors-origin', createCorsOrigin(apiKey, baseUrl)), writeHomepage ? setHomepageUrl(apiKey, homepageUrlValue) : undefined, - ]); - + ] as const; + const settled = await Promise.allSettled(requests); + const failures = settled.filter((result) => result.status === 'rejected'); + // A simultaneous non-auth failure must not be disguised as an auth failure. + const failure = failures.find((result) => !isConfigurationUnauthorized(result.reason)) ?? failures[0]; + if (failure) throw failure.reason; + const [redirectUri, corsOrigin, homepageUrl] = await Promise.all(requests); const results: AutoConfigResult = { redirectUri, corsOrigin, homepageUrl }; analytics.capture(INSTALLER_INTERACTION_EVENT_NAME, { @@ -330,19 +384,9 @@ export async function autoConfigureWorkOSEnvironment( return results; } catch (error) { - const message = error instanceof Error ? error.message : 'Unknown error'; - - // Provide specific guidance for common errors - if (message.includes('401') || message.includes('Invalid API key')) { - ui.log.warn('Could not configure WorkOS dashboard: Invalid API key'); - } else if (message.includes('403') || message.includes('permission')) { - ui.log.warn('Could not configure WorkOS dashboard: API key lacks permission'); - } else if (message.includes('422') || message.includes('Validation')) { - ui.log.warn(`Could not configure WorkOS dashboard: Validation error`); - ui.log.info(` Error: ${message}`); - } else { - ui.log.warn(`Could not configure WorkOS dashboard: ${message}`); - } + if (isConfigurationUnauthorized(error)) throw error; + const message = error instanceof DashboardConfigError ? error.message : 'Configuration request failed'; + ui.log.warn(`Could not configure WorkOS dashboard: ${message}`); ui.log.info('You can configure these settings manually in the WorkOS dashboard.'); From a91baf54691e4e986e21fa85e9dd6ff992a84c2d Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Mon, 28 Sep 2026 14:38:19 -0500 Subject: [PATCH 2/3] fix(installer): recover post-agent URL setup without changing its target Reach AUTH-6735 recovery from the current post-agent Next.js and other-framework URL setup. Pin environment/application identity and transport across the single retry, re-read/reconcile partial writes, and keep validation plus final read-back mandatory. Adopt an accepted sandbox pair only after atomically replacing the known JS .env.local file, then update shared state and downstream options. Non-JS post-agent key replacement stays manual because generated credential formats are not authoritative. Preserve structured auth/cancel exit codes, explicit CI/JSON policy and the shared TUI prompt host. Tests exercise actual post-agent orchestration, target ambiguity/change, same or rejected credentials, partial writes, file/state consistency, manual/cancel/headless paths and secret-safe failures. Attribution for the original PR219 concept and forwarding work is recorded in the preceding commit. Refs: AUTH-6735 --- src/commands/install.spec.ts | 17 + src/commands/install.ts | 4 +- src/lib/authkit-application-setup.spec.ts | 400 +++++++++++++++++++++- src/lib/authkit-application-setup.ts | 129 ++++++- src/lib/env-writer.spec.ts | 32 +- src/lib/env-writer.ts | 35 ++ src/lib/run-with-core.setup.spec.ts | 1 + src/lib/run-with-core.ts | 95 +++-- 8 files changed, 662 insertions(+), 51 deletions(-) diff --git a/src/commands/install.spec.ts b/src/commands/install.spec.ts index 41d50f62..97e79d91 100644 --- a/src/commands/install.spec.ts +++ b/src/commands/install.spec.ts @@ -95,6 +95,23 @@ describe('handleInstall', () => { }); }); + it.each([ + ['auth_required', 4], + ['cancelled', 2], + ] as const)('preserves %s exit conventions in human and JSON modes', async (code, exitCode) => { + const { InstallDeclinedError } = await import('../lib/installer-errors.js'); + vi.mocked(runInstaller).mockRejectedValue( + new InstallDeclinedError('Callback unverified; recover access or configure manually.', code), + ); + for (const json of [false, true]) { + vi.mocked(isJsonMode).mockReturnValue(json); + await expect(handleInstall({ _: ['install'], $0: 'workos' } as any)).rejects.toMatchObject({ exitCode }); + if (json) + expect(exitWithError).toHaveBeenCalledWith({ code, message: expect.stringContaining('Callback unverified') }); + } + expect(maybeRunSetupAfter).not.toHaveBeenCalled(); + }); + it('exits non-zero without extra output in human mode (guidance already printed)', async () => { const { InstallDeclinedError } = await import('../lib/installer-errors.js'); vi.mocked(runInstaller).mockRejectedValue(new InstallDeclinedError('Next.js 14 is unsupported')); diff --git a/src/commands/install.ts b/src/commands/install.ts index 8da40186..eea92efb 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -2,7 +2,7 @@ import { runInstaller } from '../run.js'; import type { InstallerArgs } from '../run.js'; import ui from '../utils/ui.js'; import { exitWithError, isJsonMode } from '../utils/output.js'; -import { ExitCode, exitWithCode } from '../utils/exit-codes.js'; +import { ExitCode, exitWithCode, resolveErrorCode } from '../utils/exit-codes.js'; import { isCiMode } from '../utils/interaction-mode.js'; import type { ArgumentsCamelCase } from 'yargs'; import { InstallDeclinedError } from '../lib/installer-errors.js'; @@ -48,7 +48,7 @@ export async function handleInstall(argv: ArgumentsCamelCase): Pr if (isJsonMode()) { exitWithError({ code: err.code, message: err.message }); } - exitWithCode(ExitCode.GENERAL_ERROR); + exitWithCode(resolveErrorCode(err.code).exit); } const { getLogFilePath } = await import('../utils/debug.js'); diff --git a/src/lib/authkit-application-setup.spec.ts b/src/lib/authkit-application-setup.spec.ts index c9552487..12e33404 100644 --- a/src/lib/authkit-application-setup.spec.ts +++ b/src/lib/authkit-application-setup.spec.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { mkdtemp, rm, writeFile, readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; @@ -8,12 +8,19 @@ vi.mock('./config-store.js', async (importOriginal) => ({ getActiveEnvironment: vi.fn(), })); vi.mock('./command-auth.js', () => ({ refreshIfExpired: vi.fn() })); +vi.mock('./ensure-auth.js', () => ({ ensureAuthenticated: vi.fn() })); +vi.mock('./credentials.js', () => ({ getAccessToken: vi.fn() })); +vi.mock('./staging-api.js', () => ({ fetchStagingCredentials: vi.fn() })); +vi.mock('./validation/index.js', () => ({ validateInstallation: vi.fn() })); vi.mock('./api-key.js', () => ({ resolveApiBaseUrl: () => 'https://api.workos.com', resolveApiKey: vi.fn(), })); vi.mock('./environment-target.js', () => ({ fetchTeamEnvironments: vi.fn() })); -vi.mock('./dashboard-graphql.js', () => ({ dashboardGraphqlRequest: vi.fn() })); +vi.mock('./dashboard-graphql.js', async (importOriginal) => ({ + ...(await importOriginal()), + dashboardGraphqlRequest: vi.fn(), +})); vi.mock('../catalog/operation.js', () => ({ getOperation: (name: string) => ({ name }), resolveExecutableDocument: (operation: { name: string }) => operation.name, @@ -22,13 +29,25 @@ vi.mock('../catalog/operation.js', () => ({ import { getActiveEnvironment } from './config-store.js'; import { refreshIfExpired } from './command-auth.js'; import { fetchTeamEnvironments } from './environment-target.js'; -import { dashboardGraphqlRequest } from './dashboard-graphql.js'; +import { ensureAuthenticated } from './ensure-auth.js'; +import { getAccessToken } from './credentials.js'; +import { fetchStagingCredentials } from './staging-api.js'; +import { validateInstallation } from './validation/index.js'; +import ui, { CANCEL, setUiHost } from '../utils/ui.js'; +import { setInteractionMode, resetInteractionModeForTests } from '../utils/interaction-mode.js'; +import { setOutputMode } from '../utils/output.js'; +import { CliExit } from '../utils/cli-exit.js'; +import { dashboardGraphqlRequest, DashboardGraphqlError } from './dashboard-graphql.js'; import { buildApplicationSetup, configureAuthkitApplication, readNextjsApplicationSetup, } from './authkit-application-setup.js'; -import { configureInstallEnvironment, configureOtherApplicationUrls } from './run-with-core.js'; +import { + configureInstallEnvironment, + configureOtherApplicationUrls, + configureNextjsApplicationUrls, +} from './run-with-core.js'; import { createInstallerEventEmitter } from './events.js'; import { applicationSetupNextSteps } from './completion-data.js'; @@ -905,6 +924,379 @@ describe('native application URL setup', () => { }); }); +describe('Unauthorized configuration recovery', () => { + const unauthorized = () => new DashboardGraphqlError('private fake token', 'forbidden', 401); + const pair = { clientId: setup.clientId, apiKey: 'sk_test_fake_recovered' }; + let directory: string; + const tty = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + + beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'recovery-app-')); + Object.defineProperty(process.stdin, 'isTTY', { configurable: true, value: true }); + setInteractionMode({ mode: 'human', source: 'flag' }); + setOutputMode('human'); + vi.spyOn(ui, 'select').mockResolvedValue('retry'); + vi.spyOn(ui.log, 'info').mockImplementation(() => {}); + vi.mocked(ensureAuthenticated).mockResolvedValue({ + authenticated: true, + loginTriggered: false, + tokenRefreshed: true, + }); + vi.mocked(getAccessToken).mockReturnValue('fake-refreshed-token'); + vi.mocked(fetchStagingCredentials).mockResolvedValue(pair); + vi.mocked(validateInstallation).mockResolvedValue({ passed: true, framework: 'nextjs', issues: [], durationMs: 0 }); + }); + afterEach(async () => { + vi.restoreAllMocks(); + if (tty) Object.defineProperty(process.stdin, 'isTTY', tty); + else Reflect.deleteProperty(process.stdin, 'isTTY'); + resetInteractionModeForTests(); + setOutputMode('human'); + await rm(directory, { recursive: true, force: true }); + }); + + const contextFor = (installDir: string, integration = 'sveltekit') => ({ + options: { + installDir, + debug: false, + forceInstall: false, + local: false, + ci: false, + skipAuth: true, + redirectUri: setup.redirectUri, + }, + integration, + credentials: { apiKey: 'sk_test_fake_rejected', clientId: setup.clientId }, + emitter: createInstallerEventEmitter(), + }); + + it('recovers the real post-agent Next.js path without changing local credentials or skipping validation/read-back', async () => { + const context = contextFor(directory, 'nextjs'); + await writeFile( + join(directory, '.env.local'), + `WORKOS_API_KEY=${context.credentials.apiKey}\nWORKOS_CLIENT_ID=${setup.clientId}\nNEXT_PUBLIC_WORKOS_REDIRECT_URI=${setup.redirectUri}\n`, + ); + vi.mocked(fetchTeamEnvironments).mockRejectedValueOnce(unauthorized()); + const result = await configureNextjsApplicationUrls(context); + expect(result.verified).toBe(true); + expect(validateInstallation).toHaveBeenCalledWith('nextjs', directory, { runBuild: false }); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(fetchTeamEnvironments).toHaveBeenLastCalledWith('fake-refreshed-token'); + expect(vi.mocked(dashboardGraphqlRequest).mock.calls.at(-1)?.[0]).toBe('defaultAuthkitApplication'); + expect(context.credentials.apiKey).toBe('sk_test_fake_rejected'); + expect(fetchStagingCredentials).not.toHaveBeenCalled(); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }); + + it.each(['nextjs', 'sveltekit'])( + 'adopts an authoritative pair in %s post-agent files, state and subsequent options', + async (integration) => { + const context = contextFor(directory, integration); + const redirectKey = integration === 'nextjs' ? 'NEXT_PUBLIC_WORKOS_REDIRECT_URI' : 'WORKOS_REDIRECT_URI'; + await writeFile( + join(directory, '.env.local'), + `# keep\nWORKOS_API_KEY=${context.credentials.apiKey}\nWORKOS_CLIENT_ID=${setup.clientId}\n${redirectKey}=${setup.redirectUri}\n`, + ); + vi.mocked(refreshIfExpired).mockResolvedValue(null); + const request = vi.fn(async (_url: string, init: RequestInit) => + Response.json( + {}, + { + status: (init.headers as Record).Authorization.includes('fake_rejected') ? 401 : 201, + }, + ), + ); + vi.stubGlobal('fetch', request); + const result = + integration === 'nextjs' + ? await configureNextjsApplicationUrls(context) + : await configureOtherApplicationUrls(context, setup.clientId, context.credentials.apiKey); + expect(result).toMatchObject({ callbackRegistered: true, verified: false }); + expect(result?.reason).toContain('Homepage URL was left unchanged'); + expect(context.credentials).toEqual(pair); + expect(context.options).toMatchObject(pair); + const env = await readFile(join(directory, '.env.local'), 'utf8'); + expect(env).toContain(`WORKOS_API_KEY=${pair.apiKey}`); + expect(env).toContain(`WORKOS_CLIENT_ID=${pair.clientId}`); + expect(env).toContain('# keep'); + expect(env).not.toContain('fake_rejected'); + expect(JSON.stringify(result)).not.toContain(pair.apiKey); + expect(dashboardGraphqlRequest).not.toHaveBeenCalled(); + expect(fetchTeamEnvironments).not.toHaveBeenCalled(); + }, + ); + + it.each(['same', 'mismatch', 'production', 'repeated401', 'cancel', 'manual', 'authFailure'])( + 'leaves state and files unchanged when REST recovery fails: %s', + async (failure) => { + const context = contextFor(directory); + const before = `WORKOS_API_KEY=${context.credentials.apiKey}\nWORKOS_CLIENT_ID=${setup.clientId}\n`; + await writeFile(join(directory, '.env.local'), before); + vi.mocked(refreshIfExpired).mockResolvedValue(null); + vi.stubGlobal( + 'fetch', + vi.fn(async () => Response.json({ message: 'fake_private_key' }, { status: 401 })), + ); + if (failure === 'same') + vi.mocked(fetchStagingCredentials).mockResolvedValue({ ...pair, apiKey: context.credentials.apiKey }); + if (failure === 'mismatch') + vi.mocked(fetchStagingCredentials).mockResolvedValue({ ...pair, clientId: 'client_other' }); + if (failure === 'production') + vi.mocked(fetchStagingCredentials).mockResolvedValue({ ...pair, apiKey: 'sk_live_fake' }); + if (failure === 'cancel') vi.mocked(ui.select).mockResolvedValue(CANCEL); + if (failure === 'manual') vi.mocked(ui.select).mockResolvedValue('manual'); + if (failure === 'authFailure') vi.mocked(ensureAuthenticated).mockRejectedValue(new Error('fake_private_key')); + await expect( + configureOtherApplicationUrls(context, setup.clientId, context.credentials.apiKey), + ).rejects.toMatchObject({ + code: failure === 'cancel' ? 'cancelled' : 'auth_required', + message: expect.not.stringContaining('fake_private_key'), + }); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(globalThis.fetch).toHaveBeenCalledTimes(failure === 'repeated401' ? 2 : 1); + expect(context.credentials.apiKey).toBe('sk_test_fake_rejected'); + expect(context.options).not.toHaveProperty('apiKey'); + expect(await readFile(join(directory, '.env.local'), 'utf8')).toBe(before); + }, + ); + + it('preserves cancellation during the authentication check', async () => { + vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); + vi.mocked(ensureAuthenticated).mockRejectedValue(new CliExit(2)); + await expect(configureAuthkitApplication(setup, setup.clientId)).rejects.toMatchObject({ code: 'cancelled' }); + expect(fetchTeamEnvironments).toHaveBeenCalledTimes(1); + }); + + it('fails without adopting a key revoked between callback and CORS on the retry', async () => { + const context = contextFor(directory); + const before = `WORKOS_API_KEY=${context.credentials.apiKey}\nWORKOS_CLIENT_ID=${setup.clientId}\n`; + await writeFile(join(directory, '.env.local'), before); + vi.mocked(refreshIfExpired).mockResolvedValue(null); + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValueOnce(Response.json({}, { status: 401 })) + .mockResolvedValueOnce(Response.json({}, { status: 201 })) + .mockResolvedValueOnce(Response.json({}, { status: 401 })), + ); + await expect( + configureOtherApplicationUrls(context, setup.clientId, context.credentials.apiKey), + ).rejects.toMatchObject({ + code: 'auth_required', + message: expect.stringContaining('replacement API key was rejected'), + }); + expect(await readFile(join(directory, '.env.local'), 'utf8')).toBe(before); + expect(context.credentials.apiKey).toBe('sk_test_fake_rejected'); + expect(ui.select).toHaveBeenCalledTimes(1); + }); + + it('does not adopt a pair when the agent changed the local credential file', async () => { + const context = contextFor(directory); + const before = 'WORKOS_API_KEY=sk_test_fake_other\nWORKOS_CLIENT_ID=client_other\n'; + await writeFile(join(directory, '.env.local'), before); + vi.mocked(refreshIfExpired).mockResolvedValue(null); + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValueOnce(Response.json({}, { status: 401 })) + .mockImplementation(async () => Response.json({})), + ); + await expect( + configureOtherApplicationUrls(context, setup.clientId, context.credentials.apiKey), + ).rejects.toMatchObject({ code: 'credential_recovery_failed' }); + expect(await readFile(join(directory, '.env.local'), 'utf8')).toBe(before); + expect(context.credentials.apiKey).toBe('sk_test_fake_rejected'); + }); + + it.each(['ruby', 'go', 'dotnet'])('does not guess how to rewrite %s post-agent credentials', async (integration) => { + vi.mocked(refreshIfExpired).mockResolvedValue(null); + vi.stubGlobal( + 'fetch', + vi.fn(async () => Response.json({}, { status: 401 })), + ); + const context = contextFor(directory, integration); + await expect(configureOtherApplicationUrls(context, setup.clientId, context.credentials.apiKey)).rejects.toThrow( + 'replacement is unavailable', + ); + expect(ui.select).not.toHaveBeenCalled(); + expect(ensureAuthenticated).not.toHaveBeenCalled(); + }); + + it.each(['agent', 'ci', 'json', 'non-tty'])('does not prompt or invoke auth for %s mode', async (mode) => { + if (mode === 'json') setOutputMode('json'); + else if (mode === 'non-tty') Object.defineProperty(process.stdin, 'isTTY', { configurable: true, value: false }); + else setInteractionMode({ mode: mode as 'agent' | 'ci', source: 'flag' }); + vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); + await expect(configureAuthkitApplication(setup, setup.clientId)).rejects.toMatchObject({ + code: 'auth_required', + message: expect.stringContaining('Interactive recovery is unavailable'), + }); + expect(ui.select).not.toHaveBeenCalled(); + expect(ensureAuthenticated).not.toHaveBeenCalled(); + }); + + it('uses the shared TUI prompt host for recovery', async () => { + vi.mocked(ui.select).mockRestore(); + const prompt = vi.fn(async () => 'retry'); + setUiHost({ prompt, line: vi.fn(), status: vi.fn() }); + try { + vi.mocked(fetchTeamEnvironments).mockRejectedValueOnce(unauthorized()); + expect((await configureAuthkitApplication(setup, setup.clientId)).verified).toBe(true); + expect(prompt).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'select', message: expect.stringContaining('Unauthorized') }), + ); + } finally { + setUiHost(null); + } + }); + + it('bounds repeated dashboard Unauthorized and reports partial configuration honestly', async () => { + const original = vi.mocked(dashboardGraphqlRequest).getMockImplementation()!; + vi.mocked(dashboardGraphqlRequest).mockImplementation(async (name, options) => { + if (name === 'updateAuthkitApplication') throw unauthorized(); + return original(name, options); + }); + const result = await configureAuthkitApplication(setup, setup.clientId, 'sk_test_fake'); + expect(result).toMatchObject({ + callbackRegistered: true, + verified: false, + reason: expect.stringContaining('exhausted after one retry'), + }); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(ensureAuthenticated).toHaveBeenCalledTimes(1); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }); + + it('honors explicit --ci even when the interaction mode is human and stdin is a TTY', async () => { + const context = contextFor(directory); + context.options.ci = true; + vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); + await expect( + configureOtherApplicationUrls(context, setup.clientId, context.credentials.apiKey), + ).rejects.toMatchObject({ code: 'auth_required' }); + expect(ui.select).not.toHaveBeenCalled(); + expect(ensureAuthenticated).not.toHaveBeenCalled(); + }); + + it('does not retry an unchanged rejected dashboard session', async () => { + vi.mocked(getAccessToken).mockReturnValue('test-token'); + vi.mocked(ensureAuthenticated).mockResolvedValue({ + authenticated: true, + loginTriggered: false, + tokenRefreshed: false, + }); + vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); + await expect(configureAuthkitApplication(setup, setup.clientId)).rejects.toThrow('session is unchanged'); + expect(fetchTeamEnvironments).toHaveBeenCalledTimes(1); + expect(ui.log.info).toHaveBeenCalledWith(expect.stringContaining('no new login')); + }); + + it.each(['forbidden', 'network', 'message401'])('does not recover non-auth failures (%s)', async (failure) => { + vi.mocked(fetchTeamEnvironments).mockRejectedValue( + failure === 'forbidden' + ? new DashboardGraphqlError('forbidden', 'forbidden', 403) + : new Error(failure === 'network' ? 'network failed' : 'Unauthorized HTTP 401'), + ); + await expect(configureAuthkitApplication(setup, setup.clientId)).rejects.toThrow('Callback'); + expect(ui.select).not.toHaveBeenCalled(); + }); + + it('reconciles partial writes on the same target, without replaying logout or falling back to REST', async () => { + const original = vi.mocked(dashboardGraphqlRequest).getMockImplementation()!; + let rejected = false; + vi.mocked(dashboardGraphqlRequest).mockImplementation(async (name, options) => { + if (name === 'updateAuthkitApplication' && !rejected) { + rejected = true; + throw unauthorized(); + } + return original(name, options); + }); + const result = await configureAuthkitApplication(setup, setup.clientId, 'sk_test_fake'); + expect(result.verified).toBe(true); + expect(writes().filter(([name]) => name === 'setAuthkitApplicationLogoutUris')).toHaveLength(1); + expect(application.appHomepageUrl).toBe('https://existing.example/'); + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(fetchTeamEnvironments).toHaveBeenCalledTimes(2); + expect(ui.select).toHaveBeenCalledTimes(1); + }); + + it('re-reads a callback write whose response was Unauthorized, rather than replaying the stale list', async () => { + application.redirectUris = [{ uri: 'https://old.example/callback', isDefault: true }]; + const original = vi.mocked(dashboardGraphqlRequest).getMockImplementation()!; + let rejected = false; + vi.mocked(dashboardGraphqlRequest).mockImplementation(async (name, options) => { + const result = await original(name, options); + if (name === 'setRedirectUris' && !(options.variables!.input as { dryRun: boolean }).dryRun && !rejected) { + rejected = true; + application.redirectUris.push({ uri: 'https://concurrent.example/callback', isDefault: false }); + throw unauthorized(); + } + return result; + }); + expect((await configureAuthkitApplication(setup, setup.clientId, 'sk_test_fake')).verified).toBe(true); + expect(writes().filter(([name]) => name === 'setRedirectUris')).toHaveLength(1); + expect(application.redirectUris).toEqual([ + { uri: 'https://old.example/callback', isDefault: true }, + { uri: setup.redirectUri, isDefault: false }, + { uri: 'https://concurrent.example/callback', isDefault: false }, + ]); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }); + + it.each(['missing', 'ambiguous', 'environment', 'application', 'production'])( + 'rejects a changed recovery target (%s) after a partial write', + async (change) => { + const original = vi.mocked(dashboardGraphqlRequest).getMockImplementation()!; + vi.mocked(dashboardGraphqlRequest).mockImplementation(async (name, options) => { + if (name === 'updateAuthkitApplication') { + if (change === 'application') application.id = 'app_other'; + else + vi.mocked(fetchTeamEnvironments).mockResolvedValue( + change === 'missing' + ? [] + : change === 'ambiguous' + ? [ + { id: 'env_app', name: 'A', clientId: setup.clientId, sandbox: true }, + { id: 'env_other', name: 'B', clientId: setup.clientId, sandbox: true }, + ] + : [ + { + id: change === 'environment' ? 'env_other' : 'env_app', + name: 'Changed', + clientId: setup.clientId, + sandbox: change !== 'production', + }, + ], + ); + throw unauthorized(); + } + return original(name, options); + }); + const outcome = configureAuthkitApplication(setup, setup.clientId, 'sk_test_fake'); + if (change === 'production') + expect(await outcome).toMatchObject({ verified: false, reason: expect.stringContaining('sandbox') }); + else await expect(outcome).rejects.toThrow('Callback'); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(writes().filter(([name]) => name === 'setAuthkitApplicationLogoutUris')).toHaveLength(1); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }, + ); + + it('requires final read-back even after successful recovery', async () => { + vi.mocked(fetchTeamEnvironments).mockRejectedValueOnce(unauthorized()); + const original = vi.mocked(dashboardGraphqlRequest).getMockImplementation()!; + vi.mocked(dashboardGraphqlRequest).mockImplementation(async (name, options) => { + const result = await original(name, options); + if (name === 'updateAuthkitApplication') application.redirectUris = []; + return result; + }); + await expect(configureAuthkitApplication(setup, setup.clientId)).rejects.toThrow('Callback read-back'); + expect(ui.select).toHaveBeenCalledTimes(1); + }); +}); + describe('app URL derivation', () => { let directory: string; beforeEach(async () => { diff --git a/src/lib/authkit-application-setup.ts b/src/lib/authkit-application-setup.ts index 8025cdf9..be13f76e 100644 --- a/src/lib/authkit-application-setup.ts +++ b/src/lib/authkit-application-setup.ts @@ -1,3 +1,9 @@ +import { + isConfigurationUnauthorized, + recoverConfigurationAccess, + configurationRecoveryHint, + type ConfigurationCredentials, +} from './configuration-recovery.js'; import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { parseEnvFile } from '../utils/env-parser.js'; @@ -102,6 +108,82 @@ export async function configureAuthkitApplication( setup: AuthkitApplicationSetup, expectedClientId: string, apiKey?: string, + options: { + adoptCredentials?: (credentials: ConfigurationCredentials) => Promise; + interactive?: boolean; + } = {}, +): Promise { + const { adoptCredentials } = options; + const recovery: SetupRecovery = { retried: false }; + try { + return await attempt(); + } catch (error) { + if (!isConfigurationUnauthorized(error)) throw error; + if (recovery.transport === 'rest' && !adoptCredentials) { + return recovery.pending!( + `Unauthorized. Automatic key replacement is unavailable for this project's credential files. ${configurationRecoveryHint()}`, + 'auth_required', + ); + } + const recovered = await recoverConfigurationAccess( + recovery.transport === 'rest' ? { apiKey: apiKey!, clientId: expectedClientId } : { token: recovery.token ?? '' }, + options.interactive, + ); + if ('reason' in recovered) return recovery.pending!(recovered.reason, recovered.code ?? 'auth_required'); + recovery.retried = true; + recovery.token = recovered.token; + const accepted = recovered.credentials; + if (accepted) apiKey = accepted.apiKey; + let result: AuthkitApplicationSetup; + try { + result = await attempt(); + } catch (retryError) { + if (!isConfigurationUnauthorized(retryError)) throw retryError; + if (accepted) { + throw new InstallDeclinedError( + `Unauthorized recovery exhausted after one retry. The replacement API key was rejected; application credentials were left unchanged. Check any partially configured URLs. ${configurationRecoveryHint()}`, + 'auth_required', + ); + } + return recovery.pending!( + `Unauthorized recovery exhausted after one retry. ${configurationRecoveryHint()}`, + 'auth_required', + ); + } + // Never expose secrets on AuthkitApplicationSetup (it is emitted to adapters). + // The callback updates the file atomically before adopting shared state. + if (accepted) { + try { + await adoptCredentials!(accepted); + } catch { + throw new InstallDeclinedError( + 'Recovered credentials could not be saved safely. Check local credentials and dashboard URLs before retrying.', + 'credential_recovery_failed', + ); + } + } + return result; + } + + function attempt() { + return configureApplicationAttempt(setup, expectedClientId, apiKey, recovery); + } +} + +interface SetupRecovery { + retried: boolean; + transport?: 'rest' | 'dashboard'; + token?: string; + environmentId?: string; + applicationId?: string; + pending?: (reason: string, code?: string) => AuthkitApplicationSetup; +} + +async function configureApplicationAttempt( + setup: AuthkitApplicationSetup, + expectedClientId: string, + apiKey: string | undefined, + recovery: SetupRecovery, ): Promise { if (setup.initiateLoginUri === undefined) { setup = { @@ -114,12 +196,17 @@ export async function configureAuthkitApplication( // Never trust incoming registration flags; they belong to an earlier attempt. setup = { ...setup, signOutRegistered: false, ...(setup.corsOrigin !== undefined ? { corsRegistered: false } : {}) }; let callbackRegistered = false; - const pending = (reason: string): AuthkitApplicationSetup => { + const pending = (reason: string, code = 'callback_unregistered'): AuthkitApplicationSetup => { if (!callbackRegistered) { - throw new InstallDeclinedError(`Callback URL is not registered or verified. ${reason}`, 'callback_unregistered'); + throw new InstallDeclinedError(`Callback URL is not registered or verified. ${reason}`, code); } return { ...setup, callbackRegistered, verified: false, reason }; }; + recovery.pending = (reason, code) => + pending( + `${reason} For application ${setup.clientId}, check Callback URL: ${setup.redirectUri}; Sign-out URI: ${setup.signOutUri}${setup.initiateLoginUri ? `; Initiate login URI: ${setup.initiateLoginUri}` : ''}${setup.corsOrigin ? `; CORS origin: ${setup.corsOrigin}` : ''}.`, + code, + ); const isSignOutDestination = (uri: string): boolean => { try { return new URL(uri).href === new URL(setup.signOutUri).href; @@ -131,6 +218,7 @@ export async function configureAuthkitApplication( return pending('The app client ID changed during installation. Confirm the application before configuring it.'); } const registerApiCallback = async (): Promise => { + recovery.transport = 'rest'; if (!apiKey) return pending( 'No usable dashboard environment or API key is available. Configure the callback in the dashboard.', @@ -143,7 +231,8 @@ export async function configureAuthkitApplication( try { const { createWorkOSClient } = await import('./workos-client.js'); await createWorkOSClient(apiKey).redirectUris.add(setup.redirectUri); - } catch { + } catch (error) { + if (isConfigurationUnauthorized(error)) throw error; return pending('Could not register the callback URL. Check the API key and connection, then retry setup.'); } callbackRegistered = true; @@ -151,7 +240,8 @@ export async function configureAuthkitApplication( try { await createCorsOrigin(apiKey, setup.corsOrigin); setup = { ...setup, corsRegistered: true }; - } catch { + } catch (error) { + if (isConfigurationUnauthorized(error)) throw error; return pending( 'Callback registered using the API key, but CORS origin setup failed. Check the application URLs in the dashboard.', ); @@ -166,7 +256,8 @@ export async function configureAuthkitApplication( } try { await setHomepageUrl(apiKey, setup.homepageUrl ?? new URL(setup.redirectUri).origin); - } catch { + } catch (error) { + if (isConfigurationUnauthorized(error)) throw error; return pending( 'Callback registered using the API key, but homepage setup failed. Check the Homepage URL, Sign-out URI and Initiate login URI in the dashboard.', ); @@ -175,25 +266,35 @@ export async function configureAuthkitApplication( 'Callback registered using the API key. Sign-out URI and Initiate login URI still require dashboard setup and verification. Sign in to the correct team (and claim the environment if needed) to manage those settings.', ); }; - const session = await refreshIfExpired().catch(() => { - throw new InstallDeclinedError( - 'Callback URL is not registered or verified. Could not check the dashboard session. Retry setup.', - 'callback_unregistered', - ); - }); + if (recovery.transport === 'rest') return registerApiCallback(); + const session = + recovery.retried && recovery.token + ? { accessToken: recovery.token } + : await refreshIfExpired().catch((error: unknown) => { + if (isConfigurationUnauthorized(error)) throw error; + throw new InstallDeclinedError( + 'Callback URL is not registered or verified. Could not check the dashboard session. Retry setup.', + 'callback_unregistered', + ); + }); if (!session) return registerApiCallback(); + recovery.token = session.accessToken; + recovery.transport = 'dashboard'; try { const environments = await fetchTeamEnvironments(session.accessToken); const matches = environments.filter((environment) => environment.clientId === setup.clientId); // A session for another team must not disable API-key-only onboarding. No // dashboard mutation has happened, and this branch returns before any can. - if (matches.length === 0) return registerApiCallback(); + if (matches.length === 0 && !recovery.retried) return registerApiCallback(); if (matches.length !== 1) return pending('Could not uniquely match the app client ID to a WorkOS environment.'); const environment = matches[0]; // Already validated by the team catalog and the application read below. // Do not resolve again: that would re-fetch and mutate stored profiles. const environmentId = environment.id; + if (recovery.environmentId && recovery.environmentId !== environmentId) + return pending('The WorkOS environment changed during recovery. No new target was configured.'); + recovery.environmentId = environmentId; const request = (name: string, variables: Record): Promise => dashboardGraphqlRequest(resolveExecutableDocument(getOperation(name)), { token: session.accessToken, @@ -209,6 +310,7 @@ export async function configureAuthkitApplication( !application || application.clientId !== setup.clientId || !application.id || + (recovery.applicationId !== undefined && application.id !== recovery.applicationId) || !Array.isArray(application.logoutUris) || !Array.isArray(application.redirectUris) || !(application.initiateLoginUri === null || typeof application.initiateLoginUri === 'string') || @@ -223,6 +325,7 @@ export async function configureAuthkitApplication( return application; }; let original = await readApplication(); + recovery.applicationId = original.id; if (environment.sandbox !== true) { // Production can use an already registered callback, but is read-only here. callbackRegistered = original.redirectUris.some((uri) => uri.uri === setup.redirectUri); @@ -410,7 +513,7 @@ export async function configureAuthkitApplication( verified: true, }; } catch (error) { - if (error instanceof InstallDeclinedError) throw error; + if (error instanceof InstallDeclinedError || isConfigurationUnauthorized(error)) throw error; // Callback failures are fatal. Once it is confirmed, other settings may be // reported as incomplete, without exposing private backend errors or switching targets. return pending( diff --git a/src/lib/env-writer.spec.ts b/src/lib/env-writer.spec.ts index 1c8aef55..ceb05e10 100644 --- a/src/lib/env-writer.spec.ts +++ b/src/lib/env-writer.spec.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { writeCredentialsEnv, writeEnvLocal } from './env-writer.js'; +import { writeCredentialsEnv, writeEnvLocal, replaceRecoveredEnvCredentials } from './env-writer.js'; /** POSIX permission bits of `path`. */ const modeOf = (path: string): number => statSync(path).mode & 0o777; @@ -21,6 +21,36 @@ describe('writeEnvLocal', () => { rmSync(testDir, { recursive: true, force: true }); }); + it('atomically replaces a recovered pair while preserving unrelated content and permissions', async () => { + const previous = { apiKey: 'sk_test_fake_old', clientId: 'client_fake' }; + const replacement = { apiKey: 'sk_test_fake_new', clientId: 'client_fake' }; + const path = join(testDir, '.env.local'); + writeFileSync( + path, + `# keep\nWORKOS_API_KEY=${previous.apiKey}\nWORKOS_CLIENT_ID=${previous.clientId}\nOTHER=value\n`, + { mode: 0o600 }, + ); + await replaceRecoveredEnvCredentials(testDir, previous, replacement); + expect(readFileSync(path, 'utf8')).toBe( + `# keep\nWORKOS_API_KEY=${replacement.apiKey}\nWORKOS_CLIENT_ID=${replacement.clientId}\nOTHER=value\n`, + ); + if (process.platform !== 'win32') expect(modeOf(path)).toBe(0o600); + expect(readFileSync(join(testDir, '.gitignore'), 'utf8')).toContain('.env.local.recovery-*'); + }); + + it.each(['changed', 'duplicate'])('leaves files untouched when replacement is unsafe: %s', async (failure) => { + const previous = { apiKey: 'sk_test_fake_old', clientId: 'client_fake' }; + const replacement = { apiKey: 'sk_test_fake_new', clientId: 'client_fake' }; + const path = join(testDir, '.env.local'); + const before = + failure === 'changed' + ? 'WORKOS_API_KEY=sk_test_fake_other\nWORKOS_CLIENT_ID=client_other\n' + : `WORKOS_API_KEY=${previous.apiKey}\nWORKOS_API_KEY=${previous.apiKey}\nWORKOS_CLIENT_ID=${previous.clientId}\n`; + writeFileSync(path, before); + await expect(replaceRecoveredEnvCredentials(testDir, previous, replacement)).rejects.toThrow(); + expect(readFileSync(path, 'utf8')).toBe(before); + }); + it('creates .env.local when none exists', () => { writeEnvLocal(testDir, { WORKOS_CLIENT_ID: 'client_123', diff --git a/src/lib/env-writer.ts b/src/lib/env-writer.ts index a7c67a38..9f01b314 100644 --- a/src/lib/env-writer.ts +++ b/src/lib/env-writer.ts @@ -1,5 +1,7 @@ import { existsSync, readFileSync, statSync, writeFileSync } from 'fs'; import { basename, join } from 'path'; +import { lstat, readFile, writeFile, rename, rm } from 'node:fs/promises'; +import type { ConfigurationCredentials } from './configuration-recovery.js'; import { parseEnvFile } from '../utils/env-parser.js'; const ENV_LOCAL_COVERING_PATTERNS = ['.env.local', '.env*.local', '.env*']; @@ -185,6 +187,39 @@ export function writeEnvLocal(installDir: string, envVars: Partial): vo writeSecretFile(envPath, upsertEnvLines(existingContent, vars), envExisted); } +/** Replace a recovered pair only in the known installer-owned file, before state adoption. */ +export async function replaceRecoveredEnvCredentials( + installDir: string, + previous: ConfigurationCredentials, + replacement: ConfigurationCredentials, +): Promise { + const path = join(installDir, '.env.local'); + const info = await lstat(path); + if (!info.isFile()) throw new Error('Credential file is not a regular file.'); + const content = await readFile(path, 'utf8'); + const current = parseEnvFile(content); + if (current.WORKOS_API_KEY !== previous.apiKey || current.WORKOS_CLIENT_ID !== previous.clientId) + throw new Error('Local credentials changed during setup.'); + const updated = upsertEnvLines(content, { + WORKOS_API_KEY: replacement.apiKey, + WORKOS_CLIENT_ID: replacement.clientId, + }); + const parsed = parseEnvFile(updated); + if (parsed.WORKOS_API_KEY !== replacement.apiKey || parsed.WORKOS_CLIENT_ID !== replacement.clientId) + throw new Error('Cannot safely replace duplicate credential assignments.'); + // Keep temporary secrets ignored even with a narrow .env.local ignore rule. + const temporaryName = `.env.local.recovery-${crypto.randomUUID()}`; + ensureGitignore(installDir, '.env.local.recovery-*', ['.env.local.recovery-*', '.env*']); + const temporary = join(installDir, temporaryName); + try { + await writeFile(temporary, updated, { flag: 'wx', mode: info.mode & 0o777 }); + if ((await readFile(path, 'utf8')) !== content) throw new Error('Local credentials changed during recovery.'); + await rename(temporary, path); + } finally { + await rm(temporary, { force: true }); + } +} + /** * Write WorkOS credentials to the appropriate env file for the project. * Picks `.env.local` for JS projects (package.json present) or `.env` for diff --git a/src/lib/run-with-core.setup.spec.ts b/src/lib/run-with-core.setup.spec.ts index a79c07bf..cbea08de 100644 --- a/src/lib/run-with-core.setup.spec.ts +++ b/src/lib/run-with-core.setup.spec.ts @@ -366,6 +366,7 @@ describe('application URLs for SDKs other than Next.js', () => { }, 'client_a', 'sk_test_a', + { adoptCredentials: undefined, interactive: !options.ci }, ); expect(result?.verified).toBe(true); }); diff --git a/src/lib/run-with-core.ts b/src/lib/run-with-core.ts index 9eee52a6..73e41dcf 100644 --- a/src/lib/run-with-core.ts +++ b/src/lib/run-with-core.ts @@ -60,7 +60,8 @@ import { assertNextjsSignInRouteAvailable, } from '../integrations/nextjs/utils.js'; import { detectPort, getClientEnvPrefix, getSignInPath, resolveRedirectUri } from './port-detection.js'; -import { writeEnvLocal } from './env-writer.js'; +import { writeEnvLocal, replaceRecoveredEnvCredentials } from './env-writer.js'; +import type { ConfigurationCredentials } from './configuration-recovery.js'; import { getRegistry } from './registry.js'; import { observeHostFailure } from './host-probe.js'; import { formatWorkOSCommand } from '../utils/command-invocation.js'; @@ -243,6 +244,53 @@ export async function configureInstallEnvironment( step('env-vars', 'done'); } +async function credentialAdopter( + context: Pick & + Partial>, + clientId: string, + apiKey?: string, +): Promise<((pair: ConfigurationCredentials) => Promise) | undefined> { + if ( + !apiKey || + !context.credentials || + !context.integration || + (await getRegistry()).get(context.integration)?.config.metadata.language !== 'javascript' + ) + return undefined; + return async (pair) => { + await replaceRecoveredEnvCredentials(context.options.installDir, { apiKey, clientId }, pair); + Object.assign(context.credentials!, pair); + Object.assign(context.options, pair); + }; +} + +/** The actual post-agent Next.js path: validate the saved app before any URL writes. */ +export async function configureNextjsApplicationUrls( + context: Pick, +): Promise { + const { options, credentials, emitter } = context; + const setup = await readNextjsApplicationSetup(options.installDir, options.homepageUrl); + if (setup.redirectUri !== resolveRedirectUri('nextjs', options)) { + throw new Error('The app callback URL changed during installation. Confirm it before configuring WorkOS.'); + } + const validation = await validateInstallation('nextjs', options.installDir, { runBuild: false }); + if (!validation.passed) { + throw new Error( + `Application setup is incomplete:\n${validation.issues + .filter((issue) => issue.severity === 'error') + .map((issue) => `${issue.message}. ${issue.hint ?? ''}`) + .join('\n')}`, + ); + } + const adopt = await credentialAdopter(context, credentials?.clientId ?? '', credentials?.apiKey); + return reportAppUrlSetup(emitter, () => + configureAuthkitApplication(setup, credentials?.clientId ?? '', credentials?.apiKey, { + adoptCredentials: adopt, + interactive: !options.ci, + }), + ); +} + export const NO_SIGN_IN_ROUTE_REASON = 'This framework has no fixed sign-in route to use.'; /** @@ -282,7 +330,8 @@ export async function reportAppUrlSetup( * target selected here. An unregistered callback fails the install. */ export async function configureOtherApplicationUrls( - context: Pick, + context: Pick & + Partial>, clientId: string, apiKey?: string, ): Promise { @@ -307,9 +356,18 @@ export async function configureOtherApplicationUrls( delete setup.initiateLoginUri; setup.initiateLoginReason = `Client-side ${signInPath} requires browser verification. Confirm it starts sign-in without a click, then set the Initiate login URI in the WorkOS dashboard. The existing setting was left unchanged.`; } - return reportAppUrlSetup(context.emitter, () => configureAuthkitApplication(setup, clientId, apiKey), { - includeCors: true, - }); + const adopt = await credentialAdopter(context, clientId, apiKey); + return reportAppUrlSetup( + context.emitter, + () => + configureAuthkitApplication(setup, clientId, apiKey, { + adoptCredentials: adopt, + interactive: !installerOptions.ci, + }), + { + includeCors: true, + }, + ); } /** Pick the installer adapter for this process's output mode and terminal. */ @@ -486,32 +544,7 @@ export async function runWithCore(options: InstallerOptions): Promise { const summary = await runIntegrationInstallerFn(integration, agentOptions); let applicationSetup; if (integration === 'nextjs') { - applicationSetup = await readNextjsApplicationSetup( - installerOptions.installDir, - installerOptions.homepageUrl, - ); - const expectedRedirectUri = resolveRedirectUri(integration, installerOptions); - if (applicationSetup.redirectUri !== expectedRedirectUri) { - throw new Error( - 'The app callback URL changed during installation. Confirm it before configuring WorkOS.', - ); - } - // Even --no-validate must not point the dashboard at a missing route. - const validation = await validateInstallation(integration, installerOptions.installDir, { - runBuild: false, - }); - if (!validation.passed) { - throw new Error( - `Application setup is incomplete:\n${validation.issues - .filter((issue) => issue.severity === 'error') - .map((issue) => `${issue.message}. ${issue.hint ?? ''}`) - .join('\n')}`, - ); - } - const setup = applicationSetup; - applicationSetup = await reportAppUrlSetup(context.emitter, () => - configureAuthkitApplication(setup, credentials?.clientId ?? '', credentials?.apiKey), - ); + applicationSetup = await configureNextjsApplicationUrls(context); } else if (credentials?.clientId) { applicationSetup = await configureOtherApplicationUrls(context, credentials.clientId, credentials.apiKey); } From 7657b2ba218a394e9c7ece9058c0f3da81a26494 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Mon, 28 Sep 2026 16:59:15 -0500 Subject: [PATCH 3/3] fix(installer): clarify manual recovery and incomplete homepage setup Follow up PR255 review of a91baf54691e4e986e21fa85e9dd6ff992a84c2d. Reproductions confirmed the rejected-but-unexpired session stop and the legacy success headline after a skipped homepage write. P1 disposition: Nick chose option B, retaining manual session replacement rather than introducing explicit reauth policy. Explain that auth login alone can reuse a rejected session; offer user-controlled host logout/login for the same account or manual URL verification. Do not log a reused rejected session as needing no login. No auth, logout, target, retry-budget or headless policy changes. P2 disposition: retain the ownership guard. A recovered staging pair does not authorize using stale profile claim evidence to write a default homepage. The optional homepage result and post-agent unverified/manual outcome were already intentional; replace the legacy full-success headline with explicit partial guidance. Explicit homepage overrides retain their existing semantics. Offline regressions cover real unexpired-session guards and login reuse, failed/cancelled auth checks, post-agent file preservation, unclaimed-profile key replacement, preserved homepage settings and explicit overrides. Focused 202 tests and full 3290 tests pass; typecheck, lint, format:check and standalone build pass. Refs: AUTH-6735, https://github.com/workos/cli/pull/255#discussion_r4127134957, https://github.com/workos/cli/pull/255#discussion_r4127134970 --- src/commands/login.spec.ts | 26 ++++++- src/lib/authkit-application-setup.spec.ts | 89 +++++++++++++++++++---- src/lib/configuration-recovery.spec.ts | 86 +++++++++++++++++++++- src/lib/configuration-recovery.ts | 24 +++--- src/lib/workos-management.spec.ts | 48 ++++++++++++ src/lib/workos-management.ts | 12 ++- 6 files changed, 256 insertions(+), 29 deletions(-) diff --git a/src/commands/login.spec.ts b/src/commands/login.spec.ts index 42f33e6c..f31bd7a5 100644 --- a/src/commands/login.spec.ts +++ b/src/commands/login.spec.ts @@ -109,7 +109,7 @@ const { getConfig, saveConfig, setInsecureConfigStorage, clearConfig } = await i const { provisionStagingEnvironment, runLogin } = await import('./login.js'); const { maybeRunSetupAfter } = await import('./setup.js'); const { isJsonMode, outputJson } = await import('../utils/output.js'); -const { clearCredentials, setInsecureStorage } = await import('../lib/credentials.js'); +const { clearCredentials, setInsecureStorage, saveCredentials, getCredentials } = await import('../lib/credentials.js'); const { resetInteractionModeForTests, setInteractionMode } = await import('../utils/interaction-mode.js'); const uiMod = await import('../utils/ui.js'); @@ -151,6 +151,30 @@ describe('login', () => { } catch {} }); + it('reuses a locally unexpired session even when a dashboard request previously rejected it', async () => { + const rejectedSession = { + accessToken: 'fake_revoked_but_unexpired_token', + refreshToken: 'fake_refresh_token', + expiresAt: Date.now() + 3_600_000, + userId: 'fake_user', + }; + saveCredentials(rejectedSession); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + try { + await runLogin(); + expect(log).toHaveBeenCalledWith(expect.stringContaining('Already logged in')); + expect(getCredentials()).toMatchObject(rejectedSession); + expect(mockOpen).not.toHaveBeenCalled(); + expect(mockRequestDeviceCode).not.toHaveBeenCalled(); + expect(mockPollForToken).not.toHaveBeenCalled(); + expect(mockFetchStagingCredentials).not.toHaveBeenCalled(); + expect(mockTryResolveProfileEnvironmentId).not.toHaveBeenCalled(); + expect(maybeRunSetupAfter).not.toHaveBeenCalled(); + } finally { + log.mockRestore(); + } + }); + describe('provisionStagingEnvironment', () => { it('creates a staging environment on success', async () => { mockFetchStagingCredentials.mockResolvedValueOnce({ diff --git a/src/lib/authkit-application-setup.spec.ts b/src/lib/authkit-application-setup.spec.ts index 12e33404..691678cc 100644 --- a/src/lib/authkit-application-setup.spec.ts +++ b/src/lib/authkit-application-setup.spec.ts @@ -989,9 +989,15 @@ describe('Unauthorized configuration recovery', () => { }); it.each(['nextjs', 'sveltekit'])( - 'adopts an authoritative pair in %s post-agent files, state and subsequent options', + 'adopts an authoritative pair in %s but leaves the unclaimed profile homepage explicitly incomplete', async (integration) => { const context = contextFor(directory, integration); + vi.mocked(getActiveEnvironment).mockReturnValue({ + name: 'fake-unclaimed', + type: 'unclaimed', + ...context.credentials, + claimToken: 'fake_claim_token', + }); const redirectKey = integration === 'nextjs' ? 'NEXT_PUBLIC_WORKOS_REDIRECT_URI' : 'WORKOS_REDIRECT_URI'; await writeFile( join(directory, '.env.local'), @@ -1023,6 +1029,10 @@ describe('Unauthorized configuration recovery', () => { expect(JSON.stringify(result)).not.toContain(pair.apiKey); expect(dashboardGraphqlRequest).not.toHaveBeenCalled(); expect(fetchTeamEnvironments).not.toHaveBeenCalled(); + expect(vi.mocked(getActiveEnvironment).mock.results.at(-1)?.value.apiKey).toBe('sk_test_fake_rejected'); + expect( + request.mock.calls.every(([url]) => !url.endsWith('/app_homepage_url') && !url.endsWith('/claim-nonces')), + ).toBe(true); }, ); @@ -1060,6 +1070,36 @@ describe('Unauthorized configuration recovery', () => { }, ); + it.each(['failed', 'cancelled', 'not-authenticated'])( + 'leaves post-agent files and target untouched when dashboard auth checking is %s', + async (outcome) => { + const context = contextFor(directory, 'nextjs'); + const before = `WORKOS_API_KEY=${context.credentials.apiKey}\nWORKOS_CLIENT_ID=${setup.clientId}\nNEXT_PUBLIC_WORKOS_REDIRECT_URI=${setup.redirectUri}\n`; + await writeFile(join(directory, '.env.local'), before); + vi.mocked(fetchTeamEnvironments).mockRejectedValueOnce(unauthorized()); + if (outcome === 'not-authenticated') + vi.mocked(ensureAuthenticated).mockResolvedValue({ + authenticated: false, + loginTriggered: true, + tokenRefreshed: false, + }); + else + vi.mocked(ensureAuthenticated).mockRejectedValue( + outcome === 'cancelled' ? new CliExit(2) : new Error('fake_private_auth_error'), + ); + await expect(configureNextjsApplicationUrls(context)).rejects.toMatchObject({ + code: outcome === 'cancelled' ? 'cancelled' : 'auth_required', + message: expect.not.stringContaining('fake_private_auth_error'), + }); + expect(await readFile(join(directory, '.env.local'), 'utf8')).toBe(before); + expect(context.credentials.apiKey).toBe('sk_test_fake_rejected'); + expect(fetchTeamEnvironments).toHaveBeenCalledTimes(1); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(dashboardGraphqlRequest).not.toHaveBeenCalled(); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }, + ); + it('preserves cancellation during the authentication check', async () => { vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); vi.mocked(ensureAuthenticated).mockRejectedValue(new CliExit(2)); @@ -1180,18 +1220,41 @@ describe('Unauthorized configuration recovery', () => { expect(ensureAuthenticated).not.toHaveBeenCalled(); }); - it('does not retry an unchanged rejected dashboard session', async () => { - vi.mocked(getAccessToken).mockReturnValue('test-token'); - vi.mocked(ensureAuthenticated).mockResolvedValue({ - authenticated: true, - loginTriggered: false, - tokenRefreshed: false, - }); - vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); - await expect(configureAuthkitApplication(setup, setup.clientId)).rejects.toThrow('session is unchanged'); - expect(fetchTeamEnvironments).toHaveBeenCalledTimes(1); - expect(ui.log.info).toHaveBeenCalledWith(expect.stringContaining('no new login')); - }); + it.each([false, true])( + 'keeps unchanged-session recovery manual, with callbackRegistered=%s', + async (callbackRegistered) => { + vi.mocked(getAccessToken).mockReturnValue('test-token'); + vi.mocked(ensureAuthenticated).mockResolvedValue({ + authenticated: true, + loginTriggered: false, + tokenRefreshed: false, + }); + if (callbackRegistered) { + const original = vi.mocked(dashboardGraphqlRequest).getMockImplementation()!; + vi.mocked(dashboardGraphqlRequest).mockImplementation(async (name, options) => { + if (name === 'updateAuthkitApplication') throw unauthorized(); + return original(name, options); + }); + } else vi.mocked(fetchTeamEnvironments).mockRejectedValue(unauthorized()); + const outcome = configureAuthkitApplication(setup, setup.clientId); + if (callbackRegistered) { + expect(await outcome).toMatchObject({ + callbackRegistered: true, + verified: false, + reason: expect.stringContaining('auth login` alone may reuse'), + }); + } else { + await expect(outcome).rejects.toMatchObject({ + code: 'auth_required', + message: expect.stringContaining('auth login` alone may reuse'), + }); + } + expect(fetchTeamEnvironments).toHaveBeenCalledTimes(1); + expect(ui.select).toHaveBeenCalledTimes(1); + expect(ui.log.info).not.toHaveBeenCalled(); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }, + ); it.each(['forbidden', 'network', 'message401'])('does not recover non-auth failures (%s)', async (failure) => { vi.mocked(fetchTeamEnvironments).mockRejectedValue( diff --git a/src/lib/configuration-recovery.spec.ts b/src/lib/configuration-recovery.spec.ts index fc7e8aae..d4bacb4c 100644 --- a/src/lib/configuration-recovery.spec.ts +++ b/src/lib/configuration-recovery.spec.ts @@ -1,8 +1,90 @@ -import { describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { UnauthorizedException } from '@workos-inc/node'; import { DashboardGraphqlError } from './dashboard-graphql.js'; import { WorkOSApiError } from './workos-api.js'; -import { DashboardConfigError, isConfigurationUnauthorized } from './configuration-recovery.js'; +import { + DashboardConfigError, + isConfigurationUnauthorized, + recoverConfigurationAccess, +} from './configuration-recovery.js'; +import { getCredentials, clearCredentials, saveCredentials, updateTokens } from './credential-store.js'; +import { runLogin } from '../commands/login.js'; +import { refreshAccessToken } from './token-refresh-client.js'; +import ui, { CANCEL } from '../utils/ui.js'; +import { setInteractionMode, resetInteractionModeForTests } from '../utils/interaction-mode.js'; +import { setOutputMode } from '../utils/output.js'; + +// Keep the real credential expiry logic, refresh guard, and ensureAuthenticated. +// Only persistence and external authentication are mocked: no keychain/home reads. +vi.mock('./credential-store.js', () => ({ + getCredentials: vi.fn(), + hasCredentials: vi.fn(() => true), + clearCredentials: vi.fn(), + saveCredentials: vi.fn(), + updateTokens: vi.fn(), +})); +vi.mock('../commands/login.js', () => ({ runLogin: vi.fn() })); +vi.mock('./token-refresh-client.js', () => ({ refreshAccessToken: vi.fn() })); +vi.mock('./host-probe.js', () => ({ warnIfSandboxed: vi.fn() })); + +const fakeSession = { + accessToken: 'fake_revoked_but_unexpired_token', + refreshToken: 'fake_refresh_token', + expiresAt: Date.now() + 3_600_000, + userId: 'fake_user', +}; + +describe('revoked but locally unexpired session policy', () => { + const tty = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getCredentials).mockReturnValue(fakeSession); + Object.defineProperty(process.stdin, 'isTTY', { configurable: true, value: true }); + setInteractionMode({ mode: 'human', source: 'flag' }); + setOutputMode('human'); + vi.spyOn(ui, 'select').mockResolvedValue('retry'); + vi.spyOn(ui.log, 'info').mockImplementation(() => {}); + }); + afterEach(() => { + vi.restoreAllMocks(); + if (tty) Object.defineProperty(process.stdin, 'isTTY', tty); + else Reflect.deleteProperty(process.stdin, 'isTTY'); + resetInteractionModeForTests(); + setOutputMode('human'); + }); + + it.each(['human', 'json', 'agent', 'ci'] as const)( + 'gives actionable manual guidance without replacing a rejected session in %s mode', + async (mode) => { + if (mode === 'json') setOutputMode('json'); + else setInteractionMode({ mode, source: 'flag' }); + const result = await recoverConfigurationAccess({ token: fakeSession.accessToken }); + expect(result).toEqual({ reason: expect.stringContaining('auth login` alone may reuse') }); + if ('reason' in result) { + expect(result.reason).toContain('auth logout'); + expect(result.reason).toContain('same account'); + expect(result.reason).toContain('will not log you out automatically'); + expect(result.reason).not.toContain(fakeSession.accessToken); + } + expect(ui.select).toHaveBeenCalledTimes(mode === 'human' ? 1 : 0); + expect(ui.log.info).not.toHaveBeenCalled(); + expect(runLogin).not.toHaveBeenCalled(); + expect(refreshAccessToken).not.toHaveBeenCalled(); + expect(clearCredentials).not.toHaveBeenCalled(); + expect(saveCredentials).not.toHaveBeenCalled(); + expect(updateTokens).not.toHaveBeenCalled(); + }, + ); + + it.each(['manual', CANCEL])('does not run authentication when recovery is declined or cancelled', async (choice) => { + vi.mocked(ui.select).mockResolvedValue(choice); + const result = await recoverConfigurationAccess({ token: fakeSession.accessToken }); + expect(result).toMatchObject({ reason: expect.stringContaining(choice === CANCEL ? 'cancelled' : 'declined') }); + expect(getCredentials).not.toHaveBeenCalled(); + expect(runLogin).not.toHaveBeenCalled(); + expect(clearCredentials).not.toHaveBeenCalled(); + }); +}); describe('configuration Unauthorized classification', () => { it.each([ diff --git a/src/lib/configuration-recovery.ts b/src/lib/configuration-recovery.ts index 3d43cb25..b20cbcfe 100644 --- a/src/lib/configuration-recovery.ts +++ b/src/lib/configuration-recovery.ts @@ -29,7 +29,10 @@ export interface ConfigurationCredentials { clientId: string; } -export function configurationRecoveryHint(): string { +export function configurationRecoveryHint(dashboardSession = false): string { + if (dashboardSession) { + return `A dashboard session can be rejected before its local expiry; \`${formatWorkOSCommand('auth login')}\` alone may reuse it. If you choose to replace this CLI session, run \`${formatWorkOSCommand('auth logout')}\` then \`${formatWorkOSCommand('auth login')}\` in your host terminal, sign in to the same account, and retry setup for the same application. The installer will not log you out automatically. Alternatively, configure and verify the application's URLs manually in the WorkOS dashboard.`; + } return `Run \`${formatWorkOSCommand('auth login')}\` to check dashboard access, then retry setup for the same application, or configure its URLs manually in the WorkOS dashboard. A dashboard login does not itself replace a rejected API key.`; } @@ -42,7 +45,7 @@ export async function recoverConfigurationAccess( rejected: { token: string } | { apiKey: string; clientId?: string }, interactive = true, ): Promise { - const hint = configurationRecoveryHint(); + const hint = configurationRecoveryHint('token' in rejected); if (!interactive || !isPromptAllowed() || isJsonMode() || !process.stdin.isTTY) { return { reason: `Unauthorized. Interactive recovery is unavailable. ${hint}` }; } @@ -64,6 +67,14 @@ export async function recoverConfigurationAccess( const { ensureAuthenticated } = await import('./ensure-auth.js'); const auth = await ensureAuthenticated(); if (!auth.authenticated) return { reason: `Authentication check failed. ${hint}` }; + const { getAccessToken } = await import('./credentials.js'); + const token = getAccessToken(); + if (!token) return { reason: `No usable dashboard session. ${hint}` }; + if ('token' in rejected && token === rejected.token) { + // A local expiry check is not proof of server acceptance. Keep the session + // intact; replacing a rejected but unexpired login remains a manual choice. + return { reason: `The dashboard session is unchanged; the rejected session was not retried. ${hint}` }; + } ui.log.info( auth.loginTriggered ? 'Signed in to WorkOS.' @@ -71,14 +82,7 @@ export async function recoverConfigurationAccess( ? 'Dashboard session refreshed.' : 'Using the existing dashboard session; no new login was needed.', ); - const { getAccessToken } = await import('./credentials.js'); - const token = getAccessToken(); - if (!token) return { reason: `No usable dashboard session. ${hint}` }; - if ('token' in rejected) { - return token === rejected.token - ? { reason: `The dashboard session is unchanged; the rejected session was not retried. ${hint}` } - : { token }; - } + if ('token' in rejected) return { token }; if (!rejected.clientId) return { reason: `Cannot verify replacement credentials without the intended client ID. ${hint}` }; const { fetchStagingCredentials } = await import('./staging-api.js'); diff --git a/src/lib/workos-management.spec.ts b/src/lib/workos-management.spec.ts index da8d5159..4da46267 100644 --- a/src/lib/workos-management.spec.ts +++ b/src/lib/workos-management.spec.ts @@ -241,6 +241,54 @@ describe('workos-management', () => { expect(ui.select).not.toHaveBeenCalled(); }); + it.each([undefined, 'https://requested.example/'])( + 'preserves default-vs-explicit homepage semantics after replacing an unclaimed profile key (%s)', + async (homepageUrl) => { + const profile: EnvironmentConfig = { ...unclaimedEnv, clientId: pair.clientId }; + getActiveEnvironment.mockReturnValue(profile); + let homepage = 'https://existing.example/'; + const calls: FetchCall[] = []; + const request = vi.fn(async (url: string, init: RequestInit) => { + calls.push({ url, method: init.method! }); + if (url.endsWith('/claim-nonces')) return Response.json({ nonce: 'fake_claim_nonce' }); + const rejected = (init.headers as Record).Authorization === `Bearer ${API_KEY}`; + if (rejected) return Response.json({}, { status: 401 }); + if (url === HOMEPAGE_ENDPOINT) { + if (init.method === 'GET') return Response.json({ url: homepage }); + homepage = JSON.parse(init.body as string).url; + } + return Response.json({}, { status: 201 }); + }); + vi.stubGlobal('fetch', request); + + const result = await autoConfigureWorkOSEnvironment(API_KEY, INTEGRATION, PORT, { + clientId: pair.clientId, + homepageUrl, + }); + expect(result?.recoveredCredentials).toEqual(pair); + expect(result?.redirectUri.success).toBe(true); + expect(result?.corsOrigin.success).toBe(true); + expect(profile.apiKey).toBe(API_KEY); + expect(ui.select).toHaveBeenCalledTimes(1); + if (homepageUrl) { + expect(result?.homepageUrl).toEqual({ success: true, alreadyExists: false }); + expect(homepage).toBe(homepageUrl); + expect(homepageCalls(calls, 'PUT')).toHaveLength(1); + expect(ui.log.success).toHaveBeenCalledWith('WorkOS dashboard configured'); + } else { + // A same-client staging pair does not transfer the old claim token's + // ownership or prove the environment is STILL unclaimed after login. + expect(result?.homepageUrl).toBeUndefined(); + expect(homepage).toBe('https://existing.example/'); + expect(homepageCalls(calls, 'PUT')).toHaveLength(0); + expect(calls.filter(({ url }) => url.endsWith('/claim-nonces'))).toHaveLength(1); + expect(ui.log.success).not.toHaveBeenCalled(); + expect(ui.log.warn).toHaveBeenCalledWith(expect.stringContaining('homepage left unchanged')); + expect(rowFor('Homepage URL').status).toContain('not changed'); + } + }, + ); + it('never writes the homepage after its GET rejects authorization', async () => { const request = vi.fn(async (url: string) => Response.json({}, { status: url === HOMEPAGE_ENDPOINT ? 401 : 201 }), diff --git a/src/lib/workos-management.ts b/src/lib/workos-management.ts index 58a75d7e..e62e7d07 100644 --- a/src/lib/workos-management.ts +++ b/src/lib/workos-management.ts @@ -28,7 +28,7 @@ export interface AutoConfigResult { recoveredCredentials?: ConfigurationCredentials; redirectUri: { success: boolean; alreadyExists: boolean }; corsOrigin: { success: boolean; alreadyExists: boolean }; - /** Absent when the homepage was left alone (see `autoConfigureWorkOSEnvironment`). */ + /** Absent when the homepage was left alone; callback/CORS success does not imply complete setup. */ homepageUrl?: { success: boolean; alreadyExists: boolean }; } @@ -304,7 +304,9 @@ async function autoConfigureOnce( // someone already chose one. Write only an explicit homepage or a default // for an environment the server still reports as unclaimed. Local claim // status can be stale. With a login, the later dashboard step reads the - // current value and fills an empty one. + // current value and fills an empty one. Recheck on recovery: a replacement + // staging pair does not transfer the stored claim token or prove the target + // is still unclaimed. Leave the default alone when ownership is unverified. const writeHomepage = Boolean(options.homepageUrl) || (await isUnclaimedEnvironmentKey(apiKey)); ui.log.step('Configuring WorkOS dashboard settings...'); @@ -352,7 +354,11 @@ async function autoConfigureOnce( // Aligned key/value feedback: value in accent, a dim status for "already // existed" vs. a green status for a fresh create/update. The provenance row // comes first — it is the context for the three rows below it. - ui.log.success('WorkOS dashboard configured'); + if (homepageUrl) ui.log.success('WorkOS dashboard configured'); + else { + ui.log.warn('Callback and CORS configured; homepage left unchanged.'); + ui.log.info('Check the homepage in the WorkOS dashboard, or supply --homepage-url to explicitly override it.'); + } ui.rows([ { key: 'Environment', value: describeCredentialProvenance(apiKey), statusKind: 'muted' }, {