diff --git a/packages/better-auth/src/plugins/oidc-provider/authorize.ts b/packages/better-auth/src/plugins/oidc-provider/authorize.ts index f0804f4620..b9c6c8f76d 100644 --- a/packages/better-auth/src/plugins/oidc-provider/authorize.ts +++ b/packages/better-auth/src/plugins/oidc-provider/authorize.ts @@ -3,6 +3,7 @@ import { APIError } from "@better-auth/core/error"; import { isBrowserFetchRequest } from "@better-auth/core/utils/fetch-metadata"; import { getSessionFromCtx } from "../../api"; import { generateRandomString } from "../../crypto"; +import { InvalidClient, InvalidRequest } from "./error"; import { getClient } from "./index"; import type { AuthorizationQuery, OIDCOptions } from "./types"; import { parsePrompt } from "./utils/prompt"; @@ -58,15 +59,38 @@ export async function authorize( error: "invalid_request", }); } + const query = ctx.query as AuthorizationQuery; const session = await getSessionFromCtx(ctx); if (!session) { // Handle prompt=none per OIDC spec - must return error instead of redirecting - const query = ctx.query as AuthorizationQuery; const promptSet = parsePrompt(query.prompt ?? ""); - if (promptSet.has("none") && query.redirect_uri) { + if (promptSet.has("none")) { + if (!query.redirect_uri) { + throw new InvalidRequest( + "redirect_uri is required when prompt=none and must be usable to return errors without displaying UI", + ); + } + if (!query.client_id) { + throw new InvalidClient("client_id is required"); + } + const client = await getClient( + query.client_id, + options.trustedClients || [], + ); + if (!client) { + throw new InvalidClient("client_id is required"); + } + const validRedirectURI = client.redirectUrls.find( + (url) => url === query.redirect_uri, + ); + if (!validRedirectURI) { + throw new InvalidRequest( + "redirect_uri is invalid or not registered for this client", + ); + } return handleRedirect( formatErrorURL( - query.redirect_uri, + validRedirectURI, "login_required", "Authentication required but prompt is none", ), @@ -91,7 +115,6 @@ export async function authorize( return handleRedirect(`${options.loginPage}?${queryFromURL}`); } - const query = ctx.query as AuthorizationQuery; if (!query.client_id) { const errorURL = getErrorURL( ctx, diff --git a/packages/better-auth/src/plugins/oidc-provider/error.ts b/packages/better-auth/src/plugins/oidc-provider/error.ts index 696a803d9d..5d122e39c6 100644 --- a/packages/better-auth/src/plugins/oidc-provider/error.ts +++ b/packages/better-auth/src/plugins/oidc-provider/error.ts @@ -5,9 +5,20 @@ class OIDCProviderError extends APIError {} export class InvalidRequest extends OIDCProviderError { constructor(error_description: string, error_detail?: string) { super("BAD_REQUEST", { - message: "invalid_request", + message: error_description, + error: "invalid_request", error_description, error_detail, }); } } + +export class InvalidClient extends OIDCProviderError { + constructor(error_description: string) { + super("BAD_REQUEST", { + message: error_description, + error: "invalid_client", + error_description, + }); + } +} diff --git a/packages/better-auth/src/plugins/oidc-provider/oidc.test.ts b/packages/better-auth/src/plugins/oidc-provider/oidc.test.ts index 6ee3e60926..ffa1269339 100644 --- a/packages/better-auth/src/plugins/oidc-provider/oidc.test.ts +++ b/packages/better-auth/src/plugins/oidc-provider/oidc.test.ts @@ -460,24 +460,22 @@ describe("oidc", async () => { it("should return login_required error when prompt=none and user not authenticated", async ({ expect, }) => { - // Create an unauthenticated client - const unauthClient = createAuthClient({ - plugins: [oidcClient()], - baseURL: "http://localhost:3000", - fetchOptions: { - customFetchImpl, - }, + // Create an OAuth client + const testClient = await serverClient.oauth2.register({ + client_name: "test-login-required-prompt-none", + redirect_uris: [ + "http://localhost:3000/api/auth/oauth2/callback/login-required", + ], }); + const clientId = testClient.data?.client_id ?? ""; + const redirectUri = testClient.data?.redirect_uris?.[0] ?? ""; // Try to authorize with prompt=none const authUrl = new URL( "http://localhost:3000/api/auth/oauth2/authorize", ); - authUrl.searchParams.set("client_id", application.clientId); - authUrl.searchParams.set( - "redirect_uri", - application.redirectUrls[0] || "", - ); + authUrl.searchParams.set("client_id", clientId); + authUrl.searchParams.set("redirect_uri", redirectUri); authUrl.searchParams.set("response_type", "code"); authUrl.searchParams.set("scope", "openid profile email"); authUrl.searchParams.set("state", "test-state"); @@ -485,13 +483,11 @@ describe("oidc", async () => { authUrl.searchParams.set("code_challenge", "test-challenge"); authUrl.searchParams.set("code_challenge_method", "S256"); - let redirectURI = ""; - await unauthClient.$fetch(authUrl.toString(), { + const response = await customFetchImpl(authUrl.toString(), { method: "GET", - onError(context) { - redirectURI = context.response.headers.get("Location") || ""; - }, + redirect: "manual", }); + const redirectURI = response.headers.get("Location") || ""; expect(redirectURI).toContain("error=login_required"); expect(redirectURI).toContain("error_description=Authentication"); @@ -499,6 +495,57 @@ describe("oidc", async () => { expect(redirectURI).toContain("none"); }); + it("should not redirect to invalid redirect_uri when prompt=none", async ({ + expect, + }) => { + const attackerRedirect = "https://malicious.com/callback"; + const authUrl = new URL( + "http://localhost:3000/api/auth/oauth2/authorize", + ); + authUrl.searchParams.set("client_id", application.clientId); + authUrl.searchParams.set("redirect_uri", attackerRedirect); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "x"); + authUrl.searchParams.set("prompt", "none"); + + const response = await customFetchImpl(authUrl.toString(), { + method: "GET", + redirect: "manual", + }); + + const location = response.headers.get("Location") || ""; + expect(location === null || location === "").not.toContain( + "malicious.com", + ); + expect([400, 302]).toContain(response.status); + }); + + it("should return 400 invalid_request when prompt=none without redirect_uri", async ({ + expect, + }) => { + const authUrl = new URL( + "http://localhost:3000/api/auth/oauth2/authorize", + ); + authUrl.searchParams.set("client_id", application.clientId); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "x"); + authUrl.searchParams.set("prompt", "none"); + // No redirect_uri - must not fall through to login page + + const response = await customFetchImpl(authUrl.toString(), { + method: "GET", + redirect: "manual", + }); + + expect(response.status).toBe(400); + const location = response.headers.get("Location") || ""; + expect(location).not.toContain("/login"); + const body = await response.json().catch(() => ({})); + expect(body.error ?? body.code).toBe("invalid_request"); + }); + it("should return consent_required error when prompt=none and consent needed", async ({ expect, }) => {