Skip to content

fix(http): authenticate /mcp before method dispatch#1070

Merged
andreasronge merged 1 commit into
mainfrom
claude/1069-auth-before-method-check
Jun 3, 2026
Merged

fix(http): authenticate /mcp before method dispatch#1070
andreasronge merged 1 commit into
mainfrom
claude/1069-auth-before-method-check

Conversation

@andreasronge

Copy link
Copy Markdown
Owner

Summary

  • Hoist authenticate/2 ahead of the HTTP method check in route_mcp/2 so every /mcp request runs the full host → origin → bearer pipeline regardless of method.
  • Method-specific handling now lives in a new dispatch_method/3 that receives the already-resolved owner (POST/DELETE bodies unchanged; any other method → 405 + Allow: POST, DELETE).
  • Centralizes the auth / origin / host / rate-limit error branches in route_mcp/2; the inner POST/DELETE clauses keep only their method-specific error branches.

Behavior change

  • Unauthenticated GET /mcp401 with WWW-Authenticate: Bearer (was 405), so endpoint/method probing now consumes the AuthRateLimiter.
  • Authenticated GET /mcp → still 405 with Allow: POST, DELETE.
  • GET now also gets host/origin enforcement (bad Host → 403 forbidden before any 405).
  • Authenticated POST / DELETE → unchanged.

Test plan

  • Updated "GET /mcp returns 405 with Allow""authenticated GET /mcp returns 405 with Allow" (sends auth).
  • Added "unauthenticated GET /mcp returns 401 bearer challenge".
  • Added "GET /mcp with bad Host returns 403 before 405".
  • mix precommit from mcp_server/ (format, compile, credo, 529 tests, 0 failures).

Closes #1069

🤖 Generated with Claude Code

Hoist authenticate/2 ahead of the HTTP method check in route_mcp/2 so
every /mcp request runs the full host -> origin -> bearer pipeline
regardless of method. Method dispatch now happens in dispatch_method/3
with the resolved owner.

Unauthenticated GET /mcp now returns 401 (bearer challenge) instead of
405, and endpoint/method probing consumes the AuthRateLimiter.
Authenticated GET still returns 405 with Allow: POST, DELETE.

Verified with mix precommit in mcp_server/ (format, credo, tests).

Closes #1069

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

PR Review: fix(http): authenticate /mcp before method dispatch

Summary

Hoists authenticate/2 ahead of method dispatch in route_mcp/2 so every /mcp request runs the full host → origin → bearer pipeline before any method-specific handling. Clean, well-scoped security hardening that closes #1069. Overall a solid change.

What's Good

  • Matches the issue's solution outline exactly: single auth call in route_mcp/2, new dispatch_method/3 branching on method, centralized auth/origin/host/rate-limit error branches with method-specific branches kept local.
  • Removes duplication: the POST clause previously repeated all four auth/origin/host/rate-limit error branches (router.ex:92-104 before); these now live once in route_mcp/2 (router.ex:55-61).
  • Closes the probing gap: unauthenticated GET /mcp no longer leaks method/endpoint existence via a 405 that skips auth; it now returns 401 + WWW-Authenticate: Bearer and consumes the AuthRateLimiter, consistent with POST/DELETE.
  • Test coverage matches the behavior change: the three edge cases from the issue's test plan are all covered — authenticated GET → 405+Allow (http_router_test.exs:45), unauthenticated GET → 401 challenge (:51), bad Host on GET → 403 before 405 (:57). Tests reuse existing auth/1, with_host/2, and call/1 helpers — no test duplication.

Issues (Must Fix)

None.

Suggestions (Optional)

None blocking. The catch-all dispatch_method/3 (router.ex:107) correctly preserves the 405 + Allow: POST, DELETE response for unknown methods.

Security

No concerns — this is a net security improvement. Auth/host/origin enforcement now precedes method dispatch uniformly, eliminating the unauthenticated 405 probe path. The rate_limit_error/2 path still correctly omits WWW-Authenticate (router.ex:315-321), so a rate-limited source isn't told whether its token was missing or invalid. The added rate-limiter consumption on GET failures matches existing POST/DELETE behavior and is intentional per the issue.

Documentation

No updates needed — internal refactor of private functions; no public API or DSL changes.

Note on file size: http_router_test.exs is 890 lines (>800 threshold), but this PR adds only +16 lines, so no split is warranted here.

Verdict

Approve — clean refactor that fulfills the issue's requirements with matching test coverage and no untracked debt.

@claude claude Bot added the ready-to-merge PR is ready to be merged label Jun 3, 2026
@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Auto-Triage Summary

No issues found in review — PR received an Approve verdict with no must-fix items and no blocking suggestions.

Issue Decision Action
(none)

Added label.

@andreasronge andreasronge enabled auto-merge (squash) June 3, 2026 08:01
@andreasronge andreasronge merged commit 287f2f8 into main Jun 3, 2026
10 checks passed
@andreasronge andreasronge deleted the claude/1069-auth-before-method-check branch June 3, 2026 08:01
andreasronge added a commit that referenced this pull request Jun 4, 2026
Hoist authenticate/2 ahead of the HTTP method check in route_mcp/2 so
every /mcp request runs the full host -> origin -> bearer pipeline
regardless of method. Method dispatch now happens in dispatch_method/3
with the resolved owner.

Unauthenticated GET /mcp now returns 401 (bearer challenge) instead of
405, and endpoint/method probing consumes the AuthRateLimiter.
Authenticated GET still returns 405 with Allow: POST, DELETE.

Verified with mix precommit in mcp_server/ (format, credo, tests).

Closes #1069
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[From PR #1044] Move auth before method check for strict per-endpoint enforcement on GET /mcp

1 participant