diff --git a/.husky/pre-commit b/.husky/pre-commit new file mode 100644 index 00000000..2312dc58 --- /dev/null +++ b/.husky/pre-commit @@ -0,0 +1 @@ +npx lint-staged diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 00000000..6731faa1 --- /dev/null +++ b/.prettierignore @@ -0,0 +1,6 @@ +node_modules +.next +dist +.yarn +coverage +*.lock diff --git a/package.json b/package.json index 596dec44..c86b457d 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,8 @@ "test": "yarn workspaces run test", "lint": "yarn workspaces run lint", "type-check": "yarn workspaces run type-check", - "clean": "yarn workspaces run clean && rm -rf node_modules && pm2 delete all" + "clean": "yarn workspaces run clean && rm -rf node_modules && pm2 delete all", + "prepare": "husky" }, "engines": { "node": ">=18.0.0", @@ -57,7 +58,13 @@ "author": "", "license": "MIT", "devDependencies": { - "pm2": "^6.0.14" + "husky": "^9.1.7", + "lint-staged": "^16.2.7", + "pm2": "^6.0.14", + "prettier": "^3.8.1" + }, + "lint-staged": { + "*.{ts,tsx,js,jsx,json,css,md}": "prettier --write" }, "packageManager": "yarn@4.12.0" } diff --git a/packages/api/src/data/supabase/client.ts b/packages/api/src/data/supabase/client.ts index 2cfe3be2..13048956 100644 --- a/packages/api/src/data/supabase/client.ts +++ b/packages/api/src/data/supabase/client.ts @@ -5,6 +5,19 @@ import type { Database } from './types'; let supabaseClient: SupabaseClient | null = null; +/** + * Singleton Supabase client for PostgREST data queries using the service role key. + * + * ⚠️ DO NOT call session-mutating auth methods on this client: + * auth.refreshSession(), auth.signIn*(), auth.signUp(), auth.setSession() + * + * These methods overwrite the Authorization header from service_role to a user JWT, + * silently subjecting all subsequent PostgREST queries to RLS. persistSession:false + * does NOT prevent this — it only skips disk storage, the in-memory session is still set. + * + * Safe: .from('table').*, auth.getUser(jwt), auth.admin.* + * Ref: https://github.com/orgs/supabase/discussions/30146 + */ export function createSupabaseClient(): SupabaseClient { if (supabaseClient) { return supabaseClient; @@ -13,7 +26,7 @@ export function createSupabaseClient(): SupabaseClient { try { supabaseClient = createClient(env.SUPABASE_URL, env.SUPABASE_SECRET_KEY, { auth: { - autoRefreshToken: true, + autoRefreshToken: false, persistSession: false, }, }); diff --git a/packages/api/src/data/supabase/types.ts b/packages/api/src/data/supabase/types.ts index efc6e004..52c7240a 100644 --- a/packages/api/src/data/supabase/types.ts +++ b/packages/api/src/data/supabase/types.ts @@ -1,6 +1,3 @@ -// Database types - Auto-generated from Supabase schema -// Run `mcp__supabase__generate_typescript_types` to regenerate - export type Json = | string | number @@ -410,7 +407,6 @@ export type Database = { change_summary: string | null change_type: string | null changed_by_agent_id: string | null - changed_by_identity_id: string | null changed_by_user_id: string | null content: string created_at: string | null @@ -423,7 +419,6 @@ export type Database = { change_summary?: string | null change_type?: string | null changed_by_agent_id?: string | null - changed_by_identity_id?: string | null changed_by_user_id?: string | null content: string created_at?: string | null @@ -436,7 +431,6 @@ export type Database = { change_summary?: string | null change_type?: string | null changed_by_agent_id?: string | null - changed_by_identity_id?: string | null changed_by_user_id?: string | null content?: string created_at?: string | null @@ -459,84 +453,6 @@ export type Database = { referencedRelation: "users" referencedColumns: ["id"] }, - { - foreignKeyName: "artifact_history_changed_by_identity_id_fkey" - columns: ["changed_by_identity_id"] - isOneToOne: false - referencedRelation: "agent_identities" - referencedColumns: ["id"] - }, - ] - } - artifact_comments: { - Row: { - artifact_id: string - content: string - created_at: string | null - created_by_agent_id: string | null - created_by_identity_id: string | null - deleted_at: string | null - id: string - metadata: Json | null - parent_comment_id: string | null - updated_at: string | null - user_id: string - } - Insert: { - artifact_id: string - content: string - created_at?: string | null - created_by_agent_id?: string | null - created_by_identity_id?: string | null - deleted_at?: string | null - id?: string - metadata?: Json | null - parent_comment_id?: string | null - updated_at?: string | null - user_id: string - } - Update: { - artifact_id?: string - content?: string - created_at?: string | null - created_by_agent_id?: string | null - created_by_identity_id?: string | null - deleted_at?: string | null - id?: string - metadata?: Json | null - parent_comment_id?: string | null - updated_at?: string | null - user_id?: string - } - Relationships: [ - { - foreignKeyName: "artifact_comments_artifact_id_fkey" - columns: ["artifact_id"] - isOneToOne: false - referencedRelation: "artifacts" - referencedColumns: ["id"] - }, - { - foreignKeyName: "artifact_comments_created_by_identity_id_fkey" - columns: ["created_by_identity_id"] - isOneToOne: false - referencedRelation: "agent_identities" - referencedColumns: ["id"] - }, - { - foreignKeyName: "artifact_comments_parent_comment_id_fkey" - columns: ["parent_comment_id"] - isOneToOne: false - referencedRelation: "artifact_comments" - referencedColumns: ["id"] - }, - { - foreignKeyName: "artifact_comments_user_id_fkey" - columns: ["user_id"] - isOneToOne: false - referencedRelation: "users" - referencedColumns: ["id"] - }, ] } artifacts: { @@ -547,7 +463,6 @@ export type Database = { content_type: string | null created_at: string | null created_by_agent_id: string | null - created_by_identity_id: string | null id: string metadata: Json | null tags: string[] | null @@ -565,7 +480,6 @@ export type Database = { content_type?: string | null created_at?: string | null created_by_agent_id?: string | null - created_by_identity_id?: string | null id?: string metadata?: Json | null tags?: string[] | null @@ -583,7 +497,6 @@ export type Database = { content_type?: string | null created_at?: string | null created_by_agent_id?: string | null - created_by_identity_id?: string | null id?: string metadata?: Json | null tags?: string[] | null @@ -602,13 +515,6 @@ export type Database = { referencedRelation: "users" referencedColumns: ["id"] }, - { - foreignKeyName: "artifacts_created_by_identity_id_fkey" - columns: ["created_by_identity_id"] - isOneToOne: false - referencedRelation: "agent_identities" - referencedColumns: ["id"] - }, ] } audit_log: { @@ -1136,7 +1042,7 @@ export type Database = { last_used_at: string | null refresh_token: string scopes: string[] | null - supabase_refresh_token: string + supabase_refresh_token: string | null updated_at: string | null user_id: string } @@ -1148,7 +1054,7 @@ export type Database = { last_used_at?: string | null refresh_token: string scopes?: string[] | null - supabase_refresh_token: string + supabase_refresh_token?: string | null updated_at?: string | null user_id: string } @@ -1160,7 +1066,7 @@ export type Database = { last_used_at?: string | null refresh_token?: string scopes?: string[] | null - supabase_refresh_token?: string + supabase_refresh_token?: string | null updated_at?: string | null user_id?: string } diff --git a/packages/api/src/mcp/auth/pcp-auth-provider.test.ts b/packages/api/src/mcp/auth/pcp-auth-provider.test.ts index 99cffcbf..80e13ca7 100644 --- a/packages/api/src/mcp/auth/pcp-auth-provider.test.ts +++ b/packages/api/src/mcp/auth/pcp-auth-provider.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import crypto from 'crypto'; +import jwt from 'jsonwebtoken'; // --------------------------------------------------------------------------- // Mocks @@ -10,7 +11,6 @@ const mockUpdate = vi.fn(); const mockDelete = vi.fn(); const mockSelect = vi.fn(); const mockGetUser = vi.fn(); -const mockRefreshSession = vi.fn(); // Build a chainable mock for Supabase queries function mockChain(terminalData: unknown = null, terminalError: unknown = null) { @@ -32,7 +32,6 @@ vi.mock('@supabase/supabase-js', () => ({ createClient: vi.fn(() => ({ auth: { getUser: mockGetUser, - refreshSession: mockRefreshSession, }, from: vi.fn((table: string) => { if (table === 'users') return currentUserChain; @@ -42,10 +41,13 @@ vi.mock('@supabase/supabase-js', () => ({ })), })); +const TEST_JWT_SECRET = 'test-jwt-secret-that-is-at-least-32-characters-long'; + vi.mock('../../config/env', () => ({ env: { SUPABASE_URL: 'http://localhost:54321', SUPABASE_SECRET_KEY: 'test-key', + JWT_SECRET: 'test-jwt-secret-that-is-at-least-32-characters-long', }, })); @@ -105,7 +107,6 @@ async function runFullAuthFlow(provider: PcpAuthProvider) { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'supabase-jwt-123', - refreshToken: 'supabase-rt-456', }); if ('error' in callbackResult) throw new Error(`Callback failed: ${callbackResult.error}`); @@ -140,12 +141,20 @@ describe('PcpAuthProvider', () => { // ========================================================================= describe('createPendingAuth', () => { - it('should return a pendingId starting with "pending-"', () => { + it('should return a valid signed JWT', () => { const pendingId = setupPendingAuth(provider); - expect(pendingId).toMatch(/^pending-/); + // JWT format: three base64url segments separated by dots + expect(pendingId.split('.')).toHaveLength(3); + + // Should be verifiable with the secret + const decoded = jwt.verify(pendingId, TEST_JWT_SECRET) as Record; + expect(decoded.type).toBe('pending_auth'); + expect(decoded.clientId).toBe('test-client'); + expect(decoded.redirectUri).toBe('http://localhost:3001/callback'); + expect(decoded.state).toBe('test-state'); }); - it('should generate unique IDs for each call', () => { + it('should generate unique tokens for each call', () => { const id1 = setupPendingAuth(provider); const id2 = setupPendingAuth(provider); expect(id1).not.toBe(id2); @@ -164,7 +173,6 @@ describe('PcpAuthProvider', () => { const result = await provider.handleAuthCallback({ pendingId, accessToken: 'supabase-jwt', - refreshToken: 'supabase-rt', }); expect('code' in result).toBe(true); @@ -179,7 +187,6 @@ describe('PcpAuthProvider', () => { const result = await provider.handleAuthCallback({ pendingId: 'nonexistent', accessToken: 'jwt', - refreshToken: 'rt', }); expect(result).toEqual({ @@ -198,7 +205,6 @@ describe('PcpAuthProvider', () => { const result = await provider.handleAuthCallback({ pendingId, accessToken: 'bad-token', - refreshToken: 'rt', }); expect(result).toEqual({ @@ -232,7 +238,6 @@ describe('PcpAuthProvider', () => { const result = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', }); expect(result).toHaveProperty('code'); @@ -251,7 +256,6 @@ describe('PcpAuthProvider', () => { const result = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', }); expect(result).toEqual({ @@ -260,29 +264,19 @@ describe('PcpAuthProvider', () => { }); }); - // Regression: web portal was redirecting to /mcp/auth/callback without - // refresh_token, causing "Missing refresh token" error for MCP clients. - // The auth callback MUST receive both access_token and refresh_token - // from the web portal so the token exchange can store the Supabase - // refresh token for later use. - it('should require refresh_token for successful callback (regression)', async () => { + it('should work without refresh_token (no longer needed)', async () => { const pendingId = setupPendingAuth(provider); mockSuccessfulAuth(); - // Callback with access_token but NO refresh_token should still - // produce an auth code — the provider doesn't validate this, the - // HTTP layer does. But verify the stored refresh token propagates - // through to the code exchange. const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'supabase-jwt', - refreshToken: 'supabase-rt-required', }); expect('code' in callbackResult).toBe(true); if (!('code' in callbackResult)) return; - // Exchange the code and verify refresh token was stored + // Exchange the code currentMcpTokensChain = mockChain(); mockInsert.mockReturnValue({ error: null }); @@ -293,34 +287,75 @@ describe('PcpAuthProvider', () => { }); expect('access_token' in tokenResult).toBe(true); - if (!('access_token' in tokenResult)) return; - // The insert call should contain the supabase refresh token + // supabase_refresh_token should be null (self-issued JWTs, no Supabase dependency) expect(mockInsert).toHaveBeenCalled(); const insertArgs = mockInsert.mock.calls[0]?.[0]; - expect(insertArgs).toHaveProperty('supabase_refresh_token', 'supabase-rt-required'); + expect(insertArgs).toHaveProperty('supabase_refresh_token', null); }); - it('should consume the pending auth after successful callback', async () => { + it('should accept the same JWT on re-callback (stateless, PKCE prevents replay)', async () => { const pendingId = setupPendingAuth(provider); mockSuccessfulAuth(); - await provider.handleAuthCallback({ + const result1 = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', }); - // Second call with same pendingId should fail - const result = await provider.handleAuthCallback({ + expect('code' in result1).toBe(true); + + // Second call with same pendingId should also succeed (stateless JWT) + mockSuccessfulAuth(); + const result2 = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', + }); + + expect('code' in result2).toBe(true); + }); + + it('should return error for expired JWT', async () => { + // Sign a JWT that's already expired + const expiredToken = jwt.sign( + { + type: 'pending_auth', + clientId: 'test', + codeChallenge: 'ch', + redirectUri: 'http://localhost/cb', + state: 's', + }, + TEST_JWT_SECRET, + { expiresIn: 0 } + ); + + // Wait a tick so the token is expired + await new Promise((r) => setTimeout(r, 10)); + + const result = await provider.handleAuthCallback({ + pendingId: expiredToken, + accessToken: 'jwt', }); expect(result).toEqual({ error: 'invalid_request', - error_description: 'Invalid or expired authorization request', + error_description: 'Authorization request expired', + }); + }); + + it('should return error for wrong JWT type', async () => { + const wrongTypeToken = jwt.sign({ type: 'wrong_type', clientId: 'test' }, TEST_JWT_SECRET, { + expiresIn: 600, + }); + + const result = await provider.handleAuthCallback({ + pendingId: wrongTypeToken, + accessToken: 'jwt', + }); + + expect(result).toEqual({ + error: 'invalid_request', + error_description: 'Invalid authorization request', }); }); }); @@ -330,7 +365,7 @@ describe('PcpAuthProvider', () => { // ========================================================================= describe('exchangeAuthorizationCode', () => { - it('should return access_token and refresh_token on success', async () => { + it('should return a self-signed JWT access_token and refresh_token on success', async () => { const pendingId = setupPendingAuth(provider); mockSuccessfulAuth(); @@ -340,7 +375,6 @@ describe('PcpAuthProvider', () => { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'supabase-jwt-123', - refreshToken: 'supabase-rt-456', }); expect('code' in callbackResult).toBe(true); @@ -354,15 +388,23 @@ describe('PcpAuthProvider', () => { expect('access_token' in result).toBe(true); if ('access_token' in result) { - expect(result.access_token).toBe('supabase-jwt-123'); + // Access token is a self-signed JWT (3 dot-separated segments) + expect(result.access_token.split('.')).toHaveLength(3); + // Verify JWT payload + const decoded = jwt.verify(result.access_token, TEST_JWT_SECRET) as Record; + expect(decoded.type).toBe('mcp_access'); + expect(decoded.sub).toBe('user-123'); + expect(decoded.email).toBe('test@example.com'); + expect(decoded.scope).toBe('mcp:tools'); + expect(result.refresh_token).toMatch(/^pcp-rt-/); expect(result.token_type).toBe('Bearer'); - expect(result.expires_in).toBe(3600); + expect(result.expires_in).toBe(30 * 24 * 60 * 60); expect(result.scope).toBe('mcp:tools'); } }); - it('should store refresh token in database', async () => { + it('should store refresh token in database with null supabase_refresh_token', async () => { const pendingId = setupPendingAuth(provider); mockSuccessfulAuth(); @@ -372,7 +414,6 @@ describe('PcpAuthProvider', () => { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'supabase-rt', }); if (!('code' in callbackResult)) return; @@ -382,8 +423,9 @@ describe('PcpAuthProvider', () => { clientId: 'test-client', }); - // Verify insert was called with correct data expect(mockInsert).toHaveBeenCalled(); + const insertArgs = mockInsert.mock.calls[0]?.[0]; + expect(insertArgs).toHaveProperty('supabase_refresh_token', null); }); it('should return error for invalid code', async () => { @@ -406,7 +448,6 @@ describe('PcpAuthProvider', () => { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', }); if (!('code' in callbackResult)) return; @@ -422,6 +463,28 @@ describe('PcpAuthProvider', () => { }); }); + it('should return error for client_id mismatch', async () => { + const pendingId = setupPendingAuth(provider); + mockSuccessfulAuth(); + + const callbackResult = await provider.handleAuthCallback({ + pendingId, + accessToken: 'jwt', + }); + if (!('code' in callbackResult)) return; + + const result = await provider.exchangeAuthorizationCode({ + code: callbackResult.code, + codeVerifier: 'test-verifier', + clientId: 'different-client', // doesn't match 'test-client' from setupPendingAuth + }); + + expect(result).toEqual({ + error: 'invalid_grant', + error_description: 'Client ID mismatch', + }); + }); + it('should return error when DB insert fails', async () => { const pendingId = setupPendingAuth(provider); mockSuccessfulAuth(); @@ -433,7 +496,6 @@ describe('PcpAuthProvider', () => { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', }); if (!('code' in callbackResult)) return; @@ -459,7 +521,6 @@ describe('PcpAuthProvider', () => { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'jwt', - refreshToken: 'rt', }); if (!('code' in callbackResult)) return; @@ -489,31 +550,19 @@ describe('PcpAuthProvider', () => { // ========================================================================= describe('exchangeRefreshToken', () => { - it('should return new access_token on successful refresh', async () => { - // Mock: find the token in DB + it('should return a self-signed JWT access_token on successful refresh', async () => { const futureDate = new Date(Date.now() + 86400000).toISOString(); currentMcpTokensChain = mockChain({ id: 'token-1', user_id: 'user-123', client_id: 'test-client', refresh_token: 'pcp-rt-abc', - supabase_refresh_token: 'supabase-rt-old', + supabase_refresh_token: null, scopes: ['mcp:tools'], expires_at: futureDate, + users: { email: 'test@example.com' }, }); - // Mock: Supabase refresh succeeds - mockRefreshSession.mockResolvedValue({ - data: { - session: { - access_token: 'fresh-supabase-jwt', - refresh_token: 'supabase-rt-new', - }, - }, - error: null, - }); - - // Mock: update succeeds mockUpdate.mockReturnValue({ eq: vi.fn(() => ({ error: null })), }); @@ -525,33 +574,30 @@ describe('PcpAuthProvider', () => { expect('access_token' in result).toBe(true); if ('access_token' in result) { - expect(result.access_token).toBe('fresh-supabase-jwt'); - expect(result.refresh_token).toBe('pcp-rt-abc'); // Same refresh token + // Verify it's a self-signed JWT + const decoded = jwt.verify(result.access_token, TEST_JWT_SECRET) as Record; + expect(decoded.type).toBe('mcp_access'); + expect(decoded.sub).toBe('user-123'); + expect(decoded.email).toBe('test@example.com'); + expect(decoded.scope).toBe('mcp:tools'); + + expect(result.refresh_token).toBe('pcp-rt-abc'); expect(result.token_type).toBe('Bearer'); - expect(result.expires_in).toBe(3600); + expect(result.expires_in).toBe(30 * 24 * 60 * 60); } }); - it('should call supabase.auth.refreshSession with stored token', async () => { + it('should not call supabase.auth.refreshSession (no Supabase dependency)', async () => { const futureDate = new Date(Date.now() + 86400000).toISOString(); currentMcpTokensChain = mockChain({ id: 'token-1', user_id: 'user-123', client_id: 'test-client', refresh_token: 'pcp-rt-abc', - supabase_refresh_token: 'supabase-rt-stored', + supabase_refresh_token: null, scopes: ['mcp:tools'], expires_at: futureDate, - }); - - mockRefreshSession.mockResolvedValue({ - data: { - session: { - access_token: 'new-jwt', - refresh_token: 'new-supabase-rt', - }, - }, - error: null, + users: { email: 'test@example.com' }, }); mockUpdate.mockReturnValue({ @@ -563,9 +609,8 @@ describe('PcpAuthProvider', () => { clientId: 'test-client', }); - expect(mockRefreshSession).toHaveBeenCalledWith({ - refresh_token: 'supabase-rt-stored', - }); + // No Supabase auth calls should be made during refresh + expect(mockGetUser).not.toHaveBeenCalled(); }); it('should return error for unknown refresh token', async () => { @@ -589,9 +634,10 @@ describe('PcpAuthProvider', () => { user_id: 'user-123', client_id: 'original-client', refresh_token: 'pcp-rt-abc', - supabase_refresh_token: 'supabase-rt', + supabase_refresh_token: null, scopes: ['mcp:tools'], expires_at: futureDate, + users: { email: 'test@example.com' }, }); const result = await provider.exchangeRefreshToken({ @@ -612,12 +658,12 @@ describe('PcpAuthProvider', () => { user_id: 'user-123', client_id: 'test-client', refresh_token: 'pcp-rt-abc', - supabase_refresh_token: 'supabase-rt', + supabase_refresh_token: null, scopes: ['mcp:tools'], expires_at: pastDate, + users: { email: 'test@example.com' }, }); - // Mock: delete chain mockDelete.mockReturnValue({ eq: vi.fn(() => ({ error: null })), }); @@ -632,39 +678,6 @@ describe('PcpAuthProvider', () => { error_description: 'Refresh token expired', }); }); - - it('should return error when Supabase refresh fails', async () => { - const futureDate = new Date(Date.now() + 86400000).toISOString(); - currentMcpTokensChain = mockChain({ - id: 'token-1', - user_id: 'user-123', - client_id: 'test-client', - refresh_token: 'pcp-rt-abc', - supabase_refresh_token: 'supabase-rt-revoked', - scopes: ['mcp:tools'], - expires_at: futureDate, - }); - - mockRefreshSession.mockResolvedValue({ - data: { session: null }, - error: { message: 'Token has been revoked' }, - }); - - // Mock: update to expire the token - mockUpdate.mockReturnValue({ - eq: vi.fn(() => ({ error: null })), - }); - - const result = await provider.exchangeRefreshToken({ - refreshToken: 'pcp-rt-abc', - clientId: 'test-client', - }); - - expect(result).toEqual({ - error: 'invalid_grant', - error_description: 'Unable to refresh session. Please re-authenticate.', - }); - }); }); // ========================================================================= @@ -672,48 +685,68 @@ describe('PcpAuthProvider', () => { // ========================================================================= describe('verifyAccessToken', () => { - it('should return user info for valid token', async () => { - mockGetUser.mockResolvedValue({ - data: { user: { email: 'test@example.com' } }, - error: null, - }); - currentUserChain = mockChain({ id: 'user-123', email: 'test@example.com' }); - - const result = await provider.verifyAccessToken('Bearer valid-jwt'); - + it('should return user info for valid self-signed JWT', () => { + const token = jwt.sign( + { type: 'mcp_access', sub: 'user-123', email: 'test@example.com', scope: 'mcp:tools' }, + TEST_JWT_SECRET, + { expiresIn: '30d' } + ); + + const result = provider.verifyAccessToken(`Bearer ${token}`); expect(result).toEqual({ userId: 'user-123', email: 'test@example.com' }); }); - it('should return null for missing auth header', async () => { - const result = await provider.verifyAccessToken(undefined); + it('should return null for missing auth header', () => { + const result = provider.verifyAccessToken(undefined); expect(result).toBeNull(); }); - it('should return null for non-Bearer auth header', async () => { - const result = await provider.verifyAccessToken('Basic abc123'); + it('should return null for non-Bearer auth header', () => { + const result = provider.verifyAccessToken('Basic abc123'); expect(result).toBeNull(); }); - it('should return null for invalid Supabase token', async () => { - mockGetUser.mockResolvedValue({ - data: { user: null }, - error: { message: 'Invalid token' }, - }); + it('should return null for expired JWT', () => { + const token = jwt.sign( + { type: 'mcp_access', sub: 'user-123', email: 'test@example.com', scope: 'mcp:tools' }, + TEST_JWT_SECRET, + { expiresIn: 0 } + ); - const result = await provider.verifyAccessToken('Bearer expired-jwt'); + const result = provider.verifyAccessToken(`Bearer ${token}`); expect(result).toBeNull(); }); - it('should return null when PCP user not found', async () => { - mockGetUser.mockResolvedValue({ - data: { user: { email: 'unknown@example.com' } }, - error: null, + it('should return null for wrong JWT type', () => { + const token = jwt.sign({ type: 'pending_auth', clientId: 'test' }, TEST_JWT_SECRET, { + expiresIn: '1h', }); - currentUserChain = mockChain(null); - const result = await provider.verifyAccessToken('Bearer valid-jwt'); + const result = provider.verifyAccessToken(`Bearer ${token}`); expect(result).toBeNull(); }); + + it('should return null for JWT signed with wrong secret', () => { + const token = jwt.sign( + { type: 'mcp_access', sub: 'user-123', email: 'test@example.com', scope: 'mcp:tools' }, + 'wrong-secret-that-is-at-least-32-characters-long', + { expiresIn: '30d' } + ); + + const result = provider.verifyAccessToken(`Bearer ${token}`); + expect(result).toBeNull(); + }); + + it('should not make any Supabase calls', () => { + const token = jwt.sign( + { type: 'mcp_access', sub: 'user-123', email: 'test@example.com', scope: 'mcp:tools' }, + TEST_JWT_SECRET, + { expiresIn: '30d' } + ); + + provider.verifyAccessToken(`Bearer ${token}`); + expect(mockGetUser).not.toHaveBeenCalled(); + }); }); // ========================================================================= @@ -721,7 +754,7 @@ describe('PcpAuthProvider', () => { // ========================================================================= describe('full OAuth flow', () => { - it('should complete authorize → callback → code exchange → refresh', async () => { + it('should complete authorize → callback → code exchange → refresh → verify', async () => { // Step 1: Create pending auth const pendingId = setupPendingAuth(provider); @@ -730,7 +763,6 @@ describe('PcpAuthProvider', () => { const callbackResult = await provider.handleAuthCallback({ pendingId, accessToken: 'supabase-jwt-original', - refreshToken: 'supabase-rt-original', }); expect('code' in callbackResult).toBe(true); if (!('code' in callbackResult)) return; @@ -746,7 +778,11 @@ describe('PcpAuthProvider', () => { }); expect('access_token' in tokens).toBe(true); if (!('access_token' in tokens)) return; - expect(tokens.access_token).toBe('supabase-jwt-original'); + + // Access token is a self-signed JWT + const decoded = jwt.verify(tokens.access_token, TEST_JWT_SECRET) as Record; + expect(decoded.type).toBe('mcp_access'); + expect(decoded.sub).toBe('user-123'); expect(tokens.refresh_token).toMatch(/^pcp-rt-/); // Step 4: Refresh the token @@ -756,19 +792,10 @@ describe('PcpAuthProvider', () => { user_id: 'user-123', client_id: 'test-client', refresh_token: tokens.refresh_token, - supabase_refresh_token: 'supabase-rt-original', + supabase_refresh_token: null, scopes: ['mcp:tools'], expires_at: futureDate, - }); - - mockRefreshSession.mockResolvedValue({ - data: { - session: { - access_token: 'supabase-jwt-refreshed', - refresh_token: 'supabase-rt-rotated', - }, - }, - error: null, + users: { email: 'test@example.com' }, }); mockUpdate.mockReturnValue({ @@ -781,15 +808,23 @@ describe('PcpAuthProvider', () => { }); expect('access_token' in refreshResult).toBe(true); - if ('access_token' in refreshResult) { - expect(refreshResult.access_token).toBe('supabase-jwt-refreshed'); - expect(refreshResult.refresh_token).toBe(tokens.refresh_token); // Same opaque token - } - - // Verify Supabase refresh was called with original token - expect(mockRefreshSession).toHaveBeenCalledWith({ - refresh_token: 'supabase-rt-original', - }); + if (!('access_token' in refreshResult)) return; + + // Refreshed token is also a self-signed JWT + const refreshedDecoded = jwt.verify(refreshResult.access_token, TEST_JWT_SECRET) as Record< + string, + unknown + >; + expect(refreshedDecoded.type).toBe('mcp_access'); + expect(refreshedDecoded.sub).toBe('user-123'); + expect(refreshResult.refresh_token).toBe(tokens.refresh_token); + + // Step 5: Verify the access token + const verified = provider.verifyAccessToken(`Bearer ${refreshResult.access_token}`); + expect(verified).toEqual({ userId: 'user-123', email: 'test@example.com' }); + + // No Supabase auth calls after initial callback + expect(mockGetUser).toHaveBeenCalledTimes(1); // Only during handleAuthCallback }); }); }); diff --git a/packages/api/src/mcp/auth/pcp-auth-provider.ts b/packages/api/src/mcp/auth/pcp-auth-provider.ts index a7dae2fe..489e5976 100644 --- a/packages/api/src/mcp/auth/pcp-auth-provider.ts +++ b/packages/api/src/mcp/auth/pcp-auth-provider.ts @@ -1,15 +1,16 @@ /** * PCP OAuth Provider for MCP Authentication * - * Handles the OAuth 2.0 authorization code flow with refresh token support. - * Uses Supabase as the identity provider, issuing our own opaque refresh tokens - * backed by stored Supabase refresh tokens for server-side session renewal. + * Issues self-signed JWTs as MCP access tokens (30-day expiry). + * Supabase is used only for initial identity verification during login. + * After that, all token operations are local (sign/verify with JWT_SECRET). * - * Token chain: MCP client refresh_token -> this provider -> supabase.auth.refreshSession() -> fresh JWT + * Token chain: MCP client refresh_token (opaque, DB-backed) -> jwt.sign() -> self-issued JWT */ import { createClient, type SupabaseClient } from '@supabase/supabase-js'; import crypto from 'crypto'; +import jwt from 'jsonwebtoken'; import { env } from '../../config/env'; import { logger } from '../../utils/logger'; import type { Database } from '../../data/supabase/types'; @@ -26,12 +27,27 @@ export interface PendingAuth { expiresAt: number; } +/** JWT payload for pending auth tokens (replaces in-memory Map) */ +interface PendingAuthPayload { + type: 'pending_auth'; + clientId: string; + codeChallenge: string; + redirectUri: string; + state: string; +} + +/** JWT payload for self-issued MCP access tokens */ +interface McpAccessTokenPayload { + type: 'mcp_access'; + sub: string; // PCP user ID + email: string; + scope: string; +} + export interface AuthCode { clientId: string; codeChallenge: string; redirectUri: string; - supabaseToken: string; - supabaseRefreshToken: string; userId: string; userEmail: string; expiresAt: number; @@ -60,31 +76,23 @@ export interface AuthCallbackResult { // Constants // ============================================================================ -// TODO: Consider extending Supabase JWT expiry to 30 days (2592000s) in dashboard -// and updating this constant to match. Current 1-hour expiry works via refresh -// tokens, but a longer JWT reduces refresh frequency for MCP clients. -const ACCESS_TOKEN_LIFETIME = 3600; // 1 hour (Supabase JWT default) +const ACCESS_TOKEN_LIFETIME_SECONDS = 30 * 24 * 60 * 60; // 30 days const REFRESH_TOKEN_LIFETIME_DAYS = 90; const REFRESH_TOKEN_LIFETIME_MS = REFRESH_TOKEN_LIFETIME_DAYS * 24 * 60 * 60 * 1000; const AUTH_CODE_LIFETIME_MS = 10 * 60 * 1000; // 10 minutes -const PENDING_AUTH_LIFETIME_MS = 10 * 60 * 1000; // 10 minutes +const PENDING_AUTH_LIFETIME_SECONDS = 600; // 10 minutes // ============================================================================ // Provider // ============================================================================ export class PcpAuthProvider { - private pendingAuths = new Map(); private authCodes = new Map(); private supabase: SupabaseClient; constructor() { this.supabase = createClient(env.SUPABASE_URL, env.SUPABASE_SECRET_KEY, { auth: { - // CRITICAL: persistSession must be false in server contexts. - // Without this, auth.refreshSession() stores a user session internally, - // causing subsequent PostgREST queries to use that user's JWT instead of - // the service role key — which subjects them to RLS and breaks lookups. autoRefreshToken: false, persistSession: false, }, @@ -101,15 +109,18 @@ export class PcpAuthProvider { redirectUri: string; state: string; }): string { - const pendingId = `pending-${Date.now()}-${crypto.randomBytes(8).toString('hex')}`; + const payload: PendingAuthPayload = { + type: 'pending_auth', + clientId: params.clientId, + codeChallenge: params.codeChallenge, + redirectUri: params.redirectUri, + state: params.state, + }; - this.pendingAuths.set(pendingId, { - ...params, - expiresAt: Date.now() + PENDING_AUTH_LIFETIME_MS, + return jwt.sign(payload, env.JWT_SECRET, { + expiresIn: PENDING_AUTH_LIFETIME_SECONDS, + jwtid: crypto.randomBytes(8).toString('hex'), }); - - this.cleanupExpired(this.pendingAuths); - return pendingId; } // -------------------------------------------------------------------------- @@ -119,21 +130,30 @@ export class PcpAuthProvider { async handleAuthCallback(params: { pendingId: string; accessToken: string; - refreshToken: string; + refreshToken?: string; }): Promise { - const pending = this.pendingAuths.get(params.pendingId); - if (!pending) { - return { error: 'invalid_request', error_description: 'Invalid or expired authorization request' }; - } - - if (Date.now() > pending.expiresAt) { - this.pendingAuths.delete(params.pendingId); - return { error: 'invalid_request', error_description: 'Authorization request expired' }; + // Verify the signed pending auth JWT + let pending: PendingAuthPayload; + try { + const decoded = jwt.verify(params.pendingId, env.JWT_SECRET); + if (typeof decoded === 'string' || (decoded as PendingAuthPayload).type !== 'pending_auth') { + return { error: 'invalid_request', error_description: 'Invalid authorization request' }; + } + pending = decoded as PendingAuthPayload; + } catch (err) { + const desc = + err instanceof jwt.TokenExpiredError + ? 'Authorization request expired' + : 'Invalid or expired authorization request'; + return { error: 'invalid_request', error_description: desc }; } try { // Verify Supabase token and resolve PCP user - const { data: { user }, error: authError } = await this.supabase.auth.getUser(params.accessToken); + const { + data: { user }, + error: authError, + } = await this.supabase.auth.getUser(params.accessToken); if (authError || !user) { logger.error('Supabase auth verification failed in callback', { error: authError }); return { error: 'access_denied', error_description: 'Authentication failed' }; @@ -158,7 +178,9 @@ export class PcpAuthProvider { if (createError) { // Check if user was created by another request (race condition or unique violation) if (createError.code === '23505') { - logger.info('User already exists (race condition), retrying lookup', { email: user.email }); + logger.info('User already exists (race condition), retrying lookup', { + email: user.email, + }); const { data: existingUser, error: retryError } = await this.supabase .from('users') .select('id, email') @@ -166,7 +188,10 @@ export class PcpAuthProvider { .single(); if (retryError || !existingUser) { - logger.error('Failed to fetch existing user after unique violation', { email: user.email, error: retryError }); + logger.error('Failed to fetch existing user after unique violation', { + email: user.email, + error: retryError, + }); return { error: 'server_error', error_description: 'User lookup failed' }; } @@ -190,14 +215,11 @@ export class PcpAuthProvider { clientId: pending.clientId, codeChallenge: pending.codeChallenge, redirectUri: pending.redirectUri, - supabaseToken: params.accessToken, - supabaseRefreshToken: params.refreshToken, userId: pcpUser.id, userEmail: pcpUser.email || '', expiresAt: Date.now() + AUTH_CODE_LIFETIME_MS, }); - this.pendingAuths.delete(params.pendingId); this.cleanupExpired(this.authCodes); logger.info('MCP auth callback complete', { userId: pcpUser.id, email: pcpUser.email }); @@ -234,6 +256,14 @@ export class PcpAuthProvider { // Fall back to the client_id stored in the auth code (from /authorize). // Some clients (e.g. Codex) don't send client_id in the token exchange body. + // If a different client_id is explicitly provided, reject it. + if (params.clientId && params.clientId !== codeData.clientId) { + logger.warn('client_id mismatch in code exchange', { + expected: codeData.clientId, + received: params.clientId, + }); + return { error: 'invalid_grant', error_description: 'Client ID mismatch' }; + } const clientId = params.clientId || codeData.clientId; // Verify PKCE @@ -257,16 +287,14 @@ export class PcpAuthProvider { const expiresAt = new Date(Date.now() + REFRESH_TOKEN_LIFETIME_MS); // Store in database - const { error: dbError } = await this.supabase - .from('mcp_tokens') - .insert({ - user_id: codeData.userId, - client_id: clientId, - refresh_token: refreshToken, - supabase_refresh_token: codeData.supabaseRefreshToken, - scopes: ['mcp:tools'], - expires_at: expiresAt.toISOString(), - }); + const { error: dbError } = await this.supabase.from('mcp_tokens').insert({ + user_id: codeData.userId, + client_id: clientId, + refresh_token: refreshToken, + supabase_refresh_token: null, + scopes: ['mcp:tools'], + expires_at: expiresAt.toISOString(), + }); if (dbError) { logger.error('Failed to store MCP token', { error: dbError }); @@ -276,6 +304,18 @@ export class PcpAuthProvider { // Consume the authorization code this.authCodes.delete(params.code); + // Sign our own JWT as the access token + const accessToken = jwt.sign( + { + type: 'mcp_access', + sub: codeData.userId, + email: codeData.userEmail, + scope: 'mcp:tools', + } satisfies McpAccessTokenPayload, + env.JWT_SECRET, + { expiresIn: ACCESS_TOKEN_LIFETIME_SECONDS } + ); + logger.info('MCP tokens issued', { userId: codeData.userId, email: codeData.userEmail, @@ -284,10 +324,10 @@ export class PcpAuthProvider { }); return { - access_token: codeData.supabaseToken, + access_token: accessToken, refresh_token: refreshToken, token_type: 'Bearer', - expires_in: ACCESS_TOKEN_LIFETIME, + expires_in: ACCESS_TOKEN_LIFETIME_SECONDS, scope: 'mcp:tools', }; } @@ -300,10 +340,10 @@ export class PcpAuthProvider { refreshToken: string; clientId: string; }): Promise { - // Look up token in database + // Look up token in database, joining users table for email const { data: tokenRecord, error: lookupError } = await this.supabase .from('mcp_tokens') - .select('*') + .select('*, users(email)') .eq('refresh_token', params.refreshToken) .single(); @@ -328,31 +368,25 @@ export class PcpAuthProvider { return { error: 'invalid_grant', error_description: 'Refresh token expired' }; } - // Use stored Supabase refresh token to get a fresh session - const { data: sessionData, error: refreshError } = await this.supabase.auth.refreshSession({ - refresh_token: tokenRecord.supabase_refresh_token, - }); - - if (refreshError || !sessionData.session) { - logger.error('Supabase token refresh failed', { - error: refreshError, - userId: tokenRecord.user_id, - }); - // Expire the token so it's cleaned up, but don't delete (user can re-auth) - await this.supabase - .from('mcp_tokens') - .update({ expires_at: new Date().toISOString() }) - .eq('id', tokenRecord.id); - return { error: 'invalid_grant', error_description: 'Unable to refresh session. Please re-authenticate.' }; - } - - // Update stored Supabase refresh token (Supabase rotates on use) + // Resolve user email from the join + const userEmail = (tokenRecord.users as unknown as { email: string | null })?.email || ''; + + // Sign a fresh self-issued JWT — no Supabase call needed + const accessToken = jwt.sign( + { + type: 'mcp_access', + sub: tokenRecord.user_id, + email: userEmail, + scope: 'mcp:tools', + } satisfies McpAccessTokenPayload, + env.JWT_SECRET, + { expiresIn: ACCESS_TOKEN_LIFETIME_SECONDS } + ); + + // Update last_used_at await this.supabase .from('mcp_tokens') - .update({ - supabase_refresh_token: sessionData.session.refresh_token, - last_used_at: new Date().toISOString(), - }) + .update({ last_used_at: new Date().toISOString() }) .eq('id', tokenRecord.id); logger.info('MCP token refreshed', { @@ -361,10 +395,10 @@ export class PcpAuthProvider { }); return { - access_token: sessionData.session.access_token, - refresh_token: params.refreshToken, // Keep same refresh token (don't rotate) + access_token: accessToken, + refresh_token: params.refreshToken, token_type: 'Bearer', - expires_in: ACCESS_TOKEN_LIFETIME, + expires_in: ACCESS_TOKEN_LIFETIME_SECONDS, scope: tokenRecord.scopes?.join(' ') || 'mcp:tools', }; } @@ -373,32 +407,18 @@ export class PcpAuthProvider { // Token verification (for /mcp endpoint auth) // -------------------------------------------------------------------------- - async verifyAccessToken(authHeader: string | undefined): Promise<{ userId: string; email: string } | null> { + verifyAccessToken(authHeader: string | undefined): { userId: string; email: string } | null { if (!authHeader?.startsWith('Bearer ')) return null; const token = authHeader.substring(7); try { - const { data: { user }, error } = await this.supabase.auth.getUser(token); - - if (error || !user) { - logger.debug('Supabase token validation failed', { error: error?.message }); + const decoded = jwt.verify(token, env.JWT_SECRET); + if (typeof decoded === 'string' || (decoded as McpAccessTokenPayload).type !== 'mcp_access') { return null; } - - const { data: pcpUser } = await this.supabase - .from('users') - .select('id, email') - .eq('email', user.email!) - .single(); - - if (!pcpUser) { - logger.warn('Supabase user not found in PCP', { email: user.email }); - return null; - } - - return { userId: pcpUser.id, email: pcpUser.email || '' }; - } catch (error) { - logger.error('Error verifying access token', { error }); + const payload = decoded as McpAccessTokenPayload; + return { userId: payload.sub, email: payload.email }; + } catch { return null; } } diff --git a/packages/api/src/mcp/server.ts b/packages/api/src/mcp/server.ts index 663514c8..1665042a 100644 --- a/packages/api/src/mcp/server.ts +++ b/packages/api/src/mcp/server.ts @@ -8,12 +8,27 @@ import { MCP_SERVER_NAME, MCP_SERVER_VERSION, MCP_SERVER_DESCRIPTION } from '../ import { env } from '../config/env'; import { logger } from '../utils/logger'; import type { DataComposer } from '../data/composer'; -import { registerAllTools, setMiniAppsRegistry, setTelegramListener, registerChannelListener } from './tools'; -import { loadMiniApps, registerMiniAppTools, getMiniAppsInfo, type LoadedMiniApp } from '../mini-apps'; +import { + registerAllTools, + setMiniAppsRegistry, + setTelegramListener, + registerChannelListener, +} from './tools'; +import { + loadMiniApps, + registerMiniAppTools, + getMiniAppsInfo, + type LoadedMiniApp, +} from '../mini-apps'; import adminRouter, { setWhatsAppListener } from '../routes/admin'; import agentTriggerRouter, { getAgentGateway } from '../routes/agent-trigger'; import { createChatRouter } from '../routes/chat'; -import { ChannelGateway, createChannelGateway, type ChannelGatewayConfig, type IncomingMessageHandler } from '../channels/gateway'; +import { + ChannelGateway, + createChannelGateway, + type ChannelGatewayConfig, + type IncomingMessageHandler, +} from '../channels/gateway'; import { runWithRequestContext } from '../utils/request-context'; import { PcpAuthProvider } from './auth/pcp-auth-provider'; @@ -35,7 +50,13 @@ export class MCPServer { private httpServer: Server | null = null; private miniApps: Map = new Map(); - private miniAppsInfo: Array<{ name: string; version: string; description: string; triggers: string[]; functions: string[] }> = []; + private miniAppsInfo: Array<{ + name: string; + version: string; + description: string; + triggers: string[]; + functions: string[]; + }> = []; private toolsVersion = 0; private channelGateway: ChannelGateway | null = null; private config: MCPServerConfig; @@ -50,7 +71,7 @@ export class MCPServer { this.miniApps = loadMiniApps(); setMiniAppsRegistry(this.miniApps); this.miniAppsInfo = getMiniAppsInfo(this.miniApps); - logger.info(`Mini-apps loaded: ${this.miniAppsInfo.map(a => a.name).join(', ') || 'none'}`); + logger.info(`Mini-apps loaded: ${this.miniAppsInfo.map((a) => a.name).join(', ') || 'none'}`); // Create primary server instance (used for stdio; HTTP creates per-session servers) this.server = this.createMcpServerInstance(); @@ -119,10 +140,12 @@ export class MCPServer { const app = express(); // Enable CORS for web portal, MCP clients, and agents - app.use(cors({ - origin: ['http://localhost:3001', 'http://localhost:3002', 'http://localhost:3003'], - credentials: true, - })); + app.use( + cors({ + origin: ['http://localhost:3001', 'http://localhost:3002', 'http://localhost:3003'], + credentials: true, + }) + ); // ============================================================================ // Streamable HTTP MCP endpoint (stateless) @@ -150,7 +173,8 @@ export class MCPServer { challengeParts.push('error="invalid_token"'); } - res.status(401) + res + .status(401) .set('WWW-Authenticate', challengeParts.join(', ')) .json({ jsonrpc: '2.0', @@ -167,9 +191,7 @@ export class MCPServer { // Use request-scoped context (AsyncLocalStorage) instead of global state // to prevent identity leaking across concurrent stateless requests. - const ctx = userData - ? { userId: userData.userId, email: userData.email } - : {}; + const ctx = userData ? { userId: userData.userId, email: userData.email } : {}; await runWithRequestContext(ctx, async () => { let transport: StreamableHTTPServerTransport | undefined; @@ -213,7 +235,10 @@ export class MCPServer { // ============================================================================ app.get('/health', async (_req, res) => { const startTime = Date.now(); - const checks: Record = {}; + const checks: Record< + string, + { status: 'ok' | 'error'; latencyMs?: number; error?: string; details?: unknown } + > = {}; try { const dbStart = Date.now(); @@ -222,24 +247,32 @@ export class MCPServer { ? { status: 'error', error: error.message } : { status: 'ok', latencyMs: Date.now() - dbStart }; } catch (err) { - checks.database = { status: 'error', error: err instanceof Error ? err.message : 'Unknown error' }; + checks.database = { + status: 'error', + error: err instanceof Error ? err.message : 'Unknown error', + }; } if (this.channelGateway) { const gatewayStatus = this.channelGateway.getStatus(); checks.telegram = { - status: gatewayStatus.telegram.enabled && gatewayStatus.telegram.connected ? 'ok' : 'error', - ...(gatewayStatus.telegram.enabled && !gatewayStatus.telegram.connected && { error: 'Not connected' }), + status: + gatewayStatus.telegram.enabled && gatewayStatus.telegram.connected ? 'ok' : 'error', + ...(gatewayStatus.telegram.enabled && + !gatewayStatus.telegram.connected && { error: 'Not connected' }), ...(!gatewayStatus.telegram.enabled && { error: 'Disabled' }), }; checks.whatsapp = { - status: gatewayStatus.whatsapp.enabled && gatewayStatus.whatsapp.connected ? 'ok' : 'error', - ...(gatewayStatus.whatsapp.enabled && !gatewayStatus.whatsapp.connected && { error: 'Not connected' }), + status: + gatewayStatus.whatsapp.enabled && gatewayStatus.whatsapp.connected ? 'ok' : 'error', + ...(gatewayStatus.whatsapp.enabled && + !gatewayStatus.whatsapp.connected && { error: 'Not connected' }), ...(!gatewayStatus.whatsapp.enabled && { error: 'Disabled' }), }; checks.discord = { status: gatewayStatus.discord.enabled && gatewayStatus.discord.connected ? 'ok' : 'error', - ...(gatewayStatus.discord.enabled && !gatewayStatus.discord.connected && { error: 'Not connected' }), + ...(gatewayStatus.discord.enabled && + !gatewayStatus.discord.connected && { error: 'Not connected' }), ...(!gatewayStatus.discord.enabled && { error: 'Disabled' }), }; } @@ -307,26 +340,25 @@ export class MCPServer { }); const webPortalUrl = process.env.WEB_PORTAL_URL || 'http://localhost:3002'; - const mcpCallback = `${baseUrl}/mcp/auth/callback`; const loginUrl = new URL(`${webPortalUrl}/login`); - loginUrl.searchParams.set('redirect', mcpCallback); loginUrl.searchParams.set('pending_id', pendingId); - logger.info('MCP /authorize redirecting to web portal', { pendingId, loginUrl: loginUrl.toString() }); + logger.info('MCP /authorize redirecting to web portal', { + pendingId, + loginUrl: loginUrl.toString(), + }); res.redirect(loginUrl.toString()); }); - // Auth callback — receives Supabase tokens from web portal, creates auth code + // Auth callback — receives Supabase access token from web portal, creates auth code app.get('/mcp/auth/callback', async (req, res) => { const pendingId = req.query.pending_id as string; const accessToken = req.query.access_token as string; - const refreshToken = req.query.refresh_token as string; logger.info('MCP /mcp/auth/callback called', { pendingId, hasAccessToken: !!accessToken, - hasRefreshToken: !!refreshToken, }); if (!accessToken) { @@ -334,21 +366,14 @@ export class MCPServer { return; } - if (!refreshToken) { - res.status(400).send('Missing refresh token. Please try logging in again.'); - return; - } - const result = await this.authProvider.handleAuthCallback({ pendingId, accessToken, - refreshToken, }); if ('error' in result) { - const statusCode = result.error === 'server_error' ? 500 - : result.error === 'access_denied' ? 403 - : 400; + const statusCode = + result.error === 'server_error' ? 500 : result.error === 'access_denied' ? 403 : 400; res.status(statusCode).send(result.error_description || result.error); return; } @@ -458,13 +483,15 @@ export class MCPServer { } // Kindle routes (registered below after import) - import('../routes/kindle.js').then(({ createKindleRouter }) => { - const kindleRouter = createKindleRouter(); - app.use('/api/kindle', kindleRouter); - logger.info('Kindle API routes registered at /api/kindle'); - }).catch((err) => { - logger.warn('Kindle routes not loaded:', err.message); - }); + import('../routes/kindle.js') + .then(({ createKindleRouter }) => { + const kindleRouter = createKindleRouter(); + app.use('/api/kindle', kindleRouter); + logger.info('Kindle API routes registered at /api/kindle'); + }) + .catch((err) => { + logger.warn('Kindle routes not loaded:', err.message); + }); app.post('/refresh-tools', async (_req, res) => { try { @@ -492,9 +519,12 @@ export class MCPServer { }); // Periodic cleanup of expired MCP refresh tokens (every 6 hours) - setInterval(() => { - this.authProvider.cleanupExpiredDatabaseTokens(); - }, 6 * 60 * 60 * 1000); + setInterval( + () => { + this.authProvider.cleanupExpiredDatabaseTokens(); + }, + 6 * 60 * 60 * 1000 + ); // Initialize channel gateway if message handler is configured if (this.config.messageHandler) { @@ -627,10 +657,12 @@ export class MCPServer { getToolsVersion(): number { return this.toolsVersion; } - } -export async function createMCPServer(dataComposer: DataComposer, config?: MCPServerConfig): Promise { +export async function createMCPServer( + dataComposer: DataComposer, + config?: MCPServerConfig +): Promise { return new MCPServer(dataComposer, config); } diff --git a/packages/web/.env.example b/packages/web/.env.example index b8f49bfa..008b9280 100644 --- a/packages/web/.env.example +++ b/packages/web/.env.example @@ -1,6 +1,6 @@ -# Supabase configuration -NEXT_PUBLIC_SUPABASE_URL=your_supabase_url -NEXT_PUBLIC_SUPABASE_ANON_KEY=your_supabase_anon_key +# Supabase configuration (server-only — not exposed to the browser) +SUPABASE_URL=your_supabase_url +SUPABASE_PUBLISHABLE_KEY=your_supabase_publishable_key # API server URL (MCP server with admin routes, default port 3001) API_URL=http://localhost:3001 diff --git a/packages/web/package.json b/packages/web/package.json index d916c6eb..1227eaac 100644 --- a/packages/web/package.json +++ b/packages/web/package.json @@ -8,7 +8,10 @@ "build": "next build", "start": "next start -p 3002", "lint": "next lint", - "type-check": "tsc --noEmit" + "type-check": "tsc --noEmit", + "test": "vitest run", + "test:watch": "vitest", + "test:integration": "vitest run --config vitest.integration.config.ts" }, "dependencies": { "@mantine/core": "^8.2.4", @@ -49,9 +52,13 @@ "@types/node": "^20.10.6", "@types/react": "^19.0.0", "@types/react-dom": "^19.0.0", + "@vitejs/plugin-react": "^5.1.4", "autoprefixer": "^10.4.20", + "dotenv": "^17.2.4", "postcss": "^8.4.49", "tailwindcss": "^3.4.17", - "typescript": "^5.3.3" + "typescript": "^5.3.3", + "vite": "^7.3.1", + "vitest": "^4.0.18" } } diff --git a/packages/web/src/app/(auth)/login/login-form.tsx b/packages/web/src/app/(auth)/login/login-form.tsx index 6c478f93..aa6b799b 100644 --- a/packages/web/src/app/(auth)/login/login-form.tsx +++ b/packages/web/src/app/(auth)/login/login-form.tsx @@ -5,26 +5,22 @@ import { useSearchParams, useRouter } from 'next/navigation'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; -import { createClient } from '@/lib/supabase/client'; +import { signInWithPassword, signInWithOtp } from '@/lib/auth/actions'; type AuthMode = 'magic-link' | 'password'; // Map common error messages to user-friendly text function getErrorMessage(error: string): string { const errorMap: Record = { - 'auth': 'Authentication failed. Please try again.', + auth: 'Authentication failed. Please try again.', 'code challenge does not match previously saved code verifier': 'Your magic link expired or was opened in a different browser. Please request a new one using the same browser.', 'Email link is invalid or has expired': 'This magic link has expired. Please request a new one.', - 'No authentication code provided': - 'Invalid login link. Please request a new magic link.', - 'Invalid login credentials': - 'Invalid email or password. Please try again.', - 'Email not confirmed': - 'Please confirm your email address before signing in.', - 'rate limit': - 'Too many requests. Please try signing in with password instead.', + 'No authentication code provided': 'Invalid login link. Please request a new magic link.', + 'Invalid login credentials': 'Invalid email or password. Please try again.', + 'Email not confirmed': 'Please confirm your email address before signing in.', + 'rate limit': 'Too many requests. Please try signing in with password instead.', }; // Check for partial matches @@ -48,27 +44,8 @@ export default function LoginForm() { const [mcpRedirecting, setMcpRedirecting] = useState(false); // MCP OAuth redirect params - const mcpRedirect = searchParams.get('redirect'); const mcpPendingId = searchParams.get('pending_id'); - const isMcpAuth = !!(mcpRedirect && mcpPendingId); - - // If already logged in and this is an MCP auth flow, redirect immediately - useEffect(() => { - if (!isMcpAuth) return; - - const checkExistingSession = async () => { - const supabase = createClient(); - const { data: { session } } = await supabase.auth.getSession(); - if (session?.access_token && session?.refresh_token) { - setMcpRedirecting(true); - redirectToMcp(); - } - // If session exists but refresh_token is missing, let user re-auth - // via the login form to get a fresh session with both tokens. - }; - - checkExistingSession(); - }, [isMcpAuth]); // eslint-disable-line react-hooks/exhaustive-deps + const isMcpAuth = !!mcpPendingId; // Check for error in URL params on mount useEffect(() => { @@ -81,55 +58,30 @@ export default function LoginForm() { setAuthMode('password'); } // Clear the error from URL without reload (preserve MCP params) - const newUrl = isMcpAuth - ? `/login?redirect=${encodeURIComponent(mcpRedirect!)}&pending_id=${mcpPendingId}` - : '/login'; + const newUrl = isMcpAuth ? `/login?pending_id=${mcpPendingId}` : '/login'; window.history.replaceState({}, '', newUrl); } - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); // Run once on mount — searchParams causes infinite loop when URL is modified - // Redirect to MCP callback with access token - const redirectToMcp = async () => { - if (!isMcpAuth) return; - - const supabase = createClient(); - const { data: { session } } = await supabase.auth.getSession(); - - if (session?.access_token && session?.refresh_token) { - const callbackUrl = new URL(mcpRedirect!); - callbackUrl.searchParams.set('pending_id', mcpPendingId!); - callbackUrl.searchParams.set('access_token', session.access_token); - callbackUrl.searchParams.set('refresh_token', session.refresh_token); - window.location.href = callbackUrl.toString(); - } - }; - const handleMagicLink = async () => { - const supabase = createClient(); - - // For MCP auth, include the redirect info in the callback URL + // For MCP auth, include the pending_id in the callback URL const callbackUrl = isMcpAuth - ? `${window.location.origin}/auth/callback?mcp_redirect=${encodeURIComponent(mcpRedirect!)}&mcp_pending_id=${mcpPendingId}` + ? `${window.location.origin}/auth/callback?mcp_pending_id=${mcpPendingId}` : `${window.location.origin}/auth/callback`; - const { error } = await supabase.auth.signInWithOtp({ - email, - options: { - emailRedirectTo: callbackUrl, - }, - }); + const result = await signInWithOtp(email, callbackUrl); - if (error) { + if ('error' in result) { // If rate limited, suggest password mode - if (error.message.toLowerCase().includes('rate')) { + if (result.error.toLowerCase().includes('rate')) { setMessage({ type: 'error', - text: 'Rate limit reached. Please sign in with password instead.' + text: 'Rate limit reached. Please sign in with password instead.', }); setAuthMode('password'); } else { - setMessage({ type: 'error', text: error.message }); + setMessage({ type: 'error', text: result.error }); } } else { setMessage({ @@ -140,25 +92,18 @@ export default function LoginForm() { }; const handlePassword = async () => { - const supabase = createClient(); - const { error } = await supabase.auth.signInWithPassword({ - email, - password, - }); - - if (error) { - setMessage({ type: 'error', text: getErrorMessage(error.message) }); + const result = await signInWithPassword(email, password, mcpPendingId); + + if ('error' in result) { + setMessage({ type: 'error', text: getErrorMessage(result.error) }); + } else if ('mcpRedirectUrl' in result) { + // MCP flow: redirect to callback with tokens + setMcpRedirecting(true); + window.location.href = result.mcpRedirectUrl; } else { - // Successful login - if (isMcpAuth) { - // Show granting access view, then redirect to MCP callback - setMcpRedirecting(true); - await redirectToMcp(); - } else { - // Normal dashboard redirect - router.push('/'); - router.refresh(); - } + // Normal dashboard redirect + router.push('/'); + router.refresh(); } }; @@ -292,8 +237,8 @@ export default function LoginForm() { {isLoading ? 'Signing in...' : authMode === 'magic-link' - ? 'Send Magic Link' - : 'Sign In'} + ? 'Send Magic Link' + : 'Sign In'} diff --git a/packages/web/src/app/api/auth/me/route.test.ts b/packages/web/src/app/api/auth/me/route.test.ts new file mode 100644 index 00000000..143691cc --- /dev/null +++ b/packages/web/src/app/api/auth/me/route.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock Supabase server client +const mockGetUser = vi.fn(); + +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn().mockResolvedValue({ + auth: { + getUser: () => mockGetUser(), + }, + }), +})); + +import { GET } from './route'; + +describe('GET /api/auth/me', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns authenticated user when session exists', async () => { + mockGetUser.mockResolvedValue({ + data: { + user: { + id: 'user-uuid-123', + email: 'user@test.com', + }, + }, + }); + + const response = await GET(); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(body).toEqual({ + authenticated: true, + user: { id: 'user-uuid-123', email: 'user@test.com' }, + }); + }); + + it('returns 401 when no user session exists', async () => { + mockGetUser.mockResolvedValue({ + data: { user: null }, + }); + + const response = await GET(); + const body = await response.json(); + + expect(response.status).toBe(401); + expect(body).toEqual({ authenticated: false }); + }); + + it('does not leak extra user fields', async () => { + mockGetUser.mockResolvedValue({ + data: { + user: { + id: 'user-uuid-123', + email: 'user@test.com', + role: 'admin', + app_metadata: { provider: 'email' }, + user_metadata: { full_name: 'Test User' }, + }, + }, + }); + + const response = await GET(); + const body = await response.json(); + + expect(body.user).toEqual({ id: 'user-uuid-123', email: 'user@test.com' }); + expect(body.user).not.toHaveProperty('role'); + expect(body.user).not.toHaveProperty('app_metadata'); + expect(body.user).not.toHaveProperty('user_metadata'); + }); +}); diff --git a/packages/web/src/app/api/auth/me/route.ts b/packages/web/src/app/api/auth/me/route.ts new file mode 100644 index 00000000..e9d45c24 --- /dev/null +++ b/packages/web/src/app/api/auth/me/route.ts @@ -0,0 +1,18 @@ +import { NextResponse } from 'next/server'; +import { createClient } from '@/lib/supabase/server'; + +export async function GET() { + const supabase = await createClient(); + const { + data: { user }, + } = await supabase.auth.getUser(); + + if (!user) { + return NextResponse.json({ authenticated: false }, { status: 401 }); + } + + return NextResponse.json({ + authenticated: true, + user: { id: user.id, email: user.email }, + }); +} diff --git a/packages/web/src/app/auth/callback/route.ts b/packages/web/src/app/auth/callback/route.ts index d79c03e6..8fade27c 100644 --- a/packages/web/src/app/auth/callback/route.ts +++ b/packages/web/src/app/auth/callback/route.ts @@ -7,9 +7,8 @@ export async function GET(request: Request) { const next = searchParams.get('next') ?? '/'; // MCP OAuth redirect params (passed through from magic link) - const mcpRedirect = searchParams.get('mcp_redirect'); const mcpPendingId = searchParams.get('mcp_pending_id'); - const isMcpAuth = !!(mcpRedirect && mcpPendingId); + const isMcpAuth = !!mcpPendingId; // Check for error from Supabase (e.g., expired link) const error = searchParams.get('error'); @@ -17,9 +16,9 @@ export async function GET(request: Request) { if (error) { const errorParam = encodeURIComponent(errorDescription || error); - // Preserve MCP params in error redirect + // Preserve MCP pending_id in error redirect const loginUrl = isMcpAuth - ? `${origin}/login?error=${errorParam}&redirect=${encodeURIComponent(mcpRedirect!)}&pending_id=${mcpPendingId}` + ? `${origin}/login?error=${errorParam}&pending_id=${mcpPendingId}` : `${origin}/login?error=${errorParam}`; return NextResponse.redirect(loginUrl); } @@ -29,12 +28,13 @@ export async function GET(request: Request) { const { data, error: exchangeError } = await supabase.auth.exchangeCodeForSession(code); if (!exchangeError && data.session) { - // If this is MCP auth, redirect to MCP callback with access token + // If this is MCP auth, redirect to MCP callback with tokens if (isMcpAuth) { - const mcpCallbackUrl = new URL(mcpRedirect); + const apiUrl = + process.env.API_URL || `http://localhost:${process.env.PCP_PORT_BASE || 3001}`; + const mcpCallbackUrl = new URL(`${apiUrl}/mcp/auth/callback`); mcpCallbackUrl.searchParams.set('pending_id', mcpPendingId!); mcpCallbackUrl.searchParams.set('access_token', data.session.access_token); - mcpCallbackUrl.searchParams.set('refresh_token', data.session.refresh_token); return NextResponse.redirect(mcpCallbackUrl.toString()); } @@ -45,11 +45,13 @@ export async function GET(request: Request) { // Pass specific error message const errorMessage = encodeURIComponent(exchangeError?.message || 'Failed to exchange code'); const loginUrl = isMcpAuth - ? `${origin}/login?error=${errorMessage}&redirect=${encodeURIComponent(mcpRedirect!)}&pending_id=${mcpPendingId}` + ? `${origin}/login?error=${errorMessage}&pending_id=${mcpPendingId}` : `${origin}/login?error=${errorMessage}`; return NextResponse.redirect(loginUrl); } // No code provided - return NextResponse.redirect(`${origin}/login?error=${encodeURIComponent('No authentication code provided')}`); + return NextResponse.redirect( + `${origin}/login?error=${encodeURIComponent('No authentication code provided')}` + ); } diff --git a/packages/web/src/app/kindle/[token]/page.tsx b/packages/web/src/app/kindle/[token]/page.tsx index 09596303..fb39852c 100644 --- a/packages/web/src/app/kindle/[token]/page.tsx +++ b/packages/web/src/app/kindle/[token]/page.tsx @@ -6,7 +6,6 @@ import { Button } from '@/components/ui/button'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Badge } from '@/components/ui/badge'; import { Sparkles } from 'lucide-react'; -import { createClient } from '@/lib/supabase/client'; import { apiPost } from '@/lib/api'; interface TokenInfo { @@ -52,12 +51,15 @@ export default function KindleLandingPage() { fetchToken(); }, [token]); - // Check auth status + // Check auth status via server endpoint useEffect(() => { async function checkAuth() { - const supabase = createClient(); - const { data: { user } } = await supabase.auth.getUser(); - setIsAuthenticated(!!user); + try { + const res = await fetch('/api/auth/me'); + setIsAuthenticated(res.ok); + } catch { + setIsAuthenticated(false); + } } checkAuth(); }, []); @@ -71,7 +73,9 @@ export default function KindleLandingPage() { setRedeeming(true); try { - const result = await apiPost<{ kindleId: string; agentId: string }>('/api/kindle/redeem', { token }); + const result = await apiPost<{ kindleId: string; agentId: string }>('/api/kindle/redeem', { + token, + }); // Redirect to onboarding chat router.push(`/kindle/onboarding?kindleId=${result.kindleId}&agentId=${result.agentId}`); } catch (err) { @@ -123,8 +127,8 @@ export default function KindleLandingPage() { {parentName && (

- {parentName} wants to kindle a new SB for you — one that shares - their core values but will grow to be uniquely yours. + {parentName} wants to kindle a new SB for you — one that shares their core values but + will grow to be uniquely yours.

)} @@ -145,24 +149,18 @@ export default function KindleLandingPage() {

- What happens next: You'll have a conversation with - your nascent SB. They'll ask a few questions about what matters to - you, explore your values together, and then choose a name. After - that, your SB is yours. + What happens next: You'll have a conversation with your nascent + SB. They'll ask a few questions about what matters to you, explore your values + together, and then choose a name. After that, your SB is yours.

- {!isAuthenticated && ( diff --git a/packages/web/src/components/layout/sidebar.tsx b/packages/web/src/components/layout/sidebar.tsx index b4f4d3e7..84da97ac 100644 --- a/packages/web/src/components/layout/sidebar.tsx +++ b/packages/web/src/components/layout/sidebar.tsx @@ -16,8 +16,7 @@ import { MessageSquare, } from 'lucide-react'; import { cn } from '@/lib/utils'; -import { createClient } from '@/lib/supabase/client'; -import { useRouter } from 'next/navigation'; +import { signOut } from '@/lib/auth/actions'; const navigation = [ { name: 'Dashboard', href: '/', icon: Home }, @@ -34,13 +33,6 @@ const navigation = [ export function Sidebar() { const pathname = usePathname(); - const router = useRouter(); - - const handleSignOut = async () => { - const supabase = createClient(); - await supabase.auth.signOut(); - router.push('/login'); - }; return (
@@ -71,7 +63,7 @@ export function Sidebar() {