From 826a42d8d6c4877431474c27e14f0f1d4ec092d7 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 14 Feb 2026 10:55:14 +0700 Subject: [PATCH] fix(cliproxy): harden kiro device-code auth flow consistency --- src/cliproxy/auth/auth-types.ts | 4 +- src/management/oauth-port-diagnostics.ts | 52 ++++++++++--------- src/web-server/routes/cliproxy-auth-routes.ts | 13 +++++ .../cliproxy/provider-capabilities.test.ts | 19 +++++++ .../web-server/cliproxy-auth-routes.test.ts | 16 ++++++ .../components/account/add-account-dialog.tsx | 48 +++++++++++------ ui/src/hooks/use-cliproxy-auth-flow.ts | 2 +- ui/src/hooks/use-device-code.ts | 3 +- ui/src/lib/provider-config.ts | 8 +++ 9 files changed, 122 insertions(+), 43 deletions(-) create mode 100644 tests/unit/web-server/cliproxy-auth-routes.test.ts diff --git a/src/cliproxy/auth/auth-types.ts b/src/cliproxy/auth/auth-types.ts index 234a00ba..1f66114f 100644 --- a/src/cliproxy/auth/auth-types.ts +++ b/src/cliproxy/auth/auth-types.ts @@ -15,19 +15,19 @@ import { AccountInfo } from '../account-manager'; * - Gemini: Authorization Code Flow with local callback server on port 8085 * - Codex: Authorization Code Flow with local callback server on port 1455 * - Agy: Authorization Code Flow with local callback server on port 51121 - * - Kiro: Authorization Code Flow with local callback server on port 9876 * - iFlow: Authorization Code Flow with local callback server on port 11451 * - Claude: Authorization Code Flow with local callback server on port 54545 (Anthropic OAuth) + * - Kiro: Device Code Flow (polling-based, NO callback port needed) * - Qwen: Device Code Flow (polling-based, NO callback port needed) * - GHCP: Device Code Flow (polling-based, NO callback port needed) */ export const OAUTH_CALLBACK_PORTS: Partial> = { gemini: 8085, - kiro: 9876, codex: 1455, agy: 51121, iflow: 11451, claude: 54545, + // kiro: Device Code Flow - no callback port // qwen: Device Code Flow - no callback port // ghcp: Device Code Flow - no callback port }; diff --git a/src/management/oauth-port-diagnostics.ts b/src/management/oauth-port-diagnostics.ts index d50c16cf..8a13628d 100644 --- a/src/management/oauth-port-diagnostics.ts +++ b/src/management/oauth-port-diagnostics.ts @@ -9,7 +9,7 @@ * - Codex: 1455 * - Agy: 51121 * - iFlow: 11451 - * - Kiro: 9876 + * - Kiro: Device Code Flow (no port needed) * - Claude: 54545 * - Qwen: Device Code Flow (no port needed) * - GHCP: Device Code Flow (no port needed) @@ -26,40 +26,44 @@ import { } from '../utils/port-utils'; import { CLIProxyProvider } from '../cliproxy/types'; import { CLIPROXY_PROFILES } from '../auth/profile-detector'; +import { + CLIPROXY_PROVIDER_IDS, + getOAuthCallbackPort, + getOAuthFlowType, + type OAuthFlowType as ProviderOAuthFlowType, +} from '../cliproxy/provider-capabilities'; /** - * OAuth callback ports for each provider - * Extracted from CLIProxyAPI source + * Build provider-indexed records from canonical provider capabilities. + * Keeps diagnostics in sync with runtime OAuth flow metadata. */ -export const OAUTH_CALLBACK_PORTS: Record = { - gemini: 8085, - codex: 1455, - agy: 51121, - qwen: null, // Device Code Flow - no callback port - iflow: 11451, // Authorization Code Flow - kiro: 9876, // Authorization Code Flow - ghcp: null, // Device Code Flow - no callback port - claude: 54545, // Authorization Code Flow (Anthropic OAuth) -}; +function buildProviderMap( + valueFor: (provider: CLIProxyProvider) => T +): Record { + return CLIPROXY_PROVIDER_IDS.reduce( + (acc, provider) => { + acc[provider] = valueFor(provider); + return acc; + }, + {} as Record + ); +} + +export const OAUTH_CALLBACK_PORTS: Record = buildProviderMap( + (provider) => getOAuthCallbackPort(provider) +); /** * OAuth flow types */ -export type OAuthFlowType = 'authorization_code' | 'device_code'; +export type OAuthFlowType = ProviderOAuthFlowType; /** * OAuth flow type per provider */ -export const OAUTH_FLOW_TYPES: Record = { - gemini: 'authorization_code', - codex: 'authorization_code', - agy: 'authorization_code', - qwen: 'device_code', - iflow: 'authorization_code', - kiro: 'authorization_code', - ghcp: 'device_code', - claude: 'authorization_code', -}; +export const OAUTH_FLOW_TYPES: Record = buildProviderMap( + (provider) => getOAuthFlowType(provider) +); /** * Port diagnostic result diff --git a/src/web-server/routes/cliproxy-auth-routes.ts b/src/web-server/routes/cliproxy-auth-routes.ts index 302323ba..0930e873 100644 --- a/src/web-server/routes/cliproxy-auth-routes.ts +++ b/src/web-server/routes/cliproxy-auth-routes.ts @@ -38,6 +38,7 @@ import { CLIPROXY_CALLBACK_PROVIDER_MAP, CLIPROXY_AUTH_URL_PROVIDER_MAP, } from '../../cliproxy/auth/auth-types'; +import { getOAuthFlowType } from '../../cliproxy/provider-capabilities'; import type { CLIProxyProvider } from '../../cliproxy/types'; import { CLIPROXY_PROFILES } from '../../auth/profile-detector'; @@ -46,6 +47,13 @@ const router = Router(); // Valid providers list - derived from canonical CLIPROXY_PROFILES const validProviders: CLIProxyProvider[] = [...CLIPROXY_PROFILES]; +export function getStartUrlUnsupportedReason(provider: CLIProxyProvider): string | null { + if (getOAuthFlowType(provider) === 'device_code') { + return `Provider '${provider}' uses Device Code flow. Use /api/cliproxy/auth/${provider}/start instead.`; + } + return null; +} + /** * GET /api/cliproxy/auth - Get auth status for built-in CLIProxy profiles * Also fetches CLIProxyAPI stats to update lastUsedAt for active providers @@ -557,6 +565,11 @@ router.post('/:provider/start-url', async (req: Request, res: Response): Promise res.status(400).json({ error: `Invalid provider: ${provider}` }); return; } + const unsupportedReason = getStartUrlUnsupportedReason(provider as CLIProxyProvider); + if (unsupportedReason) { + res.status(400).json({ error: unsupportedReason }); + return; + } try { const authUrlProvider = diff --git a/tests/unit/cliproxy/provider-capabilities.test.ts b/tests/unit/cliproxy/provider-capabilities.test.ts index 7aa13e7e..3434c344 100644 --- a/tests/unit/cliproxy/provider-capabilities.test.ts +++ b/tests/unit/cliproxy/provider-capabilities.test.ts @@ -2,11 +2,17 @@ import { describe, expect, it } from 'bun:test'; import { CLIPROXY_PROVIDER_IDS, getOAuthCallbackPort, + getOAuthFlowType, getProviderDisplayName, getProvidersByOAuthFlow, isCLIProxyProvider, mapExternalProviderName, } from '../../../src/cliproxy/provider-capabilities'; +import { + OAUTH_CALLBACK_PORTS as DIAGNOSTIC_CALLBACK_PORTS, + OAUTH_FLOW_TYPES, +} from '../../../src/management/oauth-port-diagnostics'; +import { OAUTH_CALLBACK_PORTS as AUTH_CALLBACK_PORTS } from '../../../src/cliproxy/auth/auth-types'; describe('provider-capabilities', () => { it('keeps canonical provider IDs backward-compatible', () => { @@ -56,4 +62,17 @@ describe('provider-capabilities', () => { expect(getOAuthCallbackPort('gemini')).toBe(8085); expect(getProviderDisplayName('agy')).toBe('AntiGravity'); }); + + it('keeps diagnostics flow metadata in sync with provider capabilities', () => { + for (const provider of CLIPROXY_PROVIDER_IDS) { + expect(OAUTH_FLOW_TYPES[provider]).toBe(getOAuthFlowType(provider)); + expect(DIAGNOSTIC_CALLBACK_PORTS[provider]).toBe(getOAuthCallbackPort(provider)); + } + }); + + it('does not define callback ports for device code providers in auth constants', () => { + for (const provider of getProvidersByOAuthFlow('device_code')) { + expect(AUTH_CALLBACK_PORTS[provider]).toBeUndefined(); + } + }); }); diff --git a/tests/unit/web-server/cliproxy-auth-routes.test.ts b/tests/unit/web-server/cliproxy-auth-routes.test.ts new file mode 100644 index 00000000..025c0e8b --- /dev/null +++ b/tests/unit/web-server/cliproxy-auth-routes.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'bun:test'; +import { getStartUrlUnsupportedReason } from '../../../src/web-server/routes/cliproxy-auth-routes'; + +describe('cliproxy-auth-routes start-url guard', () => { + it('rejects device code providers', () => { + expect(getStartUrlUnsupportedReason('kiro')).toContain("Provider 'kiro' uses Device Code flow"); + expect(getStartUrlUnsupportedReason('ghcp')).toContain("Provider 'ghcp' uses Device Code flow"); + expect(getStartUrlUnsupportedReason('qwen')).toContain("Provider 'qwen' uses Device Code flow"); + }); + + it('allows authorization code providers', () => { + expect(getStartUrlUnsupportedReason('gemini')).toBeNull(); + expect(getStartUrlUnsupportedReason('codex')).toBeNull(); + expect(getStartUrlUnsupportedReason('claude')).toBeNull(); + }); +}); diff --git a/ui/src/components/account/add-account-dialog.tsx b/ui/src/components/account/add-account-dialog.tsx index 55ef8079..abae2ba8 100644 --- a/ui/src/components/account/add-account-dialog.tsx +++ b/ui/src/components/account/add-account-dialog.tsx @@ -1,7 +1,7 @@ /** * Add Account Dialog Component * Uses /start-url to get OAuth URL + polls for completion via management API. - * For Device Code flows (ghcp, qwen): Uses /start endpoint which spawns CLIProxy + * For Device Code flows (ghcp, qwen, kiro): Uses /start endpoint which spawns CLIProxy * binary and emits WebSocket events. DeviceCodeDialog handles user code display. * Shows auth URL + callback paste field. Polling auto-closes on success. * For Kiro: Also shows "Import from IDE" option. @@ -22,7 +22,7 @@ import { Loader2, ExternalLink, User, Download, Copy, Check } from 'lucide-react import { useKiroImport } from '@/hooks/use-cliproxy'; import { useCliproxyAuthFlow } from '@/hooks/use-cliproxy-auth-flow'; import { applyDefaultPreset } from '@/lib/preset-utils'; -import { isDeviceCodeProvider } from '@/lib/provider-config'; +import { isDeviceCodeProvider, isNicknameRequiredProvider } from '@/lib/provider-config'; import { toast } from 'sonner'; interface AddAccountDialogProps { @@ -44,18 +44,23 @@ export function AddAccountDialog({ const [nickname, setNickname] = useState(''); const [callbackUrl, setCallbackUrl] = useState(''); const [copied, setCopied] = useState(false); + const [localError, setLocalError] = useState(null); const wasAuthenticatingRef = useRef(false); const authFlow = useCliproxyAuthFlow(); const kiroImportMutation = useKiroImport(); const isKiro = provider === 'kiro'; const isDeviceCode = isDeviceCodeProvider(provider); + const requiresNickname = isNicknameRequiredProvider(provider); const isPending = authFlow.isAuthenticating || kiroImportMutation.isPending; + const nicknameTrimmed = nickname.trim(); + const errorMessage = localError || authFlow.error; const resetAndClose = () => { setNickname(''); setCallbackUrl(''); setCopied(false); + setLocalError(null); wasAuthenticatingRef.current = false; onClose(); }; @@ -103,13 +108,18 @@ export function AddAccountDialog({ }; /** - * Authenticate via /start-url + polling only. - * Does NOT call /start (which spawns a local CLIProxy binary that kills running instances). - * /start-url uses the management API to get auth URL, then polls for completion. + * Start auth flow using provider capabilities. + * - Device code providers use /start and rely on WebSocket events for code display. + * - Authorization code providers use /start-url and polling. */ const handleAuthenticate = () => { + if (requiresNickname && !nicknameTrimmed) { + setLocalError(`Nickname is required for ${displayName} accounts.`); + return; + } + setLocalError(null); wasAuthenticatingRef.current = true; - authFlow.startAuth(provider, { nickname: nickname.trim() || undefined }); + authFlow.startAuth(provider, { nickname: nicknameTrimmed || undefined }); }; const handleKiroImport = () => { @@ -157,20 +167,27 @@ export function AddAccountDialog({ {/* Nickname input - only show before auth starts */} {!showAuthUI && (
- +
setNickname(e.target.value)} + onChange={(e) => { + setNickname(e.target.value); + setLocalError(null); + }} placeholder="e.g., work, personal" disabled={isPending} className="flex-1" />

