mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-19 11:24:57 -05:00
fix(oauth): log OAuth token-exchange failures with provider context
validateAuthorizationCode failures were logged with an empty message
and no provider id, and a provider that answers 200 with an error
body (Slack `{ok:false,error:"invalid_code"}`, or an
`error`/`error_description` pair) produced no log at all until the
unrelated getUserInfo step failed later. Add the provider id to the
catch-site log message and log when a resolved token set has neither
accessToken nor idToken, using only the provider-supplied error
fields (never the raw token response, which may carry secrets).
This commit is contained in:
@@ -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.
|
||||
@@ -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<Record<string, never>>;
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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?: {
|
||||
|
||||
Reference in New Issue
Block a user