From 45b2f6e2222d6f00a61752f3e5857c9526409e82 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 26 Mar 2026 00:15:28 -0400 Subject: [PATCH] fix: read-only scoped API keys cannot access MCP (#11875) --- server/models/ApiKey.test.ts | 19 ++++++++ server/models/ApiKey.ts | 6 +++ .../models/oauth/OAuthAuthentication.test.ts | 48 +++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 server/models/oauth/OAuthAuthentication.test.ts diff --git a/server/models/ApiKey.test.ts b/server/models/ApiKey.test.ts index 8b62cf6545..09a53e8018 100644 --- a/server/models/ApiKey.test.ts +++ b/server/models/ApiKey.test.ts @@ -1,4 +1,5 @@ import { randomString } from "@shared/random"; +import { Scope } from "@shared/types"; import { buildApiKey } from "@server/test/factories"; import ApiKey from "./ApiKey"; @@ -110,5 +111,23 @@ describe("#ApiKey", () => { expect(apiKey.canAccess("/api/documents.create")).toBe(false); expect(apiKey.canAccess("/api/collections.create")).toBe(false); }); + + it("should allow MCP access for scoped API keys", async () => { + const apiKey = await buildApiKey({ + name: "Dev", + scope: [Scope.Read], + }); + + expect(apiKey.canAccess("/mcp")).toBe(true); + expect(apiKey.canAccess("/mcp/")).toBe(true); + }); + + it("should allow MCP access for unscoped API keys", async () => { + const apiKey = await buildApiKey({ + name: "Dev", + }); + + expect(apiKey.canAccess("/mcp")).toBe(true); + }); }); }); diff --git a/server/models/ApiKey.ts b/server/models/ApiKey.ts index d9ccf42a07..cb3a7d5770 100644 --- a/server/models/ApiKey.ts +++ b/server/models/ApiKey.ts @@ -176,6 +176,12 @@ class ApiKey extends ParanoidModel< return true; } + // MCP endpoint access is allowed if the key has any valid scope. + // Fine-grained scope enforcement happens at the tool level. + if (path.startsWith("/mcp")) { + return this.scope.length > 0; + } + return AuthenticationHelper.canAccess(path, this.scope); }; } diff --git a/server/models/oauth/OAuthAuthentication.test.ts b/server/models/oauth/OAuthAuthentication.test.ts new file mode 100644 index 0000000000..978edf079e --- /dev/null +++ b/server/models/oauth/OAuthAuthentication.test.ts @@ -0,0 +1,48 @@ +import { Scope } from "@shared/types"; +import { buildOAuthAuthentication, buildUser } from "@server/test/factories"; + +describe("OAuthAuthentication", () => { + describe("canAccess", () => { + it("should allow MCP access for scoped tokens", async () => { + const user = await buildUser(); + const authentication = await buildOAuthAuthentication({ + user, + scope: [Scope.Read], + }); + + expect(authentication.canAccess("/mcp")).toBe(true); + expect(authentication.canAccess("/mcp/")).toBe(true); + }); + + it("should deny MCP access for tokens with empty scope", async () => { + const user = await buildUser(); + const authentication = await buildOAuthAuthentication({ + user, + scope: [], + }); + + expect(authentication.canAccess("/mcp")).toBe(false); + }); + + it("should always allow the revoke endpoint", async () => { + const user = await buildUser(); + const authentication = await buildOAuthAuthentication({ + user, + scope: [Scope.Read], + }); + + expect(authentication.canAccess("/oauth/revoke")).toBe(true); + }); + + it("should check scopes for API paths", async () => { + const user = await buildUser(); + const authentication = await buildOAuthAuthentication({ + user, + scope: [Scope.Read], + }); + + expect(authentication.canAccess("/api/documents.list")).toBe(true); + expect(authentication.canAccess("/api/documents.update")).toBe(false); + }); + }); +});