diff --git a/.env.example b/.env.example index fbd32428..882a6dc8 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.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..def3155c 100644 --- a/apps/server/src/core/auth/services/auth.service.ts +++ b/apps/server/src/core/auth/services/auth.service.ts @@ -57,7 +57,23 @@ 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, }); @@ -84,6 +100,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/integrations/mcp/mcp-auth.helpers.ts b/apps/server/src/integrations/mcp/mcp-auth.helpers.ts new file mode 100644 index 00000000..6eb17ab0 --- /dev/null +++ b/apps/server/src/integrations/mcp/mcp-auth.helpers.ts @@ -0,0 +1,379 @@ +// 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 { JwtType } from '../../core/auth/dto/jwt-payload'; + +/** + * 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>; + // 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. + */ +export function isCredentialsFailure(err: unknown): boolean { + return ( + err instanceof UnauthorizedException && + typeof err.message === 'string' && + err.message.toLowerCase().includes('email or password does not match') + ); +} + +// 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 }; +} + +/** 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.'); + } + + // 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 any prior failure budget. + deps.limiter.reset(ipKey); + deps.limiter.reset(ipEmailKey); + deps.limiter.reset(emailKey); + 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..1b4a0969 --- /dev/null +++ b/apps/server/src/integrations/mcp/mcp.service.spec.ts @@ -0,0 +1,396 @@ +import { BadRequestException, UnauthorizedException } from '@nestjs/common'; +import { + parseBasicAuth, + FailedLoginLimiter, + resolveMcpSessionConfig, + isCredentialsFailure, + verifyBearerAccess, + McpAuthDeps, +} from './mcp-auth.helpers'; + +// 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' }), + 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); + }); +}); + +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); + }); +}); diff --git a/apps/server/src/integrations/mcp/mcp.service.ts b/apps/server/src/integrations/mcp/mcp.service.ts index be67e228..a18e2d6d 100644 --- a/apps/server/src/integrations/mcp/mcp.service.ts +++ b/apps/server/src/integrations/mcp/mcp.service.ts @@ -1,8 +1,26 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { + Injectable, + Logger, + UnauthorizedException, +} from '@nestjs/common'; import { pathToFileURL } from 'node:url'; +import { timingSafeEqual } from 'node:crypto'; +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 { JwtType, JwtPayload } from '../../core/auth/dto/jwt-payload'; +import { + FailedLoginLimiter, + resolveMcpSessionConfig, + verifyBearerAccess, + DocmostMcpConfig, + ResolvedMcpAuth, +} from './mcp-auth.helpers'; // Minimal shape of the embedded MCP HTTP handler exported by @docmost/mcp/http. interface McpHttpHandler { @@ -13,14 +31,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 @@ -37,13 +64,23 @@ export class McpService { 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); + 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, ) {} // 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 +117,102 @@ export class McpService { } } + // 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-prefix via early-exit string comparison; it requires equal + // buffer lengths, so a length mismatch is treated as a non-match WITHOUT + // calling timingSafeEqual (which would throw on unequal lengths). + private 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); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); + } + + // Best-effort client IP for the failed-login limiter key. Prefer Fastify's + // req.ip (which honours a configured trustProxy chain) and the socket address + // over a raw X-Forwarded-For hop, since XFF is client-forgeable when no + // trusted proxy is configured. The first XFF hop is only used as a last + // resort. NOTE: 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. + private clientIp(req: FastifyRequest): 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'; + } + + // 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, { + verifyJwt: (t) => + this.tokenService.verifyJwt(t, JwtType.ACCESS) as 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). Only the INIT request + // should run the full, session-minting login(); subsequent requests only + // re-validate credentials (anti-fixation) with no side effects. + const isSessionInit = !req.headers['mcp-session-id']; + return resolveMcpSessionConfig(authHeader, { + apiUrl: this.getApiUrl(), + email: this.getEmail(), + password: this.getPassword(), + findWorkspace: () => this.workspaceRepo.findFirst(), + 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: this.clientIp(req), + isSessionInit, + }); + } + // 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 +226,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 +261,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 (!this.sharedTokenMatches(sharedToken, provided)) { res.status(401).send({ error: 'Unauthorized' }); return; } @@ -129,20 +278,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/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); +});