From 1febf5e6042e864c2befe07790e1ceff14cc5af9 Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Mon, 17 Nov 2025 12:30:43 -0800 Subject: [PATCH] fix: sync --- .../better-auth/src/plugins/mcp/authorize.ts | 219 ++++++++++++++---- 1 file changed, 176 insertions(+), 43 deletions(-) diff --git a/packages/better-auth/src/plugins/mcp/authorize.ts b/packages/better-auth/src/plugins/mcp/authorize.ts index 401bf6a9f9..d3f7fb2c20 100644 --- a/packages/better-auth/src/plugins/mcp/authorize.ts +++ b/packages/better-auth/src/plugins/mcp/authorize.ts @@ -8,13 +8,25 @@ import type { Client, OIDCOptions, } from "../oidc-provider/types"; +import { parsePrompt } from "../oidc-provider/utils/prompt"; -function redirectErrorURL(url: string, error: string, description: string) { +function formatErrorURL(url: string, error: string, description: string) { return `${ url.includes("?") ? "&" : "?" }error=${error}&error_description=${description}`; } +function getErrorURL( + ctx: GenericEndpointContext, + error: string, + description: string, +) { + const baseURL = + ctx.context.options.onAPIError?.errorURL || `${ctx.context.baseURL}/error`; + const formattedURL = formatErrorURL(baseURL, error, description); + return formattedURL; +} + export async function authorizeMCPOAuth( ctx: GenericEndpointContext, options: OIDCOptions, @@ -23,6 +35,19 @@ export async function authorizeMCPOAuth( ctx.setHeader("Access-Control-Allow-Methods", "POST, OPTIONS"); ctx.setHeader("Access-Control-Allow-Headers", "Content-Type, Authorization"); ctx.setHeader("Access-Control-Max-Age", "86400"); + + const handleRedirect = (url: string) => { + const fromFetch = ctx.request?.headers.get("sec-fetch-mode") === "cors"; + if (fromFetch) { + return ctx.json({ + redirect: true, + url, + }); + } else { + throw ctx.redirect(url); + } + }; + const opts = { codeExpiresIn: 600, defaultScope: "openid", @@ -43,6 +68,19 @@ export async function authorizeMCPOAuth( } 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) { + return handleRedirect( + formatErrorURL( + query.redirect_uri, + "login_required", + "Authentication required but prompt is none", + ), + ); + } + /** * If the user is not logged in, we need to redirect them to the * login page. @@ -58,22 +96,26 @@ export async function authorizeMCPOAuth( }, ); const queryFromURL = ctx.request.url?.split("?")[1]!; - throw ctx.redirect(`${options.loginPage}?${queryFromURL}`); + return handleRedirect(`${options.loginPage}?${queryFromURL}`); } const query = ctx.query as AuthorizationQuery; if (!query.client_id) { - throw ctx.redirect(`${ctx.context.baseURL}/error?error=invalid_client`); + const errorURL = getErrorURL( + ctx, + "invalid_client", + "client_id is required", + ); + throw ctx.redirect(errorURL); } if (!query.response_type) { - throw ctx.redirect( - redirectErrorURL( - `${ctx.context.baseURL}/error`, - "invalid_request", - "response_type is required", - ), + const errorURL = getErrorURL( + ctx, + "invalid_request", + "response_type is required", ); + throw ctx.redirect(errorURL); } const client = await ctx.context.adapter @@ -97,7 +139,12 @@ export async function authorizeMCPOAuth( } as Client; }); if (!client) { - throw ctx.redirect(`${ctx.context.baseURL}/error?error=invalid_client`); + const errorURL = getErrorURL( + ctx, + "invalid_client", + "client_id is required", + ); + throw ctx.redirect(errorURL); } const redirectURI = client.redirectUrls.find( (url) => url === ctx.query.redirect_uri, @@ -112,13 +159,17 @@ export async function authorizeMCPOAuth( }); } if (client.disabled) { - throw ctx.redirect(`${ctx.context.baseURL}/error?error=client_disabled`); + const errorURL = getErrorURL(ctx, "client_disabled", "client is disabled"); + throw ctx.redirect(errorURL); } if (query.response_type !== "code") { - throw ctx.redirect( - `${ctx.context.baseURL}/error?error=unsupported_response_type`, + const errorURL = getErrorURL( + ctx, + "unsupported_response_type", + "unsupported response type", ); + throw ctx.redirect(errorURL); } const requestScope = @@ -127,8 +178,8 @@ export async function authorizeMCPOAuth( return !opts.scopes.includes(scope); }); if (invalidScopes.length) { - throw ctx.redirect( - redirectErrorURL( + return handleRedirect( + formatErrorURL( query.redirect_uri, "invalid_scope", `The following scopes are invalid: ${invalidScopes.join(", ")}`, @@ -140,12 +191,8 @@ export async function authorizeMCPOAuth( (!query.code_challenge || !query.code_challenge_method) && options.requirePKCE ) { - throw ctx.redirect( - redirectErrorURL( - query.redirect_uri, - "invalid_request", - "pkce is required", - ), + return handleRedirect( + formatErrorURL(query.redirect_uri, "invalid_request", "pkce is required"), ); } @@ -159,8 +206,8 @@ export async function authorizeMCPOAuth( options.allowPlainCodeChallengeMethod ? "plain" : "s256", ].includes(query.code_challenge_method?.toLowerCase() || "") ) { - throw ctx.redirect( - redirectErrorURL( + return handleRedirect( + formatErrorURL( query.redirect_uri, "invalid_request", "invalid code_challenge method", @@ -171,6 +218,69 @@ export async function authorizeMCPOAuth( const code = generateRandomString(32, "a-z", "A-Z", "0-9"); const codeExpiresInMs = opts.codeExpiresIn * 1000; const expiresAt = new Date(Date.now() + codeExpiresInMs); + + // Determine if consent is required + // Consent is ALWAYS required unless: + // 1. The client is trusted (skipConsent = true) + // 2. The user has already consented and prompt is not "consent" + const skipConsentForTrustedClient = client.skipConsent; + const hasAlreadyConsented = await ctx.context.adapter + .findOne<{ + consentGiven: boolean; + }>({ + model: "oauthConsent", + where: [ + { + field: "clientId", + value: client.clientId, + }, + { + field: "userId", + value: session.user.id, + }, + ], + }) + .then((res) => !!res?.consentGiven); + + const promptSet = parsePrompt(query.prompt ?? ""); + + // Handle prompt=none per OIDC spec 3.1.2.1 + // The Authorization Server MUST NOT display any authentication or consent UI + if (promptSet.has("none")) { + // If consent is required, return consent_required error + if (!skipConsentForTrustedClient && !hasAlreadyConsented) { + return handleRedirect( + formatErrorURL( + query.redirect_uri, + "consent_required", + "Consent required but prompt is none", + ), + ); + } + // If we reach here, user is authenticated and consent is satisfied + // Continue without any UI interaction + } + + // Handle max_age parameter per OIDC spec 3.1.2.1 + // max_age=0 is equivalent to prompt=login + let requireLogin = promptSet.has("login"); + if (query.max_age !== undefined) { + const maxAge = Number(query.max_age); + if (Number.isInteger(maxAge) && maxAge >= 0) { + const sessionAge = + (Date.now() - new Date(session.session.createdAt).getTime()) / 1000; + if (sessionAge > maxAge) { + // Session is older than max_age, force reauthentication + requireLogin = true; + } + } + // If max_age is invalid (not a non-negative integer), ignore it per OIDC spec + } + + const requireConsent = + !skipConsentForTrustedClient && + (!hasAlreadyConsented || promptSet.has("consent")); + try { /** * Save the code in the database @@ -183,19 +293,16 @@ export async function authorizeMCPOAuth( userId: session.user.id, authTime: new Date(session.session.createdAt).getTime(), /** - * If the prompt is set to `consent`, then we need - * to require the user to consent to the scopes. + * Consent is required per OIDC spec unless: + * 1. Client is trusted (skipConsent = true) + * 2. User has already consented (and prompt is not "consent") * - * This means the code now needs to be treated as a - * consent request. - * - * once the user consents, the code will be updated - * with the actual code. This is to prevent the - * client from using the code before the user - * consents. + * When consent is required, the code needs to be treated as a + * consent request. Once the user consents, the code will be + * updated with the actual authorization code. */ - requireConsent: query.prompt === "consent", - state: query.prompt === "consent" ? query.state : null, + requireConsent, + state: requireConsent ? query.state : null, codeChallenge: query.code_challenge, codeChallengeMethod: query.code_challenge_method, nonce: query.nonce, @@ -204,8 +311,8 @@ export async function authorizeMCPOAuth( expiresAt, }); } catch (e) { - throw ctx.redirect( - redirectErrorURL( + return handleRedirect( + formatErrorURL( query.redirect_uri, "server_error", "An error occurred while processing the request", @@ -213,15 +320,41 @@ export async function authorizeMCPOAuth( ); } - // Consent is NOT required - redirect with the code immediately - if (query.prompt !== "consent") { + if (requireLogin) { + await ctx.setSignedCookie( + "oidc_login_prompt", + JSON.stringify(ctx.query), + ctx.context.secret, + { + maxAge: 600, + path: "/", + sameSite: "lax", + }, + ); + await ctx.setSignedCookie("oidc_consent_prompt", code, ctx.context.secret, { + maxAge: 600, + path: "/", + sameSite: "lax", + }); + + const loginURI = `${options.loginPage}?${new URLSearchParams({ + client_id: client.clientId, + code, + state: query.state, + }).toString()}`; + return handleRedirect(loginURI); + } + + // If consent is not required, redirect with the code immediately + if (!requireConsent) { const redirectURIWithCode = new URL(redirectURI); redirectURIWithCode.searchParams.set("code", code); redirectURIWithCode.searchParams.set("state", ctx.query.state); - throw ctx.redirect(redirectURIWithCode.toString()); + return handleRedirect(redirectURIWithCode.toString()); } - // Consent is REQUIRED - redirect to consent page or show consent HTML + // Consent is required - redirect to consent page or show consent HTML + if (options?.consentPage) { // Set cookie to support cookie-based consent flows await ctx.setSignedCookie("oidc_consent_prompt", code, ctx.context.secret, { @@ -230,19 +363,19 @@ export async function authorizeMCPOAuth( sameSite: "lax", }); - // Pass the consent code as a URL parameter to support URL-BASED consent flows + // Pass the consent code as a URL parameter to support URL-based consent flows const urlParams = new URLSearchParams(); urlParams.set("consent_code", code); urlParams.set("client_id", client.clientId); urlParams.set("scope", requestScope.join(" ")); const consentURI = `${options.consentPage}?${urlParams.toString()}`; - throw ctx.redirect(consentURI); + return handleRedirect(consentURI); } // No consent page configured - fall back to direct redirect with code const redirectURIWithCode = new URL(redirectURI); redirectURIWithCode.searchParams.set("code", code); redirectURIWithCode.searchParams.set("state", ctx.query.state); - throw ctx.redirect(redirectURIWithCode.toString()); + return handleRedirect(redirectURIWithCode.toString()); }