diff --git a/src/app/token/route.test.ts b/src/app/token/route.test.ts index 2dad6c47..b28ba8d6 100644 --- a/src/app/token/route.test.ts +++ b/src/app/token/route.test.ts @@ -395,4 +395,76 @@ describe("POST /token", () => { expect(missingRefreshResponse.status).toBe(400); expect(missingRefresh.calls.persisted).toHaveLength(0); }); + + test("records the client and a bounded provider error code on rejection", async () => { + const deps = dependencies({ + clerkStatus: 400, + clerkTokens: { + error: "invalid_grant", + error_description: "Code already redeemed", + }, + }); + + const response = await tokenRequest( + request({ + grant_type: "authorization_code", + client_id: "client_1", + code: "already-used", + code_verifier: "verifier_1", + }), + deps.value, + ); + + expect(response.status).toBe(400); + expect(deps.calls.outcomes[0]).toMatchObject({ + stage: "provider_exchange", + outcome: "error", + clientId: "client_1", + providerStatusCode: 400, + providerErrorCode: "invalid_grant", + }); + // The description can carry anything, so it is deliberately not recorded. + expect(JSON.stringify(deps.calls.outcomes[0])).not.toContain("redeemed"); + }); + + test("records an unrecognized provider error code as unknown", async () => { + const deps = dependencies({ + clerkStatus: 400, + clerkTokens: { error: "user_42_is_rate_limited_until_2026" }, + }); + + await tokenRequest( + request({ + grant_type: "authorization_code", + client_id: "client_1", + code: "rejected", + code_verifier: "verifier_1", + }), + deps.value, + ); + + expect(deps.calls.outcomes[0]).toMatchObject({ + providerErrorCode: "unknown", + }); + expect(JSON.stringify(deps.calls.outcomes[0])).not.toContain("user_42"); + }); + + test("records the client on a successful exchange and no provider failure fields", async () => { + const deps = dependencies(); + + await tokenRequest( + request({ + grant_type: "authorization_code", + client_id: "client_1", + code: "code_1", + code_verifier: "verifier_1", + }), + deps.value, + ); + + const outcome = deps.calls.outcomes[0]; + expect(outcome).toMatchObject({ outcome: "success", clientId: "client_1" }); + expect(outcome.providerStatusCode).toBeUndefined(); + expect(outcome.providerErrorCode).toBeUndefined(); + }); }); diff --git a/src/app/token/route.ts b/src/app/token/route.ts index 051bd85e..f395ba52 100644 --- a/src/app/token/route.ts +++ b/src/app/token/route.ts @@ -161,6 +161,33 @@ async function oauthErrorCode(response: NextResponse): Promise { return response.status >= 500 ? "server_error" : "invalid_grant"; } +// RFC 6749 section 5.2 token endpoint error codes. The provider's code is a +// coarse signal: `invalid_grant` alone covers expired, revoked, redirect- +// mismatched and wrong-client grants, so it narrows a failure rather than +// identifying it. Anything outside this set is recorded as `unknown`, because +// the field is untrusted input and must not carry free text, PII or unbounded +// cardinality into logs and analytics. The description is never recorded. +const PROVIDER_ERROR_CODES = new Set([ + "invalid_request", + "invalid_client", + "invalid_grant", + "unauthorized_client", + "unsupported_grant_type", + "invalid_scope", +]); + +async function readProviderErrorCode( + response: Response, +): Promise { + try { + const body = (await response.json()) as { error?: unknown }; + if (typeof body.error !== "string") return undefined; + return PROVIDER_ERROR_CODES.has(body.error) ? body.error : "unknown"; + } catch { + return undefined; + } +} + export async function tokenRequest( request: NextRequest, dependencies: TokenDependencies = tokenDependencies, @@ -172,6 +199,9 @@ export async function tokenRequest( "unknown"; let accessScopeForAnalytics: OAuthTokenExchangeAnalytics["accessScope"] = "unknown"; + let clientIdForAnalytics: string | undefined; + let providerStatusCode: number | undefined; + let providerErrorCode: string | undefined; let stage: OAuthTokenExchangeAnalytics["stage"] = "request_validation"; const finish = ( @@ -182,10 +212,13 @@ export async function tokenRequest( dependencies.recordExchange?.({ grantType: grantTypeForAnalytics, clientType: clientTypeForAnalytics, + ...(clientIdForAnalytics ? { clientId: clientIdForAnalytics } : {}), accessScope: accessScopeForAnalytics, stage, outcome: response.ok ? "success" : "error", ...(errorCode ? { errorCode } : {}), + ...(providerStatusCode ? { providerStatusCode } : {}), + ...(providerErrorCode ? { providerErrorCode } : {}), statusCode: response.status, durationMs: Date.now() - startedAt, }); @@ -229,6 +262,7 @@ export async function tokenRequest( grantTypeForAnalytics = normalizedGrantType(grantType); const { clientId } = clientCredentials(request, body, params); clientTypeForAnalytics = clientType(clientId); + clientIdForAnalytics = clientId || undefined; if (!clientId) { return fail("invalid_request", "Missing required parameter: client_id"); } @@ -296,6 +330,8 @@ export async function tokenRequest( }, ); if (!clerkResponse.ok) { + providerStatusCode = clerkResponse.status; + providerErrorCode = await readProviderErrorCode(clerkResponse); return fail( "invalid_grant", grantType === "refresh_token" diff --git a/src/lib/mcp/analytics.test.ts b/src/lib/mcp/analytics.test.ts index fd019e5e..39a2c068 100644 --- a/src/lib/mcp/analytics.test.ts +++ b/src/lib/mcp/analytics.test.ts @@ -560,6 +560,37 @@ describe("captureOAuthTokenExchange", () => { expect(JSON.stringify(captured)).not.toContain("refresh_token_hash"); expect(JSON.stringify(captured)).not.toContain("code_verifier"); }); + + test("maps client and provider failure fields onto their PostHog keys", () => { + const captured: unknown[] = []; + const fakePosthog = { + capture: (event: unknown) => captured.push(event), + } as unknown as PostHog; + + captureOAuthTokenExchange( + { + grantType: "authorization_code", + clientType: "registered_client", + clientId: "client_1", + accessScope: "organization", + stage: "provider_exchange", + outcome: "error", + errorCode: "invalid_grant", + providerStatusCode: 400, + providerErrorCode: "invalid_grant", + statusCode: 400, + durationMs: 17, + }, + fakePosthog, + ); + + const properties = (captured[0] as { properties: Record }) + .properties; + + expect(properties.oauth_client_id).toBe("client_1"); + expect(properties.oauth_provider_status_code).toBe(400); + expect(properties.oauth_provider_error_code).toBe("invalid_grant"); + }); }); describe("captureMcpConnectionScopeFailure", () => { diff --git a/src/lib/mcp/analytics.ts b/src/lib/mcp/analytics.ts index 3a366bd8..c03778a8 100644 --- a/src/lib/mcp/analytics.ts +++ b/src/lib/mcp/analytics.ts @@ -39,6 +39,8 @@ const projectToken = process.env.POSTHOG_PROJECT_TOKEN; export type OAuthTokenExchangeAnalytics = { grantType: "authorization_code" | "refresh_token" | "unknown"; clientType: "kernel_cli" | "registered_client" | "unknown"; + /** OAuth client_id, so a failure can be attributed to the client that caused it. */ + clientId?: string; accessScope: "organization" | "project" | "unknown"; stage: | "request_validation" @@ -54,6 +56,11 @@ export type OAuthTokenExchangeAnalytics = { | "invalid_grant" | "unsupported_grant_type" | "server_error"; + /** Upstream status when the provider rejected the exchange. */ + providerStatusCode?: number; + /** Coarse RFC 6749 section 5.2 error code from the provider, or `unknown` + * for anything outside that set. Narrows a failure; does not identify it. */ + providerErrorCode?: string; statusCode: number; durationMs: number; }; @@ -541,10 +548,13 @@ export function captureOAuthTokenExchange( const properties = { oauth_grant_type: exchange.grantType, oauth_client_type: exchange.clientType, + oauth_client_id: exchange.clientId, oauth_access_scope: exchange.accessScope, oauth_stage: exchange.stage, oauth_outcome: exchange.outcome, oauth_error_code: exchange.errorCode, + oauth_provider_status_code: exchange.providerStatusCode, + oauth_provider_error_code: exchange.providerErrorCode, http_status_code: exchange.statusCode, duration_ms: exchange.durationMs, };