From 2d64fe38aa9d375edd2f0a230888ed89c366cc84 Mon Sep 17 00:00:00 2001 From: Bereket Engida <86073083+Bekacru@users.noreply.github.com> Date: Tue, 30 Sep 2025 12:54:15 -0700 Subject: [PATCH] fix(oauth-proxy): should skip state check for oauth proxy (#4991) --- docs/content/docs/plugins/oauth-proxy.mdx | 4 +- packages/better-auth/src/init.ts | 6 ++ packages/better-auth/src/oauth2/state.ts | 11 ++- .../src/plugins/oauth-proxy/index.ts | 40 +++++++++- .../plugins/oauth-proxy/oauth-proxy.test.ts | 76 ++++++++++++++----- 5 files changed, 110 insertions(+), 27 deletions(-) diff --git a/docs/content/docs/plugins/oauth-proxy.mdx b/docs/content/docs/plugins/oauth-proxy.mdx index 6c516dcc70..3bc2beecb0 100644 --- a/docs/content/docs/plugins/oauth-proxy.mdx +++ b/docs/content/docs/plugins/oauth-proxy.mdx @@ -60,8 +60,10 @@ await authClient.signIn.social({ When the OAuth provider returns the user to your server, the plugin automatically redirects them to the intended callback URL. - To share cookies between the proxy server and your main server it uses URL query parameters to pass the cookies encrypted in the URL. This is secure as the cookies are encrypted and can only be decrypted by the server. + + +This plugin requires skipping the state cookie check. This has security implications and should only be used in dev or staging environments. If `baseURL` and `productionURL` are the same, the plugin will not proxy the request. ## Options diff --git a/packages/better-auth/src/init.ts b/packages/better-auth/src/init.ts index f45f052a6e..107183f982 100644 --- a/packages/better-auth/src/init.ts +++ b/packages/better-auth/src/init.ts @@ -189,6 +189,12 @@ export type AuthContext = { appName: string; baseURL: string; trustedOrigins: string[]; + oauthConfig?: { + /** + * This is dangerous and should only be used in dev or staging environments. + */ + skipStateCookieCheck?: boolean; + }; /** * New session that will be set after the request * meaning: there is a `set-cookie` header that will set diff --git a/packages/better-auth/src/oauth2/state.ts b/packages/better-auth/src/oauth2/state.ts index d31c7ab963..464424caeb 100644 --- a/packages/better-auth/src/oauth2/state.ts +++ b/packages/better-auth/src/oauth2/state.ts @@ -101,7 +101,16 @@ export async function parseState(c: GenericEndpointContext) { stateCookie.name, c.context.secret, ); - if (!stateCookieValue || stateCookieValue !== state) { + /** + * This is generally cause security issue and should only be used in + * dev or staging environments. It's currently used by the oauth-proxy + * plugin + */ + const skipStateCookieCheck = c.context.oauthConfig?.skipStateCookieCheck; + if ( + !skipStateCookieCheck && + (!stateCookieValue || stateCookieValue !== state) + ) { const errorURL = c.context.options.onAPIError?.errorURL || `${c.context.baseURL}/error`; throw c.redirect(`${errorURL}?error=state_mismatch`); diff --git a/packages/better-auth/src/plugins/oauth-proxy/index.ts b/packages/better-auth/src/plugins/oauth-proxy/index.ts index de218c445a..505f219136 100644 --- a/packages/better-auth/src/plugins/oauth-proxy/index.ts +++ b/packages/better-auth/src/plugins/oauth-proxy/index.ts @@ -54,6 +54,19 @@ export const oAuthProxy = (opts?: OAuthProxyOptions) => { ); }; + const checkSkipProxy = (ctx: EndpointContext) => { + // if skip proxy header is set, we don't need to proxy + const skipProxy = ctx.request?.headers.get("x-skip-oauth-proxy"); + if (skipProxy) { + return true; + } + const productionURL = opts?.productionURL || env.BETTER_AUTH_URL; + if (productionURL === ctx.context.options.baseURL) { + return true; + } + return false; + }; + return { id: "oauth-proxy", options: opts, @@ -189,6 +202,26 @@ export const oAuthProxy = (opts?: OAuthProxyOptions) => { }, ], before: [ + { + matcher() { + return true; + }, + handler: createAuthMiddleware(async (ctx) => { + const skipProxy = checkSkipProxy(ctx); + if (skipProxy || ctx.path !== "/callback/:id") { + return; + } + return { + context: { + context: { + oauthConfig: { + skipStateCookieCheck: true, + }, + }, + }, + }; + }), + }, { matcher(context) { return ( @@ -197,14 +230,13 @@ export const oAuthProxy = (opts?: OAuthProxyOptions) => { ); }, handler: createAuthMiddleware(async (ctx) => { - // if skip proxy header is set, we don't need to proxy - const skipProxy = ctx.request?.headers.get("x-skip-oauth-proxy"); + const skipProxy = checkSkipProxy(ctx); + console.log("skipProxy", skipProxy); if (skipProxy) { return; } const url = resolveCurrentURL(ctx); - const productionURL = opts?.productionURL || env.BETTER_AUTH_URL; - if (productionURL === ctx.context.options.baseURL) { + if (!ctx.body) { return; } ctx.body.callbackURL = `${url.origin}${ diff --git a/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts b/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts index 3480916de3..d91a1df71b 100644 --- a/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts +++ b/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts @@ -42,21 +42,20 @@ vi.mock("../../oauth2", async (importOriginal) => { }); describe("oauth-proxy", async () => { - const { client, cookieSetter } = await getTestInstance({ - plugins: [ - oAuthProxy({ - currentURL: "http://preview-localhost:3000", - }), - ], - socialProviders: { - google: { - clientId: "test", - clientSecret: "test", - }, - }, - }); - it("should redirect to proxy url", async () => { + const { client, cookieSetter } = await getTestInstance({ + plugins: [ + oAuthProxy({ + currentURL: "http://preview-localhost:3000", + }), + ], + socialProviders: { + google: { + clientId: "test", + clientSecret: "test", + }, + }, + }); const headers = new Headers(); const res = await client.signIn.social( { @@ -65,7 +64,6 @@ describe("oauth-proxy", async () => { }, { throw: true, - onSuccess: cookieSetter(headers), }, ); const state = new URL(res.url!).searchParams.get("state"); @@ -108,7 +106,6 @@ describe("oauth-proxy", async () => { ); const state = new URL(res.url!).searchParams.get("state"); await client.$fetch(`/callback/google?code=test&state=${state}`, { - headers, onError(context) { const location = context.response.headers.get("location"); if (!location) { @@ -121,7 +118,7 @@ describe("oauth-proxy", async () => { }); it("should proxy to the original request url", async () => { - const { client, cookieSetter } = await getTestInstance({ + const { client } = await getTestInstance({ baseURL: "https://myapp.com", plugins: [ oAuthProxy({ @@ -135,7 +132,6 @@ describe("oauth-proxy", async () => { }, }, }); - const headers = new Headers(); const res = await client.signIn.social( { provider: "google", @@ -143,12 +139,10 @@ describe("oauth-proxy", async () => { }, { throw: true, - onSuccess: cookieSetter(headers), }, ); const state = new URL(res.url!).searchParams.get("state"); await client.$fetch(`/callback/google?code=test&state=${state}`, { - headers, onError(context) { const location = context.response.headers.get("location"); if (!location) { @@ -163,10 +157,50 @@ describe("oauth-proxy", async () => { }); }); + it("should require state cookie if it's not in proxy url", async () => { + const { client } = await getTestInstance({ + baseURL: "https://myapp.com", + plugins: [ + oAuthProxy({ + productionURL: "https://myapp.com", + }), + ], + socialProviders: { + google: { + clientId: "test", + clientSecret: "test", + }, + }, + }); + const res = await client.signIn.social( + { + provider: "google", + callbackURL: "/dashboard", + }, + { + throw: true, + }, + ); + const state = new URL(res.url!).searchParams.get("state"); + await client.$fetch(`/callback/google?code=test&state=${state}`, { + onError(context) { + const location = context.response.headers.get("location"); + if (!location) { + throw new Error("Location header not found"); + } + expect(location).toContain("state_mismatch"); + }, + }); + }); + it("shouldn't redirect to proxy url on same origin", async () => { const { client, cookieSetter } = await getTestInstance({ baseURL: "https://myapp.com", - plugins: [oAuthProxy()], + plugins: [ + oAuthProxy({ + productionURL: "https://myapp.com", + }), + ], socialProviders: { google: { clientId: "test",