mirror of
https://github.com/better-auth/better-auth.git
synced 2026-08-26 21:49:22 -05:00
fix(origin-check): support fragments in relative redirect URLs (#10983)
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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()}`,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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,<script>alert('xss')</script>",
|
||||
];
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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=<code>`). 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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
},
|
||||
});
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user