Commit Graph

4 Commits

Author SHA1 Message Date
jubnl 86129bbfbc feat: migrate OAuth public endpoints to MCP SDK auth handlers
Fixes issue #959 — two bugs causing ChatGPT's custom MCP connector to fail:

1. RFC 9728 path-based PRM: ChatGPT requests
   /.well-known/oauth-protected-resource/mcp (path-aware URL per RFC 9728
   §5). The old TREK handler only registered the base path; requests for
   the path variant fell through to the SPA catch-all and returned HTML.
   mcpAuthMetadataRouter registers the path-aware URL automatically.

2. DCR without scope: ChatGPT never sends scope during Dynamic Client
   Registration (RFC 7591 makes it optional). The old handler returned
   400 for missing scope. clientRegistrationHandler accepts it;
   trekClientsStore.registerClient defaults to ALL_SCOPES when absent,
   and the user still grants only what they approve at the consent UI
   (scopeSelectable=true for DCR clients is unchanged).

Hybrid approach: SDK handles /.well-known, /oauth/authorize (redirect to
consent SPA), and /oauth/register. TREK keeps its own /oauth/token and
/oauth/revoke because SDK clientAuth does plain-text secret comparison
while TREK uses SHA-256 hashing — incompatible without a full clientAuth
rewrite.

SPA consent page renamed /oauth/authorize → /oauth/consent to avoid
routing conflict with the SDK's backend authorize handler now mounted at
that path. Existing URL paths (/oauth/token etc.) are unchanged so
active Claude.ai connections are unaffected.

Other: lazy-init SDK metadata router so getAppUrl() (DB query) is not
called at createApp() time; path-aware mcpAddonGate so only /.well-known
returns 404 when MCP is disabled (previously a blanket middleware blocked
all routes including static files); /api/oauth mounted before the SDK
middleware chain so SPA-facing routes with their own 403 gates are
reached correctly.
2026-05-05 13:01:32 +02:00
jubnl 7c0a0d5f39 security(oauth): harden OAuth 2.1/MCP implementation (Critical + High + Medium findings)
Address 14 security findings from internal review of the OAuth 2.1 + MCP layer:

Critical:
- C1: Scope-gate all MCP resources (trips, budget, packing, collab, atlas, vacay, etc.)
- C2: Wire token/session revocation into active MCP session lifecycle per (user, client_id)
- C3: Refresh-token replay detection via parent_token_id chain + cascade revoke on replay

High:
- H1: Validate PKCE code_challenge (43-char base64url) and code_verifier (43–128 chars) format
- H2: Rate-limit /oauth/token (30/min), /authorize/validate (30/min), /oauth/revoke (10/min)
- H3: Strip client metadata from unauthenticated /authorize/validate responses (oracle prevention)
- H4: Constant-time secret comparison via crypto.timingSafeEqual (prevents timing attacks)
- H5: Collapse all invalid_grant cases to a single generic message; log specifics server-side

Medium:
- M1: Set Cache-Control: no-store + Pragma: no-cache on token endpoint responses
- M2: Return 404 (not 200/403) on discovery + revoke endpoints when MCP addon is disabled
- M4: Audit-log all OAuth lifecycle events (create, consent, issue, refresh, revoke, replay)
- M5: Union consent scopes on re-authorization instead of replacing existing grants
- M7: Require httpOnly cookie auth (not Bearer JWT) on all state-mutating OAuth endpoints
- M8: Strict Bearer scheme check in MCP token verification

Refactoring:
- Extract MCP session management (sessions Map, revokeUserSessions, revokeUserSessionsForClient)
  into mcp/sessionManager.ts to break the circular dependency between oauthService and mcp/index
- Extract verifyJwtAndLoadUser helper in auth middleware, shared by authenticate and new
  requireCookieAuth middleware

Tests:
- Fix all existing integration tests broken by the security hardening (OAUTH-019 to OAUTH-032)
- Add 13 new integration tests covering M1, M2, H1, H3, H5, M5, M7, C3
- Add 14 new unit tests covering C2, C3, H1, H3, M5 behaviors in oauthService
2026-04-10 02:03:27 +02:00
jubnl 5b44fe68b1 fix(mcp): narrow OAuth scope to allowed intersection instead of rejecting
When a client requests scopes it is not permitted for, silently drop
them rather than failing the entire authorization flow. The token is
issued with only the intersection of requested and allowed scopes.

Also fix /authorize/validate to always return HTTP 200 so the consent
page can surface the actual error_description instead of a generic
axios failure message.
2026-04-09 23:48:05 +02:00
jubnl f2908fdd65 test(mcp): add tests for OAuth 2.1, addon gating, and budget reorder
Covers OAuth integration flow, scope enforcement, addon-gated tool access,
oauthService unit tests, and budget reorder/permission/reservation-sync scenarios.
2026-04-09 23:12:59 +02:00