From 79904f0be8fadb939743e90a61bbfee207c152e1 Mon Sep 17 00:00:00 2001
From: Taesu <166604494+bytaesu@users.noreply.github.com>
Date: Tue, 25 Aug 2026 10:12:21 +0000
Subject: [PATCH] fix(origin-check): support fragments in relative redirect
URLs (#10983)
---
.changeset/parse-relative-redirect-urls.md | 5 +-
AGENTS.md | 11 ++
docs/content/docs/reference/security.mdx | 2 +-
.../better-auth/src/api/routes/callback.ts | 20 ++-
.../src/api/routes/email-verification.ts | 9 +-
packages/better-auth/src/api/routes/error.ts | 13 +-
.../src/auth/trusted-origins.test.ts | 7 +-
.../better-auth/src/auth/trusted-origins.ts | 9 +-
packages/better-auth/src/oauth2/errors.ts | 6 +-
packages/better-auth/src/oauth2/state.test.ts | 34 +++++
packages/better-auth/src/oauth2/state.ts | 2 +-
packages/better-auth/src/social.test.ts | 8 +-
packages/core/src/utils/url.test.ts | 74 ++++++++-
packages/core/src/utils/url.ts | 43 ++++++
packages/oauth-provider/src/authorize.test.ts | 16 +-
packages/oauth-provider/src/authorize.ts | 3 +-
packages/sso/src/oidc.test.ts | 10 +-
packages/sso/src/routes/sso.ts | 141 ++++++++----------
18 files changed, 292 insertions(+), 121 deletions(-)
diff --git a/.changeset/parse-relative-redirect-urls.md b/.changeset/parse-relative-redirect-urls.md
index f3456c87a9..cacedd52e4 100644
--- a/.changeset/parse-relative-redirect-urls.md
+++ b/.changeset/parse-relative-redirect-urls.md
@@ -1,5 +1,8 @@
---
"better-auth": patch
+"@better-auth/core": patch
+"@better-auth/oauth-provider": patch
+"@better-auth/sso": patch
---
-Allow relative callback and redirect URLs to use standard path and query syntax while preserving open-redirect protections.
+Allow relative callback and redirect URLs to use standard path, query, and fragment syntax while preserving open-redirect protections.
diff --git a/AGENTS.md b/AGENTS.md
index 969fb0bb05..2d0dd78cde 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -32,6 +32,17 @@ This is the Better Auth repository - a comprehensive authentication framework fo
- JSDoc comments for public APIs
- Plugins should be as independent as possible. When working on a plugin, prefer modifying the plugin over changing core.
+### URL Composition
+
+- When appending query parameters to callback or redirect URLs, use `appendQueryParams` from `@better-auth/core/utils/url`. Keep origin and trust validation separate.
+
+```ts
+const params = new URLSearchParams({ error });
+const redirectURL = appendQueryParams(errorURL, params);
+
+throw ctx.redirect(redirectURL);
+```
+
### Placeholder Emails
`User.email` is currently required and unique, which is a limitation of the current architecture.
diff --git a/docs/content/docs/reference/security.mdx b/docs/content/docs/reference/security.mdx
index 4bd9c5e7e5..72bb03bbc5 100644
--- a/docs/content/docs/reference/security.mdx
+++ b/docs/content/docs/reference/security.mdx
@@ -221,7 +221,7 @@ If you're serving your app from multiple approved domains, you'll typically want
Trusted origins prevent CSRF attacks and block open redirects. You can set a list of trusted origins in the `trustedOrigins` configuration option. Requests from origins not on this list are automatically blocked.
-Relative callback and redirect URLs support standard path and query syntax, but must begin with a single `/`. Better Auth rejects fragments, protocol-relative URLs (`//...`), backslashes, control characters, and encoded path separators in the path.
+Relative callback and redirect URLs support standard path, query, and fragment syntax, but must begin with a single `/`. Better Auth rejects protocol-relative URLs (`//...`), backslashes, control characters, and encoded path separators in the path.
### Basic Usage
diff --git a/packages/better-auth/src/api/routes/callback.ts b/packages/better-auth/src/api/routes/callback.ts
index 4aec78d01e..72629c3a93 100644
--- a/packages/better-auth/src/api/routes/callback.ts
+++ b/packages/better-auth/src/api/routes/callback.ts
@@ -3,6 +3,7 @@ import type { AccountKey } from "@better-auth/core/db";
import type { OAuth2Tokens } from "@better-auth/core/oauth2";
import { mergeScopes } from "@better-auth/core/oauth2";
import { safeJSONParse } from "@better-auth/core/utils/json";
+import { appendQueryParams } from "@better-auth/core/utils/url";
import * as z from "zod";
import { getAwaitableValue } from "../../context/helpers";
import { setSessionCookie } from "../../cookies";
@@ -86,7 +87,12 @@ export const callbackOAuth = createAuthEndpoint(
}
} catch (e) {
c.context.logger.error("INVALID_CALLBACK_REQUEST", e);
- throw c.redirect(`${defaultErrorURL}?error=invalid_callback_request`);
+ const params = new URLSearchParams({
+ error: "invalid_callback_request",
+ });
+ const redirectURL = appendQueryParams(defaultErrorURL, params);
+
+ throw c.redirect(redirectURL);
}
const {
@@ -120,9 +126,10 @@ export const callbackOAuth = createAuthEndpoint(
if (!state) {
c.context.logger.error("State not found", error);
- const sep = defaultErrorURL.includes("?") ? "&" : "?";
- const url = `${defaultErrorURL}${sep}error=state_not_found`;
- throw c.redirect(url);
+ const params = new URLSearchParams({ error: "state_not_found" });
+ const redirectURL = appendQueryParams(defaultErrorURL, params);
+
+ throw c.redirect(redirectURL);
}
const {
@@ -141,10 +148,9 @@ export const callbackOAuth = createAuthEndpoint(
const params = new URLSearchParams({ error });
if (description) params.set("error_description", description);
- const sep = baseURL.includes("?") ? "&" : "?";
- const url = `${baseURL}${sep}${params.toString()}`;
+ const redirectURL = appendQueryParams(baseURL, params);
- throw c.redirect(url);
+ throw c.redirect(redirectURL);
}
if (error) {
diff --git a/packages/better-auth/src/api/routes/email-verification.ts b/packages/better-auth/src/api/routes/email-verification.ts
index 873a39a0a4..367538266b 100644
--- a/packages/better-auth/src/api/routes/email-verification.ts
+++ b/packages/better-auth/src/api/routes/email-verification.ts
@@ -1,6 +1,7 @@
import type { GenericEndpointContext } from "@better-auth/core";
import { createAuthEndpoint } from "@better-auth/core/api";
import { APIError, BASE_ERROR_CODES } from "@better-auth/core/error";
+import { appendQueryParams } from "@better-auth/core/utils/url";
import type { JWTPayload, JWTVerifyResult } from "jose";
import { jwtVerify } from "jose";
import { JWTExpired } from "jose/errors";
@@ -291,10 +292,10 @@ export const verifyEmail = createAuthEndpoint(
async (ctx) => {
function redirectOnError(error: { code: string; message: string }) {
if (ctx.query.callbackURL) {
- if (ctx.query.callbackURL.includes("?")) {
- throw ctx.redirect(`${ctx.query.callbackURL}&error=${error.code}`);
- }
- throw ctx.redirect(`${ctx.query.callbackURL}?error=${error.code}`);
+ const params = new URLSearchParams({ error: error.code });
+ const redirectURL = appendQueryParams(ctx.query.callbackURL, params);
+
+ throw ctx.redirect(redirectURL);
}
throw APIError.from("UNAUTHORIZED", error);
}
diff --git a/packages/better-auth/src/api/routes/error.ts b/packages/better-auth/src/api/routes/error.ts
index a67ab84f1b..8133b56766 100644
--- a/packages/better-auth/src/api/routes/error.ts
+++ b/packages/better-auth/src/api/routes/error.ts
@@ -1,6 +1,7 @@
import type { BetterAuthOptions } from "@better-auth/core";
import { createAuthEndpoint } from "@better-auth/core/api";
import { isProduction } from "@better-auth/core/env";
+import { appendQueryParams } from "@better-auth/core/utils/url";
import { HIDE_METADATA } from "../../utils/hide-metadata";
function sanitize(input: string): string {
@@ -406,20 +407,22 @@ export const error = createAuthEndpoint(
? sanitize(unsanitizedDescription)
: null;
- const queryParams = new URLSearchParams();
- queryParams.set("error", safeCode);
+ const params = new URLSearchParams();
+ params.set("error", safeCode);
if (unsanitizedDescription) {
- queryParams.set("error_description", unsanitizedDescription);
+ params.set("error_description", unsanitizedDescription);
}
const options = c.context.options;
const errorURL = options.onAPIError?.errorURL;
if (errorURL) {
+ const redirectURL = appendQueryParams(errorURL, params);
+
return new Response(null, {
status: 302,
headers: {
- Location: `${errorURL}${errorURL.includes("?") ? "&" : "?"}${queryParams.toString()}`,
+ Location: redirectURL,
},
});
}
@@ -428,7 +431,7 @@ export const error = createAuthEndpoint(
return new Response(null, {
status: 302,
headers: {
- Location: `/?${queryParams.toString()}`,
+ Location: `/?${params.toString()}`,
},
});
}
diff --git a/packages/better-auth/src/auth/trusted-origins.test.ts b/packages/better-auth/src/auth/trusted-origins.test.ts
index 2ee131d175..de5db03531 100644
--- a/packages/better-auth/src/auth/trusted-origins.test.ts
+++ b/packages/better-auth/src/auth/trusted-origins.test.ts
@@ -235,9 +235,11 @@ describe("trusted origins", () => {
const { isTrustedOrigin } = await createAuthTestInstance();
const relativeURLs = [
"/docs/!$&'()*+,;=:@~",
- "/café/profile",
+ "/café#profile",
"/search?next=/settings?tab=security",
"/callback?next=%2Fdashboard",
+ "/profile#section?tab=security",
+ "/#%2f%2fevil.com",
];
for (const url of relativeURLs) {
@@ -270,6 +272,7 @@ describe("trusted origins", () => {
"/safe/%2F/evil.com",
"/safe/%5c/evil.com",
"/safe/%5C/evil.com",
+ "/%2f/evil.com#section",
`/\\/\\/evil.com`,
"/..%2F..%2Fevil.com",
`/\u0000evil.com`,
@@ -278,8 +281,6 @@ describe("trusted origins", () => {
`/\t/evil.com`,
`/\n/evil.com`,
`/\r/evil.com`,
- "/profile#section",
- "/profile#section?tab=security",
"javascript:alert('xss')",
"data:text/html,",
];
diff --git a/packages/better-auth/src/auth/trusted-origins.ts b/packages/better-auth/src/auth/trusted-origins.ts
index e4d433630e..a03d05edfc 100644
--- a/packages/better-auth/src/auth/trusted-origins.ts
+++ b/packages/better-auth/src/auth/trusted-origins.ts
@@ -74,11 +74,6 @@ const ENCODED_PATH_SEPARATOR_PATTERN = /%2[fF]|%5[cC]/;
* @see https://url.spec.whatwg.org/#concept-basic-url-parser
*/
const isSafeRelativeURL = (value: string): boolean => {
- // Fragments would swallow query parameters appended by downstream redirects.
- if (value.includes("#")) {
- return false;
- }
-
if (
!value.startsWith("/") ||
value.startsWith("//") ||
@@ -88,8 +83,8 @@ const isSafeRelativeURL = (value: string): boolean => {
return false;
}
- const queryStart = value.indexOf("?");
- const path = queryStart === -1 ? value : value.slice(0, queryStart);
+ const pathEnd = value.search(/[?#]/);
+ const path = pathEnd === -1 ? value : value.slice(0, pathEnd);
if (ENCODED_PATH_SEPARATOR_PATTERN.test(path)) {
return false;
}
diff --git a/packages/better-auth/src/oauth2/errors.ts b/packages/better-auth/src/oauth2/errors.ts
index 86d86033d1..d452944b3e 100644
--- a/packages/better-auth/src/oauth2/errors.ts
+++ b/packages/better-auth/src/oauth2/errors.ts
@@ -1,4 +1,5 @@
import type { GenericEndpointContext } from "@better-auth/core";
+import { appendQueryParams } from "@better-auth/core/utils/url";
/**
* Error codes used in OAuth callback redirects (`?error=`). These are
@@ -43,8 +44,9 @@ export function redirectOnError(
): never {
const params = new URLSearchParams({ error });
if (description) params.set("error_description", description);
- const sep = errorURL.includes("?") ? "&" : "?";
- throw ctx.redirect(`${errorURL}${sep}${params.toString()}`);
+ const redirectURL = appendQueryParams(errorURL, params);
+
+ throw ctx.redirect(redirectURL);
}
/**
diff --git a/packages/better-auth/src/oauth2/state.test.ts b/packages/better-auth/src/oauth2/state.test.ts
index a6b8907aa2..87910f9fdc 100644
--- a/packages/better-auth/src/oauth2/state.test.ts
+++ b/packages/better-auth/src/oauth2/state.test.ts
@@ -94,6 +94,24 @@ describe("parseState error mapping", () => {
);
});
+ /**
+ * @see https://github.com/better-auth/better-auth/issues/10022
+ */
+ it("appends error parameters before the URL fragment", async () => {
+ const { StateError } = await import("../state");
+ errorToThrow = new StateError("state_invalid", { code: "state_invalid" });
+
+ const { parseState } = await import("./state");
+ const { ctx, redirectCalls } = createMockContext(
+ "https://example.com/error?source=oauth#retry",
+ );
+ await parseState(ctx as unknown as GenericEndpointContext).catch(() => {});
+
+ expect(redirectCalls[0]).toBe(
+ "https://example.com/error?source=oauth&error=state_invalid#retry",
+ );
+ });
+
/**
* The per-flow `errorCallbackURL` recovered from the state takes precedence
* over the default error page, and the error parameter is appended with the
@@ -116,4 +134,20 @@ describe("parseState error mapping", () => {
"/oauth-error?source=expo&error=state_mismatch",
);
});
+
+ it("falls back to the default error URL when the recovered URL is empty", async () => {
+ const { StateError } = await import("../state");
+ errorToThrow = new StateError("State mismatch", {
+ code: "state_security_mismatch",
+ errorURL: "",
+ });
+
+ const { parseState } = await import("./state");
+ const { ctx, redirectCalls } = createMockContext();
+ await parseState(ctx as unknown as GenericEndpointContext).catch(() => {});
+
+ expect(redirectCalls[0]).toBe(
+ "http://localhost:3000/api/auth/error?error=state_mismatch",
+ );
+ });
});
diff --git a/packages/better-auth/src/oauth2/state.ts b/packages/better-auth/src/oauth2/state.ts
index e435133ae3..50dac50dab 100644
--- a/packages/better-auth/src/oauth2/state.ts
+++ b/packages/better-auth/src/oauth2/state.ts
@@ -100,7 +100,7 @@ export async function parseState(c: GenericEndpointContext) {
error.code === "state_security_mismatch"
? "state_mismatch"
: error.code;
- redirectErrorURL = error.errorURL ?? errorURL;
+ redirectErrorURL = error.errorURL || errorURL;
}
redirectOnError(c, redirectErrorURL, code);
}
diff --git a/packages/better-auth/src/social.test.ts b/packages/better-auth/src/social.test.ts
index 2ad94ed32e..4e2631c6b7 100644
--- a/packages/better-auth/src/social.test.ts
+++ b/packages/better-auth/src/social.test.ts
@@ -319,13 +319,14 @@ describe("Social Providers", async (c) => {
* state-cookie check fails, and it was already origin-validated at sign-in.
*
* @see https://github.com/better-auth/better-auth/issues/5467
+ * @see https://github.com/better-auth/better-auth/issues/10022
*/
it("redirects to the per-flow errorCallbackURL when state validation fails", async () => {
const headers = new Headers();
const signInRes = await client.signIn.social({
provider: "google",
callbackURL: "/callback",
- errorCallbackURL: "/oauth-error",
+ errorCallbackURL: "/oauth-error?source=oauth#retry",
fetchOptions: {
onSuccess: cookieSetter(headers),
},
@@ -343,8 +344,9 @@ describe("Social Providers", async (c) => {
onError(context) {
expect(context.response.status).toBe(302);
const location = context.response.headers.get("location") ?? "";
- expect(location).toContain("/oauth-error");
- expect(location).toContain("error=state_mismatch");
+ expect(location).toContain(
+ "/oauth-error?source=oauth&error=state_mismatch#retry",
+ );
expect(location).not.toContain("/api/auth/error");
},
});
diff --git a/packages/core/src/utils/url.test.ts b/packages/core/src/utils/url.test.ts
index f62635e43b..07e428c847 100644
--- a/packages/core/src/utils/url.test.ts
+++ b/packages/core/src/utils/url.test.ts
@@ -3,7 +3,79 @@ import {
isReverseDomainPrivateUseRedirectUri,
SafeUrlSchema,
} from "./redirect-uri";
-import { isSafeUrlScheme, normalizePathname } from "./url";
+import { appendQueryParams, isSafeUrlScheme, normalizePathname } from "./url";
+
+describe("appendQueryParams", () => {
+ it("should append query parameters before the fragment", () => {
+ const params = new URLSearchParams({ error: "access denied" });
+
+ expect(appendQueryParams("/login#step2", params)).toBe(
+ "/login?error=access+denied#step2",
+ );
+ expect(
+ appendQueryParams("https://example.com/login?lang=ko#step2", params),
+ ).toBe("https://example.com/login?lang=ko&error=access+denied#step2");
+ expect(appendQueryParams("myapp://callback#step2", params)).toBe(
+ "myapp://callback?error=access+denied#step2",
+ );
+ });
+
+ it("should preserve existing query encoding", () => {
+ const params = new URLSearchParams({ error: "access_denied" });
+
+ expect(
+ appendQueryParams("/search?q=hello%20world&next=~#results", params),
+ ).toBe("/search?q=hello%20world&next=~&error=access_denied#results");
+ });
+
+ it("should reuse a trailing query separator", () => {
+ const params = new URLSearchParams({ error: "access_denied" });
+
+ expect(appendQueryParams("/login?source=oauth&#retry", params)).toBe(
+ "/login?source=oauth&error=access_denied#retry",
+ );
+ });
+
+ it("should preserve empty fragment markers", () => {
+ const params = new URLSearchParams({ error: "access_denied" });
+
+ expect(appendQueryParams("/login#", params)).toBe(
+ "/login?error=access_denied#",
+ );
+ expect(appendQueryParams("https://example.com/login#", params)).toBe(
+ "https://example.com/login?error=access_denied#",
+ );
+ });
+
+ it("should preserve backslashes in the query and fragment", () => {
+ const params = new URLSearchParams({ error: "access_denied" });
+
+ expect(appendQueryParams(`/callback?next=\\foo#\\bar`, params)).toBe(
+ `/callback?next=\\foo&error=access_denied#\\bar`,
+ );
+ });
+
+ it("should preserve the input when no parameters are provided", () => {
+ expect(appendQueryParams("/login?#step2", new URLSearchParams())).toBe(
+ "/login?#step2",
+ );
+ });
+
+ it.each([
+ new URLSearchParams({ error: "access_denied" }),
+ new URLSearchParams(),
+ ])("should reject ambiguous relative URLs", (params) => {
+ for (const input of [
+ "//evil.example.com",
+ "//better-auth.invalid/path",
+ `/\\better-auth.invalid/path`,
+ ]) {
+ expect(() => appendQueryParams(input, params)).toThrow(
+ "Expected an absolute or root-relative URL",
+ );
+ }
+ });
+});
describe("isSafeUrlScheme", () => {
it("rejects code-execution schemes", () => {
diff --git a/packages/core/src/utils/url.ts b/packages/core/src/utils/url.ts
index 63326cb297..4c0058b0b8 100644
--- a/packages/core/src/utils/url.ts
+++ b/packages/core/src/utils/url.ts
@@ -48,6 +48,49 @@ export function normalizePathname(
return pathname;
}
+const URL_REFERENCE_ORIGIN = "https://better-auth.invalid";
+
+/**
+ * Appends query parameters before the fragment of an absolute or root-relative URL.
+ * Existing query text is retained without parsing it into name-value pairs.
+ *
+ * This function only composes URLs. Callers must validate untrusted input.
+ *
+ * @throws TypeError if parsing fails or a relative input changes authority.
+ */
+export function appendQueryParams(
+ input: string,
+ params: URLSearchParams,
+): string {
+ const relative = input.startsWith("/");
+ const hasAuthorityPrefix = input.startsWith("//") || input.startsWith("/\\");
+ if (hasAuthorityPrefix) {
+ throw new TypeError("Expected an absolute or root-relative URL");
+ }
+
+ const parsedURL = relative
+ ? new URL(input, URL_REFERENCE_ORIGIN)
+ : new URL(input);
+
+ if (relative && parsedURL.origin !== URL_REFERENCE_ORIGIN) {
+ throw new TypeError("Expected an absolute or root-relative URL");
+ }
+
+ const query = params.toString();
+ if (!query) {
+ return input;
+ }
+
+ const separator = parsedURL.search.endsWith("&") ? "" : "&";
+ parsedURL.search = parsedURL.search
+ ? `${parsedURL.search}${separator}${query}`
+ : query;
+
+ return relative
+ ? parsedURL.href.slice(parsedURL.origin.length)
+ : parsedURL.href;
+}
+
/**
* 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
diff --git a/packages/oauth-provider/src/authorize.test.ts b/packages/oauth-provider/src/authorize.test.ts
index a5ee49aaa0..921f1bd41f 100644
--- a/packages/oauth-provider/src/authorize.test.ts
+++ b/packages/oauth-provider/src/authorize.test.ts
@@ -8,7 +8,7 @@ import { jwt } from "better-auth/plugins/jwt";
import { getTestInstance } from "better-auth/test";
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import * as z from "zod";
-import { validateIssuerUrl } from "./authorize";
+import { formatErrorURL, validateIssuerUrl } from "./authorize";
import { oauthProviderClient } from "./client";
import { oauthProvider } from "./oauth";
import {
@@ -85,6 +85,20 @@ describe("validateIssuerUrl (RFC 9207)", () => {
});
});
+describe("formatErrorURL", () => {
+ it("should append query errors before the URL fragment", () => {
+ expect(
+ formatErrorURL(
+ "/error?source=oauth#retry",
+ "invalid_request",
+ "Missing parameter",
+ ),
+ ).toBe(
+ "/error?source=oauth&error=invalid_request&error_description=Missing+parameter#retry",
+ );
+ });
+});
+
describe("oauth signed query signatures", () => {
afterEach(() => {
vi.unstubAllGlobals();
diff --git a/packages/oauth-provider/src/authorize.ts b/packages/oauth-provider/src/authorize.ts
index c9c5e69248..0a6f9e7eb1 100644
--- a/packages/oauth-provider/src/authorize.ts
+++ b/packages/oauth-provider/src/authorize.ts
@@ -1,6 +1,7 @@
import type { GenericEndpointContext } from "@better-auth/core";
import { isBrowserFetchRequest } from "@better-auth/core/utils/fetch-metadata";
import { isLoopbackHost, isLoopbackIP } from "@better-auth/core/utils/host";
+import { appendQueryParams } from "@better-auth/core/utils/url";
import { getSessionFromCtx } from "better-auth/api";
import { generateRandomString, makeSignature } from "better-auth/crypto";
import type { Verification } from "better-auth/db";
@@ -89,7 +90,7 @@ export function formatErrorURL(
if (mode === "fragment") {
return `${url}#${searchParams.toString()}`;
}
- return `${url}${url.includes("?") ? "&" : "?"}${searchParams.toString()}`;
+ return appendQueryParams(url, searchParams);
}
/**
diff --git a/packages/sso/src/oidc.test.ts b/packages/sso/src/oidc.test.ts
index 1c53050362..7eb26e181e 100644
--- a/packages/sso/src/oidc.test.ts
+++ b/packages/sso/src/oidc.test.ts
@@ -2456,7 +2456,10 @@ describe("SSO OIDC hook rejection redirect", async () => {
fetchOptions: { customFetchImpl },
});
- it("should redirect to cross-origin errorCallbackURL when a session hook throws APIError", async () => {
+ /**
+ * @see https://github.com/better-auth/better-auth/issues/10022
+ */
+ it("should preserve the errorCallbackURL query and fragment when a session hook throws", async () => {
const { headers: adminHeaders } = await signInWithTestUser();
await auth.api.registerSSOProvider({
body: {
@@ -2484,7 +2487,8 @@ describe("SSO OIDC hook rejection redirect", async () => {
const res = await authClient.signIn.sso({
providerId: "hook-reject",
callbackURL: "https://frontend.example.com/dashboard",
- errorCallbackURL: "https://frontend.example.com/auth-error",
+ errorCallbackURL:
+ "https://frontend.example.com/auth-error?source=sso#retry",
fetchOptions: {
throw: true,
onSuccess: cookieSetter(signInHeaders),
@@ -2513,10 +2517,12 @@ describe("SSO OIDC hook rejection redirect", async () => {
const url = new URL(callbackURL);
expect(url.origin).toBe("https://frontend.example.com");
expect(url.pathname).toBe("/auth-error");
+ expect(url.searchParams.get("source")).toBe("sso");
expect(url.searchParams.get("error")).toBe("HOOK_REJECTED");
expect(url.searchParams.get("error_description")).toBe(
"SSO hook rejected this user",
);
+ expect(url.hash).toBe("#retry");
});
});
diff --git a/packages/sso/src/routes/sso.ts b/packages/sso/src/routes/sso.ts
index 45be4b4884..e495625ddd 100644
--- a/packages/sso/src/routes/sso.ts
+++ b/packages/sso/src/routes/sso.ts
@@ -3,6 +3,7 @@ import {
runWithTransaction,
} from "@better-auth/core/context";
import { isAPIError } from "@better-auth/core/utils/is-api-error";
+import { appendQueryParams } from "@better-auth/core/utils/url";
import type {
PrivateKeyJwtSigningAlgorithm,
TokenEndpointAuth,
@@ -1316,7 +1317,10 @@ async function handleOIDCCallback(
const errorURL =
ctx.context.options.onAPIError?.errorURL ||
`${ctx.context.baseURL}/error`;
- throw ctx.redirect(`${errorURL}?error=invalid_state`);
+ const params = new URLSearchParams({ error: "invalid_state" });
+ const redirectURL = appendQueryParams(errorURL, params);
+
+ throw ctx.redirect(redirectURL);
}
const providerReference =
parsedProviderReference ??
@@ -1324,28 +1328,23 @@ async function handleOIDCCallback(
stateData.serverContext?.[SSO_PROVIDER_STATE_KEY],
);
const { callbackURL, errorURL, newUserURL, requestSignUp } = stateData;
- const redirectOIDCError = (error: string, description: string): never => {
+ const redirectOIDCError = (error: string, description?: string): never => {
const baseURL = errorURL || callbackURL;
- const params = new URLSearchParams({
- error,
- error_description: description,
- });
- const separator = baseURL.includes("?") ? "&" : "?";
- throw ctx.redirect(`${baseURL}${separator}${params.toString()}`);
+ const params = new URLSearchParams({ error });
+ if (description) params.set("error_description", description);
+ const redirectURL = appendQueryParams(baseURL, params);
+
+ throw ctx.redirect(redirectURL);
};
if (!code || error) {
- redirectOIDCError(
+ return redirectOIDCError(
error || "invalid_request",
error_description || (error ? error : "authorization_code_not_found"),
);
}
const provider = await resolveOIDCProvider(ctx, options, providerId);
if (!provider) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=provider not found`,
- );
+ return redirectOIDCError("invalid_provider", "provider not found");
}
const acceptedProviderReference =
providerReference ??
@@ -1353,7 +1352,7 @@ async function handleOIDCCallback(
if (
!(await isCurrentSSOProviderReference(provider, acceptedProviderReference))
) {
- redirectOIDCError(
+ return redirectOIDCError(
"invalid_state",
"sso_provider_changed_during_authentication",
);
@@ -1371,11 +1370,7 @@ async function handleOIDCCallback(
let config = provider.oidcConfig;
if (!config) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=provider not found`,
- );
+ return redirectOIDCError("invalid_provider", "provider not found");
}
try {
@@ -1384,17 +1379,9 @@ async function handleOIDCCallback(
);
} catch (error) {
if (error instanceof DiscoveryError) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=discovery_failed&error_description=${encodeURIComponent(error.message)}`,
- );
+ return redirectOIDCError("discovery_failed", error.message);
}
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=discovery_failed&error_description=unexpected_discovery_error`,
- );
+ return redirectOIDCError("discovery_failed", "unexpected_discovery_error");
}
if (!config.scopes) {
config = {
@@ -1404,11 +1391,7 @@ async function handleOIDCCallback(
}
if (!config.tokenEndpoint) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=token_endpoint_not_found`,
- );
+ return redirectOIDCError("invalid_provider", "token_endpoint_not_found");
}
const tokenEndpoint = config.tokenEndpoint;
@@ -1445,11 +1428,7 @@ async function handleOIDCCallback(
}
if (!resolved || (!resolved.privateKeyJwk && !resolved.privateKeyPem)) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=no_private_key_available`,
- );
+ return redirectOIDCError("invalid_provider", "no_private_key_available");
}
const rawAlg = config.privateKeyAlgorithm ?? resolved.algorithm;
@@ -1504,7 +1483,7 @@ async function handleOIDCCallback(
(url) => ctx.context.isTrustedOrigin(url),
);
if (error) {
- redirectOIDCError(
+ return redirectOIDCError(
"invalid_provider",
getOIDCErrorDescription(error, "token_response_error"),
);
@@ -1519,19 +1498,15 @@ async function handleOIDCCallback(
}
ctx.context.logger.error("Error validating authorization code", e);
if (e instanceof DiscoveryError) {
- redirectOIDCError("invalid_provider", e.message);
+ return redirectOIDCError("invalid_provider", e.message);
}
- redirectOIDCError(
+ return redirectOIDCError(
"invalid_provider",
getOIDCErrorDescription(e, "token_response_error"),
);
});
if (!tokenResponse) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=token_response_not_found`,
- );
+ return redirectOIDCError("invalid_provider", "token_response_not_found");
}
type OIDCUserInfo = {
id?: string;
@@ -1558,7 +1533,7 @@ async function handleOIDCCallback(
if (tokenResponse.idToken) {
const jwksEndpoint = config.jwksEndpoint;
if (!jwksEndpoint) {
- redirectOIDCError("invalid_provider", "jwks_endpoint_not_found");
+ return redirectOIDCError("invalid_provider", "jwks_endpoint_not_found");
}
const verified = await validateOIDCIdToken(
tokenResponse.idToken,
@@ -1567,21 +1542,21 @@ async function handleOIDCCallback(
(url) => ctx.context.isTrustedOrigin(url),
).catch((error) => {
if (error instanceof DiscoveryError) {
- redirectOIDCError("invalid_provider", error.message);
+ return redirectOIDCError("invalid_provider", error.message);
}
ctx.context.logger.error(error);
return null;
});
if (!verified) {
- redirectOIDCError("invalid_provider", "token_not_verified");
+ return redirectOIDCError("invalid_provider", "token_not_verified");
}
if (!readStringClaim(verified!.payload, "sub")) {
- redirectOIDCError("invalid_provider", "id_token_subject_missing");
+ return redirectOIDCError("invalid_provider", "id_token_subject_missing");
}
verifiedIdToken = verified!;
}
if (options?.resolveUser && !verifiedIdToken) {
- redirectOIDCError(
+ return redirectOIDCError(
"invalid_provider",
"id_token_required_for_user_resolution",
);
@@ -1599,12 +1574,12 @@ async function handleOIDCCallback(
(url) => ctx.context.isTrustedOrigin(url),
).catch((e) => {
if (e instanceof DiscoveryError) {
- redirectOIDCError("invalid_provider", e.message);
+ return redirectOIDCError("invalid_provider", e.message);
}
throw e;
});
if (userInfoResponse.error) {
- redirectOIDCError(
+ return redirectOIDCError(
"invalid_provider",
userInfoResponse.error.message ||
userInfoResponse.error.statusText ||
@@ -1615,7 +1590,7 @@ async function handleOIDCCallback(
userInfoResponse.data ??
redirectOIDCError("invalid_provider", "userinfo_response_not_found");
if (verifiedIdToken && rawUserInfo.sub !== verifiedIdToken.payload.sub) {
- redirectOIDCError(
+ return redirectOIDCError(
"invalid_provider",
"id_token_userinfo_subject_mismatch",
);
@@ -1659,19 +1634,14 @@ async function handleOIDCCallback(
image: readStringClaim(idToken, mapping.image || "picture"),
};
} else {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=user_info_endpoint_not_found`,
+ return redirectOIDCError(
+ "invalid_provider",
+ "user_info_endpoint_not_found",
);
}
if (!userInfo.email || !userInfo.id) {
- throw ctx.redirect(
- `${
- errorURL || callbackURL
- }?error=invalid_provider&error_description=missing_user_info`,
- );
+ return redirectOIDCError("invalid_provider", "missing_user_info");
}
const userInfoEmail = userInfo.email;
const userInfoId = userInfo.id;
@@ -1813,20 +1783,13 @@ async function handleOIDCCallback(
if (failedAuthentication) {
linked = failedAuthentication;
} else if (isAPIError(e) && e.body?.code) {
- const baseURL = errorURL || callbackURL;
- const params = new URLSearchParams({ error: e.body.code });
- if (e.body.message) params.set("error_description", e.body.message);
- const sep = baseURL.includes("?") ? "&" : "?";
- throw ctx.redirect(`${baseURL}${sep}${params.toString()}`);
+ return redirectOIDCError(e.body.code, e.body.message);
} else {
throw e;
}
}
if (linked.error) {
- const baseURL = errorURL || callbackURL;
- const params = new URLSearchParams({ error: linked.error });
- const sep = baseURL.includes("?") ? "&" : "?";
- throw ctx.redirect(`${baseURL}${sep}${params.toString()}`);
+ return redirectOIDCError(linked.error);
}
const { session, user } = linked.data!;
@@ -2031,7 +1994,10 @@ export const callbackSSOShared = (options?: SSOOptions) => {
const errorURL =
ctx.context.options.onAPIError?.errorURL ||
`${ctx.context.baseURL}/error`;
- throw ctx.redirect(`${errorURL}?error=invalid_state`);
+ const params = new URLSearchParams({ error: "invalid_state" });
+ const redirectURL = appendQueryParams(errorURL, params);
+
+ throw ctx.redirect(redirectURL);
}
const providerReference = parseSSOProviderReference(
@@ -2039,9 +2005,13 @@ export const callbackSSOShared = (options?: SSOOptions) => {
);
if (!providerReference) {
const errorURL = stateData.errorURL || stateData.callbackURL;
- throw ctx.redirect(
- `${errorURL}?error=invalid_state&error_description=missing_sso_provider_reference`,
- );
+ const params = new URLSearchParams({
+ error: "invalid_state",
+ error_description: "missing_sso_provider_reference",
+ });
+ const redirectURL = appendQueryParams(errorURL, params);
+
+ throw ctx.redirect(redirectURL);
}
return handleOIDCCallback(
@@ -2110,7 +2080,10 @@ export const acsEndpoint = (options?: SSOOptions) => {
if (!session?.session) {
const errorURL =
ctx.context.options.onAPIError?.errorURL || `${appOrigin}/error`;
- throw ctx.redirect(`${errorURL}?error=invalid_request`);
+ const params = new URLSearchParams({ error: "invalid_request" });
+ const redirectURL = appendQueryParams(errorURL, params);
+
+ throw ctx.redirect(redirectURL);
}
const relayState = ctx.query?.RelayState as string | undefined;
throw ctx.redirect(
@@ -2242,9 +2215,13 @@ export const sloEndpoint = (options?: SSOOptions) => {
);
if (!samlRequest && !samlResponse) {
- throw ctx.redirect(
- `${safeErrorURL}?error=invalid_request&error_description=missing_logout_data`,
- );
+ const params = new URLSearchParams({
+ error: "invalid_request",
+ error_description: "missing_logout_data",
+ });
+ const redirectURL = appendQueryParams(safeErrorURL, params);
+
+ throw ctx.redirect(redirectURL);
}
const provider = await findSAMLProvider(