Skip to content

Commit 8d65d08

Browse files
ehedlundehedlund
committed
fix(core): restore auth consent in headless mode and add unit tests
1 parent 27b7fc0 commit 8d65d08

File tree

2 files changed

+110
-68
lines changed

2 files changed

+110
-68
lines changed

‎packages/core/src/utils/authConsent.test.ts‎

Lines changed: 101 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import { describe, it, expect, vi } from 'vitest';
7+
import { describe, it, expect, vi, beforeEach } from 'vitest';
88
import type { Mock } from 'vitest';
99
import readline from 'node:readline';
1010
import process from 'node:process';
@@ -27,73 +27,116 @@ vi.mock('./stdio.js', () => ({
2727
}));
2828

2929
describe('getConsentForOauth', () => {
30-
it('should use coreEvents when listeners are present', async () => {
30+
beforeEach(() => {
3131
vi.restoreAllMocks();
32-
const mockEmitConsentRequest = vi.spyOn(coreEvents, 'emitConsentRequest');
33-
const mockListenerCount = vi
34-
.spyOn(coreEvents, 'listenerCount')
35-
.mockReturnValue(1);
32+
});
3633

37-
mockEmitConsentRequest.mockImplementation((payload) => {
38-
payload.onConfirm(true);
34+
describe('in interactive mode', () => {
35+
beforeEach(() => {
36+
(isHeadlessMode as Mock).mockReturnValue(false);
3937
});
4038

41-
const result = await getConsentForOauth('Login required.');
39+
it('should emit consent request when UI listeners are present', async () => {
40+
const mockEmitConsentRequest = vi.spyOn(coreEvents, 'emitConsentRequest');
41+
vi.spyOn(coreEvents, 'listenerCount').mockReturnValue(1);
4242

43-
expect(result).toBe(true);
44-
expect(mockEmitConsentRequest).toHaveBeenCalledWith(
45-
expect.objectContaining({
46-
prompt: expect.stringContaining(
47-
'Login required. Opening authentication page in your browser.',
48-
),
49-
}),
50-
);
43+
mockEmitConsentRequest.mockImplementation((payload) => {
44+
payload.onConfirm(true);
45+
});
5146

52-
mockListenerCount.mockRestore();
53-
mockEmitConsentRequest.mockRestore();
54-
});
47+
const result = await getConsentForOauth('Login required.');
5548

56-
it('should use readline when no listeners are present and not headless', async () => {
57-
vi.restoreAllMocks();
58-
const mockListenerCount = vi
59-
.spyOn(coreEvents, 'listenerCount')
60-
.mockReturnValue(0);
61-
(isHeadlessMode as Mock).mockReturnValue(false);
62-
63-
const mockReadline = {
64-
on: vi.fn((event, callback) => {
65-
if (event === 'line') {
66-
callback('y');
67-
}
68-
}),
69-
close: vi.fn(),
70-
};
71-
(readline.createInterface as Mock).mockReturnValue(mockReadline);
72-
73-
const result = await getConsentForOauth('Login required.');
74-
75-
expect(result).toBe(true);
76-
expect(readline.createInterface).toHaveBeenCalled();
77-
expect(writeToStdout).toHaveBeenCalledWith(
78-
expect.stringContaining(
79-
'Login required. Opening authentication page in your browser.',
80-
),
81-
);
82-
83-
mockListenerCount.mockRestore();
49+
expect(result).toBe(true);
50+
expect(mockEmitConsentRequest).toHaveBeenCalledWith(
51+
expect.objectContaining({
52+
prompt: expect.stringContaining(
53+
'Login required. Opening authentication page in your browser.',
54+
),
55+
}),
56+
);
57+
});
58+
59+
it('should return false when user declines via UI', async () => {
60+
const mockEmitConsentRequest = vi.spyOn(coreEvents, 'emitConsentRequest');
61+
vi.spyOn(coreEvents, 'listenerCount').mockReturnValue(1);
62+
63+
mockEmitConsentRequest.mockImplementation((payload) => {
64+
payload.onConfirm(false);
65+
});
66+
67+
const result = await getConsentForOauth('Login required.');
68+
69+
expect(result).toBe(false);
70+
});
71+
72+
it('should throw FatalAuthenticationError when no UI listeners are present', async () => {
73+
vi.spyOn(coreEvents, 'listenerCount').mockReturnValue(0);
74+
75+
await expect(getConsentForOauth('Login required.')).rejects.toThrow(
76+
FatalAuthenticationError,
77+
);
78+
});
8479
});
8580

86-
it('should throw FatalAuthenticationError when no listeners and headless', async () => {
87-
vi.restoreAllMocks();
88-
const mockListenerCount = vi
89-
.spyOn(coreEvents, 'listenerCount')
90-
.mockReturnValue(0);
91-
(isHeadlessMode as Mock).mockReturnValue(true);
81+
describe('in non-interactive mode', () => {
82+
beforeEach(() => {
83+
(isHeadlessMode as Mock).mockReturnValue(true);
84+
});
85+
86+
it('should use readline to prompt for consent', async () => {
87+
const mockReadline = {
88+
on: vi.fn((event, callback) => {
89+
if (event === 'line') {
90+
callback('y');
91+
}
92+
}),
93+
close: vi.fn(),
94+
};
95+
(readline.createInterface as Mock).mockReturnValue(mockReadline);
96+
97+
const result = await getConsentForOauth('Login required.');
98+
99+
expect(result).toBe(true);
100+
expect(readline.createInterface).toHaveBeenCalledWith(
101+
expect.objectContaining({
102+
terminal: true,
103+
}),
104+
);
105+
expect(writeToStdout).toHaveBeenCalledWith(
106+
expect.stringContaining('Login required.'),
107+
);
108+
});
92109

93-
await expect(getConsentForOauth('Login required.')).rejects.toThrow(
94-
FatalAuthenticationError,
95-
);
110+
it('should accept empty response as "yes"', async () => {
111+
const mockReadline = {
112+
on: vi.fn((event, callback) => {
113+
if (event === 'line') {
114+
callback('');
115+
}
116+
}),
117+
close: vi.fn(),
118+
};
119+
(readline.createInterface as Mock).mockReturnValue(mockReadline);
96120

97-
mockListenerCount.mockRestore();
121+
const result = await getConsentForOauth('Login required.');
122+
123+
expect(result).toBe(true);
124+
});
125+
126+
it('should return false when user declines via readline', async () => {
127+
const mockReadline = {
128+
on: vi.fn((event, callback) => {
129+
if (event === 'line') {
130+
callback('n');
131+
}
132+
}),
133+
close: vi.fn(),
134+
};
135+
(readline.createInterface as Mock).mockReturnValue(mockReadline);
136+
137+
const result = await getConsentForOauth('Login required.');
138+
139+
expect(result).toBe(false);
140+
});
98141
});
99142
});

‎packages/core/src/utils/authConsent.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,21 @@ import { isHeadlessMode } from './headless.js';
1212

1313
/**
1414
* Requests consent from the user for OAuth login.
15-
* Handles both TTY and non-TTY environments.
15+
* Handles both interactive and non-interactive (headless) modes.
1616
*/
1717
export async function getConsentForOauth(prompt: string): Promise<boolean> {
1818
const finalPrompt = prompt + ' Opening authentication page in your browser. ';
1919

20-
if (coreEvents.listenerCount(CoreEvent.ConsentRequest) === 0) {
21-
if (isHeadlessMode()) {
22-
throw new FatalAuthenticationError(
23-
'Interactive consent could not be obtained.\n' +
24-
'Please run Gemini CLI in an interactive terminal to authenticate, or use NO_BROWSER=true for manual authentication.',
25-
);
26-
}
20+
if (isHeadlessMode()) {
2721
return getOauthConsentNonInteractive(finalPrompt);
22+
} else if (coreEvents.listenerCount(CoreEvent.ConsentRequest) > 0) {
23+
return getOauthConsentInteractive(finalPrompt);
2824
}
29-
30-
return getOauthConsentInteractive(finalPrompt);
25+
throw new FatalAuthenticationError(
26+
'Authentication consent could not be obtained.\n' +
27+
'Please run Gemini CLI in an interactive terminal to authenticate, ' +
28+
'or use NO_BROWSER=true for manual authentication.',
29+
);
3130
}
3231

3332
async function getOauthConsentNonInteractive(prompt: string) {

0 commit comments

Comments
 (0)