Add a setting to disable auth mode validation on startup (#5358)

This commit is contained in:
Billy Biggs 2025-08-01 11:49:03 -07:00 committed by GitHub
parent c725e258c6
commit 24c5a15d7a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 143 additions and 18 deletions

View File

@ -60,6 +60,7 @@ export interface Settings {
theme?: string;
customThemes?: Record<string, CustomTheme>;
selectedAuthType?: AuthType;
useExternalAuth?: boolean;
sandbox?: boolean | string;
coreTools?: string[];
excludeTools?: string[];

View File

@ -186,7 +186,10 @@ export async function main() {
: [];
const sandboxConfig = config.getSandbox();
if (sandboxConfig) {
if (settings.merged.selectedAuthType) {
if (
settings.merged.selectedAuthType &&
!settings.merged.useExternalAuth
) {
// Validate authentication here because the sandbox will interfere with the Oauth2 web redirect.
try {
const err = validateAuthMethod(settings.merged.selectedAuthType);
@ -344,6 +347,7 @@ async function loadNonInteractiveConfig(
return await validateNonInteractiveAuth(
settings.merged.selectedAuthType,
settings.merged.useExternalAuth,
finalConfig,
);
}

View File

@ -26,6 +26,7 @@ import { Tips } from './components/Tips.js';
import { checkForUpdates, UpdateObject } from './utils/updateCheck.js';
import { EventEmitter } from 'events';
import { updateEventEmitter } from '../utils/updateEventEmitter.js';
import * as auth from '../config/auth.js';
// Define a more complete mock server config based on actual Config
interface MockServerConfig {
@ -232,6 +233,10 @@ vi.mock('./utils/updateCheck.js', () => ({
checkForUpdates: vi.fn(),
}));
vi.mock('./config/auth.js', () => ({
validateAuthMethod: vi.fn(),
}));
const mockedCheckForUpdates = vi.mocked(checkForUpdates);
const { isGitRepository: mockedIsGitRepository } = vi.mocked(
await import('@google/gemini-cli-core'),
@ -1005,4 +1010,50 @@ describe('App UI', () => {
expect(lastFrame()).toContain('5 errors');
});
});
describe('auth validation', () => {
it('should call validateAuthMethod when useExternalAuth is false', async () => {
const validateAuthMethodSpy = vi.spyOn(auth, 'validateAuthMethod');
mockSettings = createMockSettings({
workspace: {
selectedAuthType: 'USE_GEMINI' as AuthType,
useExternalAuth: false,
theme: 'Default',
},
});
const { unmount } = render(
<App
config={mockConfig as unknown as ServerConfig}
settings={mockSettings}
version={mockVersion}
/>,
);
currentUnmount = unmount;
expect(validateAuthMethodSpy).toHaveBeenCalledWith('USE_GEMINI');
});
it('should NOT call validateAuthMethod when useExternalAuth is true', async () => {
const validateAuthMethodSpy = vi.spyOn(auth, 'validateAuthMethod');
mockSettings = createMockSettings({
workspace: {
selectedAuthType: 'USE_GEMINI' as AuthType,
useExternalAuth: true,
theme: 'Default',
},
});
const { unmount } = render(
<App
config={mockConfig as unknown as ServerConfig}
settings={mockSettings}
version={mockVersion}
/>,
);
currentUnmount = unmount;
expect(validateAuthMethodSpy).not.toHaveBeenCalled();
});
});
});

View File

@ -234,14 +234,19 @@ const App = ({ config, settings, startupWarnings = [], version }: AppProps) => {
} = useAuthCommand(settings, setAuthError, config);
useEffect(() => {
if (settings.merged.selectedAuthType) {
if (settings.merged.selectedAuthType && !settings.merged.useExternalAuth) {
const error = validateAuthMethod(settings.merged.selectedAuthType);
if (error) {
setAuthError(error);
openAuthDialog();
}
}
}, [settings.merged.selectedAuthType, openAuthDialog, setAuthError]);
}, [
settings.merged.selectedAuthType,
settings.merged.useExternalAuth,
openAuthDialog,
setAuthError,
]);
// Sync user tier from config when authentication changes
useEffect(() => {

View File

@ -10,6 +10,7 @@ import {
NonInteractiveConfig,
} from './validateNonInterActiveAuth.js';
import { AuthType } from '@google/gemini-cli-core';
import * as auth from './config/auth.js';
describe('validateNonInterActiveAuth', () => {
let originalEnvGeminiApiKey: string | undefined;
@ -59,7 +60,11 @@ describe('validateNonInterActiveAuth', () => {
refreshAuth: refreshAuthMock,
};
try {
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect.fail('Should have exited');
} catch (e) {
expect((e as Error).message).toContain('process.exit(1) called');
@ -75,7 +80,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.LOGIN_WITH_GOOGLE);
});
@ -84,7 +93,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.USE_GEMINI);
});
@ -95,7 +108,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.USE_VERTEX_AI);
});
@ -105,7 +122,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.USE_VERTEX_AI);
});
@ -118,7 +139,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.LOGIN_WITH_GOOGLE);
});
@ -130,7 +155,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.USE_VERTEX_AI);
});
@ -142,7 +171,11 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(undefined, nonInteractiveConfig);
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.USE_GEMINI);
});
@ -152,20 +185,24 @@ describe('validateNonInterActiveAuth', () => {
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
await validateNonInteractiveAuth(AuthType.USE_GEMINI, nonInteractiveConfig);
await validateNonInteractiveAuth(
AuthType.USE_GEMINI,
undefined,
nonInteractiveConfig,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.USE_GEMINI);
});
it('exits if validateAuthMethod returns error', async () => {
// Mock validateAuthMethod to return error
const mod = await import('./config/auth.js');
vi.spyOn(mod, 'validateAuthMethod').mockReturnValue('Auth error!');
vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!');
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
try {
await validateNonInteractiveAuth(
AuthType.USE_GEMINI,
undefined,
nonInteractiveConfig,
);
expect.fail('Should have exited');
@ -175,4 +212,28 @@ describe('validateNonInterActiveAuth', () => {
expect(consoleErrorSpy).toHaveBeenCalledWith('Auth error!');
expect(processExitSpy).toHaveBeenCalledWith(1);
});
it('skips validation if useExternalAuth is true', async () => {
// Mock validateAuthMethod to return error to ensure it's not being called
const validateAuthMethodSpy = vi
.spyOn(auth, 'validateAuthMethod')
.mockReturnValue('Auth error!');
const nonInteractiveConfig: NonInteractiveConfig = {
refreshAuth: refreshAuthMock,
};
// Even with an invalid auth type, it should not exit
// because validation is skipped.
await validateNonInteractiveAuth(
'invalid-auth-type' as AuthType,
true, // useExternalAuth = true
nonInteractiveConfig,
);
expect(validateAuthMethodSpy).not.toHaveBeenCalled();
expect(consoleErrorSpy).not.toHaveBeenCalled();
expect(processExitSpy).not.toHaveBeenCalled();
// We still expect refreshAuth to be called with the (invalid) type
expect(refreshAuthMock).toHaveBeenCalledWith('invalid-auth-type');
});
});

View File

@ -23,6 +23,7 @@ function getAuthTypeFromEnv(): AuthType | undefined {
export async function validateNonInteractiveAuth(
configuredAuthType: AuthType | undefined,
useExternalAuth: boolean | undefined,
nonInteractiveConfig: Config,
) {
const effectiveAuthType = configuredAuthType || getAuthTypeFromEnv();
@ -34,10 +35,12 @@ export async function validateNonInteractiveAuth(
process.exit(1);
}
const err = validateAuthMethod(effectiveAuthType);
if (err != null) {
console.error(err);
process.exit(1);
if (!useExternalAuth) {
const err = validateAuthMethod(effectiveAuthType);
if (err != null) {
console.error(err);
process.exit(1);
}
}
await nonInteractiveConfig.refreshAuth(effectiveAuthType);