Skip to content

Commit 0eb389f

Browse files
fix(core): allow reducing OAuth consent scopes (#2188)
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 7d28ce9 commit 0eb389f

5 files changed

Lines changed: 254 additions & 12 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"emdash": patch
3+
---
4+
5+
Fixes OAuth authorization so users can remove unnecessary requested permissions before granting an MCP client access.

docs/src/content/docs/guides/ai-tools.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ Once connected, you can ask the AI assistant to perform any of these operations
156156

157157
## Permissions
158158

159-
What you can do through an AI tool depends on your EmDash role. The AI assistant operates with the same permissions you have in the admin panel:
159+
When an MCP client opens the OAuth consent page, every permission it requested is selected by default. Clear any permission the client does not need before approving access. The resulting token is limited to the permissions you keep selected and the permissions allowed by your EmDash role, so granting fewer permissions never increases what the client can do.
160160

161161
| Role | What the AI can do |
162162
| --- | --- |

docs/src/content/docs/reference/mcp-server.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Session cookies (from the admin UI) also work but aren't practical for external
2727

2828
### Scopes
2929

30-
Tokens are scoped to limit what operations a client can perform. Scopes are requested during OAuth authorization and enforced on every tool call.
30+
Tokens are scoped to limit what operations a client can perform. Scopes are requested during OAuth authorization and enforced on every tool call. On the authorization-code consent page, all requested scopes are selected by default for compatibility; the user can remove scopes before approval but cannot add scopes the client did not request. The effective grant is also restricted by the client's registered scopes and the user's role, and EmDash rejects an empty grant.
3131

3232
| Scope | Grants access to |
3333
| --- | --- |

packages/core/src/astro/routes/api/oauth/authorize.ts

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,16 @@ function getCsrfCookie(request: Request): string | null {
6969

7070
const SCOPE_LABELS: Record<string, string> = {
7171
"content:read": "Read content (posts, pages, etc.)",
72-
"content:write": "Create, edit, and delete content",
72+
"content:write": "Create, edit, and delete content; manage menus and taxonomies",
7373
"media:read": "View media files",
7474
"media:write": "Upload and manage media files",
7575
"schema:read": "View collection schemas",
7676
"schema:write": "Create and modify collection schemas",
77+
"taxonomies:manage": "Manage taxonomies",
78+
"menus:manage": "Manage navigation menus",
79+
"settings:read": "View site settings",
80+
"settings:manage": "Modify site settings",
81+
"mcp:tools": "Use MCP tools from all enabled plugins",
7782
admin: "Full administrative access",
7883
};
7984

@@ -141,7 +146,9 @@ export const GET: APIRoute = async ({ url, request, locals }) => {
141146
}
142147

143148
// Parse and validate scopes
144-
const requestedScopes = (scope ?? "").split(" ").filter(Boolean).filter(isValidScope);
149+
const requestedScopes = [
150+
...new Set((scope ?? "").split(" ").filter(Boolean).filter(isValidScope)),
151+
];
145152

146153
if (requestedScopes.length === 0) {
147154
return new Response(renderErrorPage("No valid scopes requested."), {
@@ -201,6 +208,21 @@ export const POST: APIRoute = async ({ request, locals }) => {
201208
const v = formData.get(name);
202209
return typeof v === "string" ? v : fallback;
203210
};
211+
const requestedScopes = new Set(
212+
(new URL(request.url).searchParams.get("scope") ?? "")
213+
.split(" ")
214+
.filter(Boolean)
215+
.filter(isValidScope),
216+
);
217+
const selectedScopes = [
218+
...new Set(
219+
formData
220+
.getAll("scope")
221+
.filter((value): value is string => typeof value === "string")
222+
.filter(isValidScope)
223+
.filter((scope) => requestedScopes.has(scope)),
224+
),
225+
];
204226

205227
// SEC-18: Validate CSRF token (double-submit cookie pattern).
206228
// The form includes a hidden csrf_token field; the cookie has the same value.
@@ -285,7 +307,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
285307
response_type: field("response_type", "code"),
286308
client_id: field("client_id"),
287309
redirect_uri: redirectUri,
288-
scope: field("scope"),
310+
scope: selectedScopes.join(" "),
289311
state,
290312
code_challenge: field("code_challenge"),
291313
code_challenge_method: field("code_challenge_method", "S256"),
@@ -294,12 +316,16 @@ export const POST: APIRoute = async ({ request, locals }) => {
294316

295317
if (!result.success) {
296318
const errMsg = result.error?.message ?? "Authorization failed";
319+
const invalidScope = result.error?.code === "INVALID_SCOPE";
297320
// On error, redirect back with error params — use generic description to avoid
298321
// leaking internal error details to the (already-validated) redirect target
299322
try {
300323
const errorUrl = new URL(redirectUri);
301-
errorUrl.searchParams.set("error", "server_error");
302-
errorUrl.searchParams.set("error_description", "Authorization failed");
324+
errorUrl.searchParams.set("error", invalidScope ? "invalid_scope" : "server_error");
325+
errorUrl.searchParams.set(
326+
"error_description",
327+
invalidScope ? "No selected permission can be granted" : "Authorization failed",
328+
);
303329
if (state) errorUrl.searchParams.set("state", state);
304330
return Response.redirect(errorUrl.toString(), 302);
305331
} catch {
@@ -331,8 +357,8 @@ function renderConsentPage(params: {
331357
}): string {
332358
const scopeList = params.scopes
333359
.map((s) => {
334-
const label = SCOPE_LABELS[s] ?? s;
335-
return `<li>${escapeHtml(label)}</li>`;
360+
const label = SCOPE_LABELS[s] ?? getPluginScopeLabel(s) ?? s;
361+
return `<li><label><input type="checkbox" name="scope" value="${escapeHtml(s)}" checked><span>${escapeHtml(label)}</span></label></li>`;
336362
})
337363
.join("\n");
338364

@@ -353,6 +379,8 @@ function renderConsentPage(params: {
353379
ul { list-style: none; margin-bottom: 1.5rem; }
354380
li { padding: 0.5rem 0; border-bottom: 1px solid #262626; font-size: 0.875rem; }
355381
li:last-child { border-bottom: none; }
382+
label { display: flex; align-items: flex-start; gap: 0.625rem; cursor: pointer; }
383+
input[type="checkbox"] { margin-top: 0.125rem; accent-color: #2563eb; }
356384
.actions { display: flex; gap: 0.75rem; }
357385
button { flex: 1; padding: 0.625rem 1rem; border-radius: 8px; border: none; font-size: 0.875rem; font-weight: 500; cursor: pointer; }
358386
.approve { background: #2563eb; color: white; }
@@ -366,14 +394,13 @@ function renderConsentPage(params: {
366394
<h1>Authorize Application</h1>
367395
<p class="client-id">${escapeHtml(params.clientId)}</p>
368396
<p class="user">Signed in as <strong>${escapeHtml(params.userName)}</strong></p>
369-
<h2>Permissions requested</h2>
370-
<ul>${scopeList}</ul>
371397
<form method="POST">
398+
<h2>Permissions requested</h2>
399+
<ul>${scopeList}</ul>
372400
<input type="hidden" name="csrf_token" value="${escapeHtml(params.csrfToken)}">
373401
<input type="hidden" name="response_type" value="${escapeHtml(params.responseType)}">
374402
<input type="hidden" name="client_id" value="${escapeHtml(params.clientId)}">
375403
<input type="hidden" name="redirect_uri" value="${escapeHtml(params.redirectUri)}">
376-
<input type="hidden" name="scope" value="${escapeHtml(params.scopes.join(" "))}">
377404
<input type="hidden" name="state" value="${escapeHtml(params.state)}">
378405
<input type="hidden" name="code_challenge" value="${escapeHtml(params.codeChallenge)}">
379406
<input type="hidden" name="code_challenge_method" value="${escapeHtml(params.codeChallengeMethod)}">
@@ -388,6 +415,11 @@ function renderConsentPage(params: {
388415
</html>`;
389416
}
390417

418+
function getPluginScopeLabel(scope: string): string | null {
419+
const pluginId = scope.startsWith("mcp:tools:") ? scope.slice("mcp:tools:".length) : "";
420+
return pluginId ? `Use MCP tools from ${pluginId}` : null;
421+
}
422+
391423
function renderErrorPage(message: string): string {
392424
return `<!DOCTYPE html>
393425
<html lang="en">
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
import type { Kysely } from "kysely";
2+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
3+
4+
import { handleOAuthClientCreate } from "../../../src/api/handlers/oauth-clients.js";
5+
import {
6+
GET as getAuthorizationConsent,
7+
POST as postAuthorizationConsent,
8+
} from "../../../src/astro/routes/api/oauth/authorize.js";
9+
import type { Database } from "../../../src/database/types.js";
10+
import { setupTestDatabase, teardownTestDatabase } from "../../utils/test-db.js";
11+
12+
const REDIRECT_URI = "http://127.0.0.1:8080/callback";
13+
14+
describe("OAuth authorization consent", () => {
15+
let db: Kysely<Database>;
16+
17+
beforeEach(async () => {
18+
db = await setupTestDatabase();
19+
20+
await db
21+
.insertInto("users")
22+
.values({
23+
id: "user-1",
24+
email: "test@example.com",
25+
name: "Test User",
26+
role: 50,
27+
email_verified: 1,
28+
})
29+
.execute();
30+
31+
await handleOAuthClientCreate(db, {
32+
id: "test-client",
33+
name: "Test Client",
34+
redirectUris: [REDIRECT_URI],
35+
});
36+
});
37+
38+
afterEach(async () => {
39+
await teardownTestDatabase(db);
40+
});
41+
42+
it("renders every requested scope as a checked choice with accurate grant copy", async () => {
43+
const { response } = await renderConsent([
44+
"content:read",
45+
"content:write",
46+
"schema:write",
47+
"mcp:tools:calendar",
48+
]);
49+
50+
expect(response.status).toBe(200);
51+
const html = await response.text();
52+
expect(html).toContain('<input type="checkbox" name="scope" value="content:read" checked>');
53+
expect(html).toContain('<input type="checkbox" name="scope" value="schema:write" checked>');
54+
expect(html).toContain(
55+
'<input type="checkbox" name="scope" value="mcp:tools:calendar" checked>',
56+
);
57+
expect(html).toContain("Create, edit, and delete content; manage menus and taxonomies");
58+
expect(html).not.toContain('<input type="hidden" name="scope"');
59+
const form = html.slice(html.indexOf("<form"), html.indexOf("</form>"));
60+
expect(form).toContain('<input type="checkbox" name="scope" value="content:read"');
61+
});
62+
63+
it("grants only selected scopes from the original request", async () => {
64+
const consent = await renderConsent(["content:read", "schema:write"]);
65+
const response = await submitConsent(consent, ["admin", "content:read"]);
66+
67+
expect(response.status).toBe(302);
68+
expect(new URL(response.headers.get("Location")!).searchParams.get("code")).toBeTruthy();
69+
70+
const authorizationCode = await db
71+
.selectFrom("_emdash_authorization_codes")
72+
.select("scopes")
73+
.executeTakeFirstOrThrow();
74+
expect(JSON.parse(authorizationCode.scopes)).toEqual(["content:read"]);
75+
});
76+
77+
it("renders duplicate requests once so clearing a scope removes it", async () => {
78+
const consent = await renderConsent(["admin", "admin", "content:read"]);
79+
const html = await consent.response.clone().text();
80+
expect(html.match(/name="scope" value="admin"/g)).toHaveLength(1);
81+
82+
const response = await submitConsent(consent, ["content:read"]);
83+
expect(response.status).toBe(302);
84+
85+
const authorizationCode = await db
86+
.selectFrom("_emdash_authorization_codes")
87+
.select("scopes")
88+
.executeTakeFirstOrThrow();
89+
expect(JSON.parse(authorizationCode.scopes)).toEqual(["content:read"]);
90+
});
91+
92+
it("preserves client and role scope clamps after selection", async () => {
93+
await db
94+
.updateTable("_emdash_oauth_clients")
95+
.set({ scopes: JSON.stringify(["content:read", "schema:write"]) })
96+
.where("id", "=", "test-client")
97+
.execute();
98+
99+
const consent = await renderConsent(["content:read", "media:read", "schema:write"]);
100+
const response = await submitConsent(
101+
consent,
102+
["content:read", "media:read", "schema:write"],
103+
20,
104+
);
105+
106+
expect(response.status).toBe(302);
107+
const authorizationCode = await db
108+
.selectFrom("_emdash_authorization_codes")
109+
.select("scopes")
110+
.executeTakeFirstOrThrow();
111+
expect(JSON.parse(authorizationCode.scopes)).toEqual(["content:read"]);
112+
});
113+
114+
it("rejects approval when no requested scope is selected", async () => {
115+
const consent = await renderConsent(["content:read", "schema:write"]);
116+
const response = await submitConsent(consent, []);
117+
118+
expect(response.status).toBe(302);
119+
const redirect = new URL(response.headers.get("Location")!);
120+
expect(redirect.searchParams.get("error")).toBe("invalid_scope");
121+
expect(redirect.searchParams.get("error_description")).toBe(
122+
"No selected permission can be granted",
123+
);
124+
125+
const authorizationCodes = await db
126+
.selectFrom("_emdash_authorization_codes")
127+
.select("code_hash")
128+
.execute();
129+
expect(authorizationCodes).toHaveLength(0);
130+
});
131+
132+
async function renderConsent(scopes: string[]): Promise<{
133+
response: Response;
134+
url: URL;
135+
csrfToken: string;
136+
cookie: string;
137+
}> {
138+
const url = new URL("http://localhost:4321/_emdash/oauth/authorize");
139+
url.searchParams.set("response_type", "code");
140+
url.searchParams.set("client_id", "test-client");
141+
url.searchParams.set("redirect_uri", REDIRECT_URI);
142+
url.searchParams.set("scope", scopes.join(" "));
143+
url.searchParams.set("state", "state-1");
144+
url.searchParams.set("code_challenge", "challenge");
145+
url.searchParams.set("code_challenge_method", "S256");
146+
147+
const response = await getAuthorizationConsent({
148+
url,
149+
request: new Request(url),
150+
locals: {
151+
emdash: { db, config: {} },
152+
user: {
153+
id: "user-1",
154+
email: "test@example.com",
155+
name: "Test User",
156+
role: 50,
157+
},
158+
},
159+
} as Parameters<typeof getAuthorizationConsent>[0]);
160+
161+
const html = response.clone();
162+
const csrfToken = (await html.text()).match(/name="csrf_token" value="([^"]+)"/)?.[1];
163+
const cookie = response.headers.get("Set-Cookie")?.split(";")[0];
164+
if (!csrfToken || !cookie) throw new Error("Consent response omitted CSRF state");
165+
166+
return { response, url, csrfToken, cookie };
167+
}
168+
169+
async function submitConsent(
170+
consent: Awaited<ReturnType<typeof renderConsent>>,
171+
selectedScopes: string[],
172+
role = 50,
173+
): Promise<Response> {
174+
const body = new URLSearchParams({
175+
csrf_token: consent.csrfToken,
176+
response_type: "code",
177+
client_id: "test-client",
178+
redirect_uri: REDIRECT_URI,
179+
state: "state-1",
180+
code_challenge: "challenge",
181+
code_challenge_method: "S256",
182+
action: "approve",
183+
});
184+
for (const scope of selectedScopes) body.append("scope", scope);
185+
186+
const request = new Request(consent.url, {
187+
method: "POST",
188+
headers: { Cookie: consent.cookie },
189+
body,
190+
});
191+
192+
return postAuthorizationConsent({
193+
request,
194+
locals: {
195+
emdash: { db, config: {} },
196+
user: {
197+
id: "user-1",
198+
email: "test@example.com",
199+
name: "Test User",
200+
role,
201+
},
202+
},
203+
} as Parameters<typeof postAuthorizationConsent>[0]);
204+
}
205+
});

0 commit comments

Comments
 (0)