From ef704b25264aa516d44a0704caee93fcc8879948 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 11 Feb 2026 19:50:48 -0600 Subject: [PATCH] [web] Block suspended users in MCP token auth path (#548) --- .../auth/__tests__/auth-middleware.test.ts | 90 +++++++++++++++++++ apps/web/src/lib/auth/index.ts | 22 ++++- .../2026-02-11-security-posture-assessment.md | 19 ++-- 3 files changed, 125 insertions(+), 6 deletions(-) diff --git a/apps/web/src/lib/auth/__tests__/auth-middleware.test.ts b/apps/web/src/lib/auth/__tests__/auth-middleware.test.ts index e9cc386a46..d30f603ebd 100644 --- a/apps/web/src/lib/auth/__tests__/auth-middleware.test.ts +++ b/apps/web/src/lib/auth/__tests__/auth-middleware.test.ts @@ -19,6 +19,15 @@ vi.mock('@pagespace/lib/auth', () => ({ }, })); +vi.mock('@pagespace/lib/server', () => ({ + EnforcedAuthContext: class EnforcedAuthContext { + static fromSession(sessionClaims: unknown): unknown { + return sessionClaims; + } + }, + logSecurityEvent: vi.fn(), +})); + vi.mock('@pagespace/db', () => ({ db: { query: { @@ -51,6 +60,7 @@ vi.mock('../cookie-config', () => ({ })); import { sessionService } from '@pagespace/lib/auth'; +import { logSecurityEvent } from '@pagespace/lib/server'; import { db } from '@pagespace/db'; import { validateCSRF } from '../csrf-validation'; import { validateOrigin } from '../origin-validation'; @@ -213,6 +223,52 @@ describe('Auth Middleware', () => { expect(capturedSetValues).toBeDefined(); expect(capturedSetValues!.lastUsed).toBeInstanceOf(Date); }); + + it('denies and revokes MCP token for suspended users', async () => { + // Arrange + const mockMCPToken = { + id: 'token-id', + userId: 'test-user-id', + user: { + id: 'test-user-id', + role: 'user' as const, + tokenVersion: 0, + adminRoleVersion: 0, + suspendedAt: new Date('2026-01-01T00:00:00.000Z'), + }, + driveScopes: [], + }; + vi.mocked(db.query.mcpTokens.findFirst).mockResolvedValue(mockMCPToken as never); + + let capturedSetValues: Record | undefined; + const mockSet = vi.fn().mockImplementation((values: Record) => { + capturedSetValues = values; + return { + where: vi.fn().mockResolvedValue(undefined), + }; + }); + vi.mocked(db.update).mockReturnValue({ set: mockSet } as never); + + // Act + const result = await validateMCPToken('mcp_valid-token'); + + // Assert + expect(result).toBeNull(); + expect(capturedSetValues).toBeDefined(); + expect(capturedSetValues).toMatchObject({ + revokedAt: expect.any(Date), + }); + expect(logSecurityEvent).toHaveBeenCalledWith( + 'unauthorized', + expect.objectContaining({ + reason: 'mcp_token_user_suspended', + userId: 'test-user-id', + tokenId: 'token-id', + authType: 'mcp', + action: 'revoke_and_deny', + }) + ); + }); }); describe('authenticateSessionRequest', () => { @@ -363,6 +419,40 @@ describe('Auth Middleware', () => { } }); + it('returns error for suspended user MCP token', async () => { + // Arrange + const mockMCPToken = { + id: 'token-id', + userId: 'test-user-id', + user: { + id: 'test-user-id', + role: 'user' as const, + tokenVersion: 0, + adminRoleVersion: 0, + suspendedAt: new Date('2026-01-01T00:00:00.000Z'), + }, + driveScopes: [], + }; + vi.mocked(db.query.mcpTokens.findFirst).mockResolvedValue(mockMCPToken as never); + + const request = new Request('http://localhost/api/test', { + method: 'GET', + headers: { + Authorization: 'Bearer mcp_suspended-user-token', + }, + }); + + // Act + const result = await authenticateMCPRequest(request); + + // Assert + expect(isAuthError(result)).toBe(true); + if (isAuthError(result)) { + const body = await result.error.json(); + expect(body.error).toBe('Invalid MCP token'); + } + }); + it('authenticates with valid MCP token', async () => { // Arrange const mockMCPToken = { diff --git a/apps/web/src/lib/auth/index.ts b/apps/web/src/lib/auth/index.ts index 3e9f16882b..76f0cfb36f 100644 --- a/apps/web/src/lib/auth/index.ts +++ b/apps/web/src/lib/auth/index.ts @@ -1,7 +1,7 @@ import { NextResponse } from 'next/server'; import { db, mcpTokens, eq, and, isNull } from '@pagespace/db'; import { hashToken, sessionService, type SessionClaims } from '@pagespace/lib/auth'; -import { EnforcedAuthContext } from '@pagespace/lib/server'; +import { EnforcedAuthContext, logSecurityEvent } from '@pagespace/lib/server'; import { getSessionFromCookies } from './cookie-config'; const BEARER_PREFIX = 'Bearer '; @@ -82,6 +82,7 @@ export async function validateMCPToken(token: string): Promise scope.driveId); diff --git a/docs/security/2026-02-11-security-posture-assessment.md b/docs/security/2026-02-11-security-posture-assessment.md index 7b9dd7e733..1717617d20 100644 --- a/docs/security/2026-02-11-security-posture-assessment.md +++ b/docs/security/2026-02-11-security-posture-assessment.md @@ -5,6 +5,11 @@ Scope: `apps/web`, `apps/processor`, `packages/lib`, `packages/db`, desktop MCP ## Executive Summary Your proposed statement is directionally right, but not fully true as written. +Update (February 12, 2026): +- Issue #548 has been remediated. +- `validateMCPToken` now denies suspended users, logs the attempt as a security event, and revokes the presented MCP token on first detection. +- Previously issued MCP tokens are therefore invalidated at first post-suspension use (not just denied per-request). + What is true now: - Authenticated web/service operations are largely built around opaque server-issued tokens and server-side revalidation. - Session tokens are hashed at rest and revalidated against DB state (expiry, revocation, suspension, token version) on each use. @@ -13,7 +18,6 @@ What is true now: What is not true now: - Not every endpoint is session-mediated (by design: health, webhooks, monitoring ingest, unsubscribe links, etc.). - Scoped MCP token restrictions are not consistently enforced across hybrid (`session` + `mcp`) routes. -- A suspended user can still authenticate with existing MCP tokens (suspension check is present for sessions, not MCP token validation). - Page/chat content is plaintext at the application layer (intentional tradeoff for search/AI workflows). ## Can You Publish This Exact Claim? @@ -92,15 +96,20 @@ Quantified blast radius: - 53 hybrid routes found; all now have explicit MCP scope enforcement. - 17 routes were fixed in PR #553; 13 additional routes were fixed in PR #547-v2 (see Appendix A for details). -### P1: Suspended users can still use MCP tokens +### P1 (Resolved February 12, 2026): Suspended users can still use MCP tokens Risk vector: - Session validation blocks suspended users and revokes sessions. - MCP token validation does not check `suspendedAt`, so suspended accounts retain MCP API access until token revocation. -Evidence: +Evidence (before remediation): - Session suspension enforcement: `packages/lib/src/auth/session-service.ts:99` - MCP token validation path lacks suspension check and returns auth details: `apps/web/src/lib/auth/index.ts:63` +Resolution: +- `validateMCPToken` now checks `user.suspendedAt`. +- Suspended-user MCP token attempts are logged as security events. +- The token presented by a suspended user is revoked (`revokedAt`) on first detection, then denied for all subsequent requests. + ### P1: Processor file-delete endpoint lacks resource ownership authorization Risk vector: - Delete endpoint validates only content hash + scope. @@ -125,13 +134,13 @@ Evidence: 1. Local MCP server execution model: user-trusted local code execution, not sandboxed. 2. Plaintext searchable content: no app-layer content encryption for pages/chat. 3. Hybrid MCP scope inconsistency: scoped tokens may exceed intended drive boundaries. -4. MCP suspension gap: suspended users may retain MCP token access. +4. MCP suspension gap (resolved February 12, 2026): suspended users no longer retain MCP token access. 5. Processor hash-delete authorization gap: delete by hash lacks ownership check. 6. Admin stale-role path in one endpoint: bypasses role-version hardening model. ## Remediation Priority 1. Enforce MCP scope checks in all hybrid routes (or deny MCP by default unless explicit scope guard is present). -2. Add suspension enforcement to `validateMCPToken`. +2. Add suspension enforcement to `validateMCPToken`. (Completed February 12, 2026) 3. Add ownership/resource authorization in `apps/processor/src/api/delete-file.ts` (match serve RBAC pattern). 4. Migrate `gift-subscription` route to `verifyAdminAuth`. 5. Add automated tests: