github-starred/better-auth
2
0
Fork 0
mirror of https://github.com/better-auth/better-auth.git synced 2026-07-17 08:20:03 -05:00
Code Issues 2.5k Packages Projects Releases 112 Wiki Activity

fix(oauth): validate redirect_uri schemes in oidc-provider and mcp (#9838)

Browse Source
This commit is contained in:
Gustavo Valverde
2026-05-31 11:38:52 +01:00
committed by GitHub
parent df9e8b2951
commit be32012ca3
12 changed files with 240 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
.changeset/oauth-redirect-uri-scheme-validation.md Normal file
View File

@@ -0,0 +1,11 @@
---
"better-auth": patch
---
Validate the scheme of OAuth `redirect_uris` in the `oidc-provider` and `mcp` plugins.
Both plugins previously accepted any string as a `redirect_uri` at registration. They now reject the `javascript:`, `data:`, and `vbscript:` schemes, which are never valid OAuth redirect targets. The `@better-auth/oauth-provider` package already applied this check, so this change brings the two older plugins in line with it.
The redirect-URI scheme policy now lives in `@better-auth/core` as a single `SafeUrlSchema` and an `isSafeUrlScheme` helper, and the OAuth provider plugins share that one implementation. The client navigation helpers (`redirectPlugin`, one-tap, and two-factor) also skip navigation when the target uses one of these schemes.
The change is non-breaking. The `http`, `https`, loopback, and custom application schemes still register unchanged. Both `oidc-provider` and `mcp` are on the migration path to `@better-auth/oauth-provider`, which remains the route to its stricter HTTPS-or-loopback policy.

1
.cspell/tech-terms.txt
View File

@@ -65,3 +65,4 @@ Oligo
febf
fdff
fffe
vbscript

7
packages/better-auth/src/client/fetch-plugins.ts
View File

@@ -1,3 +1,4 @@
import { isSafeUrlScheme } from "@better-auth/core/utils/url";
import type { BetterFetchPlugin } from "@better-fetch/fetch";
export const redirectPlugin = {
@@ -5,7 +6,11 @@ export const redirectPlugin = {
name: "Redirect",
hooks: {
onSuccess(context) {
if (context.data?.url && context.data?.redirect) {
if (
context.data?.url &&
context.data?.redirect &&
isSafeUrlScheme(context.data.url)
) {
if (typeof window !== "undefined" && window.location) {
if (window.location) {
try {

12
packages/better-auth/src/plugins/mcp/index.ts
View File

@@ -9,6 +9,7 @@ import {
} from "@better-auth/core/api";
import { isProduction, logger } from "@better-auth/core/env";
import { safeJSONParse } from "@better-auth/core/utils/json";
import { isSafeUrlScheme } from "@better-auth/core/utils/url";
import { getWebcryptoSubtle } from "@better-auth/utils";
import { base64 } from "@better-auth/utils/base64";
import { createHash } from "@better-auth/utils/hash";
@@ -129,7 +130,16 @@ export const getMCPProtectedResourceMetadata = (
};
const registerMcpClientBodySchema = z.object({
redirect_uris: z.array(z.string()),
// This plugin is migrating to @better-auth/oauth-provider (see the deprecation
// notice in docs/plugins/mcp). It gets only the non-breaking guard that rejects
// code-execution schemes here; full https-or-loopback parity comes from
// @better-auth/oauth-provider's SafeUrlSchema, not from tightening this plugin.
redirect_uris: z.array(
z.string().refine(isSafeUrlScheme, {
message:
"redirect_uri cannot use a javascript:, data:, or vbscript: scheme",
}),
),
token_endpoint_auth_method: z
.enum(["none", "client_secret_basic", "client_secret_post"])
.default("client_secret_basic")

21
packages/better-auth/src/plugins/oidc-provider/index.ts
View File

@@ -8,6 +8,7 @@ import {
} from "@better-auth/core/api";
import { getCurrentAuthContext } from "@better-auth/core/context";
import { deprecate } from "@better-auth/core/utils/deprecate";
import { isSafeUrlScheme } from "@better-auth/core/utils/url";
import { base64 } from "@better-auth/utils/base64";
import { createHash } from "@better-auth/utils/hash";
import type { OpenAPIParameter } from "better-call";
@@ -144,10 +145,22 @@ const oAuthConsentBodySchema = z.object({
const oAuth2TokenBodySchema = z.record(z.any(), z.any());
const registerOAuthApplicationBodySchema = z.object({
redirect_uris: z.array(z.string()).meta({
description:
'A list of redirect URIs. Eg: ["https://client.example.com/callback"]',
}),
// This plugin is deprecated and removed in the next major. It gets only the
// non-breaking guard that rejects code-execution schemes (javascript:, data:,
// vbscript:). The stricter https-or-loopback policy lives in
// @better-auth/oauth-provider via SafeUrlSchema; migrating there is the path to
// full parity, so we do not tighten this plugin further.
redirect_uris: z
.array(
z.string().refine(isSafeUrlScheme, {
message:
"redirect_uri cannot use a javascript:, data:, or vbscript: scheme",
}),
)
.meta({
description:
'A list of redirect URIs. Eg: ["https://client.example.com/callback"]',
}),
token_endpoint_auth_method: z
.enum(["none", "client_secret_basic", "client_secret_post"])
.meta({

57
packages/better-auth/src/plugins/oidc-provider/redirect-uri.test.ts Normal file
View File

@@ -0,0 +1,57 @@
/**
* @see https://github.com/better-auth/better-auth/security/advisories/GHSA-86j7-9j95-vpqj
*/
import { describe, expect, it } from "vitest";
import { createAuthClient } from "../../client";
import { getTestInstance } from "../../test-utils/test-instance";
import { oidcProvider } from ".";
import { oidcClient } from "./client";
async function getClient() {
const { customFetchImpl, signInWithTestUser } = await getTestInstance({
baseURL: "http://localhost:3000",
plugins: [
oidcProvider({
loginPage: "/login",
consentPage: "/consent",
}),
],
});
const { headers } = await signInWithTestUser();
return createAuthClient({
plugins: [oidcClient()],
baseURL: "http://localhost:3000",
fetchOptions: { customFetchImpl, headers },
});
}
describe("oidc-provider redirect_uri scheme validation", () => {
it("rejects javascript:, data:, and vbscript: redirect URIs at registration", async () => {
const client = await getClient();
for (const uri of [
"javascript:fetch('/api/auth/get-session')//",
"data:text/html,<script>alert(1)</script>",
"vbscript:msgbox(1)",
]) {
const reg = await client.oauth2.register({
client_name: "Evil App",
redirect_uris: [uri],
});
expect(reg.error?.status).toBe(400);
expect(reg.data?.client_id).toBeUndefined();
}
});
it("still accepts https and loopback http redirect URIs", async () => {
const client = await getClient();
const reg = await client.oauth2.register({
client_name: "Good App",
redirect_uris: [
"https://client.example.com/callback",
"http://localhost:3000/callback",
],
});
expect(reg.error).toBeNull();
expect(reg.data?.client_id).toBeDefined();
});
});

11
packages/better-auth/src/plugins/one-tap/client.ts
View File

@@ -3,6 +3,7 @@ import type {
BetterAuthClientPlugin,
ClientFetchOption,
} from "@better-auth/core";
import { isSafeUrlScheme } from "@better-auth/core/utils/url";
import { PACKAGE_VERSION } from "../../version";
declare global {
@@ -257,7 +258,10 @@ export const oneTapClient = (options: GoogleOneTapOptions) => {
});
if ((!opts?.fetchOptions && !fetchOptions) || opts?.callbackURL) {
window.location.href = opts?.callbackURL ?? "/";
const target = opts?.callbackURL ?? "/";
if (isSafeUrlScheme(target)) {
window.location.href = target;
}
}
}
@@ -303,7 +307,10 @@ export const oneTapClient = (options: GoogleOneTapOptions) => {
});
if ((!opts?.fetchOptions && !fetchOptions) || opts?.callbackURL) {
window.location.href = opts?.callbackURL ?? "/";
const target = opts?.callbackURL ?? "/";
if (isSafeUrlScheme(target)) {
window.location.href = target;
}
}
}

7
packages/better-auth/src/plugins/two-factor/client.ts
View File

@@ -1,4 +1,5 @@
import type { BetterAuthClientPlugin } from "@better-auth/core";
import { isSafeUrlScheme } from "@better-auth/core/utils/url";
import { PACKAGE_VERSION } from "../../version";
import type { twoFactor as twoFa } from ".";
import { TWO_FACTOR_ERROR_CODES } from "./error-code";
@@ -68,7 +69,11 @@ export const twoFactorClient = (
}
// fallback for when `onTwoFactorRedirect` is not used and only `twoFactorPage` is provided
if (options?.twoFactorPage && typeof window !== "undefined") {
if (
options?.twoFactorPage &&
typeof window !== "undefined" &&
isSafeUrlScheme(options.twoFactorPage)
) {
window.location.href = options.twoFactorPage;
}
}

45
packages/core/src/utils/redirect-uri.ts Normal file
View File

@@ -0,0 +1,45 @@
import * as z from "zod";
import { isLoopbackHost } from "./host";
import { DANGEROUS_URL_SCHEMES } from "./url";
/**
* Zod schema for OAuth redirect URIs and other developer-supplied URLs that the
* server stores and later hands back to a browser.
*
* - Rejects dangerous schemes (`javascript:`, `data:`, `vbscript:`).
* - Requires HTTPS, except for loopback hosts (`127.0.0.0/8`, `[::1]`,
* `*.localhost` per RFC 6761), where HTTP is allowed for local development.
* - Allows custom schemes for mobile apps (e.g. `myapp://callback`).
*
* This is the single source of truth for redirect-URI validation across the
* OAuth provider plugins. Consume it from `@better-auth/core/utils/redirect-uri`
* rather than re-implementing the scheme policy per plugin.
*/
export const SafeUrlSchema = z.url().superRefine((val, ctx) => {
if (!URL.canParse(val)) {
ctx.addIssue({
code: "custom",
message: "URL must be parseable",
fatal: true,
});
return z.NEVER;
}
const u = new URL(val);
if (DANGEROUS_URL_SCHEMES.includes(u.protocol)) {
ctx.addIssue({
code: "custom",
message: "URL cannot use javascript:, data:, or vbscript: scheme",
});
return;
}
if (u.protocol === "http:" && !isLoopbackHost(u.host)) {
ctx.addIssue({
code: "custom",
message:
"Redirect URI must use HTTPS (HTTP allowed only for loopback hosts)",
});
}
});

49
packages/core/src/utils/url.test.ts Normal file
View File

@@ -0,0 +1,49 @@
import { describe, expect, it } from "vitest";
import { SafeUrlSchema } from "./redirect-uri";
import { isSafeUrlScheme } from "./url";
describe("isSafeUrlScheme", () => {
it("rejects code-execution schemes", () => {
expect(isSafeUrlScheme("javascript:alert(1)")).toBe(false);
expect(isSafeUrlScheme("data:text/html,<script>alert(1)</script>")).toBe(
false,
);
expect(isSafeUrlScheme("vbscript:msgbox(1)")).toBe(false);
});
it("normalizes the scheme before checking (mixed case is still blocked)", () => {
expect(isSafeUrlScheme("JavaScript:alert(1)")).toBe(false);
expect(isSafeUrlScheme("JAVASCRIPT:alert(1)")).toBe(false);
});
it("allows http(s), relative paths, and custom app schemes", () => {
expect(isSafeUrlScheme("https://example.com/callback")).toBe(true);
expect(isSafeUrlScheme("http://localhost:3000/callback")).toBe(true);
expect(isSafeUrlScheme("/dashboard")).toBe(true);
expect(isSafeUrlScheme("myapp://callback")).toBe(true);
});
});
describe("SafeUrlSchema", () => {
it("rejects dangerous schemes", () => {
expect(SafeUrlSchema.safeParse("javascript:alert(1)").success).toBe(false);
expect(SafeUrlSchema.safeParse("data:text/html,x").success).toBe(false);
expect(SafeUrlSchema.safeParse("vbscript:x").success).toBe(false);
});
it("requires https for non-loopback hosts", () => {
expect(SafeUrlSchema.safeParse("http://example.com/cb").success).toBe(
false,
);
expect(SafeUrlSchema.safeParse("https://example.com/cb").success).toBe(
true,
);
});
it("allows http for loopback hosts", () => {
expect(SafeUrlSchema.safeParse("http://localhost:3000/cb").success).toBe(
true,
);
expect(SafeUrlSchema.safeParse("http://127.0.0.1/cb").success).toBe(true);
});
});

25
packages/core/src/utils/url.ts
View File

@@ -41,3 +41,28 @@ export function normalizePathname(
return pathname;
}
/**
* Schemes that execute or embed code when navigated to or accepted as a
* redirect target. These are never safe as an OAuth `redirect_uri` or as a
* client-side navigation target (`window.location.href`, `location.assign`, ...).
*/
export const DANGEROUS_URL_SCHEMES = ["javascript:", "data:", "vbscript:"];
/**
* Returns `false` only when `value` is an absolute URL using a dangerous scheme
* (`javascript:`, `data:`, `vbscript:`). Relative URLs (e.g. `/dashboard`) and
* safe absolute schemes (`http`, `https`, custom app schemes such as
* `myapp://`) return `true`.
*
* Use this to guard browser navigation sinks and any redirect target that may
* originate from untrusted input. It is intentionally narrow: it blocks code
* execution schemes without rejecting relative paths or mobile deep links.
*/
export function isSafeUrlScheme(value: string): boolean {
if (!URL.canParse(value)) {
// Relative URLs carry no scheme to abuse.
return true;
}
return !DANGEROUS_URL_SCHEMES.includes(new URL(value).protocol);
}

38
packages/oauth-provider/src/types/zod.ts
View File

@@ -1,8 +1,5 @@
import { isLoopbackHost } from "@better-auth/core/utils/host";
import * as z from "zod";
const DANGEROUS_SCHEMES = ["javascript:", "data:", "vbscript:"];
/**
* Runtime schema for OAuthAuthorizationQuery.
* Uses passthrough to tolerate fields added by future extensions (PAR, FPA, etc.)
@@ -45,36 +42,7 @@ export const verificationValueSchema = z
.passthrough();
/**
* Reusable URL validation for OAuth redirect URIs.
* - Blocks dangerous schemes (javascript:, data:, vbscript:)
* - For http/https: requires HTTPS (HTTP allowed only for loopback hosts: 127.0.0.0/8, [::1], *.localhost per RFC 6761)
* - Allows custom schemes for mobile apps (e.g., myapp://callback)
* Re-exported from `@better-auth/core` so every OAuth provider plugin shares one
* redirect-URI scheme policy. See `@better-auth/core/utils/redirect-uri`.
*/
export const SafeUrlSchema = z.url().superRefine((val, ctx) => {
if (!URL.canParse(val)) {
ctx.addIssue({
code: "custom",
message: "URL must be parseable",
fatal: true,
});
return z.NEVER;
}
const u = new URL(val);
if (DANGEROUS_SCHEMES.includes(u.protocol)) {
ctx.addIssue({
code: "custom",
message: "URL cannot use javascript:, data:, or vbscript: scheme",
});
return;
}
if (u.protocol === "http:" && !isLoopbackHost(u.host)) {
ctx.addIssue({
code: "custom",
message:
"Redirect URI must use HTTPS (HTTP allowed only for loopback hosts)",
});
}
});
export { SafeUrlSchema } from "@better-auth/core/utils/redirect-uri";