- A friendly name to identify this account. Auto-generated from email if left empty. + {requiresNickname + ? 'Required for this provider. Use a unique friendly name (e.g., work, personal).' + : 'A friendly name to identify this account. Auto-generated from email if left empty.'}

)} @@ -191,11 +208,6 @@ export function AddAccountDialog({

- {/* Error display */} - {authFlow.error && !authFlow.authUrl && ( -

{authFlow.error}

- )} - {/* Auth URL section - only for Authorization Code flows, NOT Device Code */} {authFlow.authUrl && !authFlow.isDeviceCodeFlow && (
@@ -273,6 +285,9 @@ export function AddAccountDialog({
)} + {/* Persist error visibility outside auth-only UI states */} + {errorMessage &&

{errorMessage}

} + {/* Kiro import loading */} {kiroImportMutation.isPending && (

@@ -302,7 +317,10 @@ export function AddAccountDialog({ )} {!showAuthUI && ( - diff --git a/ui/src/hooks/use-cliproxy-auth-flow.ts b/ui/src/hooks/use-cliproxy-auth-flow.ts index 433af6e2..73a1d5c4 100644 --- a/ui/src/hooks/use-cliproxy-auth-flow.ts +++ b/ui/src/hooks/use-cliproxy-auth-flow.ts @@ -19,7 +19,7 @@ interface AuthFlowState { oauthState: string | null; /** Whether callback is being submitted */ isSubmittingCallback: boolean; - /** Whether this is a device code flow (ghcp, qwen) - dialog handled separately via WebSocket */ + /** Whether this is a device code flow (ghcp, qwen, kiro) - dialog handled separately via WebSocket */ isDeviceCodeFlow: boolean; } diff --git a/ui/src/hooks/use-device-code.ts b/ui/src/hooks/use-device-code.ts index a8492068..8c69bd77 100644 --- a/ui/src/hooks/use-device-code.ts +++ b/ui/src/hooks/use-device-code.ts @@ -3,7 +3,7 @@ * * Listens for WebSocket device code events and manages dialog state. * Similar to useProjectSelection but for Device Code OAuth flows - * (GitHub Copilot, Qwen, etc.) + * (GitHub Copilot, Qwen, Kiro, etc.) */ import { useState, useEffect, useCallback, useMemo } from 'react'; @@ -26,6 +26,7 @@ interface DeviceCodeState { /** Provider display names for user-friendly messages */ const PROVIDER_DISPLAY_NAMES: Record = { ghcp: 'GitHub Copilot', + kiro: 'Kiro (AWS)', qwen: 'Qwen Code', }; diff --git a/ui/src/lib/provider-config.ts b/ui/src/lib/provider-config.ts index bdcbd378..2da393ba 100644 --- a/ui/src/lib/provider-config.ts +++ b/ui/src/lib/provider-config.ts @@ -82,3 +82,11 @@ export const DEVICE_CODE_PROVIDERS: CLIProxyProvider[] = ['ghcp', 'kiro', 'qwen' export function isDeviceCodeProvider(provider: string): boolean { return DEVICE_CODE_PROVIDERS.includes(provider as CLIProxyProvider); } + +/** Providers that require nickname because token payload may not include email. */ +export const NICKNAME_REQUIRED_PROVIDERS: CLIProxyProvider[] = ['ghcp', 'kiro']; + +/** Check if provider requires user-supplied nickname in auth flow */ +export function isNicknameRequiredProvider(provider: string): boolean { + return NICKNAME_REQUIRED_PROVIDERS.includes(provider as CLIProxyProvider); +}