vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

feat(mcp): per-user auth for /mcp (HTTP Basic, server-validated) #13

Merged
Ghost merged 4 commits from feat/mcp-per-user-auth into develop 2026-06-20 19:32:03 +03:00
Conversation 0 Commits 4 Files Changed 13 +2158 -503
Ghost commented 2026-06-20 07:20:06 +03:00
Copy Link

Implements docs/backlog/mcp-per-user-auth.md (chosen variant L: HTTP Basic, server-validated).

What

The embedded /mcp server acted as one shared service account — every MCP client had that account's CASL and edit attribution. Now each /mcp session authenticates as the current user: tools run under the user's CASL, edits attribute to them, and the service account is no longer required.

How

  • Basic (primary): Authorization: Basic email:password → validated server-side via AuthService; the session carries the issued user JWT (not the raw password). Password may contain : (split on first only).
  • Bearer (fallback): Authorization: Bearer <access JWT> → verifyJwt(ACCESS) plus an active-session + non-disabled-user check (matching JwtStrategy), so revoked/disabled users are rejected.
  • Service account (fallback): kept for the no-creds + env-configured case (CI/scripts).
  • packages/mcp createMcpHttpHandler now accepts a per-request config resolver (static config / stdio path unchanged). Identity is bound to the mcp-session-id at init and re-validated from the caller's own credentials on every request.
  • MCP_TOKEN moved off Authorization to an X-MCP-Token header (timing-safe compare); the old credsConfigured 503 gate is replaced by a clear 401 listing accepted methods.

Reasoning / decisions (resolved the plan's open questions)

  • Server-side validation (not forwarding the raw password to the mcp package): errors surface as clean pre-hijack 401 JSON, and the password never travels further than the auth check.
  • Service account kept as an optional fallback (didn't force MCP_REQUIRE_USER_TOKEN) — preserves existing installs / headless use.
  • X-MCP-Token rather than dropping the shared guard, so a deployment can still gate the endpoint while Authorization carries user creds.

Review findings & fixes (adversarial security review)

The review confirmed the three highest-risk properties hold: no auth-less reuse of an established session; identity re-validated from the caller's own creds every request; no secret logging. Two blockers it found were fixed:

  1. login() ran on every request → a new user_sessions row + USER_LOGIN audit per request (audit spam / session-table DoS). Fixed: the session-minting login() happens once at initialize; subsequent requests re-verify credentials via a new non-side-effecting AuthService.verifyUserCredentials (lookup + password compare + email-verified + disabled checks, no session/audit). login() now reuses it, so a wrong password on a later request still 401s (anti-fixation intact) without spawning sessions.
  2. Bearer skipped revocation/disabled checks — added the active-session + disabled-user checks from JwtStrategy.
    Also: a global per-email limiter key (so X-Forwarded-For rotation can't brute one account), X-MCP-Token array-value handling + timingSafeEqual, empty-email Basic rejected, and the limiter only counts genuine credential failures (not "email not verified").

Verification

  • pnpm --filter server build, pnpm --filter @docmost/mcp build, pnpm --filter client build — clean.
  • Tests: @docmost/mcp 239 pass; server -- mcp 26 pass (Basic split incl. : in password; good/bad creds; Bearer ACCESS incl. inactive-session/disabled rejection; service-account fallback; anti-fixation distinct identities; limiter threshold/reset/global-email key; X-MCP-Token; resolver called once per session). Decision logic is in framework-free helpers because the full Nest graph doesn't load under this repo's jest (pre-existing).
  • Integration (live endpoint, raw JSON-RPC over Streamable-HTTP): no-auth → 401 (clear accepted-methods message, not 503); Basic admin → session established, tools/list (38 tools), list_spaces returned the admin's real spaces (per-user CASL via the issued JWT); wrong password → 401 "Email or password does not match"; anti-fixation — reusing the valid session id with a wrong password OR no auth both → 401; limiter trips at the 6th rapid failure (429-equivalent); no password echoed in any body.

Compatibility

stdio + the static service-account path are unchanged (the resolver is additive; static config still flows through). .env.example marks MCP_DOCMOST_EMAIL/PASSWORD optional and documents Basic/Bearer + X-MCP-Token.

🤖 Generated with Claude Code

Implements `docs/backlog/mcp-per-user-auth.md` (chosen variant L: HTTP Basic, server-validated). ## What The embedded `/mcp` server acted as one shared service account — every MCP client had that account's CASL and edit attribution. Now each `/mcp` session authenticates as the **current user**: tools run under the user's CASL, edits attribute to them, and the service account is no longer required. ## How - **Basic (primary):** `Authorization: Basic email:password` → validated server-side via `AuthService`; the session carries the **issued user JWT** (not the raw password). Password may contain `:` (split on first only). - **Bearer (fallback):** `Authorization: Bearer <access JWT>` → `verifyJwt(ACCESS)` **plus** an active-session + non-disabled-user check (matching `JwtStrategy`), so revoked/disabled users are rejected. - **Service account (fallback):** kept for the no-creds + env-configured case (CI/scripts). - `packages/mcp` `createMcpHttpHandler` now accepts a per-request **config resolver** (static config / stdio path unchanged). Identity is bound to the `mcp-session-id` at init and re-validated from the caller's **own** credentials on every request. - `MCP_TOKEN` moved off `Authorization` to an `X-MCP-Token` header (timing-safe compare); the old `credsConfigured` 503 gate is replaced by a clear 401 listing accepted methods. ## Reasoning / decisions (resolved the plan's open questions) - **Server-side validation** (not forwarding the raw password to the mcp package): errors surface as clean pre-hijack 401 JSON, and the password never travels further than the auth check. - **Service account kept** as an optional fallback (didn't force `MCP_REQUIRE_USER_TOKEN`) — preserves existing installs / headless use. - **X-MCP-Token** rather than dropping the shared guard, so a deployment can still gate the endpoint while `Authorization` carries user creds. ## Review findings & fixes (adversarial security review) The review confirmed the three highest-risk properties hold: no auth-less reuse of an established session; identity re-validated from the caller's own creds every request; no secret logging. Two **blockers** it found were fixed: 1. **`login()` ran on every request** → a new `user_sessions` row + `USER_LOGIN` audit per request (audit spam / session-table DoS). Fixed: the session-minting `login()` happens **once** at `initialize`; subsequent requests re-verify credentials via a new **non-side-effecting** `AuthService.verifyUserCredentials` (lookup + password compare + email-verified + disabled checks, no session/audit). `login()` now reuses it, so a wrong password on a later request still 401s (anti-fixation intact) without spawning sessions. 2. **Bearer skipped revocation/disabled checks** — added the active-session + disabled-user checks from `JwtStrategy`. Also: a **global per-email** limiter key (so X-Forwarded-For rotation can't brute one account), `X-MCP-Token` array-value handling + `timingSafeEqual`, empty-email Basic rejected, and the limiter only counts genuine credential failures (not "email not verified"). ## Verification - `pnpm --filter server build`, `pnpm --filter @docmost/mcp build`, `pnpm --filter client build` — clean. - Tests: `@docmost/mcp` **239 pass**; server `-- mcp` **26 pass** (Basic split incl. `:` in password; good/bad creds; Bearer ACCESS incl. inactive-session/disabled rejection; service-account fallback; anti-fixation distinct identities; limiter threshold/reset/global-email key; X-MCP-Token; resolver called once per session). Decision logic is in framework-free helpers because the full Nest graph doesn't load under this repo's jest (pre-existing). - Integration (live endpoint, raw JSON-RPC over Streamable-HTTP): no-auth → **401** (clear accepted-methods message, not 503); Basic `admin` → session established, `tools/list` (38 tools), `list_spaces` returned the admin's real spaces (**per-user CASL via the issued JWT**); wrong password → **401** "Email or password does not match"; **anti-fixation** — reusing the valid session id with a wrong password OR no auth both → **401**; **limiter** trips at the 6th rapid failure (429-equivalent); no password echoed in any body. ## Compatibility stdio + the static service-account path are unchanged (the resolver is additive; static config still flows through). `.env.example` marks `MCP_DOCMOST_EMAIL/PASSWORD` optional and documents Basic/Bearer + `X-MCP-Token`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 2 commits 2026-06-20 07:20:06 +03:00
feat(mcp): per-user auth for the embedded /mcp endpoint 4a00dfc3b2 
The embedded MCP server acted as a single service account; now each /mcp
session authenticates as the current user, so tools run under that user's
CASL and edits attribute to them.

- HTTP Basic (chosen path): Authorization: Basic email:password, validated
  server-side via AuthService; the session carries the issued user JWT (not
  the raw password). Password may contain ':' (split on first only).
- Bearer fallback: Authorization: Bearer <access JWT>, verified as ACCESS and
  additionally checked for an active session + non-disabled user (matching
  JwtStrategy), so revoked/disabled users are rejected.
- Service account stays as an optional fallback (no creds + env configured).
- packages/mcp createMcpHttpHandler accepts a per-request config resolver
  (back-compat: static config / stdio unchanged); identity is bound to the
  mcp-session-id at init and re-validated from the caller's own credentials on
  every request (anti session-fixation: a guessed session id can't be reused
  without matching creds).
- A full login (session + audit) happens only once at session init; later
  requests re-verify credentials via a new non-side-effecting
  AuthService.verifyUserCredentials (no session/audit spam).
- Failed-login limiter (5/60s, keyed per-IP, per-IP+email, and per-email so IP
  rotation can't brute one account) since direct login bypasses the controller
  throttler. Only real credential failures count.
- MCP_TOKEN shared guard moved off Authorization to an X-MCP-Token header
  (timing-safe compare); credsConfigured 503 gate replaced by a clear 401.
- No secrets logged; all auth resolved before res.hijack() so failures return
  clean 401 JSON. .env.example marks the service account optional.

Implements docs/backlog/mcp-per-user-auth.md (variant L).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
docs: remove implemented mcp-per-user-auth backlog plan 1483e021d1 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ghost added 1 commit 2026-06-20 13:27:29 +03:00
fix(mcp): close SSO/MFA bypass on Basic + stop non-init session mint bfd79b94bc 
Release-cycle review found the /mcp Basic path skipped the controller's
pre-token gates and over-eagerly minted sessions:

- SSO/MFA bypass (blocker): the Basic path called AuthService.login/
  verifyUserCredentials directly, but validateSsoEnforcement + the lazy EE MFA
  gate live in AuthController.login. Now enforceBasicLoginGate runs in the Basic
  branch BEFORE any token is minted: validateSsoEnforcement(workspace) (reject
  on enforced SSO) and the same lazy-require MFA check the controller uses
  (reject MFA users -> 'use a Bearer access token'). No EE module bundled (this
  fork) -> no MFA gate, identical to the controller; a throw from the check
  fails closed (no token). Bearer/service-account paths are not gated (those
  JWTs are minted post-gate).
- Non-init session mint: isSessionInit is now (no mcp-session-id) AND the body
  is a real JSON-RPC initialize (isInitializeRequestBody). A header-less
  non-initialize request takes the side-effect-free verifyCredentials path -> no
  user_sessions row, no USER_LOGIN audit, no lastLoginAt bump.
- FailedLoginLimiter.sweep() now runs on an unref'd 60s interval, cleared on
  module destroy (was never scheduled -> unbounded Map growth under XFF rotation).
- Subsequent (non-init) valid login no longer resets the global per-email brute
  bucket (only per-IP / per-IP+email); the email backstop is reset only on a
  deliberate init login.

Note: in a hypothetical EE build, checkMfaRequirements is called with no
FastifyReply (we only read requirement flags); a res-dereferencing EE impl would
surface as a clean rejection (fail-closed), not a bypass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ghost added 1 commit 2026-06-20 14:32:41 +03:00
test(mcp): cover X-MCP-Token/clientIp/bearer-type/creds-failure (pure seams) 3672093f56 
Release-cycle test audit: the /mcp auth's constant-time token guard, IP keying,
ACCESS-type pinning, and brute-force message coupling were untested. Extract
behavior-preserving pure helpers so they're testable and cover them:
- sharedTokenMatches: length-mismatch early-returns before timingSafeEqual
  (which throws on unequal lengths); equal-length uses timingSafeEqual; array
  header -> first element; non-string -> false.
- clientIp: req.ip > socket > first XFF hop > 'unknown' (limiter keying).
- bindAccessJwtVerifier: verifyJwt pinned to JwtType.ACCESS (rejects REFRESH).
- CREDENTIALS_MISMATCH_MESSAGE single source of truth shared by
  verifyUserCredentials and isCredentialsFailure, so a reworded auth error can't
  silently disable the /mcp brute-force counter.
- verifyUserCredentials no-side-effect contract asserted via a TS-AST spec
  (AuthService can't load under jest): its body has no createSessionAndToken/
  audit/updateLastLogin while login() has all three.
Extractions are behavior-preserving (reviewed); class delegates to the helpers,
dead code + unused imports removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ghost merged commit f72e44c9b7 into develop 2026-06-20 19:32:03 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 idea
Idea / proposal for discussion
 invalid
This doesn't seem right
 needs-human
эскалация: нужно решение человека
 question
Further information is requested
 refactor
Code cleanup / refactoring
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 security
Security / hardening issue
 status/blocked
ждёт зависимость blocked_by
 status/done
закрыто и проверено
 status/in-progress
в активной работе (мягкая заявка)
 status/ready
специфицировано, не заблокировано, ждёт исполнителя
 test
Test coverage / test infrastructure
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#13
Delete Branch "feat/mcp-per-user-auth"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?