diff --git a/.gitignore b/.gitignore index c922f4f..96b5b51 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,6 @@ npm-debug.log* # Husky .husky/_ + +# Local review artifacts +TODO.md diff --git a/src/plugin.ts b/src/plugin.ts index 277eb62..b3eab45 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -22,19 +22,19 @@ import { fetchBergetModels } from './plugin/models'; import { createPkceAuthorizeMethod } from './plugin/pkce-flow'; import { refreshAccessTokenDirect } from './plugin/token'; +type FetchInput = Request | string | URL; + /** * Wraps a native fetch call while preserving headers from both Request * objects and init, then injecting (or overwriting) Authorization. */ async function fetchWithAuth( authToken: string, - input: Request | string | URL, + input: FetchInput, init?: RequestInit, ): Promise { const request = - input instanceof Request - ? new Request(input, init) - : new Request(input.toString(), init); + input instanceof Request ? new Request(input, init) : new Request(input.toString(), init); const headers = new Headers(request.headers); headers.set('Authorization', `Bearer ${authToken}`); @@ -72,10 +72,8 @@ export const BergetAuthPlugin = async ({ client }: PluginInput): Promise const apiKey = apiAuth.key; return { apiKey, - fetch: async ( - input: Request | string | URL, - init?: RequestInit, - ): Promise => fetchWithAuth(apiKey, input, init), + fetch: async (input: Request | string | URL, init?: RequestInit): Promise => + fetchWithAuth(apiKey, input, init), }; } return {}; @@ -92,8 +90,29 @@ export const BergetAuthPlugin = async ({ client }: PluginInput): Promise apiKey: currentAuth.access || '', fetch: async (input: Request | string | URL, init?: RequestInit): Promise => { if (accessTokenExpired(currentAuth)) { + // Cache-busting: another process may have refreshed and persisted + // a fresher token. Ask the framework for the latest auth before + // initiating an HTTP refresh. + logDebug('Token expired, checking disk for fresher token...'); + try { + const diskAuth = await getAuth(); + if (isOAuthAuth(diskAuth as OAuthAuthDetails)) { + const diskOAuth = diskAuth as OAuthAuthDetails; + if (!accessTokenExpired(diskOAuth)) { + currentAuth = diskOAuth; + logDebug('Adopted fresher token from disk, skipping HTTP refresh'); + return fetchWithAuth(currentAuth.access || '', input, init); + } + logDebug('Disk token is also expired, proceeding with HTTP refresh'); + } + } catch (error) { + logDebug( + `getAuth() failed during cache-busting check: ${error instanceof Error ? error.message : String(error)}`, + ); + } + logDebug('Token expired, refreshing before request...'); - const result = await refreshAccessTokenDirect(currentAuth, client); + const result = await refreshAccessTokenDirect(currentAuth, client, getAuth); if (result.success) { currentAuth = result.auth; logDebug('Token refreshed successfully'); diff --git a/src/plugin/pkce-flow.test.ts b/src/plugin/pkce-flow.test.ts index 341d57e..e5edb1c 100644 --- a/src/plugin/pkce-flow.test.ts +++ b/src/plugin/pkce-flow.test.ts @@ -1,8 +1,9 @@ -import type { AuthOAuthResult } from './types'; import * as http from 'node:http'; import * as url from 'node:url'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { AuthOAuthResult } from './types'; + vi.mock('../constants', () => ({ ACCESS_TOKEN_EXPIRY_BUFFER_MS: 60_000, getKeycloakRealm: () => 'berget', @@ -16,19 +17,31 @@ vi.mock('./debug', () => ({ })); // Capture what arguments are passed to server.listen() -let capturedListenArgs: unknown[] | undefined; +let capturedListenArguments: undefined | unknown[]; +let shouldEmitEADDRINUSE = false; vi.mock('node:http', async () => { const actual = await vi.importActual('node:http'); return { ...actual, - createServer: (...args: Parameters) => { - const server = actual.createServer(...args); + createServer: (...arguments_: Parameters) => { + const server = actual.createServer(...arguments_); // eslint-disable-next-line @typescript-eslint/no-explicit-any - server.listen = (...listenArgs: any[]) => { - capturedListenArgs = listenArgs; + server.listen = (...listenArguments: any[]) => { + capturedListenArguments = listenArguments; + if (shouldEmitEADDRINUSE) { + setImmediate(() => { + const error = Object.assign( + new Error('address already in use') as NodeJS.ErrnoException, + { + code: 'EADDRINUSE', + }, + ); + server.emit('error', error); + }); + } return server; }; @@ -37,20 +50,20 @@ vi.mock('node:http', async () => { }; }); -// Dynamic import so the mocks apply before the subject module is loaded -async function loadSubject() { - const mod = await import('./pkce-flow'); - return mod.createPkceAuthorizeMethod; +async function loadExchangeCodeForTokens() { + const module_ = await import('./pkce-flow'); + return module_.exchangeCodeForTokens; } async function loadHandleCallbackRequest() { - const mod = await import('./pkce-flow'); - return mod.handleCallbackRequest; + const module_ = await import('./pkce-flow'); + return module_.handleCallbackRequest; } -async function loadExchangeCodeForTokens() { - const mod = await import('./pkce-flow'); - return mod.exchangeCodeForTokens; +// Dynamic import so the mocks apply before the subject module is loaded +async function loadSubject() { + const module_ = await import('./pkce-flow'); + return module_.createPkceAuthorizeMethod; } describe('handleCallbackRequest - Issue #4', () => { @@ -91,7 +104,9 @@ describe('handleCallbackRequest - Issue #4', () => { ); expect(resolvedResult).toBeDefined(); - if (typeof resolvedResult !== 'object' || resolvedResult === null) throw new Error('expected object'); + // eslint-disable-next-line sonarjs/different-types-comparison + if (typeof resolvedResult !== 'object' || resolvedResult === null) + throw new Error('expected object'); expect((resolvedResult as { type: string }).type).toBe('failed'); if ((resolvedResult as { type: string }).type !== 'failed') throw new Error('expected failed'); expect((resolvedResult as { error?: string }).error).toContain('access_denied'); @@ -127,10 +142,14 @@ describe('handleCallbackRequest - Issue #4', () => { ); expect(resolvedResult).toBeDefined(); - if (typeof resolvedResult !== 'object' || resolvedResult === null) throw new Error('expected object'); + // eslint-disable-next-line sonarjs/different-types-comparison + if (typeof resolvedResult !== 'object' || resolvedResult === null) + throw new Error('expected object'); expect((resolvedResult as { type: string }).type).toBe('failed'); if ((resolvedResult as { type: string }).type !== 'failed') throw new Error('expected failed'); - expect((resolvedResult as { error?: string }).error).toBe('Authentication failed: invalid_scope'); + expect((resolvedResult as { error?: string }).error).toBe( + 'Authentication failed: invalid_scope', + ); }); it('sets Cache-Control: no-store on error callback response', async () => { @@ -158,18 +177,19 @@ describe('handleCallbackRequest - Issue #4', () => { ); expect(mockResponse.writeHead).toHaveBeenCalledTimes(1); - const [, headers] = (mockResponse.writeHead as unknown as ReturnType).mock.calls[0]; + const [, headers] = (mockResponse.writeHead as unknown as ReturnType).mock + .calls[0]; expect(headers['Cache-Control']).toBe('no-store, no-cache, must-revalidate, proxy-revalidate'); expect(headers['Pragma']).toBe('no-cache'); expect(headers['Expires']).toBe('0'); }); }); - describe('createPkceAuthorizeMethod - Issue #1', () => { beforeEach(() => { vi.clearAllMocks(); - capturedListenArgs = undefined; + capturedListenArguments = undefined; + shouldEmitEADDRINUSE = false; delete process.env.CI; delete process.env.SSH_CONNECTION; }); @@ -186,12 +206,31 @@ describe('createPkceAuthorizeMethod - Issue #1', () => { const callbackPromise = (result.callback as any)(); callbackPromise.catch(() => {}); // ignore — we only care about listen args - expect(capturedListenArgs).toBeDefined(); - expect(capturedListenArgs).toHaveLength(3); // port, hostname, callback - if (!capturedListenArgs) throw new Error('capturedListenArgs should be defined'); - expect(capturedListenArgs[0]).toBe(8787); - expect(capturedListenArgs[1]).toBe('127.0.0.1'); - expect(typeof capturedListenArgs[2]).toBe('function'); + expect(capturedListenArguments).toBeDefined(); + expect(capturedListenArguments).toHaveLength(3); // port, hostname, callback + if (!capturedListenArguments) throw new Error('capturedListenArgs should be defined'); + expect(capturedListenArguments[0]).toBe(8787); + expect(capturedListenArguments[1]).toBe('127.0.0.1'); + expect(typeof capturedListenArguments[2]).toBe('function'); + }); + + it('returns clear EADDRINUSE error when another login is in progress', async () => { + process.env.CI = 'true'; + shouldEmitEADDRINUSE = true; + + const createPkceAuthorizeMethod = await loadSubject(); + const authorize = createPkceAuthorizeMethod(); + const result = await authorize(); + + // Trigger server creation — it will emit EADDRINUSE synchronously in listen() + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const callbackResult = await (result.callback as any)(); + + expect(callbackResult.type).toBe('failed'); + if (callbackResult.type !== 'failed') throw new Error('expected failed'); + expect(callbackResult.error).toBe( + 'Port 8787 is already in use. Another OpenCode login may be in progress. Please wait and try again, or close other OpenCode sessions.', + ); }); }); @@ -215,7 +254,11 @@ describe('exchangeCodeForTokens - Issue #3', () => { ); const exchangeCodeForTokens = await loadExchangeCodeForTokens(); - const result = await exchangeCodeForTokens('code', 'verifier', 'http://localhost:8787/callback'); + const result = await exchangeCodeForTokens( + 'code', + 'verifier', + 'http://localhost:8787/callback', + ); expect(result.type).toBe('success'); if (result.type !== 'success' || !('access' in result)) { @@ -223,8 +266,8 @@ describe('exchangeCodeForTokens - Issue #3', () => { } expect(result.access).toBe('access-123'); // Issue #5: stored expiry must be the raw timestamp, NOT pre-reduced by buffer - expect(result.expires).toBeGreaterThanOrEqual(Date.now() + 300_000 - 2_000); - expect(result.expires).toBeLessThanOrEqual(Date.now() + 300_000 + 2_000); + expect(result.expires).toBeGreaterThanOrEqual(Date.now() + 300_000 - 2000); + expect(result.expires).toBeLessThanOrEqual(Date.now() + 300_000 + 2000); expect(result.refresh).toBe('refresh-456'); }); @@ -238,7 +281,11 @@ describe('exchangeCodeForTokens - Issue #3', () => { ); const exchangeCodeForTokens = await loadExchangeCodeForTokens(); - const result = await exchangeCodeForTokens('code', 'verifier', 'http://localhost:8787/callback'); + const result = await exchangeCodeForTokens( + 'code', + 'verifier', + 'http://localhost:8787/callback', + ); expect(result.type).toBe('failed'); if (result.type !== 'failed') throw new Error('expected failed'); @@ -259,7 +306,11 @@ describe('exchangeCodeForTokens - Issue #3', () => { ); const exchangeCodeForTokens = await loadExchangeCodeForTokens(); - const result = await exchangeCodeForTokens('code', 'verifier', 'http://localhost:8787/callback'); + const result = await exchangeCodeForTokens( + 'code', + 'verifier', + 'http://localhost:8787/callback', + ); expect(result.type).toBe('failed'); if (result.type !== 'failed') throw new Error('expected failed'); @@ -275,7 +326,11 @@ describe('exchangeCodeForTokens - Issue #3', () => { ); const exchangeCodeForTokens = await loadExchangeCodeForTokens(); - const result = await exchangeCodeForTokens('code', 'verifier', 'http://localhost:8787/callback'); + const result = await exchangeCodeForTokens( + 'code', + 'verifier', + 'http://localhost:8787/callback', + ); expect(result.type).toBe('failed'); if (result.type !== 'failed') throw new Error('expected failed'); @@ -291,7 +346,11 @@ describe('exchangeCodeForTokens - Issue #3', () => { ); const exchangeCodeForTokens = await loadExchangeCodeForTokens(); - const result = await exchangeCodeForTokens('code', 'verifier', 'http://localhost:8787/callback'); + const result = await exchangeCodeForTokens( + 'code', + 'verifier', + 'http://localhost:8787/callback', + ); expect(result.type).toBe('failed'); if (result.type !== 'failed') throw new Error('expected failed'); @@ -300,8 +359,8 @@ describe('exchangeCodeForTokens - Issue #3', () => { describe('generateCodeVerifier - Issue #10', () => { it('produces a base64url string of the correct length (32 bytes => 43 chars)', async () => { - const mod = await import('./pkce-flow'); - const verifier = mod.generateCodeVerifier(); + const module_ = await import('./pkce-flow'); + const verifier = module_.generateCodeVerifier(); expect(typeof verifier).toBe('string'); expect(verifier.length).toBe(43); // ceil(32 / 3) * 4 = 43 with base64url padding stripped diff --git a/src/plugin/pkce-flow.ts b/src/plugin/pkce-flow.ts index dd415ad..28af265 100644 --- a/src/plugin/pkce-flow.ts +++ b/src/plugin/pkce-flow.ts @@ -32,6 +32,136 @@ export function createPkceAuthorizeMethod(): ( return executePkceAuthorization; } +/** + * Exchanges authorization code for tokens + */ +export async function exchangeCodeForTokens( + code: string, + codeVerifier: string, + redirectUri: string, +): Promise { + const tokenUrl = `${getKeycloakUrl()}/realms/${getKeycloakRealm()}/protocol/openid-connect/token`; + + logDebug(`Exchanging code for tokens at ${tokenUrl}`); + + const response = await fetch(tokenUrl, { + body: new URLSearchParams({ + client_id: KEYCLOAK_CLIENT_ID, + code, + code_verifier: codeVerifier, + grant_type: 'authorization_code', + redirect_uri: redirectUri, + }).toString(), + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + method: 'POST', + }); + + if (!response.ok) { + const errorText = await response.text(); + logDebug(`Token exchange failed: ${errorText}`); + return { + error: `Failed to exchange code for tokens: ${errorText}`, + type: 'failed', + }; + } + + const tokenData = (await response.json()) as Record; + + if ( + typeof tokenData.access_token !== 'string' || + typeof tokenData.expires_in !== 'number' || + typeof tokenData.refresh_token !== 'string' + ) { + logDebug('Token exchange returned malformed body'); + return { + error: 'Invalid token response from authorization server', + type: 'failed', + }; + } + + const expires = Date.now() + tokenData.expires_in * 1000; + + logDebug('Successfully obtained tokens via PKCE'); + + return { + access: tokenData.access_token, + expires, + refresh: tokenData.refresh_token, + type: 'success', + }; +} + +/** + * Generate a random string for PKCE code_verifier + */ +export function generateCodeVerifier(): string { + const bytes = new Uint8Array(32); + crypto.webcrypto.getRandomValues(bytes); + return Buffer.from(bytes).toString('base64url'); +} + +/** + * Handles the OAuth callback request + */ +export async function handleCallbackRequest( + response: http.ServerResponse, + server: http.Server, + parsedUrl: url.UrlWithParsedQuery, + state: string, + codeVerifier: string, + redirectUri: string, + resolve: (value: AuthOAuthResult | PromiseLike) => void, +): Promise { + const receivedState = parsedUrl.query.state as string; + const code = parsedUrl.query.code as string; + const error = parsedUrl.query.error as string; + const errorDescription = parsedUrl.query.error_description as string | undefined; + + if (error) { + const displayMessage = errorDescription ? `${error}: ${errorDescription}` : error; + + writeHtmlResponse(response, buildHtmlResponse(false, displayMessage)); + server.close(); + resolve({ + error: `Authentication failed: ${displayMessage}`, + type: 'failed', + }); + return; + } + + if (receivedState !== state) { + writeHtmlResponse(response, buildHtmlResponse(false, 'Invalid state parameter')); + server.close(); + resolve({ + error: 'Invalid state parameter. Please try again.', + type: 'failed', + }); + return; + } + + if (!code) { + writeHtmlResponse(response, buildHtmlResponse(false, 'No authorization code received')); + server.close(); + resolve({ + error: 'No authorization code received.', + type: 'failed', + }); + return; + } + + // Exchange code for tokens + writeHtmlResponse( + response, + buildHtmlResponse(true, 'You can close this window and return to OpenCode.'), + ); + server.close(); + + const result = await exchangeCodeForTokens(code, codeVerifier, redirectUri); + resolve(result); +} + /** * Builds the HTML response for the callback page */ @@ -163,67 +293,6 @@ function createCallbackServerPromise( }); } -/** - * Exchanges authorization code for tokens - */ -export async function exchangeCodeForTokens( - code: string, - codeVerifier: string, - redirectUri: string, -): Promise { - const tokenUrl = `${getKeycloakUrl()}/realms/${getKeycloakRealm()}/protocol/openid-connect/token`; - - logDebug(`Exchanging code for tokens at ${tokenUrl}`); - - const response = await fetch(tokenUrl, { - body: new URLSearchParams({ - client_id: KEYCLOAK_CLIENT_ID, - code, - code_verifier: codeVerifier, - grant_type: 'authorization_code', - redirect_uri: redirectUri, - }).toString(), - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - method: 'POST', - }); - - if (!response.ok) { - const errorText = await response.text(); - logDebug(`Token exchange failed: ${errorText}`); - return { - error: `Failed to exchange code for tokens: ${errorText}`, - type: 'failed', - }; - } - - const tokenData = (await response.json()) as Record; - - if ( - typeof tokenData.access_token !== 'string' || - typeof tokenData.expires_in !== 'number' || - typeof tokenData.refresh_token !== 'string' - ) { - logDebug('Token exchange returned malformed body'); - return { - error: 'Invalid token response from authorization server', - type: 'failed', - }; - } - - const expires = Date.now() + tokenData.expires_in * 1000; - - logDebug('Successfully obtained tokens via PKCE'); - - return { - access: tokenData.access_token, - expires, - refresh: tokenData.refresh_token, - type: 'success', - }; -} - async function executePkceAuthorization( _inputs?: Record, ): Promise { @@ -288,85 +357,6 @@ function generateRandomHex(byteLength: number): string { return Buffer.from(bytes).toString('hex'); } -/** - * Generate a random string for PKCE code_verifier - */ -export function generateCodeVerifier(): string { - const bytes = new Uint8Array(32); - crypto.webcrypto.getRandomValues(bytes); - return Buffer.from(bytes).toString('base64url'); -} - -/** - * Sends an HTML response with cache-control headers appropriate for OAuth callbacks - */ -function writeHtmlResponse(response: http.ServerResponse, html: string): void { - response.writeHead(200, { - 'Cache-Control': 'no-store, no-cache, must-revalidate, proxy-revalidate', - 'Content-Type': 'text/html', - 'Expires': '0', - 'Pragma': 'no-cache', - }); - response.end(html); -} - -/** - * Handles the OAuth callback request - */ -export async function handleCallbackRequest( - response: http.ServerResponse, - server: http.Server, - parsedUrl: url.UrlWithParsedQuery, - state: string, - codeVerifier: string, - redirectUri: string, - resolve: (value: AuthOAuthResult | PromiseLike) => void, -): Promise { - const receivedState = parsedUrl.query.state as string; - const code = parsedUrl.query.code as string; - const error = parsedUrl.query.error as string; - const errorDescription = parsedUrl.query.error_description as string | undefined; - - if (error) { - const displayMessage = errorDescription ? `${error}: ${errorDescription}` : error; - - writeHtmlResponse(response, buildHtmlResponse(false, displayMessage)); - server.close(); - resolve({ - error: `Authentication failed: ${displayMessage}`, - type: 'failed', - }); - return; - } - - if (receivedState !== state) { - writeHtmlResponse(response, buildHtmlResponse(false, 'Invalid state parameter')); - server.close(); - resolve({ - error: 'Invalid state parameter. Please try again.', - type: 'failed', - }); - return; - } - - if (!code) { - writeHtmlResponse(response, buildHtmlResponse(false, 'No authorization code received')); - server.close(); - resolve({ - error: 'No authorization code received.', - type: 'failed', - }); - return; - } - - // Exchange code for tokens - writeHtmlResponse(response, buildHtmlResponse(true, 'You can close this window and return to OpenCode.')); - server.close(); - - const result = await exchangeCodeForTokens(code, codeVerifier, redirectUri); - resolve(result); -} - /** * Handles server startup errors */ @@ -377,7 +367,7 @@ function handleServerError( if (error.code === 'EADDRINUSE') { logDebug(`Port ${PKCE_CALLBACK_PORT} is already in use`); resolve({ - error: `Port ${PKCE_CALLBACK_PORT} is already in use. Please close other applications using this port.`, + error: `Port ${PKCE_CALLBACK_PORT} is already in use. Another OpenCode login may be in progress. Please wait and try again, or close other OpenCode sessions.`, type: 'failed', }); return; @@ -434,3 +424,16 @@ function openBrowserUrl(urlString: string): void { logDebug(`Failed to open browser: ${error}`); } } + +/** + * Sends an HTML response with cache-control headers appropriate for OAuth callbacks + */ +function writeHtmlResponse(response: http.ServerResponse, html: string): void { + response.writeHead(200, { + 'Cache-Control': 'no-store, no-cache, must-revalidate, proxy-revalidate', + 'Content-Type': 'text/html', + Expires: '0', + Pragma: 'no-cache', + }); + response.end(html); +} diff --git a/src/plugin/plugin.test.ts b/src/plugin/plugin.test.ts index 0c0fcce..4a22660 100644 --- a/src/plugin/plugin.test.ts +++ b/src/plugin/plugin.test.ts @@ -314,4 +314,146 @@ describe('BergetAuthPlugin loader', () => { 'Token refresh failed: Network error: DNS lookup failed', ); }); + + it('adopts fresher token from disk and skips HTTP refresh', async () => { + const client = createMockClient(); + const plugin = await BergetAuthPlugin({ client } as PluginInput); + const loader = plugin.auth && plugin.auth.loader; + + expect(loader).toBeDefined(); + + const expiredAuth: OAuthAuthDetails = { + access: 'expired-token', + expires: Date.now() - 1000, + refresh: 'valid-refresh-token', + type: 'oauth', + }; + + const freshAuth: OAuthAuthDetails = { + access: 'disk-fresh-token', + expires: Date.now() + 3600 * 1000, + refresh: 'valid-refresh-token', + type: 'oauth', + }; + + // First call returns expired, second call (inside fetch) returns fresh + const getAuth = vi + .fn() + .mockResolvedValueOnce(expiredAuth) + .mockResolvedValueOnce(freshAuth) as unknown as () => Promise; + + const result = await (loader as NonNullable)(getAuth, { + id: 'berget', + } as unknown as Provider); + + const mockFetch = vi.fn().mockResolvedValue(new Response('OK')); + vi.stubGlobal('fetch', mockFetch); + + const response = await (result.fetch as FetchLike)('https://api.berget.ai/v1/chat'); + + expect(getAuth).toHaveBeenCalledTimes(2); + // refresh endpoint should NOT have been called + expect(mockFetch).toHaveBeenCalledTimes(1); + const [, requestInit] = mockFetch.mock.calls[0]; + const headers = new Headers((requestInit as RequestInit | undefined)?.headers); + expect(headers.get('Authorization')).toBe('Bearer disk-fresh-token'); + expect(response.status).toBe(200); + }); + + it('proceeds with refresh when disk token is also expired', async () => { + const client = createMockClient(); + const plugin = await BergetAuthPlugin({ client } as PluginInput); + const loader = plugin.auth && plugin.auth.loader; + + expect(loader).toBeDefined(); + + const expiredAuth: OAuthAuthDetails = { + access: 'expired-token', + expires: Date.now() - 1000, + refresh: 'valid-refresh-token', + type: 'oauth', + }; + + // Both calls return expired + const getAuth = vi.fn().mockResolvedValue(expiredAuth) as unknown as () => Promise; + + const result = await (loader as NonNullable)(getAuth, { + id: 'berget', + } as unknown as Provider); + + // Mock the token refresh endpoint + const refreshFetch = vi.fn().mockResolvedValue({ + json: async () => ({ expires_in: 3600, token: 'new-token' }), + ok: true, + } as Response); + + const mockFetch = vi.fn().mockResolvedValue(new Response('OK')); + vi.stubGlobal('fetch', (...arguments_: [FetchArgument, init?: RequestInit]) => { + const url = arguments_[0]; + if (typeof url === 'string' && url.includes('/v1/auth/refresh')) { + return refreshFetch(...arguments_); + } + return mockFetch(...arguments_); + }); + + const response = await (result.fetch as FetchLike)('https://api.berget.ai/v1/chat'); + + expect(getAuth).toHaveBeenCalledTimes(2); // initial + cache-busting check + expect(refreshFetch).toHaveBeenCalledTimes(1); // refresh was triggered + expect(mockFetch).toHaveBeenCalledTimes(1); + const [, requestInit] = mockFetch.mock.calls[0]; + const headers = new Headers((requestInit as RequestInit | undefined)?.headers); + expect(headers.get('Authorization')).toBe('Bearer new-token'); + expect(response.status).toBe(200); + }); + + it('falls back to refresh when getAuth throws during check', async () => { + const client = createMockClient(); + const plugin = await BergetAuthPlugin({ client } as PluginInput); + const loader = plugin.auth && plugin.auth.loader; + + expect(loader).toBeDefined(); + + const expiredAuth: OAuthAuthDetails = { + access: 'expired-token', + expires: Date.now() - 1000, + refresh: 'valid-refresh-token', + type: 'oauth', + }; + + // First call returns expired, second call throws + const getAuth = vi + .fn() + .mockResolvedValueOnce(expiredAuth) + .mockRejectedValueOnce(new Error('Storage read error')) as unknown as () => Promise; + + const result = await (loader as NonNullable)(getAuth, { + id: 'berget', + } as unknown as Provider); + + // Mock the token refresh endpoint + const refreshFetch = vi.fn().mockResolvedValue({ + json: async () => ({ expires_in: 3600, token: 'new-token' }), + ok: true, + } as Response); + + const mockFetch = vi.fn().mockResolvedValue(new Response('OK')); + vi.stubGlobal('fetch', (...arguments_: [FetchArgument, init?: RequestInit]) => { + const url = arguments_[0]; + if (typeof url === 'string' && url.includes('/v1/auth/refresh')) { + return refreshFetch(...arguments_); + } + return mockFetch(...arguments_); + }); + + const response = await (result.fetch as FetchLike)('https://api.berget.ai/v1/chat'); + + expect(getAuth).toHaveBeenCalledTimes(2); + expect(refreshFetch).toHaveBeenCalledTimes(1); // refresh was triggered despite getAuth throwing + expect(mockFetch).toHaveBeenCalledTimes(1); + const [, requestInit] = mockFetch.mock.calls[0]; + const headers = new Headers((requestInit as RequestInit | undefined)?.headers); + expect(headers.get('Authorization')).toBe('Bearer new-token'); + expect(response.status).toBe(200); + }); }); diff --git a/src/plugin/token.test.ts b/src/plugin/token.test.ts index 66876d1..d730dbd 100644 --- a/src/plugin/token.test.ts +++ b/src/plugin/token.test.ts @@ -78,8 +78,8 @@ describe('refreshAccessTokenDirect', () => { expect(result.auth.access).toBe(newToken); expect(result.auth.refresh).toBe('refresh-token-1'); // Issue #5: stored expiry must be the raw timestamp, NOT pre-reduced by buffer - expect(result.auth.expires).toBeGreaterThanOrEqual(Date.now() + expiresIn * 1000 - 2_000); - expect(result.auth.expires).toBeLessThanOrEqual(Date.now() + expiresIn * 1000 + 2_000); + expect(result.auth.expires).toBeGreaterThanOrEqual(Date.now() + expiresIn * 1000 - 2000); + expect(result.auth.expires).toBeLessThanOrEqual(Date.now() + expiresIn * 1000 + 2000); expect(client.auth.set).toHaveBeenCalledTimes(1); expect(client.auth.set).toHaveBeenCalledWith({ @@ -480,20 +480,23 @@ describe('refreshAccessTokenDirect', () => { it('retries once on 503 and succeeds on second attempt', async () => { let callCount = 0; - vi.stubGlobal('fetch', vi.fn().mockImplementation(() => { - callCount++; - if (callCount === 1) { + vi.stubGlobal( + 'fetch', + vi.fn().mockImplementation(() => { + callCount++; + if (callCount === 1) { + return Promise.resolve({ + ok: false, + status: 503, + text: async () => 'Service Unavailable', + } as Response); + } return Promise.resolve({ - ok: false, - status: 503, - text: async () => 'Service Unavailable', + json: async () => ({ expires_in: 3600, token: 'retried-token' }), + ok: true, } as Response); - } - return Promise.resolve({ - json: async () => ({ expires_in: 3600, token: 'retried-token' }), - ok: true, - } as Response); - })); + }), + ); const auth: OAuthAuthDetails = { access: 'old', @@ -541,31 +544,109 @@ describe('refreshAccessTokenDirect', () => { expect(elapsed).toBeGreaterThanOrEqual(1500); }); - it('does not retry on 401 (client error)', async () => { + it('recovers from invalid_grant when disk has a valid token', async () => { + const { warnSpy } = suppressConsole(); + vi.stubGlobal( 'fetch', vi.fn().mockResolvedValue({ ok: false, - status: 401, - text: async () => JSON.stringify({ error: 'invalid_token' }), + status: 400, + text: async () => JSON.stringify({ error: 'invalid_grant' }), } as Response), ); + const diskAuth: OAuthAuthDetails = { + access: 'disk-fresh-token', + expires: Date.now() + 3600 * 1000, + refresh: 'disk-refresh-token', + type: 'oauth', + }; + + const getAuth = vi.fn().mockResolvedValue(diskAuth); + const auth: OAuthAuthDetails = { access: 'old', expires: Date.now() - 1000, - refresh: 'refresh-token-no-retry', + refresh: 'refresh-token-recover-valid', type: 'oauth', }; - const start = Date.now(); - const result = await refreshAccessTokenDirect(auth); - const elapsed = Date.now() - start; + const result = await refreshAccessTokenDirect(auth, undefined, getAuth); + + expect(result.success).toBe(true); + if (!result.success) throw new Error('result should be success'); + expect(result.auth.access).toBe('disk-fresh-token'); + expect(getAuth).toHaveBeenCalledTimes(1); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('fails on invalid_grant when disk auth is also expired', async () => { + const { warnSpy } = suppressConsole(); + + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: false, + status: 400, + text: async () => JSON.stringify({ error: 'invalid_grant' }), + } as Response), + ); + + const diskAuth: OAuthAuthDetails = { + access: 'disk-expired-token', + expires: Date.now() - 1000, + refresh: 'disk-refresh-token', + type: 'oauth', + }; + + const getAuth = vi.fn().mockResolvedValue(diskAuth); + + const auth: OAuthAuthDetails = { + access: 'old', + expires: Date.now() - 1000, + refresh: 'refresh-token-recover-expired', + type: 'oauth', + }; + + const result = await refreshAccessTokenDirect(auth, undefined, getAuth); expect(result.success).toBe(false); if (result.success) throw new Error('result should be failure'); expect(result.reason).toBe('Refresh token is invalid or revoked'); - // Should not have waited for retry - expect(elapsed).toBeLessThan(200); + expect(warnSpy).toHaveBeenCalledWith( + '[Berget Auth] Refresh token is invalid or revoked. Please run `opencode auth login` to reauthenticate.', + ); + }); + + it('fails on invalid_grant when getAuth throws', async () => { + const { warnSpy } = suppressConsole(); + + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: false, + status: 400, + text: async () => JSON.stringify({ error: 'invalid_grant' }), + } as Response), + ); + + const getAuth = vi.fn().mockRejectedValue(new Error('Storage read error')); + + const auth: OAuthAuthDetails = { + access: 'old', + expires: Date.now() - 1000, + refresh: 'refresh-token-recover-throw', + type: 'oauth', + }; + + const result = await refreshAccessTokenDirect(auth, undefined, getAuth); + + expect(result.success).toBe(false); + if (result.success) throw new Error('result should be failure'); + expect(result.reason).toBe('Refresh token is invalid or revoked'); + expect(warnSpy).toHaveBeenCalledWith( + '[Berget Auth] Refresh token is invalid or revoked. Please run `opencode auth login` to reauthenticate.', + ); }); }); diff --git a/src/plugin/token.ts b/src/plugin/token.ts index 75a6ee0..2897748 100644 --- a/src/plugin/token.ts +++ b/src/plugin/token.ts @@ -2,9 +2,12 @@ * Token refresh logic for Berget OAuth */ +import type { Auth } from '@opencode-ai/sdk'; + import type { OAuthAuthDetails, PluginInput, RefreshResult } from './types'; import { getTokenRefreshEndpoint } from '../constants'; +import { accessTokenExpired, isOAuthAuth } from './auth'; import { logDebug } from './debug'; // Track in-flight refresh requests to prevent duplicates @@ -17,6 +20,7 @@ const refreshInFlight = new Map>(); export async function refreshAccessTokenDirect( auth: OAuthAuthDetails, client?: PluginInput['client'], + getAuth?: () => Promise, ): Promise { const refreshToken = auth.refresh; @@ -33,7 +37,7 @@ export async function refreshAccessTokenDirect( } // Start refresh and track the promise - const refreshPromise = refreshAccessTokenInternal(auth, client); + const refreshPromise = refreshAccessTokenInternal(auth, client, getAuth); refreshInFlight.set(refreshToken, refreshPromise); try { @@ -43,6 +47,88 @@ export async function refreshAccessTokenDirect( } } +/** + * Parses a successful token refresh JSON response into a RefreshResult. + */ +function buildRefreshResult( + data: Record, + auth: OAuthAuthDetails, + client?: PluginInput['client'], +): RefreshResult { + if (typeof data.token !== 'string' || typeof data.expires_in !== 'number') { + logDebug('Refresh endpoint returned malformed body'); + return { + reason: 'Invalid token response from refresh endpoint', + success: false, + }; + } + + logDebug(`Token refreshed, expires_in=${data.expires_in}s`); + + // Build updated auth + const updatedAuth: OAuthAuthDetails = { + ...auth, + access: data.token, + expires: Date.now() + data.expires_in * 1000, + refresh: typeof data.refresh_token === 'string' ? data.refresh_token : auth.refresh, // Use new refresh token if rotated + }; + + // Persist updated tokens to OpenCode so they survive restarts + if (client) { + persistRefreshedToken(client, updatedAuth).catch(() => { + // Already logged inside persistRefreshedToken + }); + } + + return { auth: updatedAuth, success: true }; +} + +/** + * Handles the HTTP error response from the token refresh endpoint. + * Decides whether to retry (5xx), recover from disk (invalid_grant), or fail. + */ +async function handleErrorResponse( + response: Response, + errorText: string, + attempt: number, + auth: OAuthAuthDetails, + client: PluginInput['client'] | undefined, + getAuth: (() => Promise) | undefined, +): Promise { + // Retry transient 5xx errors (up to 2 attempts total) + if (response.status >= 500 && attempt <= 2) { + const delay = attempt === 1 ? 500 : 1500; + logDebug(`Refresh got HTTP ${response.status}, retrying in ${delay}ms...`); + await new Promise((resolve) => setTimeout(resolve, delay)); + return refreshAccessTokenInternal(auth, client, getAuth, attempt + 1); + } + + // Handle revoked/invalid refresh token + if (response.status === 401 || response.status === 400) { + const errorData = parseErrorResponse(errorText); + + if (errorData?.error === 'invalid_grant' || errorData?.error === 'invalid_token') { + // Try reloading from disk before failing - another process may have refreshed + const recovered = getAuth ? await tryRecoverFromDisk(getAuth) : undefined; + if (recovered) { + logDebug('Recovered from invalid_grant: valid token found on disk'); + return { auth: recovered, success: true }; + } + + const reason = 'Refresh token is invalid or revoked'; + console.warn( + '[Berget Auth] Refresh token is invalid or revoked. Please run `opencode auth login` to reauthenticate.', + ); + return { reason, success: false }; + } + + return { reason: `Token refresh failed: HTTP ${response.status}`, success: false }; + } + + // Other errors - might be temporary + return { reason: `Token refresh failed: HTTP ${response.status}`, success: false }; +} + /** * Parses error response from token endpoint */ @@ -60,22 +146,49 @@ function parseErrorResponse( } } +/** + * Persists refreshed tokens to OpenCode client. + */ +async function persistRefreshedToken( + client: PluginInput['client'], + updatedAuth: OAuthAuthDetails, +): Promise { + if (!updatedAuth.access || typeof updatedAuth.expires !== 'number') { + return; + } + + try { + await client.auth.set({ + body: { + access: updatedAuth.access, + expires: updatedAuth.expires, + refresh: updatedAuth.refresh, + type: 'oauth', + }, + path: { id: 'berget' }, + }); + logDebug('Token refresh persisted to OpenCode'); + } catch (error) { + // Non-fatal: in-memory token still works for this session + console.warn('[Berget Auth] Failed to persist token refresh:', error); + } +} + /** * Internal implementation of token refresh */ async function refreshAccessTokenInternal( auth: OAuthAuthDetails, client?: PluginInput['client'], + getAuth?: () => Promise, attempt = 1, ): Promise { - const refreshToken = auth.refresh; - logDebug('Refreshing access token'); try { const response = await fetch(getTokenRefreshEndpoint(), { body: JSON.stringify({ - refresh_token: refreshToken, + refresh_token: auth.refresh, }), headers: { 'Content-Type': 'application/json', @@ -86,77 +199,36 @@ async function refreshAccessTokenInternal( if (!response.ok) { const errorText = await response.text().catch(() => ''); logDebug(`Token refresh failed: ${response.status} ${errorText}`); - - // Retry transient 5xx errors (up to 2 attempts total) - if (response.status >= 500 && attempt <= 2) { - const delay = attempt === 1 ? 500 : 1500; - logDebug(`Refresh got HTTP ${response.status}, retrying in ${delay}ms...`); - await new Promise((resolve) => setTimeout(resolve, delay)); - return refreshAccessTokenInternal(auth, client, attempt + 1); - } - - // Handle revoked/invalid refresh token - if (response.status === 401 || response.status === 400) { - const errorData = parseErrorResponse(errorText); - - if (errorData?.error === 'invalid_grant' || errorData?.error === 'invalid_token') { - const reason = 'Refresh token is invalid or revoked'; - console.warn( - '[Berget Auth] Refresh token is invalid or revoked. Please run `opencode auth login` to reauthenticate.', - ); - return { reason, success: false }; - } - - return { reason: `Token refresh failed: HTTP ${response.status}`, success: false }; - } - - // Other errors - might be temporary - return { reason: `Token refresh failed: HTTP ${response.status}`, success: false }; + return handleErrorResponse(response, errorText, attempt, auth, client, getAuth); } const data = (await response.json()) as Record; - - if (typeof data.token !== 'string' || typeof data.expires_in !== 'number') { - logDebug('Refresh endpoint returned malformed body'); - return { - success: false, - reason: 'Invalid token response from refresh endpoint', - }; - } - - logDebug(`Token refreshed, expires_in=${data.expires_in}s`); - - // Build updated auth - const updatedAuth: OAuthAuthDetails = { - ...auth, - access: data.token, - expires: Date.now() + data.expires_in * 1000, - refresh: typeof data.refresh_token === 'string' ? data.refresh_token : refreshToken, // Use new refresh token if rotated - }; - - // Persist updated tokens to OpenCode so they survive restarts - if (client && updatedAuth.access && typeof updatedAuth.expires === 'number') { - try { - await client.auth.set({ - body: { - access: updatedAuth.access, - expires: updatedAuth.expires, - refresh: updatedAuth.refresh, - type: 'oauth', - }, - path: { id: 'berget' }, - }); - logDebug('Token refresh persisted to OpenCode'); - } catch (error) { - // Non-fatal: in-memory token still works for this session - console.warn('[Berget Auth] Failed to persist token refresh:', error); - } - } - - return { auth: updatedAuth, success: true }; + return buildRefreshResult(data, auth, client); } catch (error) { const reason = error instanceof Error ? error.message : 'Unknown network error'; console.error('Failed to refresh Berget access token:', error); return { reason: `Network error: ${reason}`, success: false }; } } + +/** + * Attempts to recover from invalid_grant/invalid_token by reloading auth from disk. + * Returns the disk auth if valid and non-expired, otherwise undefined. + */ +async function tryRecoverFromDisk( + getAuth: () => Promise, +): Promise { + try { + const diskAuth = await getAuth(); + if ( + isOAuthAuth(diskAuth) && + diskAuth.access && + !accessTokenExpired(diskAuth as unknown as OAuthAuthDetails) + ) { + return diskAuth as unknown as OAuthAuthDetails; + } + } catch { + // Fall through — no recovery possible + } + return undefined; +}