diff --git a/.changeset/oauth-callback-token-exchange-logging.md b/.changeset/oauth-callback-token-exchange-logging.md new file mode 100644 index 0000000000..557a319c77 --- /dev/null +++ b/.changeset/oauth-callback-token-exchange-logging.md @@ -0,0 +1,5 @@ +--- +"better-auth": patch +--- + +Log OAuth token-exchange failures in the `/callback/:id` handler: the authorization-code exchange error now includes the provider id, and a 200 response with neither `access_token` nor `id_token` (e.g. Slack's `{ok:false,error:"invalid_code"}`) is now logged instead of failing silently until the later user-info step. diff --git a/packages/better-auth/src/api/routes/callback.test.ts b/packages/better-auth/src/api/routes/callback.test.ts new file mode 100644 index 0000000000..0857003771 --- /dev/null +++ b/packages/better-auth/src/api/routes/callback.test.ts @@ -0,0 +1,76 @@ +import type { OAuthProvider } from "@better-auth/core/oauth2"; +import { getOAuth2Tokens } from "@better-auth/core/oauth2"; +import { describe, expect, it, vi } from "vitest"; +import { getTestInstance } from "../../test-utils/test-instance"; + +describe("callback", () => { + /** + * @see https://github.com/better-auth/better-auth/issues/1259 + */ + it("logs the provider error when a 200 token response has no access_token or id_token", async () => { + const provider = { + id: "no-token-provider", + name: "No Token Provider", + accountSubject: () => "subject", + createAuthorizationURL: ({ state }) => + new URL(`https://idp.example.com/authorize?state=${state}`), + validateAuthorizationCode: async () => + // Mirrors a provider (e.g. Slack) that answers with HTTP 200 but an + // error in the response body instead of failing the HTTP request. + getOAuth2Tokens({ ok: false, error: "invalid_code" }), + getUserInfo: async () => null, + } satisfies OAuthProvider>; + + const { client, auth, cookieSetter } = await getTestInstance({ + plugins: [ + { + id: "no-token-provider-plugin", + init: (ctx) => ({ + context: { + socialProviders: [provider, ...ctx.socialProviders], + }, + }), + }, + ], + }); + + const ctx = await auth.$context; + const errorSpy = vi.spyOn(ctx.logger, "error").mockImplementation(() => {}); + + const oAuthHeaders = new Headers(); + const signIn = await client.signIn.social( + { + provider: provider.id, + callbackURL: "/dashboard", + }, + { + throw: true, + onSuccess: cookieSetter(oAuthHeaders), + }, + ); + const state = new URL(signIn.url!).searchParams.get("state"); + + await client.$fetch(`/callback/${provider.id}?code=test&state=${state}`, { + method: "GET", + headers: oAuthHeaders, + onError() {}, + }); + + const noTokenCalls = errorSpy.mock.calls.filter( + ([message]) => + message === + "OAuth token response contained no access_token or id_token", + ); + expect(noTokenCalls).toHaveLength(1); + expect(noTokenCalls[0]?.[1]).toEqual({ + providerId: provider.id, + error: "invalid_code", + error_description: undefined, + ok: false, + }); + // The raw token response must never be logged as-is. + expect(JSON.stringify(noTokenCalls[0])).not.toContain('"raw"'); + + errorSpy.mockRestore(); + }); +}); diff --git a/packages/better-auth/src/api/routes/callback.ts b/packages/better-auth/src/api/routes/callback.ts index fd41096ec4..d938fdd7ce 100644 --- a/packages/better-auth/src/api/routes/callback.ts +++ b/packages/better-auth/src/api/routes/callback.ts @@ -204,12 +204,31 @@ export const callbackOAuth = createAuthEndpoint( redirectURI: `${c.context.baseURL}${getOAuthCallbackPath(provider)}`, }); } catch (e) { - c.context.logger.error("", e); + c.context.logger.error("Failed to exchange OAuth authorization code", { + providerId: provider.id, + error: e, + }); throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.INVALID_CODE); } if (!tokens) { throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.INVALID_CODE); } + if (!tokens.accessToken && !tokens.idToken) { + // A 200 response with neither field means the provider reported an + // error in the response body instead of the HTTP status (e.g. Slack's + // `{ok:false,error:"invalid_code"}`). Log the provider-supplied error + // fields only; never log the raw token response, which may carry + // other secrets. + c.context.logger.error( + "OAuth token response contained no access_token or id_token", + { + providerId: provider.id, + error: tokens.raw?.error, + error_description: tokens.raw?.error_description, + ok: tokens.raw?.ok, + }, + ); + } const parsedUserData = userData ? safeJSONParse<{ name?: {