Skip to content

Commit 45b2f6e

Browse files
authored
fix: read-only scoped API keys cannot access MCP (outline#11875)
1 parent b91d9e9 commit 45b2f6e

File tree

3 files changed

+73
-0
lines changed

3 files changed

+73
-0
lines changed

server/models/ApiKey.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { randomString } from "@shared/random";
2+
import { Scope } from "@shared/types";
23
import { buildApiKey } from "@server/test/factories";
34
import ApiKey from "./ApiKey";
45

@@ -110,5 +111,23 @@ describe("#ApiKey", () => {
110111
expect(apiKey.canAccess("/api/documents.create")).toBe(false);
111112
expect(apiKey.canAccess("/api/collections.create")).toBe(false);
112113
});
114+
115+
it("should allow MCP access for scoped API keys", async () => {
116+
const apiKey = await buildApiKey({
117+
name: "Dev",
118+
scope: [Scope.Read],
119+
});
120+
121+
expect(apiKey.canAccess("/mcp")).toBe(true);
122+
expect(apiKey.canAccess("/mcp/")).toBe(true);
123+
});
124+
125+
it("should allow MCP access for unscoped API keys", async () => {
126+
const apiKey = await buildApiKey({
127+
name: "Dev",
128+
});
129+
130+
expect(apiKey.canAccess("/mcp")).toBe(true);
131+
});
113132
});
114133
});

server/models/ApiKey.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ class ApiKey extends ParanoidModel<
176176
return true;
177177
}
178178

179+
// MCP endpoint access is allowed if the key has any valid scope.
180+
// Fine-grained scope enforcement happens at the tool level.
181+
if (path.startsWith("/mcp")) {
182+
return this.scope.length > 0;
183+
}
184+
179185
return AuthenticationHelper.canAccess(path, this.scope);
180186
};
181187
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { Scope } from "@shared/types";
2+
import { buildOAuthAuthentication, buildUser } from "@server/test/factories";
3+
4+
describe("OAuthAuthentication", () => {
5+
describe("canAccess", () => {
6+
it("should allow MCP access for scoped tokens", async () => {
7+
const user = await buildUser();
8+
const authentication = await buildOAuthAuthentication({
9+
user,
10+
scope: [Scope.Read],
11+
});
12+
13+
expect(authentication.canAccess("/mcp")).toBe(true);
14+
expect(authentication.canAccess("/mcp/")).toBe(true);
15+
});
16+
17+
it("should deny MCP access for tokens with empty scope", async () => {
18+
const user = await buildUser();
19+
const authentication = await buildOAuthAuthentication({
20+
user,
21+
scope: [],
22+
});
23+
24+
expect(authentication.canAccess("/mcp")).toBe(false);
25+
});
26+
27+
it("should always allow the revoke endpoint", async () => {
28+
const user = await buildUser();
29+
const authentication = await buildOAuthAuthentication({
30+
user,
31+
scope: [Scope.Read],
32+
});
33+
34+
expect(authentication.canAccess("/oauth/revoke")).toBe(true);
35+
});
36+
37+
it("should check scopes for API paths", async () => {
38+
const user = await buildUser();
39+
const authentication = await buildOAuthAuthentication({
40+
user,
41+
scope: [Scope.Read],
42+
});
43+
44+
expect(authentication.canAccess("/api/documents.list")).toBe(true);
45+
expect(authentication.canAccess("/api/documents.update")).toBe(false);
46+
});
47+
});
48+
});

0 commit comments

Comments
 (0)