fix(oauth): reject incomplete authorization links

docs/operations/oauth-signout-playbook.md

@@ -107,6 +107,14 @@ Attributes:
 
 `OAuth authorization failed: Invalid redirect URI` (both GET and POST `/oauth/authorize`) and `Redirect URI not registered for client on callback` now carry `clientId`, `redirectUri`, `registeredUris`, `clientName` as `extra` fields.
 
+`OAuth authorization request rejected: missing redirect URI` identifies GET
+`/oauth/authorize` requests with a non-empty `client_id` and a missing or empty
+`redirect_uri`. These return a local 400 before client metadata lookup, with no
+redirect or Sentry issue. The log carries `code: invalid_request` and
+`reason: missing_redirect_uri`, without the authorization URL. Ask the user to
+restart authentication in their client and open the full link. Metadata fetch
+failures for complete authorization requests still follow the existing error path.
+
 The OAuth provider itself emits high-volume logs shaped like
 `OAuth error response: 401 invalid_token - Invalid access token`. Treat those
 as the provider-level count source; do not add a second high-volume app log for

packages/mcp-cloudflare/src/server/oauth/client-metadata.integration.test.ts

@@ -1,6 +1,6 @@
 import { createExecutionContext, env } from "cloudflare:test";
 import { getOAuthApi } from "@cloudflare/workers-oauth-provider";
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import { SCOPES } from "../../constants";
 import { createPkcePair, PKCE_CODE_CHALLENGE } from "../../test-utils/pkce";
 import app from "../app";
@@ -116,6 +116,48 @@ async function exchangeCode(
 }
 
 describe("pre-registered client metadata integration", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it.each([undefined, ""])(
+    "rejects an incomplete authorization link before fetching metadata: redirect_uri=%j",
+    async (redirectUri) => {
+      const fetchSpy = vi.spyOn(globalThis, "fetch");
+      const response = await handler.fetch!(
+        createAuthorizationRequest(`${CODEX_CLIENT_ID}?other-client=1`, {
+          redirect_uri: redirectUri,
+        }),
+        workerEnv,
+        createExecutionContext(),
+      );
+
+      expect(response.status).toBe(400);
+      expect(response.headers.get("Location")).toBeNull();
+      expect(await response.text()).toContain(
+        "authorization link is incomplete",
+      );
+      expect(fetchSpy).not.toHaveBeenCalled();
+    },
+  );
+
+  it("preserves the missing client ID error for an incomplete authorization link", async () => {
+    const fetchSpy = vi.spyOn(globalThis, "fetch");
+    const response = await handler.fetch!(
+      createAuthorizationRequest(CODEX_CLIENT_ID, {
+        client_id: undefined,
+        redirect_uri: undefined,
+      }),
+      workerEnv,
+      createExecutionContext(),
+    );
+
+    expect(response.status).toBe(400);
+    expect(response.headers.get("Location")).toBeNull();
+    expect(await response.text()).toBe("client_id is required");
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
   it("authorizes Codex and exchanges and refreshes tokens when upstream metadata is blocked", async () => {
     // The network mock returns 403: a fresh provider without pre-registration
     // cannot resolve this client, so an existing cache cannot hide the outage.
@@ -194,6 +236,17 @@ describe("pre-registered client metadata integration", () => {
   it("still fetches unregistered CIMD URLs and rejects an upstream 403", async () => {
     const clientId = `${CODEX_CLIENT_ID}?other-client=1`;
 
+    const fetchSpy = vi.spyOn(globalThis, "fetch");
+    const authorizationResponse = await handler.fetch!(
+      createAuthorizationRequest(clientId),
+      workerEnv,
+      createExecutionContext(),
+    );
+    expect(authorizationResponse.status).toBe(500);
+    expect(authorizationResponse.headers.get("Location")).toBeNull();
+    expect(await authorizationResponse.text()).toBe("Internal Server Error");
+    expect(fetchSpy).toHaveBeenCalled();
+
     await expect(
       createOAuthApi().parseAuthRequest(createAuthorizationRequest(clientId)),
     ).rejects.toThrow("Failed to fetch client metadata: HTTP 403");

packages/mcp-cloudflare/src/server/oauth/routes/authorize.ts

@@ -74,6 +74,23 @@ export default new Hono<{ Bindings: Env }>()
    * This route initiates the OAuth flow when a user wants to log in.
    */
   .get("/", async (c) => {
+    const authUrl = new URL(c.req.url);
+    if (
+      authUrl.searchParams.get("client_id") &&
+      !authUrl.searchParams.get("redirect_uri")
+    ) {
+      // The provider looks up client metadata before checking redirect_uri.
+      // Reject incomplete links first so an upstream failure cannot mask them.
+      logWarn("OAuth authorization request rejected: missing redirect URI", {
+        loggerScope: ["cloudflare", "oauth", "authorize"],
+        extra: { code: "invalid_request", reason: "missing_redirect_uri" },
+      });
+      return c.text(
+        "The authorization link is incomplete. Restart authentication in your client and open the full link.",
+        400,
+      );
+    }
+
     let oauthReqInfo: AuthRequest;
     try {
       oauthReqInfo = await c.env.OAUTH_PROVIDER.parseAuthRequest(c.req.raw);
@@ -88,7 +105,6 @@ export default new Hono<{ Bindings: Env }>()
         if (errorMessage.includes("Invalid redirect URI")) {
           // parseAuthRequest threw before producing a request, so read the
           // attempted values directly from the query string.
-          const authUrl = new URL(c.req.url);
           const attemptedClientId = authUrl.searchParams.get("client_id");
           const attemptedRedirectUri = authUrl.searchParams.get("redirect_uri");
           let registeredUris: string[] | undefined;