Better Auth's MCP authorize bounces an unauthenticated client to /login carrying the original OAuth request as query params, with no returnTo. The login page only honored returnTo, so after sign-in it landed on the dashboard and the authorization never resumed: the MCP client waiting at its callback never got its code. After sign-in, if the URL carries an MCP authorize request, hand it back to /api/auth/mcp/authorize so the now authenticated request issues a code (and the consent shim takes over). The target is the same-origin authorize endpoint, which validates the client and redirect_uri, so it is not an open redirect.
Unified · Split
| @@ -1,6 +1,6 @@ | |||
| 1 | import { describe, expect, it } from "@effect/vitest"; | 1 | import { describe, expect, it } from "@effect/vitest"; |
| 2 | 2 | ||
| 3 | import { isSafeReturnTo, loginPath, safeReturnTo } from "./return-to"; | 3 | import { isSafeReturnTo, loginPath, mcpAuthorizeResumeTarget, safeReturnTo } from "./return-to"; |
| 4 | 4 | ||
| 5 | describe("isSafeReturnTo", () => { | 5 | describe("isSafeReturnTo", () => { |
| 6 | const safe = [ | 6 | const safe = [ |
| @@ -45,6 +45,34 @@ describe("safeReturnTo", () => { · 38 unmodified lines | |||
| 45 | }); | 45 | }); |
| 46 | }); | 46 | }); |
| 47 | 47 | ||
| 48 | describe("mcpAuthorizeResumeTarget", () => { | ||
| 49 | // Better Auth bounces an unauthenticated MCP authorize to /login carrying the | ||
| 50 | // OAuth params; after sign-in we resume by handing them back to the authorize | ||
| 51 | // endpoint so it issues a code (then the consent shim takes over). | ||
| 52 | it("rebuilds the authorize URL from a real MCP authorize redirect", () => { | ||
| 53 | const search = | ||
| 54 | "?response_type=code&client_id=abc&code_challenge=xyz&code_challenge_method=S256" + | ||
| 55 | "&redirect_uri=http%3A%2F%2Flocalhost%3A3118%2Fcallback&state=s1&scope=openid+profile&prompt=consent"; | ||
| 56 | const target = mcpAuthorizeResumeTarget(search); | ||
| 57 | expect(target).not.toBeNull(); | ||
| 58 | expect(target!.startsWith("/api/auth/mcp/authorize?")).toBe(true); | ||
| 59 | const params = new URLSearchParams(target!.split("?")[1]); | ||
| 60 | expect(params.get("client_id")).toBe("abc"); | ||
| 61 | expect(params.get("redirect_uri")).toBe("http://localhost:3118/callback"); | ||
| 62 | expect(params.get("response_type")).toBe("code"); | ||
| 63 | }); | ||
| 64 | |||
| 65 | it("ignores searches that are not an authorize request", () => { | ||
| 66 | expect(mcpAuthorizeResumeTarget("")).toBeNull(); | ||
| 67 | expect(mcpAuthorizeResumeTarget("?returnTo=%2Ftools")).toBeNull(); | ||
| 68 | // response_type alone is not enough: client_id and redirect_uri are required. | ||
| 69 | expect(mcpAuthorizeResumeTarget("?response_type=code&client_id=abc")).toBeNull(); | ||
| 70 | expect( | ||
| 71 | mcpAuthorizeResumeTarget("?response_type=token&client_id=abc&redirect_uri=x"), | ||
| 72 | ).toBeNull(); | ||
| 73 | }); | ||
| 74 | }); | ||
| 75 | |||
| 48 | describe("loginPath", () => { | 76 | describe("loginPath", () => { |
| 49 | it("omits returnTo for the root", () => { | 77 | it("omits returnTo for the root", () => { |
| 50 | expect(loginPath("/")).toBe("/login"); | 78 | expect(loginPath("/")).toBe("/login"); |
| @@ -12,3 +12,23 @@ export const safeReturnTo = (path: string | null | undefined): string | null => · 11 unmodified lines | |||
| 12 | 12 | ||
| 13 | export const loginPath = (returnTo: string): string => | 13 | export const loginPath = (returnTo: string): string => |
| 14 | returnTo === "/" ? "/login" : `/login?returnTo=${encodeURIComponent(returnTo)}`; | 14 | returnTo === "/" ? "/login" : `/login?returnTo=${encodeURIComponent(returnTo)}`; |
| 15 | |||
| 16 | // Better Auth's MCP authorize endpoint redirects an unauthenticated client to | ||
| 17 | // `loginPage` (/login) carrying the original OAuth request as query params | ||
| 18 | // (`response_type=code`, `client_id`, `redirect_uri`, `code_challenge`, ...). | ||
| 19 | // Unlike the integration OAuth callback (which arrives as `returnTo`), there is | ||
| 20 | // no returnTo here: the params ARE the request. After sign-in the login page | ||
| 21 | // must hand control back to the authorize endpoint so the now-authenticated | ||
| 22 | // request issues a code (and, via the consent shim, lands on /mcp-consent). | ||
| 23 | // Given a location search string, return that resume URL when it carries an MCP | ||
| 24 | // authorize request, else null. The target is our own same-origin authorize | ||
| 25 | // endpoint, which validates client_id/redirect_uri, so this is not an open | ||
| 26 | // redirect. | ||
| 27 | const MCP_AUTHORIZE_PATH = "/api/auth/mcp/authorize"; | ||
| 28 | |||
| 29 | export const mcpAuthorizeResumeTarget = (search: string): string | null => { | ||
| 30 | const params = new URLSearchParams(search); | ||
| 31 | if (params.get("response_type") !== "code") return null; | ||
| 32 | if (!params.get("client_id") || !params.get("redirect_uri")) return null; | ||
| 33 | return `${MCP_AUTHORIZE_PATH}?${params.toString()}`; | ||
| 34 | }; | ||
| @@ -5,7 +5,7 @@ import { Input } from "@executor-js/react/components/input"; · 4 unmodified lines | |||
| 5 | import { Label } from "@executor-js/react/components/label"; | 5 | import { Label } from "@executor-js/react/components/label"; |
| 6 | 6 | ||
| 7 | import { authClient } from "./auth-client"; | 7 | import { authClient } from "./auth-client"; |
| 8 | import { safeReturnTo } from "../src/auth/return-to"; | 8 | import { mcpAuthorizeResumeTarget, safeReturnTo } from "../src/auth/return-to"; |
| 9 | 9 | ||
| 10 | // Self-host login: email + password sign-in via Better Auth. On success we | 10 | // Self-host login: email + password sign-in via Better Auth. On success we |
| 11 | // reload so the shared AuthProvider re-reads /account/me and the AuthGate swaps | 11 | // reload so the shared AuthProvider re-reads /account/me and the AuthGate swaps |
| @@ -16,7 +16,15 @@ import { safeReturnTo } from "../src/auth/return-to"; · 4 unmodified lines | |||
| 16 | // redeeming an invite — either the full /join/<code> link, or by entering the | 16 | // redeeming an invite — either the full /join/<code> link, or by entering the |
| 17 | // code here ("Have an invite code?"), which forwards to the same join page. | 17 | // code here ("Have an invite code?"), which forwards to the same join page. |
| 18 | export const LoginPage = () => { | 18 | export const LoginPage = () => { |
| 19 | const returnTo = safeReturnTo(new URLSearchParams(window.location.search).get("returnTo")) ?? "/"; | 19 | const search = window.location.search; |
| 20 | // Where to go after sign-in: resume an interrupted MCP OAuth authorize if we | ||
| 21 | // arrived from one (Better Auth redirects it here with the OAuth params), | ||
| 22 | // otherwise honor a safe returnTo (e.g. an integration OAuth callback), else | ||
| 23 | // land on the dashboard. | ||
| 24 | const postLogin = | ||
| 25 | mcpAuthorizeResumeTarget(search) ?? | ||
| 26 | safeReturnTo(new URLSearchParams(search).get("returnTo")) ?? | ||
| 27 | "/"; | ||
| 20 | const [mode, setMode] = useState<"signin" | "code">("signin"); | 28 | const [mode, setMode] = useState<"signin" | "code">("signin"); |
| 21 | const [email, setEmail] = useState(""); | 29 | const [email, setEmail] = useState(""); |
| 22 | const [password, setPassword] = useState(""); | 30 | const [password, setPassword] = useState(""); |
| @@ -34,7 +42,7 @@ export const LoginPage = () => { · 11 unmodified lines | |||
| 34 | setError(result.error.message ?? "Sign in failed"); | 42 | setError(result.error.message ?? "Sign in failed"); |
| 35 | return; | 43 | return; |
| 36 | } | 44 | } |
| 37 | window.location.href = returnTo; | 45 | window.location.href = postLogin; |
| 38 | }; | 46 | }; |
| 39 | 47 | ||
| 40 | const redeem = (event: FormEvent) => { | 48 | const redeem = (event: FormEvent) => { |