From 2238802e97b824c611c07b6e1a4fafb86f0f705f Mon Sep 17 00:00:00 2001 From: Emily Hedlund Date: Fri, 30 Jan 2026 09:57:34 -0500 Subject: [PATCH] feat(core): implement interactive and non-interactive consent for OAuth (#17699) --- packages/cli/src/ui/AppContainer.test.tsx | 53 +++++++++++ packages/cli/src/ui/AppContainer.tsx | 38 ++++++-- .../src/ui/components/DialogManager.test.tsx | 7 +- .../cli/src/ui/components/DialogManager.tsx | 19 +++- .../cli/src/ui/contexts/UIStateContext.tsx | 3 +- packages/core/src/code_assist/oauth2.test.ts | 94 ++++++++++++++++++- packages/core/src/code_assist/oauth2.ts | 52 ++++++++++ packages/core/src/utils/events.test.ts | 55 +++++++++++ packages/core/src/utils/events.ts | 17 ++++ 9 files changed, 326 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index cfa1d658e8..5170b60f62 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -2510,6 +2510,59 @@ describe('AppContainer State Management', () => { expect(capturedUIState.activeHooks).toEqual(mockHooks); unmount!(); }); + + it('handles consent request events', async () => { + let unmount: () => void; + await act(async () => { + const result = renderAppContainer(); + unmount = result.unmount; + }); + await waitFor(() => expect(capturedUIState).toBeTruthy()); + + const handler = mockCoreEvents.on.mock.calls.find( + (call: unknown[]) => call[0] === CoreEvent.ConsentRequest, + )?.[1]; + expect(handler).toBeDefined(); + + const onConfirm = vi.fn(); + const payload = { + prompt: 'Do you consent?', + onConfirm, + }; + + act(() => { + handler(payload); + }); + + expect(capturedUIState.authConsentRequest).toBeDefined(); + expect(capturedUIState.authConsentRequest?.prompt).toBe( + 'Do you consent?', + ); + + act(() => { + capturedUIState.authConsentRequest?.onConfirm(true); + }); + + expect(onConfirm).toHaveBeenCalledWith(true); + expect(capturedUIState.authConsentRequest).toBeNull(); + unmount!(); + }); + + it('unsubscribes from ConsentRequest on unmount', async () => { + let unmount: () => void; + await act(async () => { + const result = renderAppContainer(); + unmount = result.unmount; + }); + await waitFor(() => expect(capturedUIState).toBeTruthy()); + + unmount!(); + + expect(mockCoreEvents.off).toHaveBeenCalledWith( + CoreEvent.ConsentRequest, + expect.any(Function), + ); + }); }); describe('Shell Interaction', () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 42f64321c7..c792094969 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -27,6 +27,7 @@ import { type HistoryItemWithoutId, type HistoryItemToolGroup, AuthState, + type ConfirmationRequest, } from './types.js'; import { MessageType, StreamingState } from './types.js'; import { ToolActionsProvider } from './contexts/ToolActionsContext.js'; @@ -68,6 +69,7 @@ import { SessionStartSource, SessionEndReason, generateSummary, + type ConsentRequestPayload, MessageBusType, type AskUserRequest, type AgentsDiscoveredPayload, @@ -885,7 +887,7 @@ Logging in with Google... Restarting Gemini CLI to continue. slashCommands, pendingHistoryItems: pendingSlashCommandHistoryItems, commandContext, - confirmationRequest, + confirmationRequest: commandConfirmationRequest, } = useSlashCommandProcessor( config, settings, @@ -902,6 +904,26 @@ Logging in with Google... Restarting Gemini CLI to continue. setCustomDialog, ); + const [authConsentRequest, setAuthConsentRequest] = + useState(null); + + useEffect(() => { + const handleConsentRequest = (payload: ConsentRequestPayload) => { + setAuthConsentRequest({ + prompt: payload.prompt, + onConfirm: (confirmed: boolean) => { + setAuthConsentRequest(null); + payload.onConfirm(confirmed); + }, + }); + }; + + coreEvents.on(CoreEvent.ConsentRequest, handleConsentRequest); + return () => { + coreEvents.off(CoreEvent.ConsentRequest, handleConsentRequest); + }; + }, []); + const performMemoryRefresh = useCallback(async () => { historyManager.addItem( { @@ -1586,7 +1608,8 @@ Logging in with Google... Restarting Gemini CLI to continue. const paddedTitle = computeTerminalTitle({ streamingState, thoughtSubject: thought?.subject, - isConfirming: !!confirmationRequest || shouldShowActionRequiredTitle, + isConfirming: + !!commandConfirmationRequest || shouldShowActionRequiredTitle, isSilentWorking: shouldShowSilentWorkingTitle, folderName: basename(config.getTargetDir()), showThoughts: !!settings.merged.ui.showStatusInTitle, @@ -1602,7 +1625,7 @@ Logging in with Google... Restarting Gemini CLI to continue. }, [ streamingState, thought, - confirmationRequest, + commandConfirmationRequest, shouldShowActionRequiredTitle, shouldShowSilentWorkingTitle, settings.merged.ui.showStatusInTitle, @@ -1682,7 +1705,8 @@ Logging in with Google... Restarting Gemini CLI to continue. shouldShowIdePrompt || isFolderTrustDialogOpen || adminSettingsChanged || - !!confirmationRequest || + !!commandConfirmationRequest || + !!authConsentRequest || !!customDialog || confirmUpdateExtensionRequests.length > 0 || !!loopDetectionConfirmationRequest || @@ -1792,7 +1816,8 @@ Logging in with Google... Restarting Gemini CLI to continue. slashCommands, pendingSlashCommandHistoryItems, commandContext, - confirmationRequest, + commandConfirmationRequest, + authConsentRequest, confirmUpdateExtensionRequests, loopDetectionConfirmationRequest, geminiMdFileCount, @@ -1890,7 +1915,8 @@ Logging in with Google... Restarting Gemini CLI to continue. slashCommands, pendingSlashCommandHistoryItems, commandContext, - confirmationRequest, + commandConfirmationRequest, + authConsentRequest, confirmUpdateExtensionRequests, loopDetectionConfirmationRequest, geminiMdFileCount, diff --git a/packages/cli/src/ui/components/DialogManager.test.tsx b/packages/cli/src/ui/components/DialogManager.test.tsx index 85cc050b21..78e292e344 100644 --- a/packages/cli/src/ui/components/DialogManager.test.tsx +++ b/packages/cli/src/ui/components/DialogManager.test.tsx @@ -80,6 +80,7 @@ describe('DialogManager', () => { isFolderTrustDialogOpen: false, loopDetectionConfirmationRequest: null, confirmationRequest: null, + consentRequest: null, isThemeDialogOpen: false, isSettingsDialogOpen: false, isModelDialogOpen: false, @@ -137,7 +138,11 @@ describe('DialogManager', () => { 'LoopDetectionConfirmation', ], [ - { confirmationRequest: { prompt: 'foo', onConfirm: vi.fn() } }, + { commandConfirmationRequest: { prompt: 'foo', onConfirm: vi.fn() } }, + 'ConsentPrompt', + ], + [ + { authConsentRequest: { prompt: 'bar', onConfirm: vi.fn() } }, 'ConsentPrompt', ], [ diff --git a/packages/cli/src/ui/components/DialogManager.tsx b/packages/cli/src/ui/components/DialogManager.tsx index b1a159c93e..527c2de983 100644 --- a/packages/cli/src/ui/components/DialogManager.tsx +++ b/packages/cli/src/ui/components/DialogManager.tsx @@ -134,11 +134,24 @@ export const DialogManager = ({ /> ); } - if (uiState.confirmationRequest) { + + // commandConfirmationRequest and authConsentRequest are kept separate + // to avoid focus deadlocks and state race conditions between the + // synchronous command loop and the asynchronous auth flow. + if (uiState.commandConfirmationRequest) { return ( + ); + } + if (uiState.authConsentRequest) { + return ( + ); diff --git a/packages/cli/src/ui/contexts/UIStateContext.tsx b/packages/cli/src/ui/contexts/UIStateContext.tsx index 6d10d76bda..f613963e4b 100644 --- a/packages/cli/src/ui/contexts/UIStateContext.tsx +++ b/packages/cli/src/ui/contexts/UIStateContext.tsx @@ -80,7 +80,8 @@ export interface UIState { slashCommands: readonly SlashCommand[] | undefined; pendingSlashCommandHistoryItems: HistoryItemWithoutId[]; commandContext: CommandContext; - confirmationRequest: ConfirmationRequest | null; + commandConfirmationRequest: ConfirmationRequest | null; + authConsentRequest: ConfirmationRequest | null; confirmUpdateExtensionRequests: ConfirmationRequest[]; loopDetectionConfirmationRequest: LoopDetectionConfirmationRequest | null; geminiMdFileCount: number; diff --git a/packages/core/src/code_assist/oauth2.test.ts b/packages/core/src/code_assist/oauth2.test.ts index c838166cc2..1ef5fc2f06 100644 --- a/packages/core/src/code_assist/oauth2.test.ts +++ b/packages/core/src/code_assist/oauth2.test.ts @@ -9,6 +9,7 @@ import type { Mock } from 'vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { getOauthClient, + getConsentForOauth, resetOauthClientForTesting, clearCachedCredentialFile, clearOauthClientCache, @@ -29,8 +30,12 @@ import { FORCE_ENCRYPTED_FILE_ENV_VAR } from '../mcp/token-storage/index.js'; import { GEMINI_DIR, homedir as pathsHomedir } from '../utils/paths.js'; import { debugLogger } from '../utils/debugLogger.js'; import { writeToStdout } from '../utils/stdio.js'; -import { FatalCancellationError } from '../utils/errors.js'; +import { + FatalAuthenticationError, + FatalCancellationError, +} from '../utils/errors.js'; import process from 'node:process'; +import { coreEvents } from '../utils/events.js'; vi.mock('node:os', async (importOriginal) => { const actual = await importOriginal(); @@ -88,6 +93,13 @@ const mockConfig = { global.fetch = vi.fn(); describe('oauth2', () => { + beforeEach(() => { + vi.spyOn(coreEvents, 'listenerCount').mockReturnValue(1); + vi.spyOn(coreEvents, 'emitConsentRequest').mockImplementation((payload) => { + payload.onConfirm(true); + }); + }); + describe('with encrypted flag false', () => { let tempHomeDir: string; @@ -1503,4 +1515,84 @@ describe('oauth2', () => { expect(fs.existsSync(credsPath)).toBe(true); // The unencrypted file should remain }); }); + + describe('getConsentForOauth', () => { + it('should use coreEvents when listeners are present', async () => { + vi.restoreAllMocks(); + const mockEmitConsentRequest = vi.spyOn(coreEvents, 'emitConsentRequest'); + const mockListenerCount = vi + .spyOn(coreEvents, 'listenerCount') + .mockReturnValue(1); + + mockEmitConsentRequest.mockImplementation((payload) => { + payload.onConfirm(true); + }); + + const result = await getConsentForOauth(); + + expect(result).toBe(true); + expect(mockEmitConsentRequest).toHaveBeenCalled(); + + mockListenerCount.mockRestore(); + mockEmitConsentRequest.mockRestore(); + }); + + it('should use readline when no listeners are present and stdin is a TTY', async () => { + vi.restoreAllMocks(); + const mockListenerCount = vi + .spyOn(coreEvents, 'listenerCount') + .mockReturnValue(0); + const originalIsTTY = process.stdin.isTTY; + Object.defineProperty(process.stdin, 'isTTY', { + value: true, + configurable: true, + }); + + const mockReadline = { + on: vi.fn((event, callback) => { + if (event === 'line') { + callback('y'); + } + }), + close: vi.fn(), + }; + (readline.createInterface as Mock).mockReturnValue(mockReadline); + + const result = await getConsentForOauth(); + + expect(result).toBe(true); + expect(readline.createInterface).toHaveBeenCalled(); + expect(writeToStdout).toHaveBeenCalledWith( + expect.stringContaining('Do you want to continue? [Y/n]: '), + ); + + mockListenerCount.mockRestore(); + Object.defineProperty(process.stdin, 'isTTY', { + value: originalIsTTY, + configurable: true, + }); + }); + + it('should throw FatalAuthenticationError when no listeners and not a TTY', async () => { + vi.restoreAllMocks(); + const mockListenerCount = vi + .spyOn(coreEvents, 'listenerCount') + .mockReturnValue(0); + const originalIsTTY = process.stdin.isTTY; + Object.defineProperty(process.stdin, 'isTTY', { + value: false, + configurable: true, + }); + + await expect(getConsentForOauth()).rejects.toThrow( + FatalAuthenticationError, + ); + + mockListenerCount.mockRestore(); + Object.defineProperty(process.stdin, 'isTTY', { + value: originalIsTTY, + configurable: true, + }); + }); + }); }); diff --git a/packages/core/src/code_assist/oauth2.ts b/packages/core/src/code_assist/oauth2.ts index a2357c9672..a0bd86c174 100644 --- a/packages/core/src/code_assist/oauth2.ts +++ b/packages/core/src/code_assist/oauth2.ts @@ -269,6 +269,11 @@ async function initOauthClient( await triggerPostAuthCallbacks(client.credentials); } else { + const userConsent = await getConsentForOauth(); + if (!userConsent) { + throw new FatalCancellationError('Authentication cancelled by user.'); + } + const webLogin = await authWithWeb(client); coreEvents.emit(CoreEvent.UserFeedback, { @@ -372,6 +377,53 @@ async function initOauthClient( return client; } +export async function getConsentForOauth(): Promise { + const prompt = + 'Code Assist login required. Opening authentication page in your browser. '; + + if (coreEvents.listenerCount(CoreEvent.ConsentRequest) === 0) { + if (!process.stdin.isTTY) { + throw new FatalAuthenticationError( + 'Code Assist login required, but interactive consent could not be obtained.\n' + + 'Please run Gemini CLI in an interactive terminal to authenticate, or use NO_BROWSER=true for manual authentication.', + ); + } + return getOauthConsentNonInteractive(prompt); + } + + return getOauthConsentInteractive(prompt); +} + +async function getOauthConsentNonInteractive(prompt: string) { + const rl = readline.createInterface({ + input: process.stdin, + output: createWorkingStdio().stdout, + terminal: true, + }); + + const fullPrompt = prompt + 'Do you want to continue? [Y/n]: '; + writeToStdout(`\n${fullPrompt}`); + + return new Promise((resolve) => { + rl.on('line', (answer) => { + rl.close(); + resolve(['y', ''].includes(answer.trim().toLowerCase())); + }); + }); +} + +async function getOauthConsentInteractive(prompt: string) { + const fullPrompt = prompt + '\n\nDo you want to continue?'; + return new Promise((resolve) => { + coreEvents.emitConsentRequest({ + prompt: fullPrompt, + onConfirm: (confirmed: boolean) => { + resolve(confirmed); + }, + }); + }); +} + export async function getOauthClient( authType: AuthType, config: Config, diff --git a/packages/core/src/utils/events.test.ts b/packages/core/src/utils/events.test.ts index 5b84af0628..ad12e79015 100644 --- a/packages/core/src/utils/events.test.ts +++ b/packages/core/src/utils/events.test.ts @@ -305,4 +305,59 @@ describe('CoreEventEmitter', () => { expect(listener).toHaveBeenCalledWith(payload); }); }); + + describe('ConsentRequest Event', () => { + it('should emit consent request immediately when a listener is present', () => { + const listener = vi.fn(); + events.on(CoreEvent.ConsentRequest, listener); + + const payload = { + prompt: 'Do you consent?', + onConfirm: vi.fn(), + }; + + events.emitConsentRequest(payload); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(payload); + }); + + it('should buffer consent requests when no listener is present', () => { + const listener = vi.fn(); + const payload = { + prompt: 'Buffered consent?', + onConfirm: vi.fn(), + }; + + // Emit while no listeners attached + events.emitConsentRequest(payload); + expect(listener).not.toHaveBeenCalled(); + + // Attach listener and drain + events.on(CoreEvent.ConsentRequest, listener); + events.drainBacklogs(); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(payload); + }); + + it('should respect the backlog size limit for consent requests', () => { + const listener = vi.fn(); + const MAX_BACKLOG_SIZE = 10000; + + for (let i = 0; i < MAX_BACKLOG_SIZE + 10; i++) { + events.emitConsentRequest({ + prompt: `Consent ${i}`, + onConfirm: vi.fn(), + }); + } + + events.on(CoreEvent.ConsentRequest, listener); + events.drainBacklogs(); + + expect(listener).toHaveBeenCalledTimes(MAX_BACKLOG_SIZE); + // Verify strictly that the FIRST call was Consent 10 (0-9 dropped) + expect(listener.mock.calls[0][0]).toMatchObject({ prompt: 'Consent 10' }); + }); + }); }); diff --git a/packages/core/src/utils/events.ts b/packages/core/src/utils/events.ts index d5f8f715aa..cea80952f9 100644 --- a/packages/core/src/utils/events.ts +++ b/packages/core/src/utils/events.ts @@ -111,6 +111,14 @@ export interface RetryAttemptPayload { model: string; } +/** + * Payload for the 'consent-request' event. + */ +export interface ConsentRequestPayload { + prompt: string; + onConfirm: (confirmed: boolean) => void; +} + /** * Payload for the 'agents-discovered' event. */ @@ -133,6 +141,7 @@ export enum CoreEvent { AgentsRefreshed = 'agents-refreshed', AdminSettingsChanged = 'admin-settings-changed', RetryAttempt = 'retry-attempt', + ConsentRequest = 'consent-request', AgentsDiscovered = 'agents-discovered', } @@ -151,6 +160,7 @@ export interface CoreEvents extends ExtensionEvents { [CoreEvent.AgentsRefreshed]: never[]; [CoreEvent.AdminSettingsChanged]: never[]; [CoreEvent.RetryAttempt]: [RetryAttemptPayload]; + [CoreEvent.ConsentRequest]: [ConsentRequestPayload]; [CoreEvent.AgentsDiscovered]: [AgentsDiscoveredPayload]; } @@ -274,6 +284,13 @@ export class CoreEventEmitter extends EventEmitter { this.emit(CoreEvent.RetryAttempt, payload); } + /** + * Requests consent from the user via the UI. + */ + emitConsentRequest(payload: ConsentRequestPayload): void { + this._emitOrQueue(CoreEvent.ConsentRequest, payload); + } + /** * Notifies subscribers that new unacknowledged agents have been discovered. */