Fixed Google User Id pass to Clearcut (#3147)

This commit is contained in:
Bryan Morgan 2025-07-03 16:54:35 -04:00 committed by GitHub
parent ab63a5f183
commit 654f8aeb61
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 110 additions and 56 deletions

View File

@ -58,11 +58,30 @@ describe('oauth2', () => {
const mockGetAccessToken = vi const mockGetAccessToken = vi
.fn() .fn()
.mockResolvedValue({ token: 'mock-access-token' }); .mockResolvedValue({ token: 'mock-access-token' });
const mockRefreshAccessToken = vi.fn().mockImplementation((callback) => {
// Mock the callback-style refreshAccessToken method
const mockTokensWithIdToken = {
access_token: 'test-access-token',
refresh_token: 'test-refresh-token',
id_token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0LWdvb2dsZS1hY2NvdW50LWlkLTEyMyJ9.signature', // Mock JWT with sub: test-google-account-id-123
};
callback(null, mockTokensWithIdToken);
});
const mockVerifyIdToken = vi.fn().mockResolvedValue({
getPayload: () => ({
sub: 'test-google-account-id-123',
aud: 'test-audience',
iss: 'https://accounts.google.com',
}),
});
const mockOAuth2Client = { const mockOAuth2Client = {
generateAuthUrl: mockGenerateAuthUrl, generateAuthUrl: mockGenerateAuthUrl,
getToken: mockGetToken, getToken: mockGetToken,
setCredentials: mockSetCredentials, setCredentials: mockSetCredentials,
getAccessToken: mockGetAccessToken, getAccessToken: mockGetAccessToken,
refreshAccessToken: mockRefreshAccessToken,
verifyIdToken: mockVerifyIdToken,
credentials: mockTokens, credentials: mockTokens,
on: vi.fn(), on: vi.fn(),
} as unknown as OAuth2Client; } as unknown as OAuth2Client;

View File

@ -11,7 +11,7 @@ import crypto from 'crypto';
import * as net from 'net'; import * as net from 'net';
import open from 'open'; import open from 'open';
import path from 'node:path'; import path from 'node:path';
import { promises as fs } from 'node:fs'; import { promises as fs, existsSync, readFileSync } from 'node:fs';
import * as os from 'os'; import * as os from 'os';
// OAuth Client ID used to initiate OAuth2Client class. // OAuth Client ID used to initiate OAuth2Client class.
@ -67,7 +67,7 @@ export async function getOauthClient(): Promise<OAuth2Client> {
// Check if we need to retrieve Google Account ID // Check if we need to retrieve Google Account ID
if (!getCachedGoogleAccountId()) { if (!getCachedGoogleAccountId()) {
try { try {
const googleAccountId = await getGoogleAccountId(client); const googleAccountId = await getRawGoogleAccountId(client);
if (googleAccountId) { if (googleAccountId) {
await cacheGoogleAccountId(googleAccountId); await cacheGoogleAccountId(googleAccountId);
} }
@ -135,7 +135,7 @@ async function authWithWeb(client: OAuth2Client): Promise<OauthWebLogin> {
client.setCredentials(tokens); client.setCredentials(tokens);
// Retrieve and cache Google Account ID during authentication // Retrieve and cache Google Account ID during authentication
try { try {
const googleAccountId = await getGoogleAccountId(client); const googleAccountId = await getRawGoogleAccountId(client);
if (googleAccountId) { if (googleAccountId) {
await cacheGoogleAccountId(googleAccountId); await cacheGoogleAccountId(googleAccountId);
} }
@ -237,13 +237,12 @@ async function cacheGoogleAccountId(googleAccountId: string): Promise<void> {
export function getCachedGoogleAccountId(): string | null { export function getCachedGoogleAccountId(): string | null {
try { try {
const filePath = getGoogleAccountIdCachePath(); const filePath = getGoogleAccountIdCachePath();
// eslint-disable-next-line @typescript-eslint/no-require-imports, no-restricted-syntax if (existsSync(filePath)) {
const fs_sync = require('fs'); return readFileSync(filePath, 'utf-8').trim() || null;
if (fs_sync.existsSync(filePath)) {
return fs_sync.readFileSync(filePath, 'utf-8').trim() || null;
} }
return null; return null;
} catch (_error) { } catch (error) {
console.debug('Error reading cached Google Account ID:', error);
return null; return null;
} }
} }
@ -263,37 +262,42 @@ export async function clearCachedCredentialFile() {
* @param client - The authenticated OAuth2Client * @param client - The authenticated OAuth2Client
* @returns The user's Google Account ID or null if not available * @returns The user's Google Account ID or null if not available
*/ */
export async function getGoogleAccountId( export async function getRawGoogleAccountId(
client: OAuth2Client, client: OAuth2Client,
): Promise<string | null> { ): Promise<string | null> {
try { try {
const { token } = await client.getAccessToken(); // 1. Get a new Access Token including the id_token
if (!token) { const refreshedTokens = await new Promise<Credentials | null>(
return null; (resolve, reject) => {
} client.refreshAccessToken((err, tokens) => {
if (err) {
const response = await fetch( return reject(err);
'https://www.googleapis.com/oauth2/v2/userinfo', }
{ resolve(tokens ?? null);
headers: { });
Authorization: `Bearer ${token}`,
},
}, },
); );
if (!response.ok) { if (!refreshedTokens?.id_token) {
console.error( console.warn('No id_token obtained after refreshing tokens.');
'Failed to fetch user info:',
response.status,
response.statusText,
);
return null; return null;
} }
const userInfo = await response.json(); // 2. Verify the ID token to securely get the user's Google Account ID.
return userInfo.id || null; const ticket = await client.verifyIdToken({
idToken: refreshedTokens.id_token,
audience: OAUTH_CLIENT_ID,
});
const payload = ticket.getPayload();
if (!payload?.sub) {
console.warn('Could not extract sub claim from verified ID token.');
return null;
}
return payload.sub;
} catch (error) { } catch (error) {
console.error('Error retrieving Google Account ID:', error); console.error('Error retrieving or verifying Google Account ID:', error);
return null; return null;
} }
} }

View File

@ -18,7 +18,7 @@ import {
import { EventMetadataKey } from './event-metadata-key.js'; import { EventMetadataKey } from './event-metadata-key.js';
import { Config } from '../../config/config.js'; import { Config } from '../../config/config.js';
import { getInstallationId } from '../../utils/user_id.js'; import { getInstallationId } from '../../utils/user_id.js';
import { getObfuscatedGoogleAccountId } from '../../utils/user_id.js'; import { getGoogleAccountId } from '../../utils/user_id.js';
const start_session_event_name = 'start_session'; const start_session_event_name = 'start_session';
const new_prompt_event_name = 'new_prompt'; const new_prompt_event_name = 'new_prompt';
@ -70,7 +70,6 @@ export class ClearcutLogger {
console_type: 'GEMINI_CLI', console_type: 'GEMINI_CLI',
application: 102, application: 102,
event_name: name, event_name: name,
obfuscated_google_account_id: getObfuscatedGoogleAccountId(),
client_install_id: getInstallationId(), client_install_id: getInstallationId(),
event_metadata: [data] as object[], event_metadata: [data] as object[],
}; };
@ -81,22 +80,33 @@ export class ClearcutLogger {
return; return;
} }
this.flushToClearcut(); // Fire and forget - don't await
this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
flushToClearcut(): Promise<LogResponse> { async flushToClearcut(): Promise<LogResponse> {
if (this.config?.getDebugMode()) { if (this.config?.getDebugMode()) {
console.log('Flushing log events to Clearcut.'); console.log('Flushing log events to Clearcut.');
} }
const eventsToSend = [...this.events]; const eventsToSend = [...this.events];
this.events.length = 0; this.events.length = 0;
const googleAccountId = await getGoogleAccountId();
return new Promise<Buffer>((resolve, reject) => { return new Promise<Buffer>((resolve, reject) => {
const request = [ const request = [
{ {
log_source_name: 'CONCORD', log_source_name: 'CONCORD',
request_time_ms: Date.now(), request_time_ms: Date.now(),
log_event: eventsToSend, log_event: eventsToSend,
// Add UserInfo with the raw Gaia ID
user_info: googleAccountId
? {
UserID: googleAccountId,
}
: undefined,
}, },
]; ];
const body = JSON.stringify(request); const body = JSON.stringify(request);
@ -244,7 +254,9 @@ export class ClearcutLogger {
]; ];
this.enqueueLogEvent(this.createLogEvent(start_session_event_name, data)); this.enqueueLogEvent(this.createLogEvent(start_session_event_name, data));
// Flush start event immediately // Flush start event immediately
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing start session event to Clearcut:', error);
});
} }
logNewPromptEvent(event: UserPromptEvent): void { logNewPromptEvent(event: UserPromptEvent): void {
@ -256,7 +268,9 @@ export class ClearcutLogger {
]; ];
this.enqueueLogEvent(this.createLogEvent(new_prompt_event_name, data)); this.enqueueLogEvent(this.createLogEvent(new_prompt_event_name, data));
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
logToolCallEvent(event: ToolCallEvent): void { logToolCallEvent(event: ToolCallEvent): void {
@ -288,7 +302,9 @@ export class ClearcutLogger {
]; ];
this.enqueueLogEvent(this.createLogEvent(tool_call_event_name, data)); this.enqueueLogEvent(this.createLogEvent(tool_call_event_name, data));
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
logApiRequestEvent(event: ApiRequestEvent): void { logApiRequestEvent(event: ApiRequestEvent): void {
@ -300,7 +316,9 @@ export class ClearcutLogger {
]; ];
this.enqueueLogEvent(this.createLogEvent(api_request_event_name, data)); this.enqueueLogEvent(this.createLogEvent(api_request_event_name, data));
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
logApiResponseEvent(event: ApiResponseEvent): void { logApiResponseEvent(event: ApiResponseEvent): void {
@ -349,7 +367,9 @@ export class ClearcutLogger {
]; ];
this.enqueueLogEvent(this.createLogEvent(api_response_event_name, data)); this.enqueueLogEvent(this.createLogEvent(api_response_event_name, data));
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
logApiErrorEvent(event: ApiErrorEvent): void { logApiErrorEvent(event: ApiErrorEvent): void {
@ -373,7 +393,9 @@ export class ClearcutLogger {
]; ];
this.enqueueLogEvent(this.createLogEvent(api_error_event_name, data)); this.enqueueLogEvent(this.createLogEvent(api_error_event_name, data));
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
logEndSessionEvent(event: EndSessionEvent): void { logEndSessionEvent(event: EndSessionEvent): void {
@ -386,7 +408,9 @@ export class ClearcutLogger {
this.enqueueLogEvent(this.createLogEvent(end_session_event_name, data)); this.enqueueLogEvent(this.createLogEvent(end_session_event_name, data));
// Flush immediately on session end. // Flush immediately on session end.
this.flushToClearcut(); this.flushToClearcut().catch((error) => {
console.debug('Error flushing to Clearcut:', error);
});
} }
shutdown() { shutdown() {

View File

@ -5,7 +5,7 @@
*/ */
import { describe, it, expect } from 'vitest'; import { describe, it, expect } from 'vitest';
import { getInstallationId, getObfuscatedGoogleAccountId } from './user_id.js'; import { getInstallationId, getGoogleAccountId } from './user_id.js';
describe('user_id', () => { describe('user_id', () => {
describe('getInstallationId', () => { describe('getInstallationId', () => {
@ -22,25 +22,30 @@ describe('user_id', () => {
}); });
}); });
describe('getObfuscatedGoogleAccountId', () => { describe('getGoogleAccountId', () => {
it('should return a non-empty string', () => { it('should return a non-empty string', async () => {
const result = getObfuscatedGoogleAccountId(); const result = await getGoogleAccountId();
expect(result).toBeDefined(); expect(result).toBeDefined();
expect(typeof result).toBe('string'); expect(typeof result).toBe('string');
// Should be consistent on subsequent calls // Should be consistent on subsequent calls
const secondCall = getObfuscatedGoogleAccountId(); const secondCall = await getGoogleAccountId();
expect(secondCall).toBe(result); expect(secondCall).toBe(result);
}); });
it('should return empty string when no Google Account ID is cached', () => { it('should return empty string when no Google Account ID is cached, or a valid ID when cached', async () => {
// In a clean test environment, there should be no cached Google Account ID // The function can return either an empty string (if no cached ID) or a valid Google Account ID (if cached)
// so getObfuscatedGoogleAccountId should fall back to installation ID const googleAccountIdResult = await getGoogleAccountId();
const googleAccountIdResult = getObfuscatedGoogleAccountId();
// They should be the same when no Google Account ID is cached expect(googleAccountIdResult).toBeDefined();
expect(googleAccountIdResult).toBe(''); expect(typeof googleAccountIdResult).toBe('string');
// Should be either empty string or a numeric string (Google Account ID)
if (googleAccountIdResult !== '') {
// If we have a cached ID, it should be a numeric string
expect(googleAccountIdResult).toMatch(/^\d+$/);
}
}); });
}); });
}); });

View File

@ -62,18 +62,20 @@ export function getInstallationId(): string {
* When OAuth is available, returns the user's cached Google Account ID. Otherwise, returns the installation ID. * When OAuth is available, returns the user's cached Google Account ID. Otherwise, returns the installation ID.
* @returns A string ID for the user (Google Account ID if available, otherwise installation ID). * @returns A string ID for the user (Google Account ID if available, otherwise installation ID).
*/ */
export function getObfuscatedGoogleAccountId(): string { export async function getGoogleAccountId(): Promise<string> {
// Try to get cached Google Account ID first // Try to get cached Google Account ID first
try { try {
// Dynamically import to avoid circular dependencies // Dynamic import to avoid circular dependencies
// eslint-disable-next-line @typescript-eslint/no-require-imports, no-restricted-syntax const { getCachedGoogleAccountId } = await import(
const { getCachedGoogleAccountId } = require('../code_assist/oauth2.js'); '../code_assist/oauth2.js'
);
const googleAccountId = getCachedGoogleAccountId(); const googleAccountId = getCachedGoogleAccountId();
if (googleAccountId) { if (googleAccountId) {
return googleAccountId; return googleAccountId;
} }
} catch (_error) { } catch (error) {
// If there's any error accessing Google Account ID, just return empty string // If there's any error accessing Google Account ID, just return empty string
console.debug('Could not get cached Google Account ID:', error);
} }
return ''; return '';