refactor(auth): save authType after successfully authenticated (#1036)

This commit is contained in:
Mingholy 2025-11-19 11:21:46 +08:00 committed by GitHub
parent 3ed93d5b5d
commit d0e76c76a8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
30 changed files with 822 additions and 518 deletions

View file

@ -9,6 +9,53 @@ import { AuthDialog } from './AuthDialog.js';
import { LoadedSettings, SettingScope } from '../../config/settings.js';
import { AuthType } from '@qwen-code/qwen-code-core';
import { renderWithProviders } from '../../test-utils/render.js';
import { UIStateContext } from '../contexts/UIStateContext.js';
import { UIActionsContext } from '../contexts/UIActionsContext.js';
import type { UIState } from '../contexts/UIStateContext.js';
import type { UIActions } from '../contexts/UIActionsContext.js';
const createMockUIState = (overrides: Partial<UIState> = {}): UIState => {
// AuthDialog only uses authError and pendingAuthType
const baseState = {
authError: null,
pendingAuthType: undefined,
} as Partial<UIState>;
return {
...baseState,
...overrides,
} as UIState;
};
const createMockUIActions = (overrides: Partial<UIActions> = {}): UIActions => {
// AuthDialog only uses handleAuthSelect
const baseActions = {
handleAuthSelect: vi.fn(),
} as Partial<UIActions>;
return {
...baseActions,
...overrides,
} as UIActions;
};
const renderAuthDialog = (
settings: LoadedSettings,
uiStateOverrides: Partial<UIState> = {},
uiActionsOverrides: Partial<UIActions> = {},
) => {
const uiState = createMockUIState(uiStateOverrides);
const uiActions = createMockUIActions(uiActionsOverrides);
return renderWithProviders(
<UIStateContext.Provider value={uiState}>
<UIActionsContext.Provider value={uiActions}>
<AuthDialog />
</UIActionsContext.Provider>
</UIStateContext.Provider>,
{ settings },
);
};
describe('AuthDialog', () => {
const wait = (ms = 50) => new Promise((resolve) => setTimeout(resolve, ms));
@ -66,13 +113,9 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog
onSelect={() => {}}
settings={settings}
initialErrorMessage="GEMINI_API_KEY environment variable not found"
/>,
);
const { lastFrame } = renderAuthDialog(settings, {
authError: 'GEMINI_API_KEY environment variable not found',
});
expect(lastFrame()).toContain(
'GEMINI_API_KEY environment variable not found',
@ -116,9 +159,7 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog onSelect={() => {}} settings={settings} />,
);
const { lastFrame } = renderAuthDialog(settings);
// Since the auth dialog only shows OpenAI option now,
// it won't show GEMINI_API_KEY messages
@ -162,9 +203,7 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog onSelect={() => {}} settings={settings} />,
);
const { lastFrame } = renderAuthDialog(settings);
expect(lastFrame()).not.toContain(
'Existing API key detected (GEMINI_API_KEY)',
@ -208,9 +247,7 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog onSelect={() => {}} settings={settings} />,
);
const { lastFrame } = renderAuthDialog(settings);
// Since the auth dialog only shows OpenAI option now,
// it won't show GEMINI_API_KEY messages
@ -255,9 +292,7 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog onSelect={() => {}} settings={settings} />,
);
const { lastFrame } = renderAuthDialog(settings);
// This is a bit brittle, but it's the best way to check which item is selected.
expect(lastFrame()).toContain('● 2. OpenAI');
@ -297,9 +332,7 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog onSelect={() => {}} settings={settings} />,
);
const { lastFrame } = renderAuthDialog(settings);
// Default is Qwen OAuth (first option)
expect(lastFrame()).toContain('● 1. Qwen OAuth');
@ -341,9 +374,7 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame } = renderWithProviders(
<AuthDialog onSelect={() => {}} settings={settings} />,
);
const { lastFrame } = renderAuthDialog(settings);
// Since the auth dialog doesn't show QWEN_DEFAULT_AUTH_TYPE errors anymore,
// it will just show the default Qwen OAuth option
@ -352,7 +383,7 @@ describe('AuthDialog', () => {
});
it('should prevent exiting when no auth method is selected and show error message', async () => {
const onSelect = vi.fn();
const handleAuthSelect = vi.fn();
const settings: LoadedSettings = new LoadedSettings(
{
settings: { ui: { customThemes: {} }, mcpServers: {} },
@ -386,8 +417,10 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame, stdin, unmount } = renderWithProviders(
<AuthDialog onSelect={onSelect} settings={settings} />,
const { lastFrame, stdin, unmount } = renderAuthDialog(
settings,
{},
{ handleAuthSelect },
);
await wait();
@ -395,16 +428,16 @@ describe('AuthDialog', () => {
stdin.write('\u001b'); // ESC key
await wait();
// Should show error message instead of calling onSelect
// Should show error message instead of calling handleAuthSelect
expect(lastFrame()).toContain(
'You must select an auth method to proceed. Press Ctrl+C again to exit.',
);
expect(onSelect).not.toHaveBeenCalled();
expect(handleAuthSelect).not.toHaveBeenCalled();
unmount();
});
it('should not exit if there is already an error message', async () => {
const onSelect = vi.fn();
const handleAuthSelect = vi.fn();
const settings: LoadedSettings = new LoadedSettings(
{
settings: { ui: { customThemes: {} }, mcpServers: {} },
@ -438,12 +471,10 @@ describe('AuthDialog', () => {
new Set(),
);
const { lastFrame, stdin, unmount } = renderWithProviders(
<AuthDialog
onSelect={onSelect}
settings={settings}
initialErrorMessage="Initial error"
/>,
const { lastFrame, stdin, unmount } = renderAuthDialog(
settings,
{ authError: 'Initial error' },
{ handleAuthSelect },
);
await wait();
@ -453,13 +484,13 @@ describe('AuthDialog', () => {
stdin.write('\u001b'); // ESC key
await wait();
// Should not call onSelect
expect(onSelect).not.toHaveBeenCalled();
// Should not call handleAuthSelect
expect(handleAuthSelect).not.toHaveBeenCalled();
unmount();
});
it('should allow exiting when auth method is already selected', async () => {
const onSelect = vi.fn();
const handleAuthSelect = vi.fn();
const settings: LoadedSettings = new LoadedSettings(
{
settings: { ui: { customThemes: {} }, mcpServers: {} },
@ -493,8 +524,10 @@ describe('AuthDialog', () => {
new Set(),
);
const { stdin, unmount } = renderWithProviders(
<AuthDialog onSelect={onSelect} settings={settings} />,
const { stdin, unmount } = renderAuthDialog(
settings,
{},
{ handleAuthSelect },
);
await wait();
@ -502,8 +535,8 @@ describe('AuthDialog', () => {
stdin.write('\u001b'); // ESC key
await wait();
// Should call onSelect with undefined to exit
expect(onSelect).toHaveBeenCalledWith(undefined, SettingScope.User);
// Should call handleAuthSelect with undefined to exit
expect(handleAuthSelect).toHaveBeenCalledWith(undefined, SettingScope.User);
unmount();
});
});

View file

@ -8,26 +8,13 @@ import type React from 'react';
import { useState } from 'react';
import { AuthType } from '@qwen-code/qwen-code-core';
import { Box, Text } from 'ink';
import { validateAuthMethod } from '../../config/auth.js';
import { type LoadedSettings, SettingScope } from '../../config/settings.js';
import { SettingScope } from '../../config/settings.js';
import { Colors } from '../colors.js';
import { useKeypress } from '../hooks/useKeypress.js';
import { OpenAIKeyPrompt } from '../components/OpenAIKeyPrompt.js';
import { RadioButtonSelect } from '../components/shared/RadioButtonSelect.js';
interface AuthDialogProps {
onSelect: (
authMethod: AuthType | undefined,
scope: SettingScope,
credentials?: {
apiKey?: string;
baseUrl?: string;
model?: string;
},
) => void;
settings: LoadedSettings;
initialErrorMessage?: string | null;
}
import { useUIState } from '../contexts/UIStateContext.js';
import { useUIActions } from '../contexts/UIActionsContext.js';
import { useSettings } from '../contexts/SettingsContext.js';
function parseDefaultAuthType(
defaultAuthType: string | undefined,
@ -41,15 +28,14 @@ function parseDefaultAuthType(
return null;
}
export function AuthDialog({
onSelect,
settings,
initialErrorMessage,
}: AuthDialogProps): React.JSX.Element {
const [errorMessage, setErrorMessage] = useState<string | null>(
initialErrorMessage || null,
);
const [showOpenAIKeyPrompt, setShowOpenAIKeyPrompt] = useState(false);
export function AuthDialog(): React.JSX.Element {
const { pendingAuthType, authError } = useUIState();
const { handleAuthSelect: onAuthSelect } = useUIActions();
const settings = useSettings();
const [errorMessage, setErrorMessage] = useState<string | null>(null);
const [selectedIndex, setSelectedIndex] = useState<number | null>(null);
const items = [
{
key: AuthType.QWEN_OAUTH,
@ -62,10 +48,17 @@ export function AuthDialog({
const initialAuthIndex = Math.max(
0,
items.findIndex((item) => {
// Priority 1: pendingAuthType
if (pendingAuthType) {
return item.value === pendingAuthType;
}
// Priority 2: settings.merged.security?.auth?.selectedType
if (settings.merged.security?.auth?.selectedType) {
return item.value === settings.merged.security?.auth?.selectedType;
}
// Priority 3: QWEN_DEFAULT_AUTH_TYPE env var
const defaultAuthType = parseDefaultAuthType(
process.env['QWEN_DEFAULT_AUTH_TYPE'],
);
@ -73,49 +66,29 @@ export function AuthDialog({
return item.value === defaultAuthType;
}
// Priority 4: default to QWEN_OAUTH
return item.value === AuthType.QWEN_OAUTH;
}),
);
const handleAuthSelect = (authMethod: AuthType) => {
if (authMethod === AuthType.USE_OPENAI) {
setShowOpenAIKeyPrompt(true);
setErrorMessage(null);
} else {
const error = validateAuthMethod(authMethod);
if (error) {
setErrorMessage(error);
} else {
setErrorMessage(null);
onSelect(authMethod, SettingScope.User);
}
}
const hasApiKey = Boolean(settings.merged.security?.auth?.apiKey);
const currentSelectedAuthType =
selectedIndex !== null
? items[selectedIndex]?.value
: items[initialAuthIndex]?.value;
const handleAuthSelect = async (authMethod: AuthType) => {
setErrorMessage(null);
await onAuthSelect(authMethod, SettingScope.User);
};
const handleOpenAIKeySubmit = (
apiKey: string,
baseUrl: string,
model: string,
) => {
setShowOpenAIKeyPrompt(false);
onSelect(AuthType.USE_OPENAI, SettingScope.User, {
apiKey,
baseUrl,
model,
});
};
const handleOpenAIKeyCancel = () => {
setShowOpenAIKeyPrompt(false);
setErrorMessage('OpenAI API key is required to use OpenAI authentication.');
const handleHighlight = (authMethod: AuthType) => {
const index = items.findIndex((item) => item.value === authMethod);
setSelectedIndex(index);
};
useKeypress(
(key) => {
if (showOpenAIKeyPrompt) {
return;
}
if (key.name === 'escape') {
// Prevent exit if there is an error message.
// This means they user is not authenticated yet.
@ -129,33 +102,11 @@ export function AuthDialog({
);
return;
}
onSelect(undefined, SettingScope.User);
onAuthSelect(undefined, SettingScope.User);
}
},
{ isActive: true },
);
const getDefaultOpenAIConfig = () => {
const fromSettings = settings.merged.security?.auth;
const modelSettings = settings.merged.model;
return {
apiKey: fromSettings?.apiKey || process.env['OPENAI_API_KEY'] || '',
baseUrl: fromSettings?.baseUrl || process.env['OPENAI_BASE_URL'] || '',
model: modelSettings?.name || process.env['OPENAI_MODEL'] || '',
};
};
if (showOpenAIKeyPrompt) {
const defaults = getDefaultOpenAIConfig();
return (
<OpenAIKeyPrompt
defaultApiKey={defaults.apiKey}
defaultBaseUrl={defaults.baseUrl}
defaultModel={defaults.model}
onSubmit={handleOpenAIKeySubmit}
onCancel={handleOpenAIKeyCancel}
/>
);
}
return (
<Box
@ -174,16 +125,26 @@ export function AuthDialog({
items={items}
initialIndex={initialAuthIndex}
onSelect={handleAuthSelect}
onHighlight={handleHighlight}
/>
</Box>
{errorMessage && (
{(authError || errorMessage) && (
<Box marginTop={1}>
<Text color={Colors.AccentRed}>{errorMessage}</Text>
<Text color={Colors.AccentRed}>{authError || errorMessage}</Text>
</Box>
)}
<Box marginTop={1}>
<Text color={Colors.AccentPurple}>(Use Enter to Set Auth)</Text>
</Box>
{hasApiKey && currentSelectedAuthType === AuthType.QWEN_OAUTH && (
<Box marginTop={1}>
<Text color={Colors.Gray}>
Note: Your existing API key in settings.json will not be cleared
when using Qwen OAuth. You can switch back to OpenAI authentication
later if needed.
</Text>
</Box>
)}
<Box marginTop={1}>
<Text>Terms of Services and Privacy Notice for Qwen Code</Text>
</Box>

View file

@ -6,27 +6,19 @@
import { useState, useCallback, useEffect } from 'react';
import type { LoadedSettings, SettingScope } from '../../config/settings.js';
import type { AuthType, Config } from '@qwen-code/qwen-code-core';
import type { Config } from '@qwen-code/qwen-code-core';
import {
AuthType,
clearCachedCredentialFile,
getErrorMessage,
logAuth,
AuthEvent,
} from '@qwen-code/qwen-code-core';
import { AuthState } from '../types.js';
import { validateAuthMethod } from '../../config/auth.js';
import { useQwenAuth } from '../hooks/useQwenAuth.js';
import type { OpenAICredentials } from '../components/OpenAIKeyPrompt.js';
export function validateAuthMethodWithSettings(
authType: AuthType,
settings: LoadedSettings,
): string | null {
const enforcedType = settings.merged.security?.auth?.enforcedType;
if (enforcedType && enforcedType !== authType) {
return `Authentication is enforced to be ${enforcedType}, but you are currently using ${authType}.`;
}
if (settings.merged.security?.auth?.useExternal) {
return null;
}
return validateAuthMethod(authType);
}
export type { QwenAuthState } from '../hooks/useQwenAuth.js';
export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
const unAuthenticated =
@ -40,6 +32,14 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
const [isAuthenticating, setIsAuthenticating] = useState(false);
const [isAuthDialogOpen, setIsAuthDialogOpen] = useState(unAuthenticated);
const [pendingAuthType, setPendingAuthType] = useState<AuthType | undefined>(
undefined,
);
const { qwenAuthState, cancelQwenAuth } = useQwenAuth(
pendingAuthType,
isAuthenticating,
);
const onAuthError = useCallback(
(error: string | null) => {
@ -52,90 +52,123 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
[setAuthError, setAuthState],
);
// Authentication flow
useEffect(() => {
const authFlow = async () => {
const authType = settings.merged.security?.auth?.selectedType;
if (isAuthDialogOpen || !authType) {
return;
const handleAuthFailure = useCallback(
(error: unknown) => {
setIsAuthenticating(false);
const errorMessage = `Failed to authenticate. Message: ${getErrorMessage(error)}`;
onAuthError(errorMessage);
// Log authentication failure
if (pendingAuthType) {
const authEvent = new AuthEvent(
pendingAuthType,
'manual',
'error',
errorMessage,
);
logAuth(config, authEvent);
}
},
[onAuthError, pendingAuthType, config],
);
const validationError = validateAuthMethodWithSettings(
authType,
settings,
);
if (validationError) {
onAuthError(validationError);
return;
}
try {
setIsAuthenticating(true);
await config.refreshAuth(authType);
console.log(`Authenticated via "${authType}".`);
setAuthError(null);
setAuthState(AuthState.Authenticated);
} catch (e) {
onAuthError(`Failed to login. Message: ${getErrorMessage(e)}`);
} finally {
setIsAuthenticating(false);
}
};
void authFlow();
}, [isAuthDialogOpen, settings, config, onAuthError]);
// Handle auth selection from dialog
const handleAuthSelect = useCallback(
const handleAuthSuccess = useCallback(
async (
authType: AuthType | undefined,
authType: AuthType,
scope: SettingScope,
credentials?: {
apiKey?: string;
baseUrl?: string;
model?: string;
},
credentials?: OpenAICredentials,
) => {
if (authType) {
await clearCachedCredentialFile();
try {
settings.setValue(scope, 'security.auth.selectedType', authType);
// Save OpenAI credentials if provided
if (credentials) {
// Update Config's internal generationConfig before calling refreshAuth
// This ensures refreshAuth has access to the new credentials
config.updateCredentials({
apiKey: credentials.apiKey,
baseUrl: credentials.baseUrl,
model: credentials.model,
});
// Also set environment variables for compatibility with other parts of the code
if (credentials.apiKey) {
// Only update credentials if not switching to QWEN_OAUTH,
// so that OpenAI credentials are preserved when switching to QWEN_OAUTH.
if (authType !== AuthType.QWEN_OAUTH && credentials) {
if (credentials?.apiKey != null) {
settings.setValue(
scope,
'security.auth.apiKey',
credentials.apiKey,
);
}
if (credentials.baseUrl) {
if (credentials?.baseUrl != null) {
settings.setValue(
scope,
'security.auth.baseUrl',
credentials.baseUrl,
);
}
if (credentials.model) {
if (credentials?.model != null) {
settings.setValue(scope, 'model.name', credentials.model);
}
await clearCachedCredentialFile();
}
settings.setValue(scope, 'security.auth.selectedType', authType);
} catch (error) {
handleAuthFailure(error);
return;
}
setIsAuthDialogOpen(false);
setAuthError(null);
setAuthState(AuthState.Authenticated);
setPendingAuthType(undefined);
setIsAuthDialogOpen(false);
setIsAuthenticating(false);
// Log authentication success
const authEvent = new AuthEvent(authType, 'manual', 'success');
logAuth(config, authEvent);
},
[settings, config],
[settings, handleAuthFailure, config],
);
const performAuth = useCallback(
async (
authType: AuthType,
scope: SettingScope,
credentials?: OpenAICredentials,
) => {
try {
await config.refreshAuth(authType);
handleAuthSuccess(authType, scope, credentials);
} catch (e) {
handleAuthFailure(e);
}
},
[config, handleAuthSuccess, handleAuthFailure],
);
const handleAuthSelect = useCallback(
async (
authType: AuthType | undefined,
scope: SettingScope,
credentials?: OpenAICredentials,
) => {
if (!authType) {
setIsAuthDialogOpen(false);
setAuthError(null);
return;
}
setPendingAuthType(authType);
setAuthError(null);
setIsAuthDialogOpen(false);
setIsAuthenticating(true);
if (authType === AuthType.USE_OPENAI) {
if (credentials) {
config.updateCredentials({
apiKey: credentials.apiKey,
baseUrl: credentials.baseUrl,
model: credentials.model,
});
await performAuth(authType, scope, credentials);
}
return;
}
await performAuth(authType, scope);
},
[config, performAuth],
);
const openAuthDialog = useCallback(() => {
@ -143,8 +176,45 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
}, []);
const cancelAuthentication = useCallback(() => {
if (isAuthenticating && pendingAuthType === AuthType.QWEN_OAUTH) {
cancelQwenAuth();
}
// Log authentication cancellation
if (isAuthenticating && pendingAuthType) {
const authEvent = new AuthEvent(pendingAuthType, 'manual', 'cancelled');
logAuth(config, authEvent);
}
// Do not reset pendingAuthType here, persist the previously selected type.
setIsAuthenticating(false);
}, []);
setIsAuthDialogOpen(true);
setAuthError(null);
}, [isAuthenticating, pendingAuthType, cancelQwenAuth, config]);
/**
/**
* We previously used a useEffect to trigger authentication automatically when
* settings.security.auth.selectedType changed. This caused problems: if authentication failed,
* the UI could get stuck, since settings.json would update before success. Now, we
* update selectedType in settings only when authentication fully succeeds.
* Authentication is triggered explicitlyeither during initial app startup or when the
* user switches methodsnot reactively through settings changes. This avoids repeated
* or broken authentication cycles.
*/
useEffect(() => {
const defaultAuthType = process.env['QWEN_DEFAULT_AUTH_TYPE'];
if (
defaultAuthType &&
![AuthType.QWEN_OAUTH, AuthType.USE_OPENAI].includes(
defaultAuthType as AuthType,
)
) {
onAuthError(
`Invalid QWEN_DEFAULT_AUTH_TYPE value: "${defaultAuthType}". Valid values are: ${[AuthType.QWEN_OAUTH, AuthType.USE_OPENAI].join(', ')}`,
);
}
}, [onAuthError]);
return {
authState,
@ -153,6 +223,8 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => {
onAuthError,
isAuthDialogOpen,
isAuthenticating,
pendingAuthType,
qwenAuthState,
handleAuthSelect,
openAuthDialog,
cancelAuthentication,