Skip to content

fix(core): return OAuth discovery 401 for unauthenticated MCP POSTs - #371

Merged
ascorbic merged 7 commits into
emdash-cms:mainfrom
pejmanjohn:contrib/emdash-cms-emdash-mcp-post-oauth-discovery
Apr 12, 2026
Merged

fix(core): return OAuth discovery 401 for unauthenticated MCP POSTs#371
ascorbic merged 7 commits into
emdash-cms:mainfrom
pejmanjohn:contrib/emdash-cms-emdash-mcp-post-oauth-discovery

Conversation

@pejmanjohn

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes OAuth discovery for unauthenticated MCP POST requests on /_emdash/api/mcp.

Some MCP clients, including Codex, probe the MCP endpoint with POST when starting auth discovery. Before this change, unauthenticated POST /_emdash/api/mcp requests without X-EmDash-Request: 1 were rejected by the generic CSRF middleware with 403 CSRF_REJECTED before the MCP auth path could return 401 Unauthorized with the WWW-Authenticate header.

That prevented OAuth-capable MCP clients from starting discovery even though EmDash already exposed valid OAuth metadata endpoints.

This change:

  • allows tokenless MCP discovery POST requests with no authenticated session to reach the existing 401 + WWW-Authenticate response
  • preserves CSRF rejection for session/cookie-backed MCP POST requests without X-EmDash-Request: 1
  • keeps non-MCP API CSRF behavior unchanged
  • adds regression tests for unauthenticated MCP POST, invalid bearer token handling, and the non-regression CSRF cases

Type of change

  • Bug fix
  • Feature (requires approved Discussion)
  • Refactor (no behavior change)
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm --silent lint:json | jq '.diagnostics | length' returns 0
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/...

AI-generated code disclosure

  • This PR includes AI-generated code

Screenshots / test output

Ran:

  • pnpm build
  • pnpm typecheck
  • pnpm typecheck:demos
  • pnpm --filter emdash exec vitest run tests/unit/auth/mcp-discovery-post.test.ts tests/unit/auth/discovery-endpoints.test.ts tests/unit/api/csrf.test.ts tests/unit/mcp/authorization.test.ts

Notes:

  • pnpm --silent lint:json | jq '.diagnostics | length' currently reports 3 existing warnings in unrelated files.
@changeset-bot

changeset-bot Bot commented Apr 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5f83a8d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/plugin-ai-moderation Patch
@emdash-cms/plugin-atproto Patch
@emdash-cms/plugin-audit-log Patch
@emdash-cms/plugin-color Patch
@emdash-cms/plugin-embeds Patch
@emdash-cms/plugin-forms Patch
@emdash-cms/plugin-webhook-notifier Patch
@emdash-cms/admin Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
create-emdash Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pejmanjohn
pejmanjohn marked this pull request as ready for review April 8, 2026 04:58
@pkg-pr-new

pkg-pr-new Bot commented Apr 9, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@371

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@371

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@371

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@371

emdash

npm i https://pkg.pr.new/emdash@371

create-emdash

npm i https://pkg.pr.new/create-emdash@371

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@371

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@371

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@371

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@371

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@371

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@371

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@371

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@371

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@371

commit: 5f83a8d

@ascorbic ascorbic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. The fix is in the right area, but I think there's a simpler approach. No MCP client will ever authenticate via session cookies, so we don't need to preserve CSRF for session-backed MCP requests: we can just make MCP bearer-only. I've made a couple of inline suggesitons, but it might need a few more as I've not tested. Basically for MCP requests we want to exempt from CSRF, but only allow token auth.

