Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions apps/web/src/lib/auth/__tests__/auth-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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<string, unknown> | undefined;
const mockSet = vi.fn().mockImplementation((values: Record<string, unknown>) => {
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', () => {
Expand Down Expand Up @@ -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 = {
Expand Down
22 changes: 21 additions & 1 deletion apps/web/src/lib/auth/index.ts
Original file line number Diff line number Diff line change
@@ -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 ';
Expand Down Expand Up @@ -82,6 +82,7 @@ export async function validateMCPToken(token: string): Promise<MCPAuthDetails |
role: true,
tokenVersion: true,
adminRoleVersion: true,
suspendedAt: true,
},
},
driveScopes: {
Expand All @@ -97,6 +98,25 @@ export async function validateMCPToken(token: string): Promise<MCPAuthDetails |
return null;
}

// Revoke previously issued MCP tokens when a suspended user attempts to authenticate.
// This makes suspension enforcement sticky for future requests.
if (user.suspendedAt) {
logSecurityEvent('unauthorized', {
reason: 'mcp_token_user_suspended',
userId: tokenRecord.userId,
tokenId: tokenRecord.id,
authType: 'mcp',
action: 'revoke_and_deny',
});

await db
.update(mcpTokens)
.set({ revokedAt: new Date() })
.where(eq(mcpTokens.id, tokenRecord.id));

return null;
}

// Extract allowed drive IDs from the scopes
const allowedDriveIds = tokenRecord.driveScopes.map(scope => scope.driveId);

Expand Down
19 changes: 14 additions & 5 deletions docs/security/2026-02-11-security-posture-assessment.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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?
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down
Loading