Harden MCP portal authentication changes

packages/gatekeeper-mcp-portal/__tests__/account-revision.test.ts

@@ -0,0 +1,54 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import {
+  McpAccountBase,
+  type ConnectedServer,
+  type ConnectOutcome,
+} from "@gadgets/mcp-shared/account";
+import { McpAccount } from "../src/portal.js";
+
+afterEach(() => vi.restoreAllMocks());
+
+describe("McpAccount configuration revision", () => {
+  it("records the current revision before handing an OAuth attempt to its callback", async () => {
+    const values = new Map<string, unknown>([["portalConfigRevision", "old"]]);
+    const ctx = {
+      id: { toString: () => "account" },
+      storage: {
+        kv: {
+          get: (key: string) => values.get(key),
+          put: (key: string, value: unknown) => values.set(key, value),
+          delete: (key: string) => values.delete(key),
+        },
+      },
+      exports: {},
+    };
+    const env = {
+      MCP_PORTAL_URL: "https://portal.example.com/mcp",
+      MCP_PORTAL_AUTH: "oauth",
+    };
+    const server: ConnectedServer = {
+      endpoint: env.MCP_PORTAL_URL,
+      serverId: "portal",
+      serverName: "Portal",
+      provenance: "deployment",
+      auth: "oauth",
+    };
+    const base = McpAccountBase.prototype as unknown as {
+      beginConnect(nonce: string, target: ConnectedServer | null): Promise<ConnectOutcome>;
+    };
+    vi.spyOn(base, "beginConnect")
+      .mockResolvedValue({ kind: "redirect", url: "https://login.example.com" });
+    const account = new McpAccount(ctx as never, env as never);
+    const internals = account as unknown as {
+      awaitingSelection(nonce: string): boolean;
+      server(): ConnectedServer | undefined;
+    };
+    vi.spyOn(internals, "awaitingSelection").mockReturnValue(true);
+    vi.spyOn(internals, "server").mockReturnValue(server);
+
+    await expect(account.beginConnect("nonce", server)).resolves.toMatchObject({ kind: "redirect" });
+
+    expect(values.get("portalConfigRevision")).not.toBe("old");
+  });
+});

packages/gatekeeper-mcp-portal/__tests__/config.test.ts

@@ -140,9 +140,13 @@ describe("portalAuthRequiresReconnect", () => {
     expect(portalAuthRequiresReconnect("token", "oauth")).toBe(true);
   });
 
-  it("allows none and oauth to differ after probing the endpoint", () => {
+  it("allows an oauth-configured portal to prove public during probing", () => {
     expect(portalAuthRequiresReconnect("none", "oauth")).toBe(false);
-    expect(portalAuthRequiresReconnect("oauth", "none")).toBe(false);
+  });
+
+  it("keeps explicitly unauthenticated mode strict", () => {
+    expect(portalAuthRequiresReconnect("oauth", "none")).toBe(true);
+    expect(portalAuthRequiresReconnect("none", "none")).toBe(false);
   });
 });
 

packages/gatekeeper-mcp-portal/src/config.ts

@@ -169,7 +169,7 @@ export function portalServer(config: PortalConfig): ConnectedServer {
 export function portalAuthRequiresReconnect(
   connected: ServerAuthKind, configured: ServerAuthKind,
 ): boolean {
-  return (connected === "token") !== (configured === "token");
+  return configured === "oauth" ? connected === "token" : connected !== configured;
 }
 
 /**

packages/gatekeeper-mcp-portal/src/portal.ts

@@ -28,7 +28,7 @@ import {
   type ToolIndex,
 } from "@gadgets/mcp-shared/client";
 import { MAX_TOOLS_PER_SERVER, type ServerTrust } from "@gadgets/mcp-shared/tools";
-import { bindingNameFragment, hostOf } from "@gadgets/mcp-shared/util";
+import { bindingNameFragment, hexEncode, hostOf } from "@gadgets/mcp-shared/util";
 import type { McpLog, McpLogFields } from "@gadgets/mcp-shared/log";
 import { generateSessionTypes, sessionTypeName } from "@gadgets/mcp-shared/schema-to-ts";
 import { McpAccountBase, type ConnectedServer, type ConnectOutcome }
@@ -38,6 +38,7 @@ import {
   fetchToolIndex,
   withClient,
   type ConnectionAccount,
+  type McpConnection,
 } from "@gadgets/mcp-shared/connection";
 import { McpSessionBase } from "@gadgets/mcp-shared/session";
 import { McpFacetBase } from "@gadgets/mcp-shared/facet";
@@ -79,6 +80,7 @@ import {
   readPortalConfig,
   requirePortalServerScope,
   toolGrantOptions,
+  type PortalConfig,
 } from "./config.js";
 import type { ConfiguratorUIOption } from "@gadgets/configurator-ui";
 import { MCP_BASE_TYPES } from "@gadgets/mcp-shared/base-types";
@@ -247,6 +249,10 @@ export class GatekeeperVendor extends WorkerEntrypoint<Env> implements Gatekeepe
  * real addition: a portal may be fronted by one instead of using OAuth.
  */
 export class McpAccount extends McpAccountBase<Env> {
+  // Counts in-flight attempts per revision. Separate Durable Object requests can overlap at the
+  // digest and portal probe, so one losing or older attempt must not clear a newer attempt's guard.
+  #connectingRevisions = new Map<string, number>();
+
   protected baseUrl(): string {
     return getBaseUrl(this.env);
   }
@@ -267,6 +273,83 @@ export class McpAccount extends McpAccountBase<Env> {
   protected override staticToken(server: ConnectedServer): string | null {
     return portalTokenFor(this.env, server.endpoint);
   }
+
+  protected override allowsOAuthFallback(server: ConnectedServer): boolean {
+    return server.auth !== "none";
+  }
+
+  protected override allowsOAuthCallback(server: ConnectedServer): boolean {
+    const config = readPortalConfig(this.env);
+    return config?.auth === "oauth" && sameEndpoint(config.endpoint, server.endpoint);
+  }
+
+  async #configurationRevision(config: PortalConfig): Promise<string> {
+    const token = config.auth === "token" ? this.env.MCP_PORTAL_TOKEN ?? "" : "";
+    const digest = await crypto.subtle.digest(
+      "SHA-256",
+      new TextEncoder().encode(`${config.auth}\u0000${token}`),
+    );
+    return hexEncode(new Uint8Array(digest));
+  }
+
+  override async beginConnect(
+    initiationNonce: string,
+    target: ConnectedServer | null,
+  ): Promise<ConnectOutcome> {
+    const config = readPortalConfig(this.env);
+    const revision = config && this.awaitingSelection(initiationNonce)
+      ? await this.#configurationRevision(config)
+      : undefined;
+    if (revision !== undefined) {
+      this.#connectingRevisions.set(
+        revision, (this.#connectingRevisions.get(revision) ?? 0) + 1);
+    }
+    try {
+      const outcome = await super.beginConnect(initiationNonce, target);
+      if (outcome.kind !== "invalid") {
+        const current = readPortalConfig(this.env);
+        const server = this.server();
+        if (current && server && sameEndpoint(current.endpoint, server.endpoint)) {
+          this.ctx.storage.kv.put(
+            "portalConfigRevision",
+            await this.#configurationRevision(current),
+          );
+        }
+      }
+      return outcome;
+    } finally {
+      if (revision !== undefined) {
+        const remaining = (this.#connectingRevisions.get(revision) ?? 1) - 1;
+        if (remaining === 0) this.#connectingRevisions.delete(revision);
+        else this.#connectingRevisions.set(revision, remaining);
+      }
+    }
+  }
+
+  override async getConnection(endpoint: string): Promise<McpConnection> {
+    const config = readPortalConfig(this.env);
+    const server = this.server();
+    if (!config || !server || !sameEndpoint(config.endpoint, endpoint)
+        || portalAuthRequiresReconnect(server.auth, config.auth)) {
+      throw new Error("This deployment's MCP portal configuration changed. Reconnect the account.");
+    }
+    const revision = await this.#configurationRevision(config);
+    const previous = this.ctx.storage.kv.get<string>("portalConfigRevision");
+    const reconnectingToCurrentRevision = this.#connectingRevisions.has(revision);
+    if (previous === undefined) {
+      // Existing token accounts predate the revision marker. Their cached session may have been
+      // minted under a different configured token, so invalidate it once before establishing the
+      // current revision as the baseline. New connects record the revision in `beginConnect()`.
+      if (!reconnectingToCurrentRevision) {
+        if (server.auth === "token") this.invalidateConnectionState();
+        this.ctx.storage.kv.put("portalConfigRevision", revision);
+      }
+    } else if (previous !== revision && !reconnectingToCurrentRevision) {
+      this.invalidateConnectionState();
+      this.ctx.storage.kv.put("portalConfigRevision", revision);
+    }
+    return super.getConnection(endpoint);
+  }
 }
 
 // ---------------------------------------------------------------------------

packages/gatekeeper-mcp-portal/vitest.config.ts

@@ -0,0 +1,13 @@
+import { fileURLToPath } from "node:url";
+import capnwebValidate from "capnweb-validate/vite";
+import { defineConfig } from "vitest/config";
+
+export default defineConfig({
+  plugins: [capnwebValidate()],
+  test: {
+    alias: {
+      "cloudflare:workers": fileURLToPath(
+        new URL("../mcp-shared/__tests__/stubs/cloudflare-workers.ts", import.meta.url)),
+    },
+  },
+});

packages/mcp-shared/src/account.ts

@@ -180,6 +180,16 @@
     return null;
   }
 
+  /** Whether an unauthenticated target may follow a 401 into the standard OAuth flow. */
+  protected allowsOAuthFallback(_server: ConnectedServer): boolean {
+    return true;
+  }
+
+  /** Whether current connector configuration still permits this pending OAuth callback. */
+  protected allowsOAuthCallback(_server: ConnectedServer): boolean {
+    return true;
+  }
+
   /** Relaxes host and scheme checks for local development against an MCP server on localhost. */
   protected fetchOptions(): FetchOptions {
     return fetchOptions(this.env);
@@ -203,6 +213,13 @@
     return generation;
   }
 
+  /** Invalidates credentials captured by facets and clears their transport session. */
+  protected invalidateConnectionState(): void {
+    this.advanceConnectionGeneration();
+    this.ctx.storage.kv.delete("mcpSessionId");
+    this.ctx.storage.kv.put("expiredNotified", false);
+  }
+
   private isCurrentConnection(server: ConnectedServer, generation: number): boolean {
     const current = this.server();
     return this.connectionGeneration() === generation && current !== undefined &&
@@ -310,18 +327,21 @@
     const generation = this.advanceConnectionGeneration();
     if (existing) this.ctx.storage.kv.delete("mcpSessionId");
     const endpointChanged = existing !== undefined && existing.endpoint !== server.endpoint;
-    if (endpointChanged) {
+    const authChanged = existing !== undefined && existing.auth !== server.auth;
+    if (endpointChanged || authChanged) {
       this.ctx.storage.kv.put("server", server);
       for (const key of [
         "tokens", "oauthClient", "oauthDiscovery", "oauthVerifier", "pendingAuth",
       ]) {
         this.ctx.storage.kv.delete(key);
       }
       this.ctx.storage.kv.put("expiredNotified", false);
-      this.log().info("portal repointed", {
-        event: "connect.repointed",
-        serverHost: hostOf(server.endpoint),
-      });
+      if (endpointChanged) {
+        this.log().info("portal repointed", {
+          event: "connect.repointed",
+          serverHost: hostOf(server.endpoint),
+        });
+      }
     }
 
     const log = this.log().with({
@@ -374,6 +394,13 @@
           `The MCP server "${server.serverName}" rejected this deployment's configured token.`,
           { cause: err });
       }
+      if (!this.allowsOAuthFallback(server)) {
+        this.restoreSelection(initiationNonce);
+        throw new Error(
+          `The MCP server "${server.serverName}" requires authorization, but this connection is ` +
+          "configured for unauthenticated access.",
+          { cause: err });
+      }
       // The endpoint answered with an authorization challenge, so OAuth is now the observed auth
       // mode even if deployment configuration optimistically called the portal public. Persist that
       // mode because `getAuthorization()` uses it to decide whether to read the tokens the callback
@@ -416,7 +443,7 @@
         throw new Error("This authorization attempt was replaced by a newer connection.");
       }
     };
     const matchesIssuer = (value: { issuer?: string } | undefined, issuer?: string) =>
       value !== undefined && (!issuer || !value.issuer || value.issuer === issuer);
 
     return {
@@ -577,6 +604,12 @@
     const pending = this.ctx.storage.kv.get<PendingAuthorization>("pendingAuth");
     if (!pending) return false;
     const server = this.requireServer();
+    if (!this.allowsOAuthCallback(server)) {
+      this.ctx.storage.kv.delete("nonce");
+      this.ctx.storage.kv.delete("pendingAuth");
+      this.ctx.storage.kv.delete("oauthVerifier");
+      return false;
+    }
     // Single-use: consumed before the exchange, so a replayed callback cannot reach the token endpoint.
     this.ctx.storage.kv.delete("nonce");
     this.ctx.storage.kv.delete("pendingAuth");