Comment on lines 212 to 216
const isOAuthConsent = url.pathname.startsWith("/_emdash/oauth/authorize");
if (
isApiRoute &&
!isTokenAuth &&
!isOAuthConsent &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const isOAuthConsent = url.pathname.startsWith("/_emdash/oauth/authorize");
const isMcpEndpoint = url.pathname === "/_emdash/api/mcp";
if (
isApiRoute &&
!isTokenAuth &&
!isOAuthConsent &&
!isMcpEndpoint &&

@pejmanjohn pejmanjohn Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ascorbic Updated this to match the broader bearer-only direction. MCP now short-circuits before session or external auth: requests without a valid bearer token return the discovery-style 401 with WWW-Authenticate, and session-backed MCP requests are no longer accepted even if the CSRF header is present.

I also updated the regression test to assert that MCP POST discovery no longer reads session state at all. Verified with pnpm --filter emdash test tests/unit/auth/mcp-discovery-post.test.ts, pnpm test tests/unit/auth/*.test.ts, pnpm typecheck, and pnpm --silent lint:json | jq .diagnostics | length.

Comment thread packages/core/src/astro/middleware/auth.ts
{
status: 401,
headers: {
"WWW-Authenticate": `Bearer resource_metadata="${url.origin}/.well-known/oauth-protected-resource"`,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to use getPublicOrigin instead of url.origin, meaning mcpUnauthorizedResponse needs to be passed the config.

@pejmanjohn pejmanjohn Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ascorbic Updated this to route the anonymous MCP discovery 401 through getPublicOrigin(...), so the WWW-Authenticate resource metadata URL now respects siteUrl / reverse-proxy deployments instead of using the raw request origin.

I also added a regression covering the configured public-origin case on the anonymous MCP POST path.

Verified with:

  • pnpm --filter emdash test tests/unit/auth/mcp-discovery-post.test.ts
  • pnpm test tests/unit/auth/*.test.ts
  • pnpm typecheck
  • pnpm --silent lint:json | jq .diagnostics | length
@pejmanjohn
pejmanjohn force-pushed the contrib/emdash-cms-emdash-mcp-post-oauth-discovery branch from 0d0526d to 5f83a8d Compare April 11, 2026 20:11
@pejmanjohn

Copy link
Copy Markdown
Contributor Author

@ascorbic rebased this branch onto current main to clear the needs-rebase conflict, and the MCP public-origin follow-up is included in the rebased history. I reran:

  • pnpm --filter emdash test tests/unit/auth/mcp-discovery-post.test.ts
  • pnpm test tests/unit/auth/*.test.ts
  • pnpm typecheck

The PR was already ready for review, so it remains ready.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts the core Astro auth middleware so unauthenticated MCP POST /_emdash/api/mcp requests can reach the MCP auth path and return the OAuth discovery-style 401 (with WWW-Authenticate) instead of being blocked earlier by CSRF enforcement.

Changes:

  • Adds an MCP-specific early 401 + WWW-Authenticate response path for non-token requests.
  • Refactors CSRF rejection response and unsafe-method detection into small helpers.
  • Adds unit tests covering unauthenticated MCP POST discovery responses and invalid bearer token handling, plus a CSRF non-regression test for non-MCP API routes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
packages/core/src/astro/middleware/auth.ts Adds MCP-specific unauthorized response path before CSRF enforcement; refactors CSRF response building.
packages/core/tests/unit/auth/mcp-discovery-post.test.ts Adds regression tests for MCP discovery via POST and invalid bearer token behavior.
.changeset/fresh-mice-battle.md Publishes a patch changeset describing the MCP discovery fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +218 to +225
// MCP discovery/tooling is bearer-only. Session/external auth should never
// be consulted for this endpoint, and unauthenticated requests must return
// the OAuth discovery-style 401 response.
const method = context.request.method.toUpperCase();
const isMcpEndpoint = url.pathname === MCP_ENDPOINT_PATH;
if (isMcpEndpoint && !isTokenAuth) {
return mcpUnauthorizedResponse(url, context.locals.emdash?.config);
}

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new MCP short-circuit runs before the CSRF check, so any non-token request to /_emdash/api/mcp (including browser requests carrying session cookies but missing X-EmDash-Request: 1) will now return the OAuth-discovery 401 instead of 403 CSRF_REJECTED. This doesn’t match the PR description’s goal of preserving CSRF rejection for session/cookie-backed MCP POSTs. If that behavior is still desired, gate this early-return to only apply to truly anonymous discovery (e.g., no Cookie/session cookie present), and add a regression test covering the cookie-present + missing-CSRF-header case.

Copilot uses AI. Check for mistakes.
Comment on lines +54 to +55
const MCP_ENDPOINT_PATH = "/_emdash/api/mcp";

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MCP_ENDPOINT_PATH is introduced, but the file still contains hard-coded "/_emdash/api/mcp" comparisons elsewhere (e.g., the invalid-token WWW-Authenticate branch and the scope rule). To avoid drift, prefer using the constant consistently in this module.

Copilot uses AI. Check for mistakes.
Comment on lines +38 to +44
beforeEach(() => {
vi.clearAllMocks();
});

afterEach(() => {
vi.clearAllMocks();
});

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vi.clearAllMocks() is called in both beforeEach and afterEach. One of these is redundant and removing the duplicate will reduce noise in the test setup while keeping the same isolation behavior.

Copilot uses AI. Check for mistakes.

@ascorbic ascorbic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks

@ascorbic
ascorbic merged commit 5320321 into emdash-cms:main Apr 12, 2026
33 checks passed
This was referenced Apr 12, 2026
fmhall pushed a commit to fmhall/emdash that referenced this pull request Apr 13, 2026
…mdash-cms#371)

* fix(core): return OAuth discovery 401 for MCP POSTs

* chore: add changeset for MCP OAuth discovery fix

* style: format

* fix(core): centralize MCP discovery auth handling

* style: format

* fix(core): make MCP auth bearer-only

* fix(core): use public origin for MCP discovery 401

---------

Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment