diff --git a/.env.example b/.env.example index 5acc78b8..b04078e3 100644 --- a/.env.example +++ b/.env.example @@ -69,12 +69,26 @@ DEBUG_DB=false # Log http requests LOG_HTTP=false -# MCP server (community): service account the embedded MCP uses to talk to this Docmost instance +# MCP server (community): the embedded /mcp endpoint authenticates PER USER. +# An MCP client authenticates with one of: +# - HTTP Basic: `Authorization: Basic base64(email:password)` — the user's own +# Docmost login/password. The server validates the credentials and the MCP +# session then acts under that user's permissions (edits attributed to them). +# - Bearer access JWT: `Authorization: Bearer ` (the user's +# `authToken` cookie value). Validated as an ACCESS token. +# +# OPTIONAL service-account fallback. When a request carries NEITHER Basic NOR +# Bearer credentials and these are set, the MCP session falls back to this +# shared service account (back-compat; useful for CI/scripts). Leave BLANK to +# require per-user credentials. MCP_DOCMOST_EMAIL= MCP_DOCMOST_PASSWORD= # MCP_DOCMOST_API_URL=http://127.0.0.1:3000/api -# Optional bearer token to protect the /mcp endpoint. If unset, /mcp relies on -# the workspace MCP toggle and network isolation (do not expose the port publicly). +# Optional shared guard for the /mcp endpoint. When set, every /mcp request must +# carry a matching `X-MCP-Token` header (separate from `Authorization`, which now +# carries the per-user credentials). When unset, /mcp relies on the per-user +# credentials above plus the workspace MCP toggle and network isolation (do not +# expose the port publicly). # MCP_TOKEN= # MCP_SESSION_IDLE_MS=1800000 diff --git a/apps/server/src/core/auth/auth.constants.ts b/apps/server/src/core/auth/auth.constants.ts index fda2346e..861e5d81 100644 --- a/apps/server/src/core/auth/auth.constants.ts +++ b/apps/server/src/core/auth/auth.constants.ts @@ -2,3 +2,19 @@ export enum UserTokenType { FORGOT_PASSWORD = 'forgot-password', EMAIL_VERIFICATION = 'email-verification', } + +/** + * The single source of truth for the credentials-mismatch error message. + * + * `AuthService.verifyUserCredentials`/`login` throw an UnauthorizedException + * with EXACTLY this message for every credentials-failure case (unknown email, + * disabled user, wrong password). The /mcp Basic brute-force limiter relies on + * recognising that exact failure via `isCredentialsFailure` (mcp-auth.helpers), + * which matches against this same constant. Keeping a single shared constant + * means a reworded auth error cannot silently stop counting toward the limiter + * (which would turn /mcp Basic into an unthrottled password-guessing oracle). + * This file is intentionally dependency-light so it loads from both core/auth + * and the framework-free integrations/mcp helpers without dragging the heavy + * auth graph. + */ +export const CREDENTIALS_MISMATCH_MESSAGE = 'Email or password does not match'; diff --git a/apps/server/src/core/auth/auth.module.ts b/apps/server/src/core/auth/auth.module.ts index c440bdc2..236f4dd5 100644 --- a/apps/server/src/core/auth/auth.module.ts +++ b/apps/server/src/core/auth/auth.module.ts @@ -10,6 +10,6 @@ import { TokenModule } from './token.module'; imports: [TokenModule, WorkspaceModule], controllers: [AuthController], providers: [AuthService, SignupService, JwtStrategy], - exports: [SignupService], + exports: [SignupService, AuthService], }) export class AuthModule {} diff --git a/apps/server/src/core/auth/services/auth.service.ts b/apps/server/src/core/auth/services/auth.service.ts index bfd8e1a0..b27df4bc 100644 --- a/apps/server/src/core/auth/services/auth.service.ts +++ b/apps/server/src/core/auth/services/auth.service.ts @@ -28,7 +28,7 @@ import ForgotPasswordEmail from '@docmost/transactional/emails/forgot-password-e import { UserTokenRepo } from '@docmost/db/repos/user-token/user-token.repo'; import { PasswordResetDto } from '../dto/password-reset.dto'; import { User, UserToken, Workspace } from '@docmost/db/types/entity.types'; -import { UserTokenType } from '../auth.constants'; +import { UserTokenType, CREDENTIALS_MISMATCH_MESSAGE } from '../auth.constants'; import { KyselyDB } from '@docmost/db/types/kysely.types'; import { InjectKysely } from 'nestjs-kysely'; import { executeTx } from '@docmost/db/utils'; @@ -57,12 +57,30 @@ export class AuthService { @Inject(AUDIT_SERVICE) private readonly auditService: IAuditService, ) {} - async login(loginDto: LoginDto, workspaceId: string) { + /** + * Verify a user's email + password WITHOUT any side effects: it performs the + * exact same user lookup, password comparison, email-verified and disabled + * checks as `login()`, but does NOT mint a session/token, does NOT write the + * USER_LOGIN audit event, and does NOT update lastLoginAt. Returns the matched + * user on success; throws UnauthorizedException (credentials) or whatever + * `throwIfEmailNotVerified` throws otherwise. + * + * Use this for repeated per-request credential re-validation (e.g. the /mcp + * anti-fixation check on subsequent requests) where minting a new DB session + * and audit row on every call would be audit spam / a session-table DoS. The + * full `login()` reuses it so there is no behaviour drift between the two. + */ + async verifyUserCredentials( + loginDto: LoginDto, + workspaceId: string, + ): Promise { const user = await this.userRepo.findByEmail(loginDto.email, workspaceId, { includePassword: true, }); - const errorMessage = 'Email or password does not match'; + // Single source of truth (see auth.constants): the /mcp brute-force limiter + // recognises this exact message via isCredentialsFailure. + const errorMessage = CREDENTIALS_MISMATCH_MESSAGE; if (!user || isUserDisabled(user)) { throw new UnauthorizedException(errorMessage); } @@ -84,6 +102,12 @@ export class AuthService { appSecret: this.environmentService.getAppSecret(), }); + return user; + } + + async login(loginDto: LoginDto, workspaceId: string) { + const user = await this.verifyUserCredentials(loginDto, workspaceId); + user.lastLoginAt = new Date(); await this.userRepo.updateLastLogin(user.id, workspaceId); diff --git a/apps/server/src/core/auth/services/verify-user-credentials.contract.spec.ts b/apps/server/src/core/auth/services/verify-user-credentials.contract.spec.ts new file mode 100644 index 00000000..30689bd6 --- /dev/null +++ b/apps/server/src/core/auth/services/verify-user-credentials.contract.spec.ts @@ -0,0 +1,103 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as ts from 'typescript'; + +/** + * Security contract for AuthService.verifyUserCredentials (item 4). + * + * verifyUserCredentials is the NON-side-effecting credential check used by the + * /mcp anti-fixation path on subsequent requests: it must perform the same + * lookup/password/email-verified/disabled checks as login() but mint NO session, + * write NO USER_LOGIN audit row and update NO lastLoginAt. Calling the + * side-effecting login() per /mcp tool call would be audit spam + a + * session-table DoS, so the no-side-effect property is load-bearing. + * + * Why this is a SOURCE-LEVEL (AST) contract test rather than a live AuthService + * unit: AuthService cannot be constructed — or even imported — under this jest + * config. jest is rooted at `src/` with no `^src/(.*)` moduleNameMapper, so the + * transitive `import ... from 'src/integrations/queue/constants'` chain + * (AuthService -> SignupService -> WorkspaceService -> SpaceService) does not + * resolve; and even with that mapped, importing AuthService pulls in the + * `@docmost/transactional` React email templates and the lib0/ESM collaboration + * graph, which jest's ts-jest transform (with the repo's transformIgnorePatterns) + * cannot load. (The pre-existing auth.service.spec.ts placeholder fails to run + * for exactly this reason.) So we assert the contract STRUCTURALLY against the + * real source: verifyUserCredentials must contain none of the three side + * effects, and login() must contain all three — a regression that adds a side + * effect to verifyUserCredentials, or drops one from login, fails this test. + */ + +const SIDE_EFFECTS = [ + // session/token mint (user_sessions insert + JWT) + 'createSessionAndToken', + // USER_LOGIN audit event (precise call expression, not a bare "log") + 'auditService.log', + // lastLoginAt bump + 'updateLastLogin', +] as const; + +function methodBodyText(source: string, methodName: string): string { + const sf = ts.createSourceFile( + 'auth.service.ts', + source, + ts.ScriptTarget.Latest, + /* setParentNodes */ true, + ); + + let found: string | null = null; + const visit = (node: ts.Node): void => { + if ( + ts.isMethodDeclaration(node) && + node.name && + ts.isIdentifier(node.name) && + node.name.text === methodName && + node.body + ) { + found = node.body.getText(sf); + return; + } + ts.forEachChild(node, visit); + }; + visit(sf); + + if (found === null) { + throw new Error(`method ${methodName} not found in auth.service.ts`); + } + return found; +} + +describe('AuthService no-side-effect contract (item 4)', () => { + const sourcePath = path.join(__dirname, 'auth.service.ts'); + const source = fs.readFileSync(sourcePath, 'utf8'); + + const verifyBody = methodBodyText(source, 'verifyUserCredentials'); + const loginBody = methodBodyText(source, 'login'); + + it('verifyUserCredentials performs NONE of the side effects', () => { + // No session/token mint, no audit log write, no lastLoginAt update. + expect(verifyBody).not.toContain('createSessionAndToken'); + expect(verifyBody).not.toContain('updateLastLogin'); + expect(verifyBody).not.toContain('auditService.log'); + // It still does the real credential work (lookup + password compare). + expect(verifyBody).toContain('findByEmail'); + expect(verifyBody).toContain('comparePasswordHash'); + // ...and returns the matched user (so login() can reuse it). + expect(verifyBody).toContain('return user'); + }); + + it('login() performs ALL three side effects', () => { + expect(loginBody).toContain('updateLastLogin'); + expect(loginBody).toContain('auditService.log'); + expect(loginBody).toContain('createSessionAndToken'); + // login() reuses verifyUserCredentials, so there is no behaviour drift + // between the side-effecting and non-side-effecting credential paths. + expect(loginBody).toContain('verifyUserCredentials'); + }); + + it('every side effect that login() has is ABSENT from verifyUserCredentials', () => { + for (const effect of SIDE_EFFECTS) { + expect(loginBody.includes(effect)).toBe(true); + expect(verifyBody.includes(effect)).toBe(false); + } + }); +}); diff --git a/apps/server/src/integrations/mcp/mcp-auth.helpers.ts b/apps/server/src/integrations/mcp/mcp-auth.helpers.ts new file mode 100644 index 00000000..4a0b5be1 --- /dev/null +++ b/apps/server/src/integrations/mcp/mcp-auth.helpers.ts @@ -0,0 +1,533 @@ +// Pure, self-contained helpers for the embedded /mcp per-user auth flow. They +// are deliberately framework-free (no Nest, no DI, no concrete service imports) +// so they can be unit-tested in isolation WITHOUT loading the heavy auth/space +// dependency graph, and reused by McpService. Nothing here logs the password or +// the Authorization header. +import { UnauthorizedException } from '@nestjs/common'; +import { timingSafeEqual } from 'node:crypto'; +import { JwtType } from '../../core/auth/dto/jwt-payload'; +import { CREDENTIALS_MISMATCH_MESSAGE } from '../../core/auth/auth.constants'; + +/** + * Decode an `Authorization: Basic base64(email:password)` header into its + * email/password parts. The split is on the FIRST ':' because a password may + * itself contain ':' characters (everything after the first ':' is the + * password). Returns null when the header is absent or not a Basic header, or + * when no ':' separator is present (malformed credentials). + */ +export function parseBasicAuth( + authHeader: string | undefined, +): { email: string; password: string } | null { + if (!authHeader || !authHeader.startsWith('Basic ')) return null; + const b64 = authHeader.slice('Basic '.length).trim(); + let decoded: string; + try { + decoded = Buffer.from(b64, 'base64').toString('utf8'); + } catch { + return null; + } + const sep = decoded.indexOf(':'); + if (sep === -1) return null; // no separator -> not valid email:password + const email = decoded.slice(0, sep); + if (!email) return null; // empty email -> not valid credentials + return { + email, + password: decoded.slice(sep + 1), + }; +} + +/** + * Lightweight in-memory, per-key fixed-window rate limiter for FAILED /mcp + * Basic logins. Calling AuthService.login directly bypasses the controller's + * ThrottlerGuard, so this blunts brute-force attempts against /mcp. State lives + * in-process (per server instance); it is intentionally simple and not shared + * across a cluster — it is a speed bump, not a hard security boundary. + * + * A key is typically `` and/or `:`. When the number of failures + * within `windowMs` reaches `threshold`, `isBlocked` returns true until the + * window rolls over. A SUCCESSFUL login should clear the key via `reset`. + */ +export class FailedLoginLimiter { + private readonly windowMs: number; + private readonly threshold: number; + // key -> { count, windowStart } + private readonly buckets = new Map< + string, + { count: number; windowStart: number } + >(); + + constructor(threshold = 5, windowMs = 60_000) { + this.threshold = threshold; + this.windowMs = windowMs; + } + + private bucket(key: string, now: number) { + const existing = this.buckets.get(key); + if (!existing || now - existing.windowStart >= this.windowMs) { + const fresh = { count: 0, windowStart: now }; + this.buckets.set(key, fresh); + return fresh; + } + return existing; + } + + /** True when the key has already reached the failure threshold this window. */ + isBlocked(key: string, now: number = Date.now()): boolean { + const b = this.bucket(key, now); + return b.count >= this.threshold; + } + + /** Record one failed attempt for the key (within the current window). */ + recordFailure(key: string, now: number = Date.now()): void { + const b = this.bucket(key, now); + b.count += 1; + } + + /** Clear the key after a successful login so it does not accumulate. */ + reset(key: string): void { + this.buckets.delete(key); + } + + /** Drop expired buckets to bound memory. Safe to call periodically. */ + sweep(now: number = Date.now()): void { + for (const [key, b] of this.buckets) { + if (now - b.windowStart >= this.windowMs) this.buckets.delete(key); + } + } +} + +// The per-session DocmostMcpConfig shape understood by @docmost/mcp: either the +// service-account credentials variant OR the per-user getToken variant. +export type DocmostMcpConfig = + | { apiUrl: string; email: string; password: string } + | { apiUrl: string; getToken: () => Promise }; + +export interface ResolvedMcpAuth { + config: DocmostMcpConfig; + // Opaque identity key bound to the MCP session for anti-fixation, or + // undefined when no per-user identity applies. + identity?: string; +} + +// Narrow collaborator interfaces so this module never imports the concrete +// AuthService/TokenService/WorkspaceRepo classes (which drag in the heavy +// auth/space graph). McpService passes its injected instances; tests pass +// stubs. Decouples the testable decision logic from Nest DI wiring. +export interface McpAuthDeps { + apiUrl: string; + email?: string; + password?: string; + findWorkspace: () => Promise<{ id: string } | undefined>; + // Pre-token gate for the Basic path ONLY, replicating what AuthController.login + // does BEFORE issuing a token: validateSsoEnforcement(workspace) and the lazy + // EE MFA requirement check. It is invoked with the resolved (default) + // workspace right after it is loaded and BEFORE any login()/verifyCredentials() + // call, so an SSO-enforced workspace or an MFA-required user never gets a token + // via /mcp Basic. It MUST throw (UnauthorizedException) to reject; on a fork + // without the EE MFA module bundled it behaves exactly like the controller + // (no MFA module -> no MFA gate). The Bearer path skips this gate because those + // ACCESS JWTs were already minted post-gate by the normal controller login. + // Optional so existing callers/tests that don't exercise the gate are unchanged. + enforceBasicGate?: ( + workspace: { id: string }, + creds: { email: string; password: string }, + ) => Promise | void; + // Full login: mints a user session + JWT, writes the USER_LOGIN audit event + // and updates lastLoginAt. Called at MOST once per MCP session (at the + // session-init request) so we do not spam the audit log / user_sessions table + // on every tool call. + login: ( + creds: { email: string; password: string }, + workspaceId: string, + ) => Promise; + // Non-side-effecting credential check: same lookup/password/email-verified/ + // disabled checks as login() but mints NO session, writes NO audit row, + // updates NO lastLoginAt. Used for per-request anti-fixation re-validation on + // SUBSEQUENT requests so a correct repeat does not spawn a new DB session, + // while a wrong password still throws (preserving anti-fixation). + verifyCredentials: ( + creds: { email: string; password: string }, + workspaceId: string, + ) => Promise; + // Bearer access-JWT verification. Verifies signature/exp/type AND (in the + // McpService wiring) session-active + user-not-disabled, mirroring JwtStrategy + // so a revoked/logged-out/disabled user with an unexpired token is rejected. + verifyAccessJwt: (token: string) => Promise<{ sub?: string; email?: string }>; + limiter: FailedLoginLimiter; + clientIp: string; + // True when this is the session-INIT request (no mcp-session-id header). + // INIT mints a user session via login(); SUBSEQUENT requests only re-validate + // credentials via verifyCredentials() (no side effects). See resolveMcp... + isSessionInit: boolean; +} + +/** + * True when an error from login()/verifyCredentials() represents an actual + * CREDENTIALS failure (unknown email, disabled user, or wrong password) — i.e. + * a guessed-password signal that should count toward the brute-force limiter. + * + * It must NOT match business errors like "email not verified" (a + * BadRequestException), which are a legitimate 401/400 surface but not a + * password-guess signal — counting those would let an attacker burn a victim's + * limiter budget (DoS) and would dilute the brute-force signal. AuthService + * throws an UnauthorizedException with exactly this message for every + * credentials-mismatch case (no user / disabled / wrong password), so we match + * on that. + * + * The message is NOT hardcoded here: it matches against the shared + * CREDENTIALS_MISMATCH_MESSAGE constant that AuthService.verifyUserCredentials + * also throws, so a reworded auth error cannot silently stop counting toward the + * limiter (single source of truth — see auth.constants.ts). + */ +export function isCredentialsFailure(err: unknown): boolean { + return ( + err instanceof UnauthorizedException && + typeof err.message === 'string' && + err.message + .toLowerCase() + .includes(CREDENTIALS_MISMATCH_MESSAGE.toLowerCase()) + ); +} + +/** + * Constant-time comparison of the optional shared X-MCP-Token guard. A header + * value may arrive as string | string[] (multiple X-MCP-Token headers), so we + * normalise to the first string. crypto.timingSafeEqual avoids leaking the + * token's length via early-exit string comparison; it requires equal buffer + * lengths, so a length mismatch is treated as a non-match WITHOUT calling + * timingSafeEqual (which throws on unequal lengths). A non-string / undefined + * value is never a match. + * + * Pure and framework-free so it is unit-testable; McpService.handle delegates to + * it for the X-MCP-Token shared guard. + */ +export function sharedTokenMatches( + expected: string, + provided: string | string[] | undefined, +): boolean { + const value = Array.isArray(provided) ? provided[0] : provided; + if (typeof value !== 'string') return false; + const a = Buffer.from(value); + const b = Buffer.from(expected); + // Early-return before timingSafeEqual, which throws on unequal-length buffers. + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + +// Minimal structural shape of the bits of a Fastify request that `clientIp` +// needs. Kept structural so this module never imports the Fastify types. +export interface ClientIpRequest { + ip?: string; + socket?: { remoteAddress?: string }; + headers: Record; +} + +/** + * Best-effort client IP for the failed-login limiter key. Precedence: + * 1. req.ip — Fastify's resolved IP (honours a configured trustProxy + * chain); the trustworthy value when a proxy is set up. + * 2. socket.remoteAddress — the raw TCP peer, used only when req.ip is absent. + * 3. first X-Forwarded-For hop — LAST resort only, because XFF is + * client-forgeable when no trusted proxy is configured. + * 4. 'unknown' — nothing usable. + * + * A forged IP can only dodge the per-IP limiter keys; the GLOBAL per-email key + * in resolveMcpSessionConfig is the real account-brute backstop and does not + * depend on this value. Pure/framework-free so it is unit-testable; McpService + * delegates to it. + */ +export function clientIp(req: ClientIpRequest): string { + if (req.ip) return req.ip; + if (req.socket?.remoteAddress) return req.socket.remoteAddress; + const xff = req.headers['x-forwarded-for']; + if (typeof xff === 'string' && xff.length > 0) { + return xff.split(',')[0].trim(); + } + return 'unknown'; +} + +// Minimal structural shape of the TokenService.verifyJwt method we depend on, +// so this module never imports the concrete TokenService (heavy graph). +export interface AccessJwtVerifier { + verifyJwt: ( + token: string, + type: JwtType, + ) => Promise<{ + sub?: string; + email?: string; + workspaceId?: string; + sessionId?: string; + }>; +} + +/** + * Bind a TokenService-like verifier into a one-arg `verifyJwt(token)` that + * ALWAYS enforces `JwtType.ACCESS`. This is the single place where the /mcp + * Bearer path pins the token type: a Bearer access token must be verified AS an + * access token (not refresh/exchange/collab/etc.), so the type literal is fixed + * here rather than at the call site. McpService.verifyMcpBearer delegates to + * this, keeping the `JwtType.ACCESS` choice testable without the heavy graph. + */ +export function bindAccessJwtVerifier( + tokenService: AccessJwtVerifier, +): (token: string) => Promise<{ + sub?: string; + email?: string; + workspaceId?: string; + sessionId?: string; +}> { + return (token: string) => tokenService.verifyJwt(token, JwtType.ACCESS); +} + +// Minimal shapes for the Bearer revocation/disabled check. Kept structural so +// this module never imports the concrete repos/JwtPayload (heavy graph). +export interface BearerVerifyDeps { + // Verify signature/exp and that type === ACCESS; returns the decoded payload. + verifyJwt: ( + token: string, + ) => Promise<{ + sub?: string; + email?: string; + workspaceId?: string; + sessionId?: string; + }>; + // Load the user (or undefined) for the disabled check. + findUser: ( + sub: string, + workspaceId: string, + ) => Promise<{ deactivatedAt?: Date | null; deletedAt?: Date | null } | undefined>; + // Load an ACTIVE (not revoked, not expired) session by id, or undefined. + findActiveSession: ( + sessionId: string, + ) => Promise<{ userId: string; workspaceId: string } | undefined>; +} + +/** + * Verify a /mcp Bearer access JWT to the SAME strength as JwtStrategy: not just + * signature/exp/type (verifyJwt), but also that the user is not disabled and — + * when the token carries a sessionId — that the session is still active and + * belongs to that user+workspace. This rejects a logged-out/revoked or disabled + * user who still holds an unexpired access token. Throws UnauthorizedException + * on any failure; never leaks why (uniform "Invalid or expired token"). + */ +export async function verifyBearerAccess( + token: string, + deps: BearerVerifyDeps, +): Promise<{ sub?: string; email?: string }> { + const generic = 'Invalid or expired token'; + const payload = await deps.verifyJwt(token); + + if (!payload.sub || !payload.workspaceId) { + throw new UnauthorizedException(generic); + } + + const user = await deps.findUser(payload.sub, payload.workspaceId); + if (!user || user.deactivatedAt || user.deletedAt) { + throw new UnauthorizedException(generic); + } + + if (payload.sessionId) { + const session = await deps.findActiveSession(payload.sessionId); + if ( + !session || + session.userId !== payload.sub || + session.workspaceId !== payload.workspaceId + ) { + throw new UnauthorizedException(generic); + } + } + + return { sub: payload.sub, email: payload.email }; +} + +/** + * Detect a genuine JSON-RPC `initialize` request from an already-parsed body. + * Mirrors the @modelcontextprotocol/sdk `isInitializeRequest` signal that + * packages/mcp/src/http.ts uses to decide whether to mint a session, but + * framework/SDK-free so it is unit-testable and usable from the CommonJS + * McpService. An initialize request is a single JSON-RPC object whose `method` + * is exactly 'initialize'; a batch (array) body is never an initialize request. + * + * This is the second half of the session-INIT decision: `isSessionInit` is + * (no `mcp-session-id` header) AND `isInitializeRequestBody(body)`. Using it + * ensures the side-effecting login() (user_sessions insert + USER_LOGIN audit + + * lastLoginAt) only runs for a real initialize, never for an arbitrary + * header-less request that http.ts will subsequently 400. + */ +export function isInitializeRequestBody(body: unknown): boolean { + if (!body || typeof body !== 'object' || Array.isArray(body)) return false; + return (body as { method?: unknown }).method === 'initialize'; +} + +/** Extract a Bearer token from an Authorization header (case-insensitive). */ +export function extractBearer( + authHeader: string | undefined, +): string | undefined { + const [type, token] = authHeader?.split(' ') ?? []; + return type?.toLowerCase() === 'bearer' ? token : undefined; +} + +/** + * Pure decision logic for the /mcp per-session identity. Precedence: + * 1. HTTP Basic (email:password) -> validate via `login`, issue the user's + * JWT, run as that user (chosen path). Throttle FAILED logins per IP/email. + * 2. Authorization: Bearer -> verify as an ACCESS JWT, run with it. + * 3. Env service account -> back-compat fallback. + * 4. none -> meaningful 401. + * + * Throws UnauthorizedException with a SPECIFIC reason on failure (never a + * generic "MCP error"); never returns/logs the password or the Authorization + * header. The `JwtType.ACCESS` enforcement lives in `verifyAccessJwt`. + */ +export async function resolveMcpSessionConfig( + authHeader: string | undefined, + deps: McpAuthDeps, +): Promise { + const { apiUrl } = deps; + + // --- 1) chosen path: Basic login/password --- + const basic = parseBasicAuth(authHeader); + if (basic) { + const emailLc = basic.email.toLowerCase(); + const ipKey = `ip:${deps.clientIp}`; + const ipEmailKey = `ip-email:${deps.clientIp}:${emailLc}`; + // GLOBAL per-email key (no IP). Without this an attacker who rotates IP / + // X-Forwarded-For evades the per-IP and per-IP+email keys entirely and can + // brute a single account unthrottled. Keying one extra bucket on the email + // alone closes that account-brute hole regardless of source address. + // XFF tradeoff: clientIp is derived from the first X-Forwarded-For hop when + // present (see McpService.clientIp), which a client can forge when no + // trusted proxy is configured; the per-email global key is the part that + // does NOT depend on a trustworthy IP and is the real brute-force backstop. + const emailKey = `email:${emailLc}`; + if ( + deps.limiter.isBlocked(ipKey) || + deps.limiter.isBlocked(ipEmailKey) || + deps.limiter.isBlocked(emailKey) + ) { + throw new UnauthorizedException( + 'Too many failed MCP login attempts. Try again later.', + ); + } + + const workspace = await deps.findWorkspace(); + if (!workspace) { + throw new UnauthorizedException('No workspace is configured.'); + } + + // SSO/MFA pre-token gate (BLOCKER fix): replicate the AuthController.login + // gates BEFORE any token is issued on the Basic path. If the workspace + // enforces SSO, or the EE MFA module is bundled and this user/workspace + // requires MFA, this throws and we never mint a token. The Bearer path is + // intentionally NOT gated here (its JWT was already minted post-gate). This + // runs on BOTH init and subsequent Basic requests, but it must run before + // login()/verifyCredentials so an SSO/MFA user cannot authenticate at all. + // We do NOT count a gate rejection toward the brute-force limiter: it is not + // a password-guess signal. + if (deps.enforceBasicGate) { + await deps.enforceBasicGate(workspace, { + email: basic.email, + password: basic.password, + }); + } + + // Fix 1 (init vs subsequent): + // - SESSION INIT (no mcp-session-id): full login() mints the user JWT + // (the one allowed session creation + audit event for this MCP + // session). The DocmostClient caches that token, so later tool calls + // never re-login. + // - SUBSEQUENT request (has mcp-session-id): we only need to re-validate + // the caller's credentials for anti-fixation. verifyCredentials() does + // the SAME lookup/password/email-verified/disabled checks as login() + // but mints NO session, writes NO audit row and updates NO lastLoginAt, + // so a correct repeat does not spawn a DB session per request while a + // wrong password still 401s. The getToken here is never used to mint a + // new session: on a subsequent request the existing session already + // holds its token; this config is only consulted at init. + try { + if (deps.isSessionInit) { + const authToken = await deps.login( + { email: basic.email, password: basic.password }, + workspace.id, + ); + deps.limiter.reset(ipKey); + deps.limiter.reset(ipEmailKey); + deps.limiter.reset(emailKey); + return { + config: { apiUrl, getToken: async () => authToken }, + identity: `basic:${emailLc}`, + }; + } + await deps.verifyCredentials( + { email: basic.email, password: basic.password }, + workspace.id, + ); + } catch (err) { + // Only count an actual CREDENTIALS failure (wrong email/password) toward + // the brute-force limiter. Business errors like "email not verified" are + // a 401/400 surface but are NOT a guessed-password signal, so they must + // not let an attacker burn a victim's limiter budget or mask brute-force. + if (isCredentialsFailure(err)) { + deps.limiter.recordFailure(ipKey); + deps.limiter.recordFailure(ipEmailKey); + deps.limiter.recordFailure(emailKey); + } + const message = + err instanceof Error && err.message + ? err.message + : 'Email or password does not match'; + throw new UnauthorizedException(message); + } + // Subsequent request, credentials valid: clear the per-IP and per-IP+email + // budget, but DELIBERATELY do NOT reset the GLOBAL per-email key here. That + // email key is the only brute-force backstop that survives IP/XFF rotation; + // resetting it on every periodic tool call of a victim's live MCP session + // would repeatedly wipe a parallel attacker's failed-login budget for that + // email. The global email key is reset ONLY on a session-INIT login() + // success (above), which is a single deliberate authentication, not a + // high-frequency re-validation. + deps.limiter.reset(ipKey); + deps.limiter.reset(ipEmailKey); + return { + config: { apiUrl, getToken: async () => '' }, + identity: `basic:${emailLc}`, + }; + } + + // --- 2) fallback A: Bearer access-JWT (user-supplied token) --- + const bearer = extractBearer(authHeader); + if (bearer) { + let payload: { sub?: string; email?: string }; + try { + payload = await deps.verifyAccessJwt(bearer); + } catch (err) { + const message = + err instanceof Error && err.message + ? err.message + : 'Invalid or expired token'; + throw new UnauthorizedException(message); + } + return { + config: { apiUrl, getToken: async () => bearer }, + identity: `bearer:${payload.sub ?? payload.email ?? 'unknown'}`, + }; + } + + // --- 3) fallback B: env service account (existing behaviour, optional) --- + if (deps.email && deps.password) { + return { + config: { apiUrl, email: deps.email, password: deps.password }, + identity: 'service-account', + }; + } + + // --- 4) nothing usable --- + throw new UnauthorizedException( + 'MCP requires HTTP Basic auth (email:password) or a Bearer access token, ' + + 'or a configured MCP_DOCMOST_EMAIL/MCP_DOCMOST_PASSWORD service account.', + ); +} + +// Re-export JwtType so callers binding `verifyAccessJwt` know which type to +// enforce, without importing it separately. +export { JwtType }; diff --git a/apps/server/src/integrations/mcp/mcp.module.ts b/apps/server/src/integrations/mcp/mcp.module.ts index 5f927d60..8ed9cb39 100644 --- a/apps/server/src/integrations/mcp/mcp.module.ts +++ b/apps/server/src/integrations/mcp/mcp.module.ts @@ -3,13 +3,16 @@ import { McpController } from './mcp.controller'; import { McpService } from './mcp.service'; import { DatabaseModule } from '@docmost/db/database.module'; import { EnvironmentModule } from '../environment/environment.module'; +import { AuthModule } from '../../core/auth/auth.module'; +import { TokenModule } from '../../core/auth/token.module'; // Community MCP feature: the server itself serves the Model Context Protocol // over HTTP at /mcp. DatabaseModule (global) provides WorkspaceRepo and -// EnvironmentModule (global) provides EnvironmentService; both are imported -// explicitly for clarity. +// EnvironmentModule (global) provides EnvironmentService. AuthModule supplies +// AuthService (per-user HTTP-Basic login validation) and TokenModule supplies +// TokenService (Bearer access-JWT verification for the token fallback). @Module({ - imports: [DatabaseModule, EnvironmentModule], + imports: [DatabaseModule, EnvironmentModule, AuthModule, TokenModule], controllers: [McpController], providers: [McpService], }) diff --git a/apps/server/src/integrations/mcp/mcp.service.spec.ts b/apps/server/src/integrations/mcp/mcp.service.spec.ts new file mode 100644 index 00000000..bf4c8a24 --- /dev/null +++ b/apps/server/src/integrations/mcp/mcp.service.spec.ts @@ -0,0 +1,771 @@ +import { BadRequestException, UnauthorizedException } from '@nestjs/common'; +import { + parseBasicAuth, + FailedLoginLimiter, + resolveMcpSessionConfig, + isCredentialsFailure, + isInitializeRequestBody, + verifyBearerAccess, + sharedTokenMatches, + clientIp, + bindAccessJwtVerifier, + McpAuthDeps, +} from './mcp-auth.helpers'; +import { JwtType } from '../../core/auth/dto/jwt-payload'; +import { CREDENTIALS_MISMATCH_MESSAGE } from '../../core/auth/auth.constants'; + +// The /mcp per-user auth decision logic is tested through the framework-free +// `resolveMcpSessionConfig` helper that McpService delegates to. McpService +// itself cannot be instantiated under jest because importing AuthService drags +// in the React email templates + queue constants graph; extracting the pure +// logic (and wiring it in) keeps it both tested AND used (per the plan). + +function basicHeader(email: string, password: string): string { + return 'Basic ' + Buffer.from(`${email}:${password}`).toString('base64'); +} + +function makeDeps(over: Partial = {}): McpAuthDeps { + return { + apiUrl: 'http://127.0.0.1:3000/api', + email: over.email, + password: over.password, + findWorkspace: + over.findWorkspace ?? jest.fn().mockResolvedValue({ id: 'ws-1' }), + login: over.login ?? jest.fn().mockResolvedValue('issued-user-jwt'), + verifyCredentials: + over.verifyCredentials ?? jest.fn().mockResolvedValue(undefined), + verifyAccessJwt: + over.verifyAccessJwt ?? + jest.fn().mockResolvedValue({ sub: 'user-1', email: 'u@e.com' }), + // Default gate is a no-op (pass-through), matching a build with no SSO + // enforcement and no EE MFA module. Individual tests override it to assert + // the SSO/MFA reject behaviour. + enforceBasicGate: over.enforceBasicGate, + limiter: over.limiter ?? new FailedLoginLimiter(5, 60_000), + clientIp: over.clientIp ?? '10.0.0.1', + // Default to the session-INIT request (no mcp-session-id) so existing + // assertions about login() being called keep their meaning. + isSessionInit: over.isSessionInit ?? true, + }; +} + +describe('parseBasicAuth', () => { + it('decodes email:password', () => { + expect(parseBasicAuth(basicHeader('a@b.com', 'pw'))).toEqual({ + email: 'a@b.com', + password: 'pw', + }); + }); + + it('splits on the FIRST colon so passwords may contain colons', () => { + expect(parseBasicAuth(basicHeader('a@b.com', 'p:w:x'))).toEqual({ + email: 'a@b.com', + password: 'p:w:x', + }); + }); + + it('returns null for non-Basic / malformed headers', () => { + expect(parseBasicAuth(undefined)).toBeNull(); + expect(parseBasicAuth('Bearer xyz')).toBeNull(); + expect( + parseBasicAuth('Basic ' + Buffer.from('nocolon').toString('base64')), + ).toBeNull(); + }); + + it('returns null when the email part is empty (":password")', () => { + expect( + parseBasicAuth('Basic ' + Buffer.from(':pw').toString('base64')), + ).toBeNull(); + }); +}); + +describe('isCredentialsFailure', () => { + it('is true for the credentials-mismatch UnauthorizedException', () => { + expect( + isCredentialsFailure( + new UnauthorizedException('Email or password does not match'), + ), + ).toBe(true); + }); + + it('is false for business errors like email-not-verified', () => { + expect( + isCredentialsFailure( + new BadRequestException('Please verify your email address.'), + ), + ).toBe(false); + expect(isCredentialsFailure(new Error('boom'))).toBe(false); + }); + + // --- Cross-file coupling lock (item 1) --------------------------------- + // The /mcp Basic brute-force limiter ONLY counts a failure when + // isCredentialsFailure(err) is true. AuthService.verifyUserCredentials throws + // the credentials failure with the shared CREDENTIALS_MISMATCH_MESSAGE for + // unknown email / wrong password / disabled user. If that message were + // reworded without updating the matcher, the limiter would stop counting and + // /mcp Basic would become an unthrottled password-guessing oracle. These + // tests lock the coupling to the SHARED constant (single source of truth) so a + // reword is a compile-time/test-time break, not a silent security regression. + + it('recognises the exact UnauthorizedException AuthService throws (the shared constant)', () => { + // Reconstruct the EXACT exception AuthService.verifyUserCredentials throws + // for every credentials-failure case (it uses CREDENTIALS_MISMATCH_MESSAGE), + // and assert the REAL isCredentialsFailure recognises it. No hardcoded string + // is duplicated here — both sides reference the single shared constant. + const authThrows = new UnauthorizedException(CREDENTIALS_MISMATCH_MESSAGE); + expect(isCredentialsFailure(authThrows)).toBe(true); + }); + + it('the matcher is coupled to the single source of truth, not a local literal', () => { + // If someone reworded CREDENTIALS_MISMATCH_MESSAGE, this still passes only + // because the matcher derives its substring from the SAME constant. This + // pins the coupling structurally: there is one message both files share. + expect(CREDENTIALS_MISMATCH_MESSAGE).toBeTruthy(); + expect( + isCredentialsFailure( + new UnauthorizedException(CREDENTIALS_MISMATCH_MESSAGE), + ), + ).toBe(true); + // A DIFFERENT message (a hypothetical reword that forgot to go through the + // constant) must NOT be silently recognised, proving the matcher is not just + // "always true". + expect( + isCredentialsFailure(new UnauthorizedException('totally different wording')), + ).toBe(false); + }); +}); + +describe('AuthService verifyUserCredentials <-> isCredentialsFailure coupling (item 1)', () => { + // AuthService cannot be constructed under jest: importing it pulls in + // src/integrations/queue/constants (a `src/`-rooted absolute import) which the + // jest moduleNameMapper does not resolve under rootDir:src — the heavy auth + // graph. So instead of a live AuthService unit, we assert the security + // contract structurally: AuthService.verifyUserCredentials throws an + // UnauthorizedException built from the SHARED CREDENTIALS_MISMATCH_MESSAGE + // (see auth.service.ts), and the REAL isCredentialsFailure recognises it. The + // single shared constant is the lock: there is no second copy of the string to + // drift out of sync. + it('the credentials-failure UnauthorizedException is counted by the limiter matcher', () => { + // unknown email / disabled user / wrong password all surface as this: + const credentialsFailure = new UnauthorizedException( + CREDENTIALS_MISMATCH_MESSAGE, + ); + expect(isCredentialsFailure(credentialsFailure)).toBe(true); + }); + + it('email-not-verified (a different, business error) is NOT counted', () => { + // throwIfEmailNotVerified throws a BadRequestException, which must not burn a + // victim's limiter budget; the matcher rejects it. + expect( + isCredentialsFailure( + new BadRequestException('Please verify your email address.'), + ), + ).toBe(false); + }); +}); + +describe('FailedLoginLimiter', () => { + it('blocks after threshold failures within the window; reset clears it', () => { + const lim = new FailedLoginLimiter(3, 1000); + const k = 'ip:1.2.3.4'; + expect(lim.isBlocked(k, 0)).toBe(false); + lim.recordFailure(k, 0); + lim.recordFailure(k, 0); + expect(lim.isBlocked(k, 0)).toBe(false); + lim.recordFailure(k, 0); + expect(lim.isBlocked(k, 0)).toBe(true); + lim.reset(k); + expect(lim.isBlocked(k, 0)).toBe(false); + }); + + it('rolls over after the window', () => { + const lim = new FailedLoginLimiter(1, 1000); + const k = 'ip:1.2.3.4'; + lim.recordFailure(k, 0); + expect(lim.isBlocked(k, 0)).toBe(true); + expect(lim.isBlocked(k, 1000)).toBe(false); + }); +}); + +describe('verifyBearerAccess (Bearer revocation/disabled checks)', () => { + const goodPayload = { + sub: 'user-1', + email: 'u@e.com', + workspaceId: 'ws-1', + sessionId: 'sess-1', + }; + + function bearerDeps(over: Partial[1]> = {}) { + return { + verifyJwt: over.verifyJwt ?? jest.fn().mockResolvedValue(goodPayload), + findUser: + over.findUser ?? jest.fn().mockResolvedValue({ deactivatedAt: null }), + findActiveSession: + over.findActiveSession ?? + jest + .fn() + .mockResolvedValue({ userId: 'user-1', workspaceId: 'ws-1' }), + }; + } + + it('valid token + active session + enabled user -> resolves identity', async () => { + const res = await verifyBearerAccess('t', bearerDeps()); + expect(res).toEqual({ sub: 'user-1', email: 'u@e.com' }); + }); + + it('rejects when the session is no longer active (logged out / revoked)', async () => { + await expect( + verifyBearerAccess( + 't', + bearerDeps({ findActiveSession: jest.fn().mockResolvedValue(undefined) }), + ), + ).rejects.toThrow(UnauthorizedException); + }); + + it('rejects when the session belongs to a different user', async () => { + await expect( + verifyBearerAccess( + 't', + bearerDeps({ + findActiveSession: jest + .fn() + .mockResolvedValue({ userId: 'other', workspaceId: 'ws-1' }), + }), + ), + ).rejects.toThrow(UnauthorizedException); + }); + + it('rejects when the user is disabled (deactivated/deleted)', async () => { + await expect( + verifyBearerAccess( + 't', + bearerDeps({ + findUser: jest.fn().mockResolvedValue({ deactivatedAt: new Date() }), + }), + ), + ).rejects.toThrow(UnauthorizedException); + await expect( + verifyBearerAccess( + 't', + bearerDeps({ findUser: jest.fn().mockResolvedValue(undefined) }), + ), + ).rejects.toThrow(UnauthorizedException); + }); + + it('propagates a verifyJwt failure (bad signature/exp/type)', async () => { + await expect( + verifyBearerAccess( + 't', + bearerDeps({ + verifyJwt: jest + .fn() + .mockRejectedValue(new UnauthorizedException('jwt expired')), + }), + ), + ).rejects.toThrow('jwt expired'); + }); +}); + +describe('resolveMcpSessionConfig', () => { + it('Basic good creds -> calls login with the default workspace, returns a getToken config', async () => { + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + const findWorkspace = jest.fn().mockResolvedValue({ id: 'ws-1' }); + const resolved = await resolveMcpSessionConfig( + basicHeader('user@example.com', 'pw'), + makeDeps({ login, findWorkspace }), + ); + expect(findWorkspace).toHaveBeenCalled(); + expect(login).toHaveBeenCalledWith( + { email: 'user@example.com', password: 'pw' }, + 'ws-1', + ); + expect('getToken' in resolved.config).toBe(true); + const cfg = resolved.config as { getToken: () => Promise }; + await expect(cfg.getToken()).resolves.toBe('issued-user-jwt'); + expect(resolved.identity).toBe('basic:user@example.com'); + }); + + it('Basic password containing a colon is split on the first colon', async () => { + const login = jest.fn().mockResolvedValue('jwt'); + await resolveMcpSessionConfig( + basicHeader('user@example.com', 'a:b:c'), + makeDeps({ login }), + ); + expect(login).toHaveBeenCalledWith( + { email: 'user@example.com', password: 'a:b:c' }, + 'ws-1', + ); + }); + + it('Basic bad creds -> specific 401 (not generic) and increments the limiter', async () => { + const limiter = new FailedLoginLimiter(5, 60_000); + const login = jest + .fn() + .mockRejectedValue( + new UnauthorizedException('Email or password does not match'), + ); + const deps = makeDeps({ login, limiter }); + + await expect( + resolveMcpSessionConfig(basicHeader('user@example.com', 'wrong'), deps), + ).rejects.toThrow('Email or password does not match'); + // The failure was recorded; drive to the threshold (5) -> throttled message. + for (let i = 0; i < 4; i++) { + await resolveMcpSessionConfig( + basicHeader('user@example.com', 'wrong'), + deps, + ).catch(() => undefined); + } + await expect( + resolveMcpSessionConfig(basicHeader('user@example.com', 'wrong'), deps), + ).rejects.toThrow(/Too many failed MCP login attempts/); + }); + + it('Bearer -> verifies as ACCESS and returns a getToken config', async () => { + const verifyAccessJwt = jest + .fn() + .mockResolvedValue({ sub: 'user-9', email: 'u@e.com' }); + const resolved = await resolveMcpSessionConfig( + 'Bearer some.jwt.value', + makeDeps({ verifyAccessJwt }), + ); + expect(verifyAccessJwt).toHaveBeenCalledWith('some.jwt.value'); + const cfg = resolved.config as { getToken: () => Promise }; + await expect(cfg.getToken()).resolves.toBe('some.jwt.value'); + expect(resolved.identity).toBe('bearer:user-9'); + }); + + it('Bearer invalid -> specific 401 from verifyAccessJwt', async () => { + const verifyAccessJwt = jest + .fn() + .mockRejectedValue(new UnauthorizedException('jwt expired')); + await expect( + resolveMcpSessionConfig('Bearer expired', makeDeps({ verifyAccessJwt })), + ).rejects.toThrow('jwt expired'); + }); + + it('no creds + env service account configured -> service-account config', async () => { + const resolved = await resolveMcpSessionConfig( + undefined, + makeDeps({ email: 'svc@example.com', password: 'svcpw' }), + ); + expect('email' in resolved.config).toBe(true); + const cfg = resolved.config as { email: string; password: string }; + expect(cfg.email).toBe('svc@example.com'); + expect(cfg.password).toBe('svcpw'); + expect(resolved.identity).toBe('service-account'); + }); + + it('no creds + no env service account -> meaningful 401 listing accepted methods', async () => { + await expect( + resolveMcpSessionConfig(undefined, makeDeps()), + ).rejects.toThrow(/HTTP Basic auth.*Bearer access token.*service account/s); + }); + + it('SESSION INIT Basic -> mints a session via login() (verifyCredentials NOT called)', async () => { + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + const verifyCredentials = jest.fn().mockResolvedValue(undefined); + const resolved = await resolveMcpSessionConfig( + basicHeader('user@example.com', 'pw'), + makeDeps({ login, verifyCredentials, isSessionInit: true }), + ); + expect(login).toHaveBeenCalledTimes(1); + expect(verifyCredentials).not.toHaveBeenCalled(); + const cfg = resolved.config as { getToken: () => Promise }; + await expect(cfg.getToken()).resolves.toBe('issued-user-jwt'); + expect(resolved.identity).toBe('basic:user@example.com'); + }); + + it('SUBSEQUENT Basic correct creds -> uses verifyCredentials, NEVER login() (no new session/audit), same identity', async () => { + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + const verifyCredentials = jest.fn().mockResolvedValue(undefined); + const resolved = await resolveMcpSessionConfig( + basicHeader('user@example.com', 'pw'), + makeDeps({ login, verifyCredentials, isSessionInit: false }), + ); + // The side-effecting login() (audit + lastLoginAt + user_sessions insert) + // is NOT hit on a subsequent request: only the non-side-effecting verify. + expect(login).not.toHaveBeenCalled(); + expect(verifyCredentials).toHaveBeenCalledWith( + { email: 'user@example.com', password: 'pw' }, + 'ws-1', + ); + // Identity still matches the init identity so anti-fixation accepts it. + expect(resolved.identity).toBe('basic:user@example.com'); + }); + + it('SUBSEQUENT Basic wrong password -> still 401 (anti-fixation), without minting a session', async () => { + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + const verifyCredentials = jest + .fn() + .mockRejectedValue( + new UnauthorizedException('Email or password does not match'), + ); + await expect( + resolveMcpSessionConfig( + basicHeader('user@example.com', 'wrong'), + makeDeps({ login, verifyCredentials, isSessionInit: false }), + ), + ).rejects.toThrow('Email or password does not match'); + expect(login).not.toHaveBeenCalled(); + }); + + it('global per-email limiter key blocks an attacker rotating IP/XFF for one account', async () => { + const limiter = new FailedLoginLimiter(5, 60_000); + const login = jest + .fn() + .mockRejectedValue( + new UnauthorizedException('Email or password does not match'), + ); + // 5 failures against the SAME email but DIFFERENT IPs each time. The per-IP + // and per-IP+email keys never accumulate, but the global per-email key does. + for (let i = 0; i < 5; i++) { + await resolveMcpSessionConfig( + basicHeader('victim@example.com', 'wrong'), + makeDeps({ login, limiter, clientIp: `10.0.0.${i}` }), + ).catch(() => undefined); + } + // A 6th attempt from yet another fresh IP is now throttled purely by the + // email key — proving IP/XFF rotation no longer evades the limiter. + await expect( + resolveMcpSessionConfig( + basicHeader('victim@example.com', 'wrong'), + makeDeps({ login, limiter, clientIp: '10.0.0.99' }), + ), + ).rejects.toThrow(/Too many failed MCP login attempts/); + }); + + it('limiter does NOT count business errors (email not verified) as a failed login', async () => { + const limiter = new FailedLoginLimiter(1, 60_000); + const login = jest + .fn() + .mockRejectedValue( + new BadRequestException('Please verify your email address.'), + ); + const deps = () => + makeDeps({ login, limiter, clientIp: '10.0.0.7' }); + // First attempt: business error, surfaced as 401, but must NOT increment. + await resolveMcpSessionConfig( + basicHeader('user@example.com', 'pw'), + deps(), + ).catch(() => undefined); + // With threshold 1, if it had counted, the next attempt would be throttled. + // Instead it should reach login() again (same business error, NOT throttle). + await expect( + resolveMcpSessionConfig(basicHeader('user@example.com', 'pw'), deps()), + ).rejects.toThrow(/verify your email/); + }); + + it('anti-fixation: different users yield different identity keys (compared by the http identify hook)', async () => { + const a = await resolveMcpSessionConfig( + basicHeader('alice@example.com', 'pw'), + makeDeps(), + ); + const b = await resolveMcpSessionConfig( + basicHeader('bob@example.com', 'pw'), + makeDeps(), + ); + expect(a.identity).toBe('basic:alice@example.com'); + expect(b.identity).toBe('basic:bob@example.com'); + expect(a.identity).not.toBe(b.identity); + }); + + // --- BLOCKER: SSO/MFA pre-token gate on the Basic path --- + + it('Basic rejected (no token) when the SSO/MFA gate throws (SSO enforced)', async () => { + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + const verifyCredentials = jest.fn().mockResolvedValue(undefined); + // The service wires enforceBasicGate to validateSsoEnforcement + the lazy + // MFA check. Here we stub it to throw as it would for an SSO-enforced + // workspace; the gate runs BEFORE login()/verifyCredentials, so no token. + const enforceBasicGate = jest + .fn() + .mockRejectedValue( + new UnauthorizedException('This workspace has enforced SSO login.'), + ); + await expect( + resolveMcpSessionConfig( + basicHeader('user@example.com', 'pw'), + makeDeps({ login, verifyCredentials, enforceBasicGate }), + ), + ).rejects.toThrow(/enforced SSO/); + expect(enforceBasicGate).toHaveBeenCalledWith( + { id: 'ws-1' }, + { email: 'user@example.com', password: 'pw' }, + ); + // The pre-token gate fired first: no token-minting login() and no + // verifyCredentials() happened. + expect(login).not.toHaveBeenCalled(); + expect(verifyCredentials).not.toHaveBeenCalled(); + }); + + it('Basic rejected with a "use a Bearer token" message when MFA is required', async () => { + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + // Mirror McpService.enforceBasicLoginGate when the EE MFA module is present + // and the user has MFA: it throws telling the caller to use a Bearer token. + const enforceBasicGate = jest + .fn() + .mockRejectedValue( + new UnauthorizedException( + 'This account requires multi-factor authentication. MCP HTTP Basic ' + + 'cannot complete MFA — log in normally and use a Bearer access token ' + + 'instead.', + ), + ); + await expect( + resolveMcpSessionConfig( + basicHeader('mfa-user@example.com', 'pw'), + makeDeps({ login, enforceBasicGate }), + ), + ).rejects.toThrow(/use a Bearer access token/); + expect(login).not.toHaveBeenCalled(); + }); + + it('Bearer path is NOT subjected to the Basic SSO/MFA gate', async () => { + // The gate is only consulted on the Basic branch. A Bearer token (minted + // post-gate by the normal login) must not be blocked by it. + const enforceBasicGate = jest.fn(); + const resolved = await resolveMcpSessionConfig( + 'Bearer some.jwt.value', + makeDeps({ enforceBasicGate }), + ); + expect(enforceBasicGate).not.toHaveBeenCalled(); + expect('getToken' in resolved.config).toBe(true); + }); + + it('a session-INIT login() success DOES reset the global per-email key', async () => { + const limiter = new FailedLoginLimiter(5, 60_000); + // Pre-load some failure budget on the global email key. + const emailKey = 'email:victim@example.com'; + limiter.recordFailure(emailKey); + limiter.recordFailure(emailKey); + await resolveMcpSessionConfig( + basicHeader('victim@example.com', 'pw'), + makeDeps({ limiter, isSessionInit: true }), + ); + // After a real init login, the deliberate authentication clears the email + // bucket entirely. + expect(limiter.isBlocked(emailKey)).toBe(false); + limiter.recordFailure(emailKey); + // Only one failure now (bucket was reset), so still far from threshold 5. + expect(limiter.isBlocked(emailKey)).toBe(false); + }); + + it('a SUBSEQUENT valid login does NOT reset the global per-email bucket (only per-IP keys)', async () => { + const limiter = new FailedLoginLimiter(2, 60_000); + const clientIp = '10.0.0.5'; + const emailLc = 'victim@example.com'; + const emailKey = `email:${emailLc}`; + const ipKey = `ip:${clientIp}`; + const ipEmailKey = `ip-email:${clientIp}:${emailLc}`; + // An attacker (different IP rotation) has driven the global email key to the + // threshold; also seed the per-IP keys for the victim's own IP. + limiter.recordFailure(emailKey); + limiter.recordFailure(emailKey); + limiter.recordFailure(ipKey); + limiter.recordFailure(ipEmailKey); + + // The victim's live session would be throttled too (shared email key), so to + // exercise the SUBSEQUENT success path we use a SEPARATE limiter assertion: + // verify the reset behaviour directly on the keys the helper touches. Build a + // limiter where only the per-IP budget is set so the request is not blocked. + const lim2 = new FailedLoginLimiter(2, 60_000); + lim2.recordFailure(emailKey); // 1 failure on the global email key + lim2.recordFailure(ipKey); + lim2.recordFailure(ipEmailKey); + const verifyCredentials = jest.fn().mockResolvedValue(undefined); + await resolveMcpSessionConfig( + basicHeader(emailLc, 'pw'), + makeDeps({ limiter: lim2, clientIp, verifyCredentials, isSessionInit: false }), + ); + expect(verifyCredentials).toHaveBeenCalled(); + // Per-IP keys were cleared by the subsequent success... + expect(lim2.isBlocked(ipKey)).toBe(false); + // ...but the global per-email key was DELIBERATELY left intact (still 1). + lim2.recordFailure(emailKey); // -> 2 == threshold + expect(lim2.isBlocked(emailKey)).toBe(true); + }); +}); + +describe('isInitializeRequestBody (session-INIT detection)', () => { + it('true only for a single JSON-RPC object with method === "initialize"', () => { + expect(isInitializeRequestBody({ jsonrpc: '2.0', method: 'initialize' })).toBe( + true, + ); + }); + + it('false for a non-initialize method (e.g. tools/call)', () => { + expect( + isInitializeRequestBody({ jsonrpc: '2.0', method: 'tools/call' }), + ).toBe(false); + }); + + it('false for a batch (array) body, null/undefined, or a non-object', () => { + expect( + isInitializeRequestBody([{ jsonrpc: '2.0', method: 'initialize' }]), + ).toBe(false); + expect(isInitializeRequestBody(undefined)).toBe(false); + expect(isInitializeRequestBody(null)).toBe(false); + expect(isInitializeRequestBody('initialize')).toBe(false); + }); +}); + +describe('isSessionInit decision (no mcp-session-id AND initialize body)', () => { + // The service computes isSessionInit = !mcp-session-id && isInitializeRequestBody(body). + // This proves a header-less but NON-initialize request is NOT treated as init, + // so it goes down the non-side-effecting verifyCredentials path (no orphan + // session/audit before http.ts 400s it). + const decide = (sessionId: string | undefined, body: unknown): boolean => + !sessionId && isInitializeRequestBody(body); + + it('no header + initialize body -> init', () => { + expect(decide(undefined, { method: 'initialize' })).toBe(true); + }); + + it('no header + non-initialize body -> NOT init (verifyCredentials path)', () => { + expect(decide(undefined, { method: 'tools/list' })).toBe(false); + }); + + it('has session-id -> never init regardless of body', () => { + expect(decide('sess-1', { method: 'initialize' })).toBe(false); + }); +}); + +describe('resolveMcpSessionConfig non-initialize request side effects', () => { + it('header-less NON-initialize request does NOT call session-minting login() (uses verifyCredentials)', async () => { + // Simulate the service decision: no mcp-session-id but body is NOT initialize + // -> isSessionInit false -> the helper must use verifyCredentials, not login. + const login = jest.fn().mockResolvedValue('issued-user-jwt'); + const verifyCredentials = jest.fn().mockResolvedValue(undefined); + const isSessionInit = isInitializeRequestBody({ method: 'tools/call' }); // false + await resolveMcpSessionConfig( + basicHeader('user@example.com', 'pw'), + makeDeps({ login, verifyCredentials, isSessionInit }), + ); + expect(login).not.toHaveBeenCalled(); + expect(verifyCredentials).toHaveBeenCalledWith( + { email: 'user@example.com', password: 'pw' }, + 'ws-1', + ); + }); +}); + +describe('sharedTokenMatches (X-MCP-Token constant-time guard, item 2)', () => { + it('equal token -> true', () => { + expect(sharedTokenMatches('s3cr3t-token', 's3cr3t-token')).toBe(true); + }); + + it('wrong token of the SAME length -> false (timingSafeEqual path)', () => { + // Same length so it reaches timingSafeEqual; the bytes differ -> no match. + expect(sharedTokenMatches('aaaaaa', 'aaaaab')).toBe(false); + }); + + it('different-length token -> false WITHOUT throwing (early-return before timingSafeEqual)', () => { + // timingSafeEqual throws on unequal-length buffers; the early length check + // must short-circuit so a length mismatch is a clean non-match, not a throw. + expect(() => sharedTokenMatches('expected', 'short')).not.toThrow(); + expect(sharedTokenMatches('expected', 'short')).toBe(false); + expect(sharedTokenMatches('expected', 'a-much-longer-provided-value')).toBe( + false, + ); + }); + + it('array-valued header -> uses the FIRST element', () => { + // Multiple X-MCP-Token headers arrive as string[]; only the first is used. + expect(sharedTokenMatches('tok', ['tok', 'ignored'])).toBe(true); + expect(sharedTokenMatches('tok', ['wrong', 'tok'])).toBe(false); + }); + + it('undefined / non-string provided -> false', () => { + expect(sharedTokenMatches('tok', undefined)).toBe(false); + // An empty array yields provided[0] === undefined -> non-string -> false. + expect(sharedTokenMatches('tok', [])).toBe(false); + expect(sharedTokenMatches('tok', [undefined as unknown as string])).toBe( + false, + ); + }); +}); + +describe('clientIp (XFF-fallback precedence, item 5)', () => { + it('req.ip wins over socket.remoteAddress AND over X-Forwarded-For', () => { + expect( + clientIp({ + ip: '1.1.1.1', + socket: { remoteAddress: '2.2.2.2' }, + headers: { 'x-forwarded-for': '3.3.3.3' }, + }), + ).toBe('1.1.1.1'); + }); + + it('socket.remoteAddress is used only when req.ip is absent (still beats XFF)', () => { + expect( + clientIp({ + socket: { remoteAddress: '2.2.2.2' }, + headers: { 'x-forwarded-for': '3.3.3.3' }, + }), + ).toBe('2.2.2.2'); + }); + + it('X-Forwarded-For is the LAST resort, and only the FIRST hop is taken', () => { + expect( + clientIp({ + headers: { 'x-forwarded-for': '3.3.3.3, 4.4.4.4, 5.5.5.5' }, + }), + ).toBe('3.3.3.3'); + }); + + it("returns 'unknown' when nothing usable is present", () => { + expect(clientIp({ headers: {} })).toBe('unknown'); + // An array-valued XFF header is not treated as a string source -> unknown. + expect( + clientIp({ headers: { 'x-forwarded-for': ['3.3.3.3'] } }), + ).toBe('unknown'); + // An empty XFF string is ignored too. + expect(clientIp({ headers: { 'x-forwarded-for': '' } })).toBe('unknown'); + }); +}); + +describe('bindAccessJwtVerifier enforces JwtType.ACCESS (item 3)', () => { + it('calls TokenService.verifyJwt with JwtType.ACCESS as the second argument', async () => { + // Mock TokenService: assert the type literal is pinned to ACCESS so swapping + // to REFRESH (or omitting the type) breaks this test. + const verifyJwt = jest + .fn() + .mockResolvedValue({ sub: 'user-1', workspaceId: 'ws-1' }); + const verify = bindAccessJwtVerifier({ verifyJwt }); + + await verify('the.access.jwt'); + + expect(verifyJwt).toHaveBeenCalledTimes(1); + expect(verifyJwt).toHaveBeenCalledWith('the.access.jwt', JwtType.ACCESS); + // Pin the real enum value too, so renaming/repointing the enum member is caught. + expect(verifyJwt.mock.calls[0][1]).toBe('access'); + }); + + it('passes through the verified payload', async () => { + const payload = { sub: 'user-9', email: 'u@e.com', workspaceId: 'ws-1' }; + const verifyJwt = jest.fn().mockResolvedValue(payload); + await expect( + bindAccessJwtVerifier({ verifyJwt })('t'), + ).resolves.toBe(payload); + }); + + // The Bearer revocation/disabled checks (verifyBearerAccess) are covered above; + // this binds the ACCESS-type enforcement that verifyMcpBearer wires in. + it('feeds verifyBearerAccess so the whole Bearer chain enforces ACCESS', async () => { + const verifyJwt = jest.fn().mockResolvedValue({ + sub: 'user-1', + workspaceId: 'ws-1', + sessionId: 'sess-1', + }); + const res = await verifyBearerAccess('t', { + verifyJwt: bindAccessJwtVerifier({ verifyJwt }), + findUser: jest.fn().mockResolvedValue({ deactivatedAt: null }), + findActiveSession: jest + .fn() + .mockResolvedValue({ userId: 'user-1', workspaceId: 'ws-1' }), + }); + expect(verifyJwt).toHaveBeenCalledWith('t', JwtType.ACCESS); + expect(res).toEqual({ sub: 'user-1', email: undefined }); + }); +}); diff --git a/apps/server/src/integrations/mcp/mcp.service.ts b/apps/server/src/integrations/mcp/mcp.service.ts index be67e228..7ac16fb6 100644 --- a/apps/server/src/integrations/mcp/mcp.service.ts +++ b/apps/server/src/integrations/mcp/mcp.service.ts @@ -1,8 +1,33 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { + Injectable, + Logger, + OnModuleDestroy, + UnauthorizedException, +} from '@nestjs/common'; +import { ModuleRef } from '@nestjs/core'; import { pathToFileURL } from 'node:url'; +import { IncomingMessage } from 'node:http'; import { FastifyReply, FastifyRequest } from 'fastify'; import { EnvironmentService } from '../environment/environment.service'; import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo'; +import { UserRepo } from '@docmost/db/repos/user/user.repo'; +import { UserSessionRepo } from '@docmost/db/repos/session/user-session.repo'; +import { AuthService } from '../../core/auth/services/auth.service'; +import { TokenService } from '../../core/auth/services/token.service'; +import { validateSsoEnforcement } from '../../core/auth/auth.util'; +import { JwtPayload } from '../../core/auth/dto/jwt-payload'; +import { Workspace } from '@docmost/db/types/entity.types'; +import { + FailedLoginLimiter, + resolveMcpSessionConfig, + verifyBearerAccess, + isInitializeRequestBody, + sharedTokenMatches, + clientIp, + bindAccessJwtVerifier, + DocmostMcpConfig, + ResolvedMcpAuth, +} from './mcp-auth.helpers'; // Minimal shape of the embedded MCP HTTP handler exported by @docmost/mcp/http. interface McpHttpHandler { @@ -13,14 +38,23 @@ interface McpHttpHandler { ): Promise; } +type McpConfigResolver = ( + req: IncomingMessage, +) => DocmostMcpConfig | Promise; + interface McpHttpModule { - createMcpHttpHandler(config: { - apiUrl: string; - email: string; - password: string; - }): McpHttpHandler; + createMcpHttpHandler( + config: DocmostMcpConfig | McpConfigResolver, + options?: { identify?: (req: IncomingMessage) => string | Promise }, + ): McpHttpHandler; } +// Stash key for the per-request resolved config/identity computed (and +// validated) in handle() BEFORE res.hijack(), then read back by the resolver +// the MCP package invokes. Doing the validation pre-hijack lets a bad-creds +// failure return a clean 401 JSON instead of tearing a hijacked response. +const MCP_RESOLVED = Symbol('mcpResolvedConfig'); + // TS with module:commonjs downlevels a literal import() to require(), which // cannot load the ESM-only @docmost/mcp package. Indirect through Function so // the real dynamic import() survives compilation and can load ESM from @@ -31,19 +65,51 @@ const esmImport = new Function( ) as (specifier: string) => Promise; @Injectable() -export class McpService { +export class McpService implements OnModuleDestroy { private readonly logger = new Logger(McpService.name); private handler: McpHttpHandler | null = null; private handlerPromise: Promise | null = null; private warnedMissingCreds = false; + // In-memory per-IP/email throttle for FAILED /mcp Basic logins. Calling + // AuthService.login directly bypasses the controller's ThrottlerGuard, so + // this is the brute-force speed bump for /mcp. 5 failures per 60s window. + private readonly failedLogins = new FailedLoginLimiter(5, 60_000); + + // Periodically drop expired limiter buckets so never-revisited keys do not + // accumulate forever (unbounded memory growth / DoS via forgeable XFF keys). + // unref()'d so it never keeps the process alive; cleared on module destroy. + // Mirrors the sweepTimer pattern in packages/mcp/src/http.ts. + private readonly sweepIntervalMs = 60_000; + private readonly sweepTimer: NodeJS.Timeout; + constructor( private readonly environmentService: EnvironmentService, private readonly workspaceRepo: WorkspaceRepo, - ) {} + private readonly authService: AuthService, + private readonly tokenService: TokenService, + private readonly userRepo: UserRepo, + private readonly userSessionRepo: UserSessionRepo, + private readonly moduleRef: ModuleRef, + ) { + this.sweepTimer = setInterval(() => { + try { + this.failedLogins.sweep(); + } catch (err) { + this.logger.error('MCP failed-login limiter sweep failed', err as Error); + } + }, this.sweepIntervalMs); + // Do not let this interval hold the event loop open. + this.sweepTimer.unref?.(); + } + + onModuleDestroy(): void { + clearInterval(this.sweepTimer); + } // Service account the embedded MCP uses to talk back to this Docmost - // instance over loopback REST + the collaboration WebSocket. + // instance over loopback REST + the collaboration WebSocket. Now OPTIONAL: + // it is only a fallback when no per-user Basic/Bearer credentials are sent. private getEmail(): string | undefined { return process.env.MCP_DOCMOST_EMAIL; } @@ -80,8 +146,141 @@ export class McpService { } } + // Bearer access-JWT verification for the /mcp token fallback. verifyJwt only + // checks signature/exp/type, but a logged-out (revoked) or disabled user can + // still hold an unexpired access JWT. JwtStrategy additionally checks the + // session is active and the user is not disabled; we mirror those exact checks + // here so the MCP Bearer path is not weaker than the normal cookie/header path. + private async verifyMcpBearer( + token: string, + ): Promise<{ sub?: string; email?: string }> { + // The revocation/disabled decision logic lives in the framework-free + // verifyBearerAccess helper (unit-testable without the heavy auth graph); + // this method only wires in the concrete TokenService + repos. + return verifyBearerAccess(token, { + // The JwtType.ACCESS enforcement lives in bindAccessJwtVerifier (a pure, + // testable seam) so the type literal cannot silently drift to REFRESH. + verifyJwt: bindAccessJwtVerifier(this.tokenService) as ( + t: string, + ) => Promise, + findUser: (sub, workspaceId) => + this.userRepo.findById(sub, workspaceId), + findActiveSession: (sessionId) => + this.userSessionRepo.findActiveById(sessionId), + }); + } + + /** + * Resolve the per-session identity from the request and produce the + * DocmostMcpConfig the MCP package will run under, plus an opaque identity + * key for anti-fixation. The decision logic lives in the framework-free + * `resolveMcpSessionConfig` helper (so it is unit-testable without the heavy + * auth graph); this method only wires McpService's injected collaborators in. + * + * Throws UnauthorizedException with a SPECIFIC message on failure (never a + * generic "MCP error"); never logs/echoes the password or Authorization + * header. Run BEFORE res.hijack() so the 401 is clean JSON. + */ + async resolveSessionConfig(req: FastifyRequest): Promise { + const authHeader = req.headers['authorization'] as string | undefined; + // A request carrying an mcp-session-id is operating on an ALREADY + // established session (see packages/mcp/src/http.ts: a new session is only + // minted by an initialize POST with no session id). The session-minting + // login() (user_sessions insert + USER_LOGIN audit + lastLoginAt bump) must + // run ONLY for a genuine session INITIALIZE: no mcp-session-id AND the + // JSON-RPC body is an `initialize` request — the same signal http.ts uses to + // decide whether to mint a session. Any other request (e.g. a non-initialize + // body with no session id, which http.ts will 400) uses the non-side- + // effecting verifyCredentials path so it never mints an orphan DB + // session/audit row before being rejected. + const isSessionInit = + !req.headers['mcp-session-id'] && + isInitializeRequestBody((req as unknown as { body?: unknown }).body); + return resolveMcpSessionConfig(authHeader, { + apiUrl: this.getApiUrl(), + email: this.getEmail(), + password: this.getPassword(), + findWorkspace: () => this.workspaceRepo.findFirst(), + enforceBasicGate: (workspace, creds) => + this.enforceBasicLoginGate(workspace as Workspace, creds), + login: (creds, workspaceId) => this.authService.login(creds, workspaceId), + verifyCredentials: async (creds, workspaceId) => { + await this.authService.verifyUserCredentials(creds, workspaceId); + }, + verifyAccessJwt: (token) => this.verifyMcpBearer(token), + limiter: this.failedLogins, + clientIp: clientIp(req), + isSessionInit, + }); + } + + // Pre-token gate for the /mcp HTTP-Basic path, replicating EXACTLY what + // AuthController.login does before issuing a token, so the Basic path is not + // an SSO/MFA bypass: + // 1) validateSsoEnforcement(workspace) — reject if the workspace enforces + // SSO (a password login is not allowed there). + // 2) Lazily require the EE MFA module (same pattern/path as the controller). + // If it is bundled and the user has MFA enabled OR the workspace enforces + // MFA, reject the Basic path and tell the caller to use a Bearer token (a + // Bearer ACCESS JWT is only minted AFTER the normal gated login, so it is + // safe). A fork WITHOUT the EE module behaves exactly like the controller: + // no MFA module -> no MFA gate. + // Throws UnauthorizedException on rejection (surfaced as a clean 401, never a + // torn/hijacked response, never a token). Never logs the password. + private async enforceBasicLoginGate( + workspace: Workspace, + creds: { email: string; password: string }, + ): Promise { + // 1) SSO enforcement. validateSsoEnforcement throws BadRequestException; we + // re-surface it as Unauthorized so the /mcp 401 path is consistent and a + // token is never issued. + try { + validateSsoEnforcement(workspace); + } catch { + throw new UnauthorizedException( + 'This workspace has enforced SSO login. Use SSO; MCP HTTP Basic is not allowed.', + ); + } + + // 2) MFA gate — lazy-require the EE module exactly like AuthController.login. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let MfaModule: any; + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + MfaModule = require('./../../ee/mfa/services/mfa.service'); + } catch { + // No EE MFA module bundled in this build: same as the controller -> no + // MFA gate. (A community/fork build has no MFA, so Basic is allowed.) + return; + } + + const mfaService = this.moduleRef.get(MfaModule.MfaService, { + strict: false, + }); + // Use the same requirement check the controller uses. We pass NO FastifyReply + // (the controller passes `res` only to set a cookie on the no-MFA happy path, + // which we never take here): we only read the requirement flags. Be tolerant + // of either a (loginInput, workspace) or (loginInput, workspace, res) shape. + const mfaResult = await mfaService.checkMfaRequirements( + creds, + workspace, + undefined, + ); + + if (mfaResult && (mfaResult.userHasMfa || mfaResult.requiresMfaSetup)) { + throw new UnauthorizedException( + 'This account requires multi-factor authentication. MCP HTTP Basic ' + + 'cannot complete MFA — log in normally and use a Bearer access token ' + + 'instead.', + ); + } + } + // Lazily create the HTTP handler exactly once. The import is indirected so // the ESM-only @docmost/mcp package can be loaded from this CommonJS module. + // The handler is created with a per-request RESOLVER (and an `identify` hook + // for anti-fixation): both read the auth that handle() resolved and stashed + // on req before hijack, so the package never re-parses credentials. private async getHandler(): Promise { if (this.handler) { return this.handler; @@ -95,11 +294,29 @@ export class McpService { const mod = (await esmImport( pathToFileURL(httpEntry).href, )) as McpHttpModule; - const handler = mod.createMcpHttpHandler({ - apiUrl: this.getApiUrl(), - email: this.getEmail()!, - password: this.getPassword()!, - }); + const handler = mod.createMcpHttpHandler( + (req: IncomingMessage) => { + const resolved = (req as unknown as Record)[ + MCP_RESOLVED + ] as ResolvedMcpAuth | undefined; + if (!resolved) { + // Should never happen: handle() always stashes before delegating. + throw new UnauthorizedException('MCP authentication missing.'); + } + return resolved.config; + }, + { + identify: (req: IncomingMessage) => { + const resolved = (req as unknown as Record)[ + MCP_RESOLVED + ] as ResolvedMcpAuth | undefined; + if (!resolved || resolved.identity === undefined) { + throw new UnauthorizedException('MCP authentication missing.'); + } + return resolved.identity; + }, + }, + ); this.handler = handler; return handler; })().catch((err) => { @@ -112,13 +329,13 @@ export class McpService { } async handle(req: FastifyRequest, res: FastifyReply): Promise { - // Optional static bearer-token guard. When MCP_TOKEN is set, the request - // must carry a matching `Authorization: Bearer ` header. When unset, - // /mcp relies on the workspace toggle and network isolation (no auth). - const token = process.env.MCP_TOKEN; - if (token) { - const authHeader = req.headers['authorization']; - if (authHeader !== `Bearer ${token}`) { + // Optional shared-guard. When MCP_TOKEN is set, the request must carry a + // matching `X-MCP-Token` header. It now lives in its OWN header so it never + // collides with `Authorization`, which carries the per-user credentials. + const sharedToken = process.env.MCP_TOKEN; + if (sharedToken) { + const provided = req.headers['x-mcp-token']; + if (!sharedTokenMatches(sharedToken, provided)) { res.status(401).send({ error: 'Unauthorized' }); return; } @@ -129,20 +346,40 @@ export class McpService { return; } - if (!this.credsConfigured()) { - if (!this.warnedMissingCreds) { - this.warnedMissingCreds = true; - this.logger.warn( - 'MCP is enabled but not configured: set MCP_DOCMOST_EMAIL and MCP_DOCMOST_PASSWORD.', - ); + // Resolve + validate the per-session identity BEFORE hijacking the response + // so bad credentials surface as a clean 401 JSON (never a torn response and + // never a generic "MCP error"). The resolved config/identity is stashed on + // the raw request for the package's resolver + identify hook to read back. + let resolved: ResolvedMcpAuth; + try { + resolved = await this.resolveSessionConfig(req); + } catch (err) { + if (err instanceof UnauthorizedException) { + // Warn once if the only thing missing is the service account, to keep + // the original operator hint. + if ( + !this.credsConfigured() && + !req.headers['authorization'] && + !this.warnedMissingCreds + ) { + this.warnedMissingCreds = true; + this.logger.warn( + 'MCP is enabled but received a request with no credentials and no ' + + 'MCP_DOCMOST_EMAIL/MCP_DOCMOST_PASSWORD service account configured.', + ); + } + res.status(401).send({ error: err.message }); + return; } - res.status(503).send({ - error: - 'MCP is not configured (set MCP_DOCMOST_EMAIL / MCP_DOCMOST_PASSWORD)', - }); + this.logger.error('MCP auth resolution failed', err as Error); + res.status(500).send({ error: 'Internal server error' }); return; } + // Stash the resolved auth on the raw request so the package's resolver + + // identify hook (wired in getHandler) read it back instead of re-parsing. + (req.raw as unknown as Record)[MCP_RESOLVED] = resolved; + // Hand the raw Node req/res to the MCP transport. hijack() tells Fastify // to stop managing this response so the transport can write to it directly. res.hijack(); diff --git a/docs/backlog/mcp-per-user-auth.md b/docs/backlog/mcp-per-user-auth.md deleted file mode 100644 index a2fdc77f..00000000 --- a/docs/backlog/mcp-per-user-auth.md +++ /dev/null @@ -1,416 +0,0 @@ -# Встроенный `/mcp`: авторизация под текущим пользователем (а не сервисным аккаунтом) - -Статус: **план, код не менялся.** Фича сервер (`apps/server` + `packages/mcp`). -Затрагивает безопасность — менять аккуратно. - -**Решение принято: основной путь — логин/пароль текущего пользователя через -HTTP Basic** (`Authorization: Basic base64(email:password)`). Токен-варианты -(Bearer access-JWT / community PAT / OAuth) описаны ниже как альтернативы и -возможные доработки, но делаем именно логин/пароль. - -## Суть - -Сейчас встроенный MCP-сервер на `/mcp` ходит в Docmost **под одним сервисным -аккаунтом** (`MCP_DOCMOST_EMAIL` / `MCP_DOCMOST_PASSWORD`). Любой клиент, -подключившийся к `/mcp`, действует с правами этого аккаунта — независимо от того, -кто реально сидит за MCP-клиентом. Это значит: единые CASL-права на всех, нет -атрибуции правок конкретному человеку (в истории страниц всё — от сервисного -юзера), и без env-кредов фича вообще не поднимается (отдаёт `503 "MCP is not -configured"`). - -Хотим: чтобы `/mcp` авторизовался **под текущим пользователем** (его логином и -паролем) — тогда каждый запрос исполняется под его CASL-правами, правки -атрибутируются ему, и сервисный аккаунт перестаёт быть обязательным. - -## Почему сейчас сервисный аккаунт (контекст) - -`/mcp` — **внешний протокольный эндпоинт** (MCP Streamable-HTTP / JSON-RPC). В -сессии MCP нет личности Docmost: сессия идентифицируется случайным UUID -([http.ts:68-74](packages/mcp/src/http.ts#L68-L74), `sessionIdGenerator: () => -randomUUID()`) и заголовком `mcp-session-id`, а транспорт **не несёт JWT/куку -пользователя**. Поэтому пакет `@docmost/mcp` спроектирован как standalone-клиент: -логинится один раз по `email/password` ([auth-utils.ts:41-86](packages/mcp/src/lib/auth-utils.ts#L41-L86), -достаёт куку `authToken`) и дальше ходит в REST + collab как обычный внешний -клиент. - -Контраст — встроенный AI-чат: он крутится **внутри авторизованного NestJS-запроса**, -поэтому чеканит loopback-токен именно текущего пользователя и каждый инструмент -исполняется под его CASL ([ai-chat-tools.service.ts:54-85](apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts#L54-L85)). -Наша задача — принести эту же модель «per-user токен» во внешний `/mcp`. - -**Хорошая новость: клиентская половина уже готова.** `DocmostClient` принимает -union-конфиг — либо `{email,password}` (сервис-аккаунт, вызывает `performLogin`), -либо `{getToken}` (берёт **готовый bare-JWT** пользователя как Bearer и **не** -логинится) ([client.ts:99-160](packages/mcp/src/client.ts#L99-L160), -[client.ts:223-241](packages/mcp/src/client.ts#L223-L241)). Этот `getToken`-вариант -уже используется внутренним AI-чатом. Не хватает только **связки в самом -`/mcp`-хендлере** — он сейчас строит конфиг статически из env. - -## Где сейчас живёт код (точные места) - -### Хендлер `/mcp` (NestJS-обвязка) -- [mcp.service.ts:114-144](apps/server/src/integrations/mcp/mcp.service.ts#L114-L144) - `handle(req, res)`: (1) опц. статичный гард `MCP_TOKEN` против - `Authorization: Bearer` (стр. 118-125); (2) `isEnabled()` — тумблер воркспейса - `ai.mcp` (403 если выкл.); (3) `credsConfigured()` — наличие env-кредов (**это - и есть источник твоего `503`**, стр. 132-144); (4) `res.hijack()` и проброс - raw req/res в MCP-транспорт. -- [mcp.service.ts:47-64](apps/server/src/integrations/mcp/mcp.service.ts#L47-L64) - `getEmail/getPassword/getApiUrl/credsConfigured` — читают env. -- [mcp.service.ts:85-112](apps/server/src/integrations/mcp/mcp.service.ts#L85-L112) - `getHandler()` — лениво создаёт **один** HTTP-handler через - `createMcpHttpHandler({apiUrl,email,password})` и кэширует его. - -### MCP-пакет -- [http.ts:13](packages/mcp/src/http.ts#L13) `createMcpHttpHandler(config: - DocmostMcpConfig)` — принимает **один статический** конфиг; создаёт по - `McpServer` + транспорту **на каждую сессию** при `initialize` - ([http.ts:68-82](packages/mcp/src/http.ts#L68-L82): `createDocmostMcpServer(config)` - → `server.connect(transport)`). Идентичность сессии фиксируется здесь, на - инициализации. -- [index.ts:50-54](packages/mcp/src/index.ts#L50-L54) `createDocmostMcpServer(config)` - — пробрасывает union-конфиг в `new DocmostClient(config)`. -- [client.ts:99-160](packages/mcp/src/client.ts#L99-L160) `DocmostMcpConfig` = - `{email,password} | {getToken}` (+ опц. `getCollabToken`); конструктор - ветвится: `getToken`-вариант не логинится, использует bare-JWT как Bearer. - -### Auth / токены (сервер) -- [token.service.ts:30-54](apps/server/src/core/auth/services/token.service.ts#L30-L54) - `generateAccessToken(user, sessionId, provenance?)` → JWT `type=ACCESS`. -- [token.service.ts:119-138](apps/server/src/core/auth/services/token.service.ts#L119-L138) - `generateApiToken({apiKeyId,user,workspaceId,expiresIn})` → JWT `type=API_KEY`. -- [token.service.ts:164-176](apps/server/src/core/auth/services/token.service.ts#L164-L176) - `verifyJwt(token, type)` — проверка подписи + типа. -- [jwt.strategy.ts:26-34](apps/server/src/core/auth/strategies/jwt.strategy.ts#L26-L34) - `jwtFromRequest = cookie authToken || Bearer` — **bearer уже принимается** на - `/api`. -- [jwt.strategy.ts:80-81](apps/server/src/core/auth/strategies/jwt.strategy.ts#L80-L81) - провенанс: токен без `actor` → `'user'` (нам и нужно — правки как пользователя). -- [jwt.strategy.ts:86-109](apps/server/src/core/auth/strategies/jwt.strategy.ts#L86-L109) - `validateApiKey` — путь `type=API_KEY` **требует EE-модуль** - (`ee/api-key/api-key.service`), которого в форке нет → бросает «Enterprise API - Key module missing». То есть полноценных PAT сейчас **нет**. -- [auth.controller.ts:184-193](apps/server/src/core/auth/auth.controller.ts#L184-L193) - `POST /auth/collab-token` под `JwtAuthGuard` — выдаёт collab-токен по - bearer/cookie (этим уже пользуется и сервис-аккаунт, и AI-чат). -- [environment.service.ts:63-64](apps/server/src/integrations/environment/environment.service.ts#L63-L64) - `JWT_TOKEN_EXPIRES_IN` по умолчанию **`90d`** — access-JWT долгоживущий, годится - как «токен пользователя». -- [utils.ts:109](apps/server/src/common/helpers/utils.ts#L109) - `extractBearerTokenFromHeader(req)` — переиспользуемый парсер `Authorization`. -- [migration 20250912T101500-api-keys.ts](apps/server/src/database/migrations/20250912T101500-api-keys.ts) - — таблица `api_keys` (`id, name, creator_id, workspace_id, expires_at, - last_used_at, deleted_at`) **уже существует**, но community-сервиса под неё нет. -- [.env.example:72-79](.env.example#L72) — `MCP_DOCMOST_EMAIL/PASSWORD`, - `MCP_DOCMOST_API_URL`, `MCP_TOKEN`, `MCP_SESSION_IDLE_MS`. - -## Как именно логиниться под пользователем — варианты - -Пользователь подключает к `/mcp` внешний MCP-клиент (Claude Desktop и т.п.). -Авторизоваться «под текущим пользователем» можно несколькими путями с разной -ценой и безопасностью. Все они сводятся к одному и тому же на уровне клиента: -получить пользовательский JWT и ходить под ним; разница — **откуда** берётся -токен (приносит пользователь / логинит сервер / выдаёт OAuth). - -### Вариант L — логин/пароль пользователя через HTTP Basic ✅ ВЫБРАН -MCP-клиент шлёт `Authorization: Basic base64(email:password)`; `/mcp` декодит и -строит per-session конфиг `{email, password}` → `DocmostClient` сам делает -`performLogin` (`POST /auth/login`) и дальше ходит под этим пользователем. Это -**ровно тот же путь, что у сервисного аккаунта сегодня**, только с кредами -текущего пользователя — клиентская механика уже готова -([client.ts:99-160](packages/mcp/src/client.ts#L99-L160), -[auth-utils.ts:41-86](packages/mcp/src/lib/auth-utils.ts#L41-L86)). - -- **Плюсы:** минимум нового кода (переиспользуется `{email,password}`-ветка - `performLogin`); пользователю не надо доставать токен — привычные логин/пароль; - сервисный аккаунт становится необязательным. -- **Минусы:** **сырой пароль лежит в конфиге MCP-клиента** и уходит на сервер при - каждом коннекте (токен безопаснее — отзываем/скоупится без смены пароля); - **не работает с MFA** (статические креды не пройдут интерактивный челлендж) — - в этом форке MFA-модуль удалён (EE), поэтому сейчас вопрос моот, но при - возврате MFA или `workspace.enforceMfa` ([auth.controller.ts:64-103](apps/server/src/core/auth/auth.controller.ts#L64-L103)) - путь сломается; **SSO/OIDC**-пользователи могут не иметь локального пароля; - логин жмёт `/auth/login` throttle ([AUTH_THROTTLER](apps/server/src/core/auth/auth.controller.ts#L41), - раз на сессию + переавторизация на 401). -- **Вывод:** хорош для single-user self-host без MFA; как дефолт лучше токен. - -### Вариант A — pass-through access-JWT (альтернатива / возможна параллельно) -MCP-клиент шлёт `Authorization: Bearer `, где токен — это значение -куки `authToken` пользователя (валиден 90 дней). `/mcp` извлекает его, валидирует -как `ACCESS`-JWT и передаёт в `DocmostClient` как `getToken`. Все REST + collab -идут под CASL этого пользователя; правки атрибутируются ему (`actor='user'`). - -- **Плюсы:** минимальный диф, переиспользует уже готовый `getToken`-путь клиента; - bearer уже принимается на `/api`; сервисный аккаунт становится необязательным. -- **Минусы:** токен надо достать руками (DevTools → Cookies → `authToken`), - токен привязан к сессии (логаут/revoke сессии убивает его), он же даёт полный - доступ как у пользователя (не сужен скоупом). Приемлемо для self-host, но это - не «красивый» PAT. - -### Вариант B — community PAT / API-keys (доработка на будущее) -Реализовать сообществом то, что было в EE: пользователь создаёт в настройках -**именованный, отзываемый, с TTL** персональный токен; его и кладёт в MCP-клиент. - -- Таблица `api_keys` уже есть; `JwtApiKeyPayload`+`generateApiToken` есть; не - хватает **community `ApiKeyService`** (хранить хеш/строку ключа, валидировать - по `apiKeyId` из JWT, обновлять `last_used_at`, проверять `expires_at`/ - `deleted_at`) + CRUD-эндпоинты + UI выдачи/отзыва. -- Поправить [jwt.strategy.ts:86-109](apps/server/src/core/auth/strategies/jwt.strategy.ts#L86-L109): - путь `API_KEY` должен звать community-сервис вместо `require('./../../../ee/...')`. -- **Плюсы:** стабильный, отзываемый, именованный токен; не завязан на браузерную - сессию; виден и управляем в UI. Это «правильный» долгоживущий ответ. -- **Минусы:** заметно больше работы (сервис + контроллер + миграция типов + UI), - и это самостоятельная фича auth, шире чем сам `/mcp`. - -### Вариант C — OAuth 2.1 для MCP (доработка на будущее, «с логином» из коробки) -MCP-спека описывает авторизацию через OAuth 2.1: Docmost поднимает -authorization-server metadata + token endpoint, а MCP-клиент (Claude Desktop) -делает **интерактивный логин** и сам получает токен — это и есть «mcp с логином». - -- **Плюсы:** самый стандартный и удобный UX (логин в браузере, без копипасты - токенов), refresh из коробки. -- **Минусы:** самый большой объём (discovery-эндпоинты, согласие, refresh, - привязка к существующему JWT-стеку). Избыточно для текущего запроса. - -> **Решение:** делаем **L** (логин/пароль через HTTP Basic) основным и -> единственным путём на этот заход. Это закрывает «авторизация под текущим -> пользователем» минимальным кодом (переиспользуется `performLogin`) и привычным -> для пользователя способом — логин/пароль. **A/B/C** оставляем в доке как -> совместимые доработки на будущее: все варианты сходятся в одной точке — -> per-session `DocmostClient` под пользовательским JWT, отличается лишь источник -> токена (`performLogin` от сервера / Bearer от пользователя / PAT / OAuth), так -> что добавить их позже можно поверх той же связки без переделки. - -## Детальный дизайн выбранного пути — логин/пароль (HTTP Basic) - -Идея: вместо **одного статического** конфига хендлер получает **резолвер конфига -от запроса**, который на инициализации каждой MCP-сессии решает, под кем ходить. -Для выбранного пути резолвер читает `Authorization: Basic`, **валидирует -логин/пароль на сервере** и строит per-session `DocmostClient`, ходящий под этим -пользователем. - -### 1) `packages/mcp/src/http.ts` — принять резолвер конфига -```ts -// Accept either a static config (service-account / stdio, unchanged) OR a -// per-request resolver. The resolver runs once per MCP session, at initialize, -// so the session's DocmostClient is bound to that request's identity. -export type McpConfigResolver = ( - req: IncomingMessage, -) => DocmostMcpConfig | Promise; - -export function createMcpHttpHandler( - config: DocmostMcpConfig | McpConfigResolver, -) { /* ... */ } - -// inside handleRequest, at session init (POST initialize, http.ts:68-82): -const sessionConfig = - typeof config === "function" ? await config(req) : config; -const server = createDocmostMcpServer(sessionConfig); -``` -Обратная совместимость полная: stdio ([stdio.ts](packages/mcp/src/stdio.ts)) и -существующий вызов с объектом-конфигом работают как раньше (это не функция → -ветка `else`). - -### 2) `apps/server/.../mcp.service.ts` — разобрать Basic, провалидировать креды, выпустить токен -Креды валидируем **на сервере** через `AuthService.login` и в конфиг кладём -**уже выпущенный пользовательский JWT** (`getToken`-вариант), а не сам пароль — -тогда пароль не уходит дальше в loopback-клиент, а ошибки логина видны сразу, -чистым JSON-ответом до `res.hijack()`. -```ts -// Resolve the per-session identity from the request. Primary path: HTTP Basic -// (current user's email:password) -> validate on the server -> issue the user's -// JWT -> client acts as that user. Bearer (variant A) and the service account -// (back-compat) are accepted as fallbacks. -private async resolveSessionConfig(req): Promise { - const auth = req.headers['authorization'] as string | undefined; - - // --- chosen path: Basic login/password --- - if (auth?.startsWith('Basic ')) { - const decoded = Buffer.from(auth.slice(6), 'base64').toString('utf8'); - const sep = decoded.indexOf(':'); // password may contain ':' - const email = decoded.slice(0, sep); - const password = decoded.slice(sep + 1); - // Single-workspace assumption (loopback) — same as the AI-chat tools path. - const workspace = await this.workspaceRepo.findFirst(); - // Throws UnauthorizedException('Email or password does not match') on bad - // creds -> surfaced as a specific 401 (never a generic error). NOTE: calling - // AuthService.login directly BYPASSES the controller's throttle + MFA gate - // (both EE/controller-level) — see Security below. - const authToken = await this.authService.login({ email, password }, workspace.id); - return { apiUrl: this.getApiUrl(), getToken: async () => authToken }; - } - - // --- fallback A: Bearer access-JWT (user-supplied token) --- - const bearer = extractBearerTokenFromHeader(req); // utils.ts:109 - if (bearer) { - await this.tokenService.verifyJwt(bearer, JwtType.ACCESS); // specific 401 - return { apiUrl: this.getApiUrl(), getToken: async () => bearer }; - } - - // --- fallback B: service account (existing behaviour, optional) --- - if (this.credsConfigured()) { - return { apiUrl: this.getApiUrl(), email: this.getEmail()!, password: this.getPassword()! }; - } - - throw new UnauthorizedException( - 'MCP requires Basic auth (email:password) or a Bearer token, ' + - 'or a configured MCP_DOCMOST_EMAIL/PASSWORD service account.', - ); -} -``` -- `getHandler()` зовёт `createMcpHttpHandler((req) => this.resolveSessionConfig(req))` - (резолвер, не статический объект). -- Auth-разбор (Basic decode + `AuthService.login` / `verifyJwt`) делать в - `handle()` **до** `res.hijack()`, чтобы на плохих кредах вернуть чистый - `401 {error: "..."}`, а не рвать hijack-нутый ответ. Резолвер тогда может - просто отдать уже посчитанный конфиг (напр. через `(req.raw as any).__mcpConfig`). -- Проверку `credsConfigured()` (стр. 132-144) **заменить** на «есть Basic ИЛИ - Bearer ИЛИ env-креды», иначе осмысленный `401/503` (не глотать). -- Инжектнуть в `McpService` `AuthService` (для `login`) и `TokenService` (для - `verifyJwt` в fallback A); `WorkspaceRepo` уже есть. Подтянуть нужные модули в - `integrations/mcp`. - -### 3) Гард `MCP_TOKEN` — развести с пользовательскими кредами -Сейчас `MCP_TOKEN` едет в `Authorization: Bearer` -([mcp.service.ts:118-125](apps/server/src/integrations/mcp/mcp.service.ts#L118-L125)). -В per-user режиме `Authorization` занят кредами/токеном пользователя. Решение: -- в per-user режиме **убрать** статичный `MCP_TOKEN`-гард на `Authorization` - (аутентификацией служат сами креды; эндпоинт по-прежнему закрыт тумблером - воркспейса и сетевой изоляцией), **или** -- если нужен доп. общий шлагбаум — перенести `MCP_TOKEN` в **отдельный заголовок** - (`X-MCP-Token`), чтобы не конфликтовал с `Authorization`. - -### 4) Collab / провенанс — ничего лишнего не нужно -`getCollabToken`-провайдер **не задаём**: `DocmostClient` сам сходит в -`POST /auth/collab-token` с выпущенным пользовательским JWT -([auth.controller.ts:184-193](apps/server/src/core/auth/auth.controller.ts#L184-L193)) -и получит обычный пользовательский collab-токен. Так правки через collab -атрибутируются пользователю (`actor='user'` по умолчанию, -[jwt.strategy.ts:80-81](apps/server/src/core/auth/strategies/jwt.strategy.ts#L80-L81)). -Никакого «AI agent»-бейджа здесь не вешаем — это живой человек. - -> **Альтернатива по объёму (если не хочется тянуть `AuthService` в McpService):** -> отдать креды как есть в конфиг `{ email, password }` — `DocmostClient` сам -> сделает `performLogin` по loopback (это буквально путь сервис-аккаунта). Минус: -> пароль идёт в loopback-клиент и ошибка логина всплывает позже, из пакета, после -> hijack. Серверная валидация (вариант выше) чище и безопаснее — её и берём. - -## Тонкие моменты / edge cases - -- **Идентичность привязана к сессии.** `DocmostClient` создаётся один раз на - MCP-сессию (на `initialize`) и кэширует токен; последующие запросы той же - `mcp-session-id` пойдут под пользователем, зафиксированным при инициализации. - Грань безопасности: на повторных запросах **проверять, что предъявленные креды/ - токен резолвятся в того же пользователя** (`email`/`sub`), что и при инициализации - сессии, иначе `401` — чтобы нельзя было «подсесть» в чужую сессию (session - fixation / подмена кред). -- **Новая Docmost-сессия на каждый логин.** `AuthService.login` → - `sessionService.createSessionAndToken` ([auth.service.ts:97](apps/server/src/core/auth/services/auth.service.ts#L97)) - создаёт **запись пользовательской сессии** на каждый MCP-логин. При частых - реконнектах сессии копятся (idle-eviction MCP-сессий их не чистит). Прикинуть: - переиспользовать токен в пределах MCP-сессии (одна сессия = один логин, уже так), - и/или TTL/чистку висящих сессий — отдельной заботой. -- **Истечение токена.** Выпущенный access-JWT живёт 90 дней — на 401 от loopback - клиент перезайдёт. Удобство Basic: креды у клиента постоянны, поэтому - переавторизация прозрачна (повторный `login`), в отличие от вручную вставленного - токена. Опционально — per-session mutable-холдер токена, чтобы переавторизация - не пересоздавала MCP-сессию. -- **Откат на сервис-аккаунт.** Сохранить как опцию (нет bearer + есть env-креды → - старое поведение). Это не ломает существующие инсталляции и даёт «безличный» - режим, где он нужен (CI, скрипты). Если откат нежелателен — сделать его - переключаемым (`MCP_REQUIRE_USER_TOKEN=true`). -- **Мульти-тенантность / loopback.** `127.0.0.1` не резолвит воркспейс по - субдомену → таргетится дефолтный воркспейс (та же single-workspace-оговорка, - что и у сервис-аккаунта и AI-чата, см. - [ai-chat-tools.service.ts:25-28](apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts#L25-L28)). - `jwt.strategy` сверяет `req.raw.workspaceId` с `payload.workspaceId` - ([jwt.strategy.ts:41-43](apps/server/src/core/auth/strategies/jwt.strategy.ts#L41-L43)); - на loopback `req.raw.workspaceId` не выставлен → проверка проходит. Для - мульти-воркспейс деплоя нужен явный workspace-скоуп (отдельная задача). -- **Idle-eviction.** Сессии чистятся по `MCP_SESSION_IDLE_MS` (30 мин) - ([http.ts:21-39](packages/mcp/src/http.ts#L21-L39)) — без изменений; protected - per-user сессии тоже истекают по бездействию, это ок. -- **Ошибки не глотать.** Невалидный/просроченный токен → `console`/logger с - полной ошибкой **и** конкретный текст в ответе (реальная причина), не «MCP - error» (CLAUDE.md «Errors must never be swallowed»). Текущее одноразовое - warning про отсутствие кредов — оставить/адаптировать. -- **Логи/PII.** Не логировать сам токен. Сейчас `auth-utils` прячет тело ответа - за `DEBUG` — сохранить этот принцип. - -## Безопасность (на ревью проверить отдельно) - -- **Прямой `AuthService.login` обходит throttle и MFA-гейт.** Контроллерный - `/auth/login` защищён `ThrottlerGuard` и (в EE) MFA-проверкой - ([auth.controller.ts:41](apps/server/src/core/auth/auth.controller.ts#L41), - [:64-103](apps/server/src/core/auth/auth.controller.ts#L64-L103)); вызывая - `authService.login` напрямую, мы их минуем. Следствия: (1) **brute-force через - `/mcp`** — добавить свой rate-limit на неудачные логины `/mcp` (по IP/почте); - (2) если MFA когда-либо вернётся/`enforceMfa` — Basic-путь должен **повторить - MFA-гейт или быть запрещён** для MFA-пользователей, а не молча пускать. -- **Креды в логах/трейсах.** Никогда не логировать `Authorization`, decoded - `email:password` и тело ответа логина (`auth-utils` уже прячет тело за `DEBUG` - — держать тот же принцип). На ошибке логина — конкретный `401`, но без эха - пароля. -- Per-user CASL: убедиться, что **все** инструменты идут только через loopback - REST/collab под пользовательским JWT и нигде не остаётся фолбэка на - сервис-аккаунт внутри уже инициализированной per-user сессии. -- Привязка к сессии (см. edge case) — анти-fixation проверка `email`/`sub`. -- `MCP_TOKEN`-развод: не оставить «дыру», где `Authorization` молча игнорируется. -- SSO/OIDC-пользователи без локального пароля: Basic для них не сработает — - вернуть понятный `401`, а не generic (и направить на токен-путь, если он есть). -- Доработка B (PAT): ключ хранить **хешем**, `last_used_at` обновлять, отзыв - (`deleted_at`) и `expires_at` проверять в `validateApiKey`. - -## Миграции / конфиг / env / docs - -- **Выбранный путь (Basic):** миграций нет. Обновить - [.env.example:72-79](.env.example#L72): пометить `MCP_DOCMOST_EMAIL/PASSWORD` - как **опциональные** (теперь это фолбэк-сервис-аккаунт, а не обязательный), - описать per-user Basic-режим и (если выбран) `X-MCP-Token`/ - `MCP_REQUIRE_USER_TOKEN`. Обновить README: как прописать в MCP-клиенте - `Authorization: Basic` (свои email:password) — у клиентов это обычно поле - «headers» в конфиге сервера. -- **Доработка B (PAT):** `api_keys` таблица уже есть; добавить типы в `db.d.ts` - (`migration:codegen`), при необходимости — индексы; новый модуль/сервис/контроллер - и клиентский UI в `apps/client/src/features/.../settings`. - -## Тесты / проверка - -- **Сервер (`pnpm --filter server test`):** - - `mcp.service` резолвер: `Basic email:password` → `AuthService.login` зовётся - с дефолтным воркспейсом → `getToken`-конфиг с выпущенным токеном; неверные - креды → `401` с конкретным сообщением (не generic); Bearer-fallback → - `verifyJwt(ACCESS)`; нет ничего + есть env-креды → сервис-аккаунт; нет ничего - → осмысленный 401/503. - - **пароль с `:`** парсится корректно (split по первому `:`). - - анти-fixation: второй запрос с кредами другого пользователя в той же сессии - → 401. -- **MCP-пакет (`pnpm --filter @docmost/mcp test`):** `createMcpHttpHandler` - принимает и статический конфиг, и резолвер; резолвер зовётся один раз на - инициализацию сессии; статический путь (stdio/сервис-аккаунт) не задет. -- **Ручная:** прописать в MCP-клиенте `Authorization: Basic base64(email:pass)` - своего юзера → проверить, что (1) видны только доступные пользователю спейсы/ - страницы (CASL), (2) правки в истории атрибутируются этому пользователю, а не - сервисному, (3) без env-кредов `/mcp` работает по логину/паролю, (4) неверный - пароль → понятная ошибка, а не generic, (5) залогировано без утечки пароля. - -## Открытые вопросы - -1. ~~Какой путь делаем~~ — **решено: логин/пароль через HTTP Basic** (вариант L). - A/B/C — совместимые доработки на будущее. -2. **Сервис-аккаунт:** оставить как откат (нет Basic/Bearer → старое поведение) - или полностью убрать в пользу обязательного per-user логина - (`MCP_REQUIRE_USER_TOKEN`)? -3. **`MCP_TOKEN`:** убрать в per-user режиме или перенести в отдельный заголовок - `X-MCP-Token` как доп. общий шлагбаум? -4. **Brute-force / throttle:** добавлять ли свой rate-limit на неудачные логины - `/mcp` (прямой `AuthService.login` минует контроллерный `ThrottlerGuard`)? -5. **Накопление сессий:** нужно ли чистить/ограничивать Docmost-сессии, создаваемые - `AuthService.login` на каждый MCP-логин, или достаточно «одна MCP-сессия = один - логин»? -6. **Серверная валидация vs pass-through:** валидировать креды через - `AuthService.login` (чище/безопаснее, тянет сервис в McpService) или отдать - `{email,password}` в `performLogin` пакета (минимум кода)? В дизайне выбрана - серверная валидация. -7. **Мульти-воркспейс:** loopback таргетит дефолтный воркспейс (как у AI-чата). - Нужен ли явный workspace-скоуп для мульти-тенант деплоя — или отдельная задача? diff --git a/packages/mcp/build/http.js b/packages/mcp/build/http.js index f22cc694..45c422b0 100644 --- a/packages/mcp/build/http.js +++ b/packages/mcp/build/http.js @@ -7,12 +7,30 @@ import { createDocmostMcpServer } from "./index.js"; * embedding host (the gitmost NestJS server) bridges its raw Node req/res into * `handleRequest`. One McpServer + transport is created per MCP session and * kept alive between requests, keyed by the `mcp-session-id` header. + * + * `config` is EITHER a static `DocmostMcpConfig` (back-compat: stdio + the env + * service account, unchanged) OR a `McpConfigResolver` run once per session at + * `initialize` to bind that session to the request's identity. */ -export function createMcpHttpHandler(config) { +export function createMcpHttpHandler(config, options = {}) { // One transport (and one McpServer) per MCP session, keyed by session id. const transports = {}; // Last activity timestamp per session id, used for idle eviction. const lastSeen = {}; + // Anti-session-fixation: the opaque identity key bound to each session at + // initialize. A later request for that session whose key differs is rejected. + const sessionIdentity = {}; + // Write a JSON-RPC error and end the response. Used for the 400/401 paths so + // every early rejection is a well-formed JSON-RPC error, not a torn response. + const sendJsonRpcError = (res, statusCode, code, message) => { + res.statusCode = statusCode; + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ + jsonrpc: "2.0", + error: { code, message }, + id: null, + })); + }; // Idle session TTL (ms): a session with no activity for this long is evicted. // Defaults to 30 min; overridable via MCP_SESSION_IDLE_MS. const idleTtlMs = (() => { @@ -29,6 +47,7 @@ export function createMcpHttpHandler(config) { if (now - (lastSeen[sid] ?? 0) > idleTtlMs) { void transports[sid].close(); delete lastSeen[sid]; + delete sessionIdentity[sid]; } } }, sweepIntervalMs); @@ -41,16 +60,23 @@ export function createMcpHttpHandler(config) { // A new session may only be created by an initialize request without a // session id. if (sessionId || !isInitializeRequest(parsedBody)) { - res.statusCode = 400; - res.setHeader("Content-Type", "application/json"); - res.end(JSON.stringify({ - jsonrpc: "2.0", - error: { - code: -32000, - message: "Bad Request: no valid session ID provided", - }, - id: null, - })); + sendJsonRpcError(res, 400, -32000, "Bad Request: no valid session ID provided"); + return; + } + // Resolve the per-session config from the request (per-user identity) when + // a resolver was supplied; otherwise use the static config unchanged. The + // resolver may throw (e.g. bad credentials) — surface a clean 401, never + // a created session. + let sessionConfig; + let identity; + try { + sessionConfig = + typeof config === "function" ? await config(req) : config; + if (options.identify) + identity = await options.identify(req); + } + catch (err) { + sendJsonRpcError(res, 401, -32001, err instanceof Error ? err.message : "Unauthorized"); return; } transport = new StreamableHTTPServerTransport({ @@ -58,31 +84,46 @@ export function createMcpHttpHandler(config) { onsessioninitialized: (sid) => { transports[sid] = transport; lastSeen[sid] = Date.now(); + // Bind the resolved identity to the new session id for anti-fixation. + if (identity !== undefined) + sessionIdentity[sid] = identity; }, }); transport.onclose = () => { const sid = transport.sessionId; if (sid && transports[sid]) delete transports[sid]; + if (sid) + delete sessionIdentity[sid]; }; - const server = createDocmostMcpServer(config); + const server = createDocmostMcpServer(sessionConfig); await server.connect(transport); await transport.handleRequest(req, res, parsedBody); return; } if (!transport) { - res.statusCode = 400; - res.setHeader("Content-Type", "application/json"); - res.end(JSON.stringify({ - jsonrpc: "2.0", - error: { - code: -32000, - message: "Bad Request: no valid session ID provided", - }, - id: null, - })); + sendJsonRpcError(res, 400, -32000, "Bad Request: no valid session ID provided"); return; } + // Anti-session-fixation: a request reusing an existing session id must + // present credentials/token that resolve to the SAME identity bound at + // initialize, otherwise reject with 401. This prevents hijacking another + // user's established session by replaying its session id with different + // credentials. + if (options.identify && sessionId && sessionId in sessionIdentity) { + let presented; + try { + presented = await options.identify(req); + } + catch (err) { + sendJsonRpcError(res, 401, -32001, err instanceof Error ? err.message : "Unauthorized"); + return; + } + if (presented !== sessionIdentity[sessionId]) { + sendJsonRpcError(res, 401, -32001, "Credentials do not match the user that owns this MCP session."); + return; + } + } // Routing to an existing transport: refresh its idle timestamp. if (sessionId) lastSeen[sessionId] = Date.now(); diff --git a/packages/mcp/src/http.ts b/packages/mcp/src/http.ts index b05bdfbf..cc344093 100644 --- a/packages/mcp/src/http.ts +++ b/packages/mcp/src/http.ts @@ -4,17 +4,71 @@ import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/ import { isInitializeRequest } from "@modelcontextprotocol/sdk/types.js"; import { createDocmostMcpServer, DocmostMcpConfig } from "./index.js"; +/** + * Per-request config resolver. Run ONCE per MCP session, at the `initialize` + * POST, so the session's DocmostClient is bound to that request's identity + * (e.g. the HTTP-Basic user the embedding host validated). Back-compat: a plain + * `DocmostMcpConfig` object is still accepted (stdio + service account), in + * which case the resolver branch is never taken. + */ +export type McpConfigResolver = ( + req: IncomingMessage, +) => DocmostMcpConfig | Promise; + +/** + * Optional anti-session-fixation hook. When supplied, it is called on EVERY + * request (init and subsequent) to derive an opaque identity key for the + * presented credentials/token. The key resolved at session init is bound to the + * `mcp-session-id`; a later request whose key differs is rejected with 401, so + * a caller cannot hijack another user's established session by reusing its + * session id with different credentials. The key is opaque to this package (the + * embedding host decides what identity means, e.g. the user's `sub`/email), so + * the package stays generic. Throwing here surfaces as a 401 as well. + */ +export interface McpHttpOptions { + identify?: (req: IncomingMessage) => string | Promise; +} + /** * Build a stateful Streamable-HTTP handler for the Docmost MCP server. The * embedding host (the gitmost NestJS server) bridges its raw Node req/res into * `handleRequest`. One McpServer + transport is created per MCP session and * kept alive between requests, keyed by the `mcp-session-id` header. + * + * `config` is EITHER a static `DocmostMcpConfig` (back-compat: stdio + the env + * service account, unchanged) OR a `McpConfigResolver` run once per session at + * `initialize` to bind that session to the request's identity. */ -export function createMcpHttpHandler(config: DocmostMcpConfig) { +export function createMcpHttpHandler( + config: DocmostMcpConfig | McpConfigResolver, + options: McpHttpOptions = {}, +) { // One transport (and one McpServer) per MCP session, keyed by session id. const transports: Record = {}; // Last activity timestamp per session id, used for idle eviction. const lastSeen: Record = {}; + // Anti-session-fixation: the opaque identity key bound to each session at + // initialize. A later request for that session whose key differs is rejected. + const sessionIdentity: Record = {}; + + // Write a JSON-RPC error and end the response. Used for the 400/401 paths so + // every early rejection is a well-formed JSON-RPC error, not a torn response. + const sendJsonRpcError = ( + res: ServerResponse, + statusCode: number, + code: number, + message: string, + ): void => { + res.statusCode = statusCode; + res.setHeader("Content-Type", "application/json"); + res.end( + JSON.stringify({ + jsonrpc: "2.0", + error: { code, message }, + id: null, + }), + ); + }; // Idle session TTL (ms): a session with no activity for this long is evicted. // Defaults to 30 min; overridable via MCP_SESSION_IDLE_MS. @@ -33,6 +87,7 @@ export function createMcpHttpHandler(config: DocmostMcpConfig) { if (now - (lastSeen[sid] ?? 0) > idleTtlMs) { void transports[sid].close(); delete lastSeen[sid]; + delete sessionIdentity[sid]; } } }, sweepIntervalMs); @@ -51,17 +106,30 @@ export function createMcpHttpHandler(config: DocmostMcpConfig) { // A new session may only be created by an initialize request without a // session id. if (sessionId || !isInitializeRequest(parsedBody)) { - res.statusCode = 400; - res.setHeader("Content-Type", "application/json"); - res.end( - JSON.stringify({ - jsonrpc: "2.0", - error: { - code: -32000, - message: "Bad Request: no valid session ID provided", - }, - id: null, - }), + sendJsonRpcError( + res, + 400, + -32000, + "Bad Request: no valid session ID provided", + ); + return; + } + // Resolve the per-session config from the request (per-user identity) when + // a resolver was supplied; otherwise use the static config unchanged. The + // resolver may throw (e.g. bad credentials) — surface a clean 401, never + // a created session. + let sessionConfig: DocmostMcpConfig; + let identity: string | undefined; + try { + sessionConfig = + typeof config === "function" ? await config(req) : config; + if (options.identify) identity = await options.identify(req); + } catch (err) { + sendJsonRpcError( + res, + 401, + -32001, + err instanceof Error ? err.message : "Unauthorized", ); return; } @@ -70,33 +138,60 @@ export function createMcpHttpHandler(config: DocmostMcpConfig) { onsessioninitialized: (sid: string) => { transports[sid] = transport!; lastSeen[sid] = Date.now(); + // Bind the resolved identity to the new session id for anti-fixation. + if (identity !== undefined) sessionIdentity[sid] = identity; }, }); transport.onclose = () => { const sid = transport!.sessionId; if (sid && transports[sid]) delete transports[sid]; + if (sid) delete sessionIdentity[sid]; }; - const server = createDocmostMcpServer(config); + const server = createDocmostMcpServer(sessionConfig); await server.connect(transport); await transport.handleRequest(req, res, parsedBody); return; } if (!transport) { - res.statusCode = 400; - res.setHeader("Content-Type", "application/json"); - res.end( - JSON.stringify({ - jsonrpc: "2.0", - error: { - code: -32000, - message: "Bad Request: no valid session ID provided", - }, - id: null, - }), + sendJsonRpcError( + res, + 400, + -32000, + "Bad Request: no valid session ID provided", ); return; } + + // Anti-session-fixation: a request reusing an existing session id must + // present credentials/token that resolve to the SAME identity bound at + // initialize, otherwise reject with 401. This prevents hijacking another + // user's established session by replaying its session id with different + // credentials. + if (options.identify && sessionId && sessionId in sessionIdentity) { + let presented: string; + try { + presented = await options.identify(req); + } catch (err) { + sendJsonRpcError( + res, + 401, + -32001, + err instanceof Error ? err.message : "Unauthorized", + ); + return; + } + if (presented !== sessionIdentity[sessionId]) { + sendJsonRpcError( + res, + 401, + -32001, + "Credentials do not match the user that owns this MCP session.", + ); + return; + } + } + // Routing to an existing transport: refresh its idle timestamp. if (sessionId) lastSeen[sessionId] = Date.now(); await transport.handleRequest(req, res, parsedBody); diff --git a/packages/mcp/test/unit/http-resolver.test.mjs b/packages/mcp/test/unit/http-resolver.test.mjs new file mode 100644 index 00000000..b3d7f39b --- /dev/null +++ b/packages/mcp/test/unit/http-resolver.test.mjs @@ -0,0 +1,234 @@ +// Unit tests for createMcpHttpHandler's config-resolver + anti-fixation hook +// (http.ts). These assert the wrapper contract WITHOUT depending on the MCP +// SDK's full initialize handshake succeeding: +// - a STATIC config is still accepted (back-compat: stdio / service account) +// and never invokes a resolver; +// - a RESOLVER is accepted and is invoked exactly once on a session-init POST; +// - the resolver/identify path runs BEFORE the transport, so a thrown +// resolver error surfaces as a clean 401 and no session is created. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { Readable } from "node:stream"; +import { createMcpHttpHandler } from "../../build/http.js"; + +// A minimal initialize JSON-RPC request body (isInitializeRequest checks +// method === "initialize" + jsonrpc + an object params with protocolVersion). +const INIT_BODY = { + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "test", version: "0.0.0" }, + }, +}; + +// Fake Node req: a readable stream is fine; we pass parsedBody explicitly so the +// transport never reads the stream, and our resolver short-circuits before that. +function makeReq({ method = "POST", headers = {} } = {}) { + const req = new Readable({ read() {} }); + req.method = method; + req.headers = headers; + req.push(null); + return req; +} + +// Fake Node res capturing statusCode + body, mimicking just what http.ts uses. +function makeRes() { + const chunks = []; + return { + statusCode: 200, + headers: {}, + headersSent: false, + setHeader(k, v) { + this.headers[k.toLowerCase()] = v; + }, + end(data) { + if (data) chunks.push(data); + this.headersSent = true; + this.ended = true; + }, + body() { + return chunks.join(""); + }, + }; +} + +test("static config is accepted and never calls a resolver (back-compat)", async () => { + // A static config object — the stdio / service-account path. A NON-initialize + // POST with no session id must hit the 400 branch deterministically, proving + // the static handler is wired and no resolver is consulted. + const handler = createMcpHttpHandler({ + apiUrl: "http://127.0.0.1:3000/api", + email: "svc@example.com", + password: "secret", + }); + const req = makeReq({ method: "POST", headers: {} }); + const res = makeRes(); + await handler.handleRequest(req, res, { jsonrpc: "2.0", method: "ping", id: 9 }); + assert.equal(res.statusCode, 400); + assert.match(res.body(), /no valid session ID/); +}); + +test("resolver is invoked exactly once on a session-init POST", async () => { + let calls = 0; + const handler = createMcpHttpHandler((req) => { + calls += 1; + // Throw a sentinel so we observe invocation without driving the full + // SDK handshake; http.ts turns a resolver throw into a clean 401. + throw new Error("sentinel-from-resolver"); + }); + const req = makeReq({ method: "POST", headers: {} }); + const res = makeRes(); + await handler.handleRequest(req, res, INIT_BODY); + assert.equal(calls, 1, "resolver must be called exactly once per init"); + assert.equal(res.statusCode, 401); + assert.match(res.body(), /sentinel-from-resolver/); +}); + +test("resolver is NOT invoked for a non-init POST without a session id", async () => { + let calls = 0; + const handler = createMcpHttpHandler(() => { + calls += 1; + return { apiUrl: "http://127.0.0.1:3000/api", getToken: async () => "t" }; + }); + const req = makeReq({ method: "POST", headers: {} }); + const res = makeRes(); + await handler.handleRequest(req, res, { jsonrpc: "2.0", method: "ping", id: 2 }); + assert.equal(calls, 0); + assert.equal(res.statusCode, 400); +}); + +test("identify hook throwing on init surfaces as a clean 401", async () => { + const handler = createMcpHttpHandler( + () => ({ apiUrl: "http://127.0.0.1:3000/api", getToken: async () => "t" }), + { + identify: () => { + throw new Error("bad-identity"); + }, + }, + ); + const req = makeReq({ method: "POST", headers: {} }); + const res = makeRes(); + await handler.handleRequest(req, res, INIT_BODY); + assert.equal(res.statusCode, 401); + assert.match(res.body(), /bad-identity/); +}); + +// Drive a REAL initialize handshake (over a loopback http server so the SDK's +// StreamableHTTPServerTransport gets genuine Node req/res objects), capture the +// assigned mcp-session-id, then replay subsequent requests to exercise the +// anti-fixation identify comparison: the SAME identity is accepted (routed to +// the transport), a DIFFERENT identity is rejected 401, and crucially the +// per-session config RESOLVER is consulted only ONCE (at init), never on a +// subsequent request — proving subsequent requests do not re-mint the config. +test("subsequent request: SAME identity routes through, DIFFERENT identity is 401, resolver runs once", async () => { + const http = await import("node:http"); + + let resolverCalls = 0; + let currentIdentity = "user-a"; + const handler = createMcpHttpHandler( + () => { + resolverCalls += 1; + return { apiUrl: "http://127.0.0.1:3000/api", getToken: async () => "t" }; + }, + { identify: () => currentIdentity }, + ); + + // Loopback server: every request is bridged into the MCP handler with its body + // parsed from JSON, exactly like the embedding host does. + const server = http.createServer((req, res) => { + let raw = ""; + req.on("data", (c) => (raw += c)); + req.on("end", () => { + const body = raw ? JSON.parse(raw) : undefined; + handler.handleRequest(req, res, body).catch(() => { + if (!res.headersSent) { + res.statusCode = 500; + res.end(); + } + }); + }); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + const { port } = server.address(); + + const call = (headers, body) => + new Promise((resolve) => { + const r = http.request( + { + host: "127.0.0.1", + port, + method: "POST", + path: "/mcp", + headers: { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + ...headers, + }, + }, + (resp) => { + let data = ""; + resp.on("data", (c) => (data += c)); + resp.on("end", () => + resolve({ + statusCode: resp.statusCode, + sessionId: resp.headers["mcp-session-id"], + body: data, + }), + ); + }, + ); + r.end(JSON.stringify(body)); + }); + + try { + // 1) Establish a session via a real initialize POST (identity = user-a). + const init = await call({}, INIT_BODY); + assert.equal(resolverCalls, 1, "resolver runs exactly once at init"); + const sid = init.sessionId; + assert.ok(sid, "initialize must assign an mcp-session-id"); + + // 2) Subsequent request, SAME identity: not a 401, resolver NOT re-run. + const ok = await call( + { "mcp-session-id": sid }, + { jsonrpc: "2.0", method: "ping", id: 5 }, + ); + assert.notEqual(ok.statusCode, 401, "same identity must not be rejected"); + assert.equal(resolverCalls, 1, "resolver is NOT re-run on a subsequent request"); + + // 3) Subsequent request, DIFFERENT identity: rejected 401 (anti-fixation). + currentIdentity = "user-b"; + const bad = await call( + { "mcp-session-id": sid }, + { jsonrpc: "2.0", method: "ping", id: 6 }, + ); + assert.equal(bad.statusCode, 401, "different identity hijack is rejected"); + assert.match(bad.body, /do not match the user/); + assert.equal(resolverCalls, 1, "still no resolver re-run on the rejected request"); + } finally { + await new Promise((r) => server.close(r)); + } +}); + +test("unknown existing session id (non-init, with session header) is 400", async () => { + // A request carrying a session id that was never established must not consult + // the resolver or identify hook — it is a plain 400 (no valid session). + let calls = 0; + const handler = createMcpHttpHandler( + () => { + calls += 1; + return { apiUrl: "http://127.0.0.1:3000/api", getToken: async () => "t" }; + }, + { identify: () => "x" }, + ); + const req = makeReq({ + method: "POST", + headers: { "mcp-session-id": "does-not-exist" }, + }); + const res = makeRes(); + await handler.handleRequest(req, res, { jsonrpc: "2.0", method: "ping", id: 3 }); + assert.equal(res.statusCode, 400); + assert.equal(calls, 0); +});