mirror of
https://github.com/better-auth/better-auth.git
synced 2026-07-16 14:38:30 -05:00
fix(oauth-proxy): reject profile callbacks with missing or expired OAuth state (#10183)
This commit is contained in:
5
.changeset/tender-rivers-serve.md
Normal file
5
.changeset/tender-rivers-serve.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"better-auth": patch
|
||||
---
|
||||
|
||||
Require OAuth proxy profile callbacks to match an issued OAuth state before creating sessions.
|
||||
@@ -108,6 +108,21 @@ type PassthroughPayload = {
|
||||
timestamp: number;
|
||||
};
|
||||
|
||||
const consumeOAuthProxyState = async (
|
||||
ctx: GenericEndpointContext,
|
||||
state: string,
|
||||
) => {
|
||||
try {
|
||||
await parseGenericState(ctx, state, {
|
||||
skipStateCookieCheck: true,
|
||||
});
|
||||
return true;
|
||||
} catch (e) {
|
||||
ctx.context.logger.warn("OAuth proxy state missing or invalid", e);
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
const oauthProxyQuerySchema = z.object({
|
||||
callbackURL: z.string().meta({
|
||||
description: "The URL to redirect to after the proxy",
|
||||
@@ -219,6 +234,7 @@ export const oAuthProxy = <O extends OAuthProxyOptions>(opts?: O) => {
|
||||
typeof payload.timestamp !== "number" ||
|
||||
!payload.userInfo ||
|
||||
!payload.account ||
|
||||
!payload.state ||
|
||||
!payload.callbackURL
|
||||
) {
|
||||
ctx.context.logger.error("Failed to parse OAuth proxy payload");
|
||||
@@ -237,13 +253,12 @@ export const oAuthProxy = <O extends OAuthProxyOptions>(opts?: O) => {
|
||||
throw redirectOnError(ctx, errorURL, "payload_expired");
|
||||
}
|
||||
|
||||
// Clean up OAuth state
|
||||
try {
|
||||
await parseGenericState(ctx, payload.state, {
|
||||
skipStateCookieCheck: true,
|
||||
});
|
||||
} catch (e) {
|
||||
ctx.context.logger.warn("Failed to clean up OAuth state", e);
|
||||
const stateConsumed = await consumeOAuthProxyState(
|
||||
ctx,
|
||||
payload.state,
|
||||
);
|
||||
if (!stateConsumed) {
|
||||
throw redirectOnError(ctx, errorURL, "state_mismatch");
|
||||
}
|
||||
|
||||
let result: Awaited<ReturnType<typeof handleOAuthUserInfo>>;
|
||||
|
||||
@@ -569,11 +569,8 @@ describe("oauth-proxy", async () => {
|
||||
// Production instance - handles OAuth callback
|
||||
const production = await getTestInstance(
|
||||
{
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
currentURL: "http://preview.example.com",
|
||||
}),
|
||||
],
|
||||
baseURL: "http://localhost:3000",
|
||||
plugins: [oAuthProxy()],
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: "test",
|
||||
@@ -590,7 +587,11 @@ describe("oauth-proxy", async () => {
|
||||
const preview = await getTestInstance(
|
||||
{
|
||||
baseURL: "http://preview.example.com",
|
||||
plugins: [oAuthProxy()],
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
productionURL: "http://localhost:3000",
|
||||
}),
|
||||
],
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: "test",
|
||||
@@ -603,8 +604,8 @@ describe("oauth-proxy", async () => {
|
||||
},
|
||||
);
|
||||
|
||||
// Step 1: Start OAuth on production
|
||||
const res = await production.client.signIn.social(
|
||||
// Step 1: Start OAuth on preview
|
||||
const res = await preview.client.signIn.social(
|
||||
{
|
||||
provider: "google",
|
||||
callbackURL: "/dashboard",
|
||||
@@ -678,11 +679,8 @@ describe("oauth-proxy", async () => {
|
||||
it("should forward result.error verbatim instead of collapsing to user_creation_failed", async () => {
|
||||
const production = await getTestInstance(
|
||||
{
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
currentURL: "http://preview.example.com",
|
||||
}),
|
||||
],
|
||||
baseURL: "http://localhost:3000",
|
||||
plugins: [oAuthProxy()],
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: "test",
|
||||
@@ -697,7 +695,11 @@ describe("oauth-proxy", async () => {
|
||||
const preview = await getTestInstance(
|
||||
{
|
||||
baseURL: "http://preview.example.com",
|
||||
plugins: [oAuthProxy()],
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
productionURL: "http://localhost:3000",
|
||||
}),
|
||||
],
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: "test",
|
||||
@@ -708,7 +710,7 @@ describe("oauth-proxy", async () => {
|
||||
{ disableTestUser: true },
|
||||
);
|
||||
|
||||
const res = await production.client.signIn.social(
|
||||
const res = await preview.client.signIn.social(
|
||||
{
|
||||
provider: "google",
|
||||
callbackURL: "/dashboard",
|
||||
@@ -1040,9 +1042,9 @@ describe("oauth-proxy", async () => {
|
||||
|
||||
const production = await getTestInstance(
|
||||
{
|
||||
baseURL: "http://localhost:3000",
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
currentURL: "http://preview.example.com",
|
||||
secret: dedicatedSecret,
|
||||
}),
|
||||
],
|
||||
@@ -1063,6 +1065,7 @@ describe("oauth-proxy", async () => {
|
||||
baseURL: "http://preview.example.com",
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
productionURL: "http://localhost:3000",
|
||||
secret: dedicatedSecret,
|
||||
}),
|
||||
],
|
||||
@@ -1078,8 +1081,8 @@ describe("oauth-proxy", async () => {
|
||||
},
|
||||
);
|
||||
|
||||
// Step 1: Start OAuth on production
|
||||
const res = await production.client.signIn.social(
|
||||
// Step 1: Start OAuth on preview
|
||||
const res = await preview.client.signIn.social(
|
||||
{
|
||||
provider: "google",
|
||||
callbackURL: "/dashboard",
|
||||
@@ -1142,7 +1145,7 @@ describe("oauth-proxy", async () => {
|
||||
expect(users[0]?.email).toBe("user@email.com");
|
||||
});
|
||||
|
||||
it("should handle existing user on preview", async () => {
|
||||
it("should reject a profile payload whose OAuth state was never issued", async () => {
|
||||
// Preview instance
|
||||
const preview = await getTestInstance(
|
||||
{
|
||||
@@ -1200,22 +1203,19 @@ describe("oauth-proxy", async () => {
|
||||
onError(context) {
|
||||
expect(context.response.status).toBe(302);
|
||||
const location = context.response.headers.get("location");
|
||||
expect(location).toContain("/dashboard");
|
||||
expect(location).toContain("error=state_mismatch");
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
// User count should still be 1 (linked account, not new user)
|
||||
const users = await previewCtx.internalAdapter.listUsers();
|
||||
expect(users.length).toBe(1);
|
||||
expect(users[0]?.email).toBe("user@email.com");
|
||||
|
||||
// Should have linked the google account
|
||||
const accounts = await previewCtx.internalAdapter.findAccounts(
|
||||
users[0]!.id,
|
||||
);
|
||||
expect(accounts.length).toBe(1);
|
||||
expect(accounts[0]?.providerId).toBe("google");
|
||||
expect(accounts.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1319,8 +1319,7 @@ describe("oauth-proxy", async () => {
|
||||
expect(users[0]?.email).toBe("user@email.com");
|
||||
});
|
||||
|
||||
it("should handle state cleanup gracefully when verification is already deleted", async () => {
|
||||
// This tests that parseGenericState errors are caught and don't break the flow
|
||||
it("should reject the callback when the OAuth state is not found in the database", async () => {
|
||||
const { client, auth } = await getTestInstance(
|
||||
{
|
||||
plugins: [
|
||||
@@ -1365,22 +1364,18 @@ describe("oauth-proxy", async () => {
|
||||
data: JSON.stringify(payload),
|
||||
});
|
||||
|
||||
// The callback should still succeed even if state cleanup fails
|
||||
await client.$fetch(
|
||||
`/oauth-proxy-callback?callbackURL=/dashboard&profile=${encodeURIComponent(encryptedProfile)}`,
|
||||
{
|
||||
onError(context) {
|
||||
const location = context.response.headers.get("location");
|
||||
// Should redirect to dashboard, not error
|
||||
expect(location).not.toContain("error=state_mismatch");
|
||||
expect(location).toContain("/dashboard");
|
||||
expect(location).toContain("error=state_mismatch");
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
// Verify user was created
|
||||
const users = await internalAdapter.listUsers();
|
||||
expect(users.length).toBe(1);
|
||||
expect(users.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1584,6 +1579,130 @@ describe("oauth-proxy", async () => {
|
||||
const users = await previewCtx.internalAdapter.listUsers();
|
||||
expect(users.length).toBe(1);
|
||||
expect(users[0]?.email).toBe("user@email.com");
|
||||
|
||||
await preview.client.$fetch(
|
||||
`/oauth-proxy-callback?callbackURL=${encodeURIComponent(callbackURL!)}&profile=${encodeURIComponent(encryptedProfile!)}`,
|
||||
{
|
||||
onError(context) {
|
||||
const location = context.response.headers.get("location");
|
||||
expect(location).toContain("error=state_mismatch");
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("cookie state strategy", () => {
|
||||
it("should validate the state cookie during proxy callback", async () => {
|
||||
const proxySecret = "shared-oauth-proxy-secret-for-stateless";
|
||||
const preview = await getTestInstance(
|
||||
{
|
||||
baseURL: "http://preview.example.com",
|
||||
database: undefined,
|
||||
secret: "preview-main-secret-for-stateless",
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
productionURL: "http://localhost:3000",
|
||||
secret: proxySecret,
|
||||
}),
|
||||
],
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: "test",
|
||||
clientSecret: "test",
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
disableTestUser: true,
|
||||
},
|
||||
);
|
||||
const production = await getTestInstance(
|
||||
{
|
||||
baseURL: "http://localhost:3000",
|
||||
database: undefined,
|
||||
secret: "production-main-secret-for-stateless",
|
||||
plugins: [
|
||||
oAuthProxy({
|
||||
secret: proxySecret,
|
||||
}),
|
||||
],
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: "test",
|
||||
clientSecret: "test",
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
disableTestUser: true,
|
||||
},
|
||||
);
|
||||
|
||||
const previewHeaders = new Headers();
|
||||
const res = await preview.client.signIn.social(
|
||||
{
|
||||
provider: "google",
|
||||
callbackURL: "/dashboard",
|
||||
},
|
||||
{
|
||||
throw: true,
|
||||
onSuccess: preview.cookieSetter(previewHeaders),
|
||||
},
|
||||
);
|
||||
|
||||
const encryptedState = new URL(res.url!).searchParams.get("state");
|
||||
expect(encryptedState).toBeTruthy();
|
||||
|
||||
let encryptedProfile: string | null = null;
|
||||
let callbackURL: string | null = null;
|
||||
await production.client.$fetch(
|
||||
`/callback/google?code=test&state=${encryptedState}`,
|
||||
{
|
||||
onError(context) {
|
||||
const location = context.response.headers.get("location");
|
||||
expect(location).toContain("/oauth-proxy-callback");
|
||||
if (location) {
|
||||
const url = new URL(location);
|
||||
encryptedProfile = url.searchParams.get("profile");
|
||||
callbackURL = url.searchParams.get("callbackURL");
|
||||
}
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(encryptedProfile).toBeTruthy();
|
||||
expect(callbackURL).toBeTruthy();
|
||||
|
||||
const previewCtx = await preview.auth.$context;
|
||||
|
||||
await preview.client.$fetch(
|
||||
`/oauth-proxy-callback?callbackURL=${encodeURIComponent(callbackURL!)}&profile=${encodeURIComponent(encryptedProfile!)}`,
|
||||
{
|
||||
onError(context) {
|
||||
const location = context.response.headers.get("location");
|
||||
expect(location).toContain("error=state_mismatch");
|
||||
},
|
||||
},
|
||||
);
|
||||
expect((await previewCtx.internalAdapter.listUsers()).length).toBe(0);
|
||||
|
||||
await preview.client.$fetch(
|
||||
`/oauth-proxy-callback?callbackURL=${encodeURIComponent(callbackURL!)}&profile=${encodeURIComponent(encryptedProfile!)}`,
|
||||
{
|
||||
headers: previewHeaders,
|
||||
onError(context) {
|
||||
preview.cookieSetter(previewHeaders)(context);
|
||||
const location = context.response.headers.get("location");
|
||||
expect(location).not.toContain("error=");
|
||||
expect(location).toContain("/dashboard");
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
const users = await previewCtx.internalAdapter.listUsers();
|
||||
expect(users.length).toBe(1);
|
||||
expect(users[0]?.email).toBe("user@email.com");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user