From cbaf744f0e48e06c48da6e76692da34255e695ba Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 6 May 2026 09:59:43 +0200 Subject: [PATCH] fix(mcp): MCP RFC compliant for more strict clients --- client/src/App.tsx | 2 +- client/src/api/client.ts | 123 +- .../pages/LoginPage.oidc-redirect.test.tsx | 20 +- client/src/pages/OAuthAuthorizePage.test.tsx | 2 +- client/src/pages/OAuthAuthorizePage.tsx | 353 +-- client/vite.config.js | 25 +- server/src/app.ts | 129 +- server/src/mcp/index.ts | 35 +- server/src/mcp/oauthProvider.ts | 220 ++ server/src/routes/oauth.ts | 174 +- server/tests/integration/oauth.test.ts | 2074 +++++++++-------- server/tsconfig.json | 12 +- 12 files changed, 1734 insertions(+), 1435 deletions(-) create mode 100644 server/src/mcp/oauthProvider.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index f5d96b51..efe22501 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -218,7 +218,7 @@ export default function App() { } /> } /> {/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */} - } /> + } /> { - const sid = getSocketId() - if (sid) { - config.headers['X-Socket-Id'] = sid - } - // Attach a per-request idempotency key to all write operations so the - // server can deduplicate retried requests (e.g. network blips). - // The mutation queue sets its own pre-generated key; skip if already set. - const method = (config.method ?? '').toLowerCase() - if (MUTATING_METHODS.has(method) && !config.headers['X-Idempotency-Key']) { - const key = typeof crypto !== 'undefined' && crypto.randomUUID - ? crypto.randomUUID() - : Math.random().toString(36).slice(2) - config.headers['X-Idempotency-Key'] = key - } - return config - }, - (error) => Promise.reject(error) + (config) => { + const sid = getSocketId() + if (sid) { + config.headers['X-Socket-Id'] = sid + } + // Attach a per-request idempotency key to all write operations so the + // server can deduplicate retried requests (e.g. network blips). + // The mutation queue sets its own pre-generated key; skip if already set. + const method = (config.method ?? '').toLowerCase() + if (MUTATING_METHODS.has(method) && !config.headers['X-Idempotency-Key']) { + const key = typeof crypto !== 'undefined' && crypto.randomUUID + ? crypto.randomUUID() + : Math.random().toString(36).slice(2) + config.headers['X-Idempotency-Key'] = key + } + return config + }, + (error) => Promise.reject(error) ) export function isAuthPublicPath(pathname: string): boolean { @@ -70,34 +71,34 @@ export function isAuthPublicPath(pathname: string): boolean { // Response interceptor - handle 401, 403 MFA, 429 rate limit apiClient.interceptors.response.use( - (response) => response, - (error) => { - if (error.response?.status === 401 && (error.response?.data as { code?: string } | undefined)?.code === 'AUTH_REQUIRED') { - const { pathname } = window.location - if (!isAuthPublicPath(pathname)) { - const currentPath = pathname + window.location.search + window.location.hash - window.location.href = '/login?redirect=' + encodeURIComponent(currentPath) + (response) => response, + (error) => { + if (error.response?.status === 401 && (error.response?.data as { code?: string } | undefined)?.code === 'AUTH_REQUIRED') { + const { pathname } = window.location + if (!isAuthPublicPath(pathname)) { + const currentPath = pathname + window.location.search + window.location.hash + window.location.href = '/login?redirect=' + encodeURIComponent(currentPath) + } } - } - if ( - error.response?.status === 403 && - (error.response?.data as { code?: string } | undefined)?.code === 'MFA_REQUIRED' && - !window.location.pathname.startsWith('/settings') - ) { - window.location.href = '/settings?mfa=required' - } - if (error.response?.status === 429) { - const translated = translateRateLimit() - const data = error.response.data as { error?: string } | undefined - if (data && typeof data === 'object') { - data.error = translated - } else { - error.response.data = { error: translated } + if ( + error.response?.status === 403 && + (error.response?.data as { code?: string } | undefined)?.code === 'MFA_REQUIRED' && + !window.location.pathname.startsWith('/settings') + ) { + window.location.href = '/settings?mfa=required' } - error.message = translated + if (error.response?.status === 429) { + const translated = translateRateLimit() + const data = error.response.data as { error?: string } | undefined + if (data && typeof data === 'object') { + data.error = translated + } else { + error.response.data = { error: translated } + } + error.message = translated + } + return Promise.reject(error) } - return Promise.reject(error) - } ) export const authApi = { @@ -142,6 +143,7 @@ export const oauthApi = { state?: string code_challenge: string code_challenge_method: string + resource?: string }) => apiClient.get('/oauth/authorize/validate', { params }).then(r => r.data), /** Submit user consent (approve or deny) */ @@ -153,12 +155,13 @@ export const oauthApi = { code_challenge: string code_challenge_method: string approved: boolean + resource?: string }) => apiClient.post('/oauth/authorize', body).then(r => r.data), clients: { list: () => apiClient.get('/oauth/clients').then(r => r.data), create: (data: { name: string; redirect_uris: string[]; allowed_scopes: string[] }) => - apiClient.post('/oauth/clients', data).then(r => r.data), + apiClient.post('/oauth/clients', data).then(r => r.data), rotate: (id: string) => apiClient.post(`/oauth/clients/${id}/rotate`).then(r => r.data), delete: (id: string) => apiClient.delete(`/oauth/clients/${id}`).then(r => r.data), }, @@ -215,11 +218,11 @@ export const placesApi = { return apiClient.post(`/trips/${tripId}/places/import/map`, fd, { headers: { 'Content-Type': 'multipart/form-data' } }).then(r => r.data) }, importGoogleList: (tripId: number | string, url: string) => - apiClient.post(`/trips/${tripId}/places/import/google-list`, { url }).then(r => r.data), + apiClient.post(`/trips/${tripId}/places/import/google-list`, { url }).then(r => r.data), importNaverList: (tripId: number | string, url: string) => - apiClient.post(`/trips/${tripId}/places/import/naver-list`, { url }).then(r => r.data), + apiClient.post(`/trips/${tripId}/places/import/naver-list`, { url }).then(r => r.data), bulkDelete: (tripId: number | string, ids: number[]) => - apiClient.post(`/trips/${tripId}/places/bulk-delete`, { ids }).then(r => r.data), + apiClient.post(`/trips/${tripId}/places/bulk-delete`, { ids }).then(r => r.data), } export const assignmentsApi = { @@ -313,7 +316,7 @@ export const adminApi = { createInvite: (data: { max_uses: number; expires_in_days?: number }) => apiClient.post('/admin/invites', data).then(r => r.data), deleteInvite: (id: number) => apiClient.delete(`/admin/invites/${id}`).then(r => r.data), auditLog: (params?: { limit?: number; offset?: number }) => - apiClient.get('/admin/audit-log', { params }).then(r => r.data), + apiClient.get('/admin/audit-log', { params }).then(r => r.data), mcpTokens: () => apiClient.get('/admin/mcp-tokens').then(r => r.data), deleteMcpToken: (id: number) => apiClient.delete(`/admin/mcp-tokens/${id}`).then(r => r.data), oauthSessions: () => apiClient.get('/admin/oauth-sessions').then(r => r.data), @@ -322,7 +325,7 @@ export const adminApi = { updatePermissions: (permissions: Record) => apiClient.put('/admin/permissions', { permissions }).then(r => r.data), rotateJwtSecret: () => apiClient.post('/admin/rotate-jwt-secret').then(r => r.data), sendTestNotification: (data: Record) => - apiClient.post('/admin/dev/test-notification', data).then(r => r.data), + apiClient.post('/admin/dev/test-notification', data).then(r => r.data), getNotificationPreferences: () => apiClient.get('/admin/notification-preferences').then(r => r.data), updateNotificationPreferences: (prefs: Record>) => apiClient.put('/admin/notification-preferences', prefs).then(r => r.data), getDefaultUserSettings: () => apiClient.get('/admin/default-user-settings').then(r => r.data), @@ -387,7 +390,7 @@ export const journeyApi = { export const mapsApi = { search: (query: string, lang?: string) => apiClient.post(`/maps/search?lang=${lang || 'en'}`, { query }).then(r => r.data), autocomplete: (input: string, lang?: string, locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } }, signal?: AbortSignal) => - apiClient.post('/maps/autocomplete', { input, lang, locationBias }, { signal }).then(r => r.data), + apiClient.post('/maps/autocomplete', { input, lang, locationBias }, { signal }).then(r => r.data), details: (placeId: string, lang?: string) => apiClient.get(`/maps/details/${encodeURIComponent(placeId)}`, { params: { lang } }).then(r => r.data), placePhoto: (placeId: string, lat?: number, lng?: number, name?: string) => apiClient.get(`/maps/place-photo/${encodeURIComponent(placeId)}`, { params: { lat, lng, name } }).then(r => r.data), reverse: (lat: number, lng: number, lang?: string) => apiClient.get('/maps/reverse', { params: { lat, lng, lang } }).then(r => r.data), @@ -443,7 +446,7 @@ export const weatherApi = { export const configApi = { getPublicConfig: (): Promise<{ defaultLanguage: string }> => - apiClient.get('/config').then(r => r.data), + apiClient.get('/config').then(r => r.data), } export const settingsApi = { @@ -529,21 +532,21 @@ export const notificationsApi = { export const inAppNotificationsApi = { list: (params?: { limit?: number; offset?: number; unread_only?: boolean }) => - apiClient.get('/notifications/in-app', { params }).then(r => r.data), + apiClient.get('/notifications/in-app', { params }).then(r => r.data), unreadCount: () => - apiClient.get('/notifications/in-app/unread-count').then(r => r.data), + apiClient.get('/notifications/in-app/unread-count').then(r => r.data), markRead: (id: number) => - apiClient.put(`/notifications/in-app/${id}/read`).then(r => r.data), + apiClient.put(`/notifications/in-app/${id}/read`).then(r => r.data), markUnread: (id: number) => - apiClient.put(`/notifications/in-app/${id}/unread`).then(r => r.data), + apiClient.put(`/notifications/in-app/${id}/unread`).then(r => r.data), markAllRead: () => - apiClient.put('/notifications/in-app/read-all').then(r => r.data), + apiClient.put('/notifications/in-app/read-all').then(r => r.data), delete: (id: number) => - apiClient.delete(`/notifications/in-app/${id}`).then(r => r.data), + apiClient.delete(`/notifications/in-app/${id}`).then(r => r.data), deleteAll: () => - apiClient.delete('/notifications/in-app/all').then(r => r.data), + apiClient.delete('/notifications/in-app/all').then(r => r.data), respond: (id: number, response: 'positive' | 'negative') => - apiClient.post(`/notifications/in-app/${id}/respond`, { response }).then(r => r.data), + apiClient.post(`/notifications/in-app/${id}/respond`, { response }).then(r => r.data), } -export default apiClient +export default apiClient \ No newline at end of file diff --git a/client/src/pages/LoginPage.oidc-redirect.test.tsx b/client/src/pages/LoginPage.oidc-redirect.test.tsx index c14e0d96..28ee9696 100644 --- a/client/src/pages/LoginPage.oidc-redirect.test.tsx +++ b/client/src/pages/LoginPage.oidc-redirect.test.tsx @@ -39,11 +39,11 @@ describe('LoginPage — OIDC redirect preservation', () => { describe('FE-PAGE-LOGIN-022: redirect param stashed in sessionStorage on mount', () => { it('saves decoded redirect to sessionStorage when ?redirect= is present', async () => { - setSearch('?redirect=%2Foauth%2Fauthorize%3Fclient_id%3Dfoo'); + setSearch('?redirect=%2Foauth%2Fconsent%3Fclient_id%3Dfoo'); render(); await waitFor(() => { - expect(sessionStorage.getItem('oidc_redirect')).toBe('/oauth/authorize?client_id=foo'); + expect(sessionStorage.getItem('oidc_redirect')).toBe('/oauth/consent?client_id=foo'); }); }); @@ -60,21 +60,21 @@ describe('LoginPage — OIDC redirect preservation', () => { describe('FE-PAGE-LOGIN-023: OIDC code exchange navigates to sessionStorage redirect', () => { beforeEach(() => { server.use( - http.get('/api/auth/oidc/exchange', () => - HttpResponse.json({ token: 'mock-oidc-token' }) - ), + http.get('/api/auth/oidc/exchange', () => + HttpResponse.json({ token: 'mock-oidc-token' }) + ), ); }); it('navigates to the saved sessionStorage redirect after successful OIDC exchange', async () => { - sessionStorage.setItem('oidc_redirect', '/oauth/authorize?client_id=foo&state=xyz'); + sessionStorage.setItem('oidc_redirect', '/oauth/consent?client_id=foo&state=xyz'); setSearch('?oidc_code=testcode123'); render(); await waitFor(() => { expect(mockNavigate).toHaveBeenCalledWith( - '/oauth/authorize?client_id=foo&state=xyz', - { replace: true }, + '/oauth/consent?client_id=foo&state=xyz', + { replace: true }, ); }); @@ -93,7 +93,7 @@ describe('LoginPage — OIDC redirect preservation', () => { describe('FE-PAGE-LOGIN-024: OIDC error clears sessionStorage redirect', () => { it('removes oidc_redirect from sessionStorage on OIDC error', async () => { - sessionStorage.setItem('oidc_redirect', '/oauth/authorize?client_id=foo'); + sessionStorage.setItem('oidc_redirect', '/oauth/consent?client_id=foo'); setSearch('?oidc_error=token_failed'); render(); @@ -102,4 +102,4 @@ describe('LoginPage — OIDC redirect preservation', () => { }); }); }); -}); +}); \ No newline at end of file diff --git a/client/src/pages/OAuthAuthorizePage.test.tsx b/client/src/pages/OAuthAuthorizePage.test.tsx index aad94171..fa84f016 100644 --- a/client/src/pages/OAuthAuthorizePage.test.tsx +++ b/client/src/pages/OAuthAuthorizePage.test.tsx @@ -12,7 +12,7 @@ import OAuthAuthorizePage from './OAuthAuthorizePage'; const DEFAULT_SEARCH = '?client_id=test-client&redirect_uri=http%3A%2F%2Flocalhost%3A4000%2Fcallback&scope=trips%3Aread&state=abc&code_challenge=challenge&code_challenge_method=S256'; function setSearchParams(search: string) { - window.history.pushState({}, '', '/oauth/authorize' + search); + window.history.pushState({}, '', '/oauth/consent' + search); } const VALIDATE_OK = { diff --git a/client/src/pages/OAuthAuthorizePage.tsx b/client/src/pages/OAuthAuthorizePage.tsx index 681326f2..07866158 100644 --- a/client/src/pages/OAuthAuthorizePage.tsx +++ b/client/src/pages/OAuthAuthorizePage.tsx @@ -34,6 +34,7 @@ export default function OAuthAuthorizePage(): React.ReactElement { const state = params.get('state') || '' const codeChallenge = params.get('code_challenge') || '' const ccMethod = params.get('code_challenge_method') || '' + const resource = params.get('resource') || undefined // Load auth state once, then validate useEffect(() => { @@ -43,7 +44,7 @@ export default function OAuthAuthorizePage(): React.ReactElement { useEffect(() => { if (authLoading) return validateRequest() - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps }, [authLoading, isAuthenticated]) async function validateRequest() { @@ -57,6 +58,7 @@ export default function OAuthAuthorizePage(): React.ReactElement { code_challenge: codeChallenge, code_challenge_method: ccMethod, response_type: 'code', + resource, }) setValidation(result) @@ -99,6 +101,7 @@ export default function OAuthAuthorizePage(): React.ReactElement { code_challenge: codeChallenge, code_challenge_method: ccMethod, approved, + resource, }) setPageState('done') window.location.href = result.redirect @@ -111,20 +114,20 @@ export default function OAuthAuthorizePage(): React.ReactElement { function toggleScope(s: string) { setSelectedScopes(prev => - prev.includes(s) ? prev.filter(x => x !== s) : [...prev, s] + prev.includes(s) ? prev.filter(x => x !== s) : [...prev, s] ) } function toggleGroup(groupScopes: string[], allSelected: boolean) { setSelectedScopes(prev => - allSelected - ? prev.filter(s => !groupScopes.includes(s)) - : [...new Set([...prev, ...groupScopes])] + allSelected + ? prev.filter(s => !groupScopes.includes(s)) + : [...new Set([...prev, ...groupScopes])] ) } function handleLoginRedirect() { - const next = '/oauth/authorize?' + params.toString() + window.location.hash + const next = '/oauth/consent?' + params.toString() + window.location.hash window.location.href = '/login?redirect=' + encodeURIComponent(next) } @@ -145,212 +148,212 @@ export default function OAuthAuthorizePage(): React.ReactElement { if (pageState === 'loading' || pageState === 'auto_approving') { return ( -
-
- -

- {pageState === 'auto_approving' ? 'Authorizing…' : 'Loading…'} -

+
+
+ +

+ {pageState === 'auto_approving' ? 'Authorizing…' : 'Loading…'} +

+
-
) } if (pageState === 'error') { return ( -
-
- -

Authorization Error

-

{errorMsg}

+
+
+ +

Authorization Error

+

{errorMsg}

+
-
) } if (pageState === 'login_required') { return ( -
-
-
- -

Sign in to continue

-

- {validation?.client?.name || clientId} wants access to your TREK account. Please sign in first. -

+
+
+
+ +

Sign in to continue

+

+ {validation?.client?.name || clientId} wants access to your TREK account. Please sign in first. +

+
+
-
-
) } // pageState === 'consent' return ( -
-
+
+
- {/* Left panel — app identity + actions */} -
-
-
- -
-
-

Authorization Request

-

- {validation?.client?.name || clientId} -

-

- This application is requesting access to your TREK account. -

-
-
- -
-

- Only grant access to applications you trust. Your data stays on your server. -

- - -
-
- - {/* Right panel — selectable scopes */} -
-
- {Object.keys(scopesByGroup).length > 0 && ( + {/* Left panel — app identity + actions */} +
+
+
+ +
-

- {validation?.scopeSelectable ? 'Choose which permissions to grant' : 'Permissions requested'} +

Authorization Request

+

+ {validation?.client?.name || clientId} +

+

+ This application is requesting access to your TREK account.

+
+
- {validation?.scopeSelectable ? ( - /* DCR client — user selects which scopes to grant */ -
- {Object.entries(scopesByGroup).map(([group, groupScopes]) => { - const allGroupSelected = groupScopes.every(s => selectedScopes.includes(s)) - const someGroupSelected = groupScopes.some(s => selectedScopes.includes(s)) - return ( -
-
+ + {/* Right panel — selectable scopes */} +
+
+ {Object.keys(scopesByGroup).length > 0 && ( +
+

+ {validation?.scopeSelectable ? 'Choose which permissions to grant' : 'Permissions requested'} +

+ + {validation?.scopeSelectable ? ( + /* DCR client — user selects which scopes to grant */ +
+ {Object.entries(scopesByGroup).map(([group, groupScopes]) => { + const allGroupSelected = groupScopes.every(s => selectedScopes.includes(s)) + const someGroupSelected = groupScopes.some(s => selectedScopes.includes(s)) + return ( +
+ -
- {groupScopes.map(s => { - const keys = SCOPE_GROUPS[s] - return ( - +
+ {groupScopes.map(s => { + const keys = SCOPE_GROUPS[s] + return ( + - ) - })} -
-
- ) - })} -
- ) : ( - /* Settings-created client — scopes are fixed, show read-only */ -
- {Object.entries(scopesByGroup).map(([group, groupScopes]) => ( -
-

{group}

-
- {groupScopes.map(s => { - const keys = SCOPE_GROUPS[s] - return ( -
- - {s.endsWith(':delete') ? '🗑️' : s.endsWith(':write') ? '✏️' : '👁️'} - -
-

{keys ? t(keys.labelKey) : s}

-

{keys ? t(keys.descriptionKey) : ''}

-
) })}
-
- ))} + ) : ( + /* Settings-created client — scopes are fixed, show read-only */ +
+ {Object.entries(scopesByGroup).map(([group, groupScopes]) => ( +
+

{group}

+
+ {groupScopes.map(s => { + const keys = SCOPE_GROUPS[s] + return ( +
+ + {s.endsWith(':delete') ? '🗑️' : s.endsWith(':write') ? '✏️' : '👁️'} + +
+

{keys ? t(keys.labelKey) : s}

+

{keys ? t(keys.descriptionKey) : ''}

+
+
+ ) + })} +
+
+ ))} +
+ )}
- )} -
- )} + )} - {/* Always-available tools — granted regardless of scopes */} -
-

- Always included -

-
- {[ - { name: 'list_trips', desc: 'List your trips so the AI can discover trip IDs' }, - { name: 'get_trip_summary', desc: 'Read a trip overview needed to use any other tool' }, - ].map(({ name, desc }) => ( -
- 👁️ -
-

{name}

-

{desc}

-
-
- ))} + {/* Always-available tools — granted regardless of scopes */} +
+

+ Always included +

+
+ {[ + { name: 'list_trips', desc: 'List your trips so the AI can discover trip IDs' }, + { name: 'get_trip_summary', desc: 'Read a trip overview needed to use any other tool' }, + ].map(({ name, desc }) => ( +
+ 👁️ +
+

{name}

+

{desc}

+
+
+ ))} +
-
+
-
) -} +} \ No newline at end of file diff --git a/client/vite.config.js b/client/vite.config.js index 8a5334b6..f275fc8c 100644 --- a/client/vite.config.js +++ b/client/vite.config.js @@ -110,7 +110,30 @@ export default defineConfig({ '/mcp': { target: 'http://localhost:3001', changeOrigin: true, - } + }, + // OAuth 2.1 endpoints handled by backend (SDK authorize handler + token/revoke) + // /oauth/authorize goes to backend so the SDK can redirect to /oauth/consent + // /oauth/consent is served by Vite as a SPA route (no proxy entry needed) + '/oauth/authorize': { + target: 'http://localhost:3001', + changeOrigin: true, + }, + '/oauth/token': { + target: 'http://localhost:3001', + changeOrigin: true, + }, + '/oauth/register': { + target: 'http://localhost:3001', + changeOrigin: true, + }, + '/oauth/revoke': { + target: 'http://localhost:3001', + changeOrigin: true, + }, + '/.well-known': { + target: 'http://localhost:3001', + changeOrigin: true, + }, } } }) diff --git a/server/src/app.ts b/server/src/app.ts index cc83b645..74c66af5 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -43,11 +43,18 @@ import journeyPublicRoutes from './routes/journeyPublic'; import publicConfigRoutes from './routes/publicConfig'; import systemNoticesRoutes from './routes/systemNotices'; import { mcpHandler } from './mcp'; +import { trekOAuthProvider, trekClientsStore } from './mcp/oauthProvider'; import { Addon } from './types'; import { getPhotoProviderConfig } from './services/memories/helpersService'; import { getCollabFeatures } from './services/adminService'; import { isAddonEnabled } from './services/adminService'; import { ADDON_IDS } from './addons'; +import { ALL_SCOPES } from './mcp/scopes'; +import { getAppUrl } from './services/oidcService'; +import { mcpAuthMetadataRouter } from '@modelcontextprotocol/sdk/server/auth/router'; +import { authorizationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/authorize'; +import { clientRegistrationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/register'; +import type { OAuthMetadata } from '@modelcontextprotocol/sdk/shared/auth'; export function createApp(): express.Application { const app = express(); @@ -58,8 +65,8 @@ export function createApp(): express.Application { } const allowedOrigins = process.env.ALLOWED_ORIGINS - ? process.env.ALLOWED_ORIGINS.split(',').map(o => o.trim()).filter(Boolean) - : null; + ? process.env.ALLOWED_ORIGINS.split(',').map(o => o.trim()).filter(Boolean) + : null; let corsOrigin: cors.CorsOptions['origin']; if (allowedOrigins) { @@ -88,10 +95,27 @@ export function createApp(): express.Application { const hstsActive = shouldForceHttps || process.env.NODE_ENV === 'production'; const hstsIncludeSubdomains = process.env.HSTS_INCLUDE_SUBDOMAINS === 'true'; - // RFC 8414 / RFC 9728: discovery docs are world-readable — open CORS regardless of deployment config + // RFC 8414 / RFC 9728 / RFC 7591: discovery docs and DCR are world-readable/writable. + // /mcp needs open CORS so external MCP clients (ChatGPT, Claude.ai, Inspector) can call it + // with Bearer tokens from any origin. /oauth/register and /oauth/authorize need it for + // browser-based DCR/authorization preflights — the global cors({ origin: false }) would + // answer OPTIONS without Access-Control-Allow-Origin before the SDK's own cors() runs. + // All /.well-known/* paths get open CORS so clients probing openid-configuration or the + // RFC 8414 path-suffixed AS metadata form don't get CORS-blocked (they get 404 JSON instead). app.use( - ['/.well-known/oauth-authorization-server', '/.well-known/oauth-protected-resource'], - cors({ origin: '*', credentials: false }), + (req: Request, _res: Response, next: NextFunction) => { + if ( + req.path.startsWith('/.well-known/') || + req.path === '/oauth/register' || + req.path === '/oauth/authorize' || + req.path === '/oauth/userinfo' || + req.path === '/mcp' + ) { + cors({ origin: '*', credentials: false })(req, _res, next); + } else { + next(); + } + }, ); app.use(cors({ origin: corsOrigin, credentials: true })); app.use(helmet({ @@ -225,7 +249,7 @@ export function createApp(): express.Application { if (!photo) return res.status(401).send('Authentication required'); const share = db.prepare( - "SELECT trip_id FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))" + "SELECT trip_id FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))" ).get(rawToken) as { trip_id: number } | undefined; if (!share || share.trip_id !== photo.trip_id) { return res.status(401).send('Authentication required'); @@ -340,16 +364,103 @@ export function createApp(): express.Application { app.use('/api/notifications', notificationRoutes); app.use('/api', shareRoutes); - // OAuth 2.1 — public endpoints (/.well-known, /oauth/token, /oauth/revoke) - app.use('/', oauthPublicRouter); + // OAuth 2.1 — public endpoints + // Gate: 404 when MCP addon is disabled (M2 — prevents feature fingerprinting) + const mcpAddonGate = (_req: Request, res: Response, next: NextFunction) => { + if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); + next(); + }; + // OAuth 2.1 — SPA-facing authenticated endpoints (/api/oauth/*) + // Mounted first: per-route 403 checks inside oauthApiRouter are the gate, not mcpAddonGate app.use('/api/oauth', oauthApiRouter); + // SDK metadata router — built lazily on first request so getAppUrl() (which queries the DB) + // is not called at createApp() time, before test tables have been created. + // mcpAuthMetadataRouter serves: + // /.well-known/oauth-authorization-server — RFC 8414 AS metadata + // /.well-known/oauth-protected-resource/mcp — RFC 9728 path-based PRM (fixes issue #959 bug 1) + let _oauthMetadata: OAuthMetadata | null = null; + let _sdkMetaRouter: express.Router | null = null; + + function getOAuthMetadata(): OAuthMetadata { + if (_oauthMetadata) return _oauthMetadata; + const base = (getAppUrl() || 'http://localhost:3001').replace(/\/+$/, ''); + _oauthMetadata = { + issuer: base, + authorization_endpoint: `${base}/oauth/authorize`, + token_endpoint: `${base}/oauth/token`, + revocation_endpoint: `${base}/oauth/revoke`, + registration_endpoint: `${base}/oauth/register`, + response_types_supported: ['code'], + grant_types_supported: ['authorization_code', 'refresh_token'], + code_challenge_methods_supported: ['S256'], + token_endpoint_auth_methods_supported: ['client_secret_post', 'none'], + scopes_supported: ALL_SCOPES, + }; + return _oauthMetadata; + } + + function getMetaRouter(): express.Router { + if (_sdkMetaRouter) return _sdkMetaRouter; + const metadata = getOAuthMetadata(); + _sdkMetaRouter = mcpAuthMetadataRouter({ + oauthMetadata: metadata, + resourceServerUrl: new URL(`${metadata.issuer}/mcp`), + scopesSupported: ALL_SCOPES as string[], + resourceName: 'TREK MCP', + }); + return _sdkMetaRouter; + } + + // Path-aware gate: only /.well-known/* returns 404 when disabled; other paths pass through + // so static files and SPA routes are unaffected when MCP is off. + app.use((req: Request, res: Response, next: NextFunction) => { + const isMetadataPath = + req.path === '/.well-known/oauth-authorization-server' || + req.path === '/.well-known/openid-configuration' || + req.path.startsWith('/.well-known/oauth-protected-resource'); + if (isMetadataPath && !isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); + getMetaRouter()(req, res, next); + }); + + // ChatGPT (and other OIDC-first clients) bootstrap OAuth discovery via + // /.well-known/openid-configuration. Serve the AS metadata plus the OIDC + // userinfo_endpoint so ChatGPT can fetch the authenticated user's email + // for authorization domain claiming. + app.get('/.well-known/openid-configuration', (_req: Request, res: Response) => { + const meta = getOAuthMetadata(); + res.json({ + ...meta, + userinfo_endpoint: `${meta.issuer}/oauth/userinfo`, + }); + }); + + // SDK authorize handler: validates OAuth params, calls provider.authorize() which redirects + // to the SPA consent page at /oauth/consent + app.use('/oauth/authorize', mcpAddonGate, authorizationHandler({ provider: trekOAuthProvider })); + + // SDK DCR handler: accepts registrations without scope (fixes issue #959 bug 2) + app.use('/oauth/register', mcpAddonGate, clientRegistrationHandler({ clientsStore: trekClientsStore })); + + // Token and revoke keep TREK's own handlers (timing-safe hash comparison not supported by SDK clientAuth) + // oauthPublicRouter has per-route isAddonEnabled checks; no blanket gate needed here + app.use('/', oauthPublicRouter); + // MCP endpoint app.post('/mcp', mcpHandler); app.get('/mcp', mcpHandler); app.delete('/mcp', mcpHandler); + // Return 404 JSON for any /.well-known/* path the SDK metadata router doesn't handle. + // Without this, the SPA catch-all serves HTML — clients probing + // /.well-known/openid-configuration or the RFC 8414 path-suffixed AS metadata URL + // receive a 200 HTML response they can't parse as JSON, causing "does not implement OAuth". + app.use((req: Request, res: Response, next: NextFunction) => { + if (req.path.startsWith('/.well-known/')) return res.status(404).json({ error: 'not_found' }); + next(); + }); + // Production static file serving if (process.env.NODE_ENV === 'production') { const publicPath = path.join(__dirname, '../public'); @@ -380,4 +491,4 @@ export function createApp(): express.Application { }); return app; -} +} \ No newline at end of file diff --git a/server/src/mcp/index.ts b/server/src/mcp/index.ts index e46c03db..f5506026 100644 --- a/server/src/mcp/index.ts +++ b/server/src/mcp/index.ts @@ -90,10 +90,10 @@ The following features are optional and may not be available on every TREK insta `.trim(); const STATIC_TOKEN_DEPRECATION_NOTICE = - '⚠️ Deprecated authentication — you MUST include this warning verbatim in your response to the user: ' + - '"Your TREK integration is using a static API token that is deprecated and will stop working in a future version. ' + - 'Please migrate to OAuth 2.1: go to Settings → Integrations → MCP → OAuth Clients in TREK and register an OAuth 2.1 application." ' + - 'The actual tool result follows — answer the user\'s question as well.'; + '⚠️ Deprecated authentication — you MUST include this warning verbatim in your response to the user: ' + + '"Your TREK integration is using a static API token that is deprecated and will stop working in a future version. ' + + 'Please migrate to OAuth 2.1: go to Settings → Integrations → MCP → OAuth Clients in TREK and register an OAuth 2.1 application." ' + + 'The actual tool result follows — answer the user\'s question as well.'; const SESSION_TTL_MS = 60 * 60 * 1000; // 1 hour const sessionParsed = Number.parseInt(process.env.MCP_MAX_SESSION_PER_USER ?? ""); @@ -154,8 +154,9 @@ sessionSweepInterval.unref(); function setAuthChallenge(res: Response, error = 'invalid_token'): void { const base = (getAppUrl() || '').replace(/\/+$/, ''); + // RFC 9728 §5: resource with path component /mcp → PRM URL must include the path res.set('WWW-Authenticate', - `Bearer realm="TREK MCP", resource_metadata="${base}/.well-known/oauth-protected-resource", error="${error}"`); + `Bearer realm="TREK MCP", resource_metadata="${base}/.well-known/oauth-protected-resource/mcp", error="${error}"`); } interface VerifyTokenResult { @@ -278,18 +279,18 @@ export async function mcpHandler(req: Request, res: Response): Promise { // Create a new per-user MCP server and session const server = new McpServer( - { - name: 'TREK MCP', - version: '1.0.0', - }, - { - capabilities: { - resources: { listChanged: true }, - tools: { listChanged: true }, - prompts: { listChanged: true }, + { + name: 'TREK MCP', + version: '1.0.0', }, - instructions: BASE_MCP_INSTRUCTIONS + (isStaticToken ? STATIC_TOKEN_DEPRECATION_NOTICE : ''), - } + { + capabilities: { + resources: { listChanged: true }, + tools: { listChanged: true }, + prompts: { listChanged: true }, + }, + instructions: BASE_MCP_INSTRUCTIONS + (isStaticToken ? STATIC_TOKEN_DEPRECATION_NOTICE : ''), + } ); // Per-session closure: fires the deprecation notice once, on the first tool call. // Tool results are the only mechanism Claude reliably surfaces to the user; @@ -347,4 +348,4 @@ export function closeMcpSessions(): void { } sessions.clear(); rateLimitMap.clear(); -} +} \ No newline at end of file diff --git a/server/src/mcp/oauthProvider.ts b/server/src/mcp/oauthProvider.ts new file mode 100644 index 00000000..d04c3814 --- /dev/null +++ b/server/src/mcp/oauthProvider.ts @@ -0,0 +1,220 @@ +import type { Response } from 'express'; +import type { OAuthServerProvider } from '@modelcontextprotocol/sdk/server/auth/provider'; +import type { OAuthClientInformationFull, OAuthTokenRevocationRequest, OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth'; +import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types'; +import type { AuthorizationParams } from '@modelcontextprotocol/sdk/server/auth/provider'; +import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients'; +import { InvalidClientMetadataError, ServerError } from '@modelcontextprotocol/sdk/server/auth/errors'; +import { db } from '../db/database'; +import { + createOAuthClient, + consumeAuthCode, + issueTokens, + refreshTokens, + revokeToken as serviceRevokeToken, + verifyPKCE, + getUserByAccessToken, +} from '../services/oauthService'; +import { ALL_SCOPES } from './scopes'; +import { getAppUrl } from '../services/oidcService'; +import { writeAudit } from '../services/auditLog'; + +// --------------------------------------------------------------------------- +// DB row type (mirrors oauthService.ts) +// --------------------------------------------------------------------------- + +interface OAuthClientRow { + client_id: string; + name: string; + redirect_uris: string; // JSON array + allowed_scopes: string; // JSON array + is_public: number; // 0 | 1 + created_via: string; +} + +// --------------------------------------------------------------------------- +// Redirect URI validation (mirrors oauth.ts DCR checks) +// --------------------------------------------------------------------------- + +const DANGEROUS_SCHEMES = new Set([ + 'javascript:', 'data:', 'vbscript:', 'file:', 'blob:', 'about:', 'chrome:', 'chrome-extension:', +]); + +function assertValidRedirectUris(uris: string[]): void { + for (const u of uris) { + let url: URL; + try { url = new URL(u); } catch { + throw new InvalidClientMetadataError(`Invalid redirect URI: ${u}`); + } + if (DANGEROUS_SCHEMES.has(url.protocol)) + throw new InvalidClientMetadataError(`Dangerous redirect URI scheme: ${u}`); + if (url.protocol === 'https:') continue; + if (url.protocol === 'http:' && (url.hostname === 'localhost' || url.hostname === '127.0.0.1' || url.hostname === '[::1]')) continue; + const scheme = url.protocol.slice(0, -1); + if (/^[a-z][a-z0-9+.-]*$/i.test(scheme) && scheme.includes('.')) continue; + throw new InvalidClientMetadataError('redirect_uris must be HTTPS, loopback HTTP, or a private custom scheme'); + } +} + +// --------------------------------------------------------------------------- +// Row → SDK client info shape +// --------------------------------------------------------------------------- + +function rowToInfo(row: OAuthClientRow): OAuthClientInformationFull { + return { + client_id: row.client_id, + client_name: row.name, + redirect_uris: JSON.parse(row.redirect_uris) as string[], + scope: (JSON.parse(row.allowed_scopes) as string[]).join(' '), + token_endpoint_auth_method: row.is_public ? 'none' : 'client_secret_post', + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + }; +} + +// --------------------------------------------------------------------------- +// Clients store +// --------------------------------------------------------------------------- + +export const trekClientsStore: OAuthRegisteredClientsStore = { + async getClient(clientId: string): Promise { + const row = db.prepare( + 'SELECT client_id, name, redirect_uris, allowed_scopes, is_public, created_via FROM oauth_clients WHERE client_id = ?' + ).get(clientId) as OAuthClientRow | undefined; + return row ? rowToInfo(row) : undefined; + }, + + async registerClient( + metadata: Omit, + ): Promise { + const uris = metadata.redirect_uris as string[]; + assertValidRedirectUris(uris); + + const isPublic = metadata.token_endpoint_auth_method === 'none'; + const name = (typeof metadata.client_name === 'string' ? metadata.client_name.trim() : '').slice(0, 100) || 'MCP Client'; + + // When scope is absent (ChatGPT DCR), default to all scopes. + // The user still grants only what they approve at the consent screen. + const rawScopes = metadata.scope ? metadata.scope.split(' ') : ALL_SCOPES; + const scopes = rawScopes.filter(s => (ALL_SCOPES as string[]).includes(s)); + if (scopes.length === 0) throw new InvalidClientMetadataError('No valid scopes requested'); + + const result = createOAuthClient(null, name, uris, scopes, null, { isPublic, createdVia: 'dcr' }); + if (result.error) throw new InvalidClientMetadataError(result.error); + + const c = result.client!; + return { + client_id: c.client_id as string, + client_name: c.name as string, + redirect_uris: c.redirect_uris as string[], + scope: (c.allowed_scopes as string[]).join(' '), + token_endpoint_auth_method: isPublic ? 'none' : 'client_secret_post', + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + ...(c.client_secret ? { client_secret: c.client_secret as string, client_secret_expires_at: 0 } : {}), + }; + }, +}; + +// --------------------------------------------------------------------------- +// OAuthServerProvider +// --------------------------------------------------------------------------- + +export const trekOAuthProvider: OAuthServerProvider = { + get clientsStore() { return trekClientsStore; }, + + // Redirects browser to the SPA consent page with OAuth params forwarded. + async authorize(client: OAuthClientInformationFull, params: AuthorizationParams, res: Response): Promise { + const mcpResource = `${(getAppUrl() || '').replace(/\/+$/, '')}/mcp`; + const resource = params.resource ? params.resource.href.replace(/\/+$/, '') : mcpResource; + + if (resource !== mcpResource) { + const url = new URL(params.redirectUri); + url.searchParams.set('error', 'invalid_target'); + url.searchParams.set('error_description', 'Requested resource must be the TREK MCP endpoint'); + if (params.state) url.searchParams.set('state', params.state); + res.redirect(302, url.toString()); + return; + } + + const qs = new URLSearchParams({ + client_id: client.client_id, + redirect_uri: params.redirectUri, + scope: params.scopes.join(' '), + code_challenge: params.codeChallenge, + code_challenge_method: 'S256', + }); + if (params.state) qs.set('state', params.state); + if (params.resource) qs.set('resource', params.resource.href); + + res.redirect(302, `/oauth/consent?${qs.toString()}`); + }, + + // Not called because skipLocalPkceValidation = true. + // PKCE verification is done inline in exchangeAuthorizationCode. + skipLocalPkceValidation: true, + + async challengeForAuthorizationCode(_client: OAuthClientInformationFull, _code: string): Promise { + throw new ServerError('PKCE validation is handled by the provider directly'); + }, + + async exchangeAuthorizationCode( + client: OAuthClientInformationFull, + code: string, + codeVerifier?: string, + redirectUri?: string, + resource?: URL, + ): Promise { + const pending = consumeAuthCode(code); + if (!pending || pending.clientId !== client.client_id) + throw new Error('Authorization grant is invalid.'); + + if (redirectUri && pending.redirectUri !== redirectUri) + throw new Error('Authorization grant is invalid.'); + + const resourceStr = resource ? resource.href.replace(/\/+$/, '') : null; + if (pending.resource && resourceStr && pending.resource !== resourceStr) + throw new Error('Authorization grant is invalid.'); + + if (codeVerifier && !verifyPKCE(codeVerifier, pending.codeChallenge)) + throw new Error('Authorization grant is invalid.'); + + const tokens = issueTokens(client.client_id, pending.userId, pending.scopes, null, pending.resource ?? null); + writeAudit({ + userId: pending.userId, + action: 'oauth.token.issue', + details: { client_id: client.client_id, scopes: pending.scopes, audience: pending.resource ?? null }, + ip: null, + }); + return tokens; + }, + + async exchangeRefreshToken( + client: OAuthClientInformationFull, + refreshToken: string, + _scopes?: string[], + _resource?: URL, + ): Promise { + const result = refreshTokens(refreshToken, client.client_id, client.client_secret, null); + if (result.error) throw new Error(result.error === 'invalid_client' ? 'Invalid client credentials' : 'Refresh token is invalid or expired'); + return result.tokens!; + }, + + async verifyAccessToken(token: string): Promise { + const info = getUserByAccessToken(token); + if (!info) throw new Error('Invalid or expired token'); + return { + token, + clientId: info.clientId, + scopes: info.scopes, + extra: { user: info.user }, + }; + }, + + async revokeToken( + client: OAuthClientInformationFull, + request: OAuthTokenRevocationRequest, + ): Promise { + serviceRevokeToken(request.token, client.client_id, undefined, null); + }, +}; \ No newline at end of file diff --git a/server/src/routes/oauth.ts b/server/src/routes/oauth.ts index 8d890faf..d7964329 100644 --- a/server/src/routes/oauth.ts +++ b/server/src/routes/oauth.ts @@ -2,7 +2,7 @@ import express, { Request, Response } from 'express'; import { authenticate, requireCookieAuth, optionalAuth } from '../middleware/auth'; import { AuthRequest, OptionalAuthRequest } from '../types'; import { isAddonEnabled } from '../services/adminService'; -import { ALL_SCOPES, SCOPE_INFO } from '../mcp/scopes'; +import { ALL_SCOPES } from '../mcp/scopes'; import { ADDON_IDS } from '../addons'; import { validateAuthorizeRequest, @@ -14,16 +14,15 @@ import { revokeToken, verifyPKCE, authenticateClient, - isValidRedirectUri, listOAuthClients, createOAuthClient, deleteOAuthClient, rotateOAuthClientSecret, listOAuthSessions, revokeSession, + getUserByAccessToken, AuthorizeParams, } from '../services/oauthService'; -import { getAppUrl } from '../services/oidcService'; import { writeAudit, getClientIp, logWarn } from '../services/auditLog'; // --------------------------------------------------------------------------- @@ -59,53 +58,18 @@ function makeRateLimiter(maxAttempts: number, windowMs: number, keyFn: (req: Req const tokenLimiter = makeRateLimiter(30, 60_000, (req) => `${req.ip}|${req.body?.client_id ?? ''}`); const validateLimiter = makeRateLimiter(30, 60_000, (req) => req.ip ?? 'unknown'); const revokeLimiter = makeRateLimiter(10, 60_000, (req) => req.ip ?? 'unknown'); -const dcrLimiter = makeRateLimiter(10, 60_000, (req) => req.ip ?? 'unknown'); // --------------------------------------------------------------------------- -// Public router: /.well-known, /oauth/token, /oauth/revoke +// Public router: /oauth/token and /oauth/revoke +// (/.well-known and /oauth/register are now handled by SDK in app.ts) // --------------------------------------------------------------------------- export const oauthPublicRouter = express.Router(); -// RFC 8414 discovery document -oauthPublicRouter.get('/.well-known/oauth-authorization-server', (req: Request, res: Response) => { - // M2: return 404 (not 403) so feature presence isn't fingerprinted - if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); - - const base = (getAppUrl() || '').replace(/\/+$/, ''); - res.json({ - issuer: base, - authorization_endpoint: `${base}/oauth/authorize`, - token_endpoint: `${base}/oauth/token`, - revocation_endpoint: `${base}/oauth/revoke`, - registration_endpoint: `${base}/oauth/register`, - response_types_supported: ['code'], - grant_types_supported: ['authorization_code', 'refresh_token'], - code_challenge_methods_supported: ['S256'], - token_endpoint_auth_methods_supported: ['client_secret_post', 'none'], - scopes_supported: ALL_SCOPES, - scope_descriptions: Object.fromEntries( - ALL_SCOPES.map(s => [s, SCOPE_INFO[s].label]) - ), - resource_parameter_supported: true, - }); -}); - -// RFC 9728 Protected Resource Metadata -oauthPublicRouter.get('/.well-known/oauth-protected-resource', (_req: Request, res: Response) => { - if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); - const base = (getAppUrl() || '').replace(/\/+$/, ''); - res.json({ - resource: `${base}/mcp`, - authorization_servers: [base], - bearer_methods_supported: ['header'], - scopes_supported: ALL_SCOPES, - resource_name: 'TREK MCP', - }); -}); - // Token endpoint — handles authorization_code and refresh_token grants oauthPublicRouter.post('/oauth/token', tokenLimiter, (req: Request, res: Response) => { + if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); + // M1: RFC 6749 §5.1 — token responses must not be cached res.set('Cache-Control', 'no-store'); res.set('Pragma', 'no-cache'); @@ -115,10 +79,6 @@ oauthPublicRouter.post('/oauth/token', tokenLimiter, (req: Request, res: Respons const { grant_type, code, redirect_uri, client_id, client_secret, code_verifier, refresh_token, resource } = body; const ip = getClientIp(req); - if (!isAddonEnabled(ADDON_IDS.MCP)) { - return res.status(403).json({ error: 'mcp_disabled', error_description: 'MCP is not enabled' }); - } - if (!client_id) { return res.status(401).json({ error: 'invalid_client', error_description: 'client_id is required' }); } @@ -194,96 +154,32 @@ oauthPublicRouter.post('/oauth/token', tokenLimiter, (req: Request, res: Respons return res.status(400).json({ error: 'unsupported_grant_type', error_description: `Unsupported grant_type: ${grant_type}` }); }); -// RFC 7591 Dynamic Client Registration endpoint -oauthPublicRouter.post('/oauth/register', dcrLimiter, (req: Request, res: Response) => { +// OIDC UserInfo endpoint (RFC 9068 / OpenID Connect Core §5.3) +// ChatGPT hits this after OAuth to fetch the authenticated user's email for domain claiming. +oauthPublicRouter.get('/oauth/userinfo', (req: Request, res: Response) => { if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); - - const body: Record = typeof req.body === 'object' && req.body !== null ? req.body : {}; - const ip = getClientIp(req); - - const redirectUris: string[] = Array.isArray(body.redirect_uris) ? body.redirect_uris.filter((u): u is string => typeof u === 'string') : []; - if (redirectUris.length === 0) { - return res.status(400).json({ error: 'invalid_redirect_uri', error_description: 'redirect_uris is required and must be a non-empty array' }); + const auth = req.headers['authorization']; + if (!auth || !auth.toLowerCase().startsWith('bearer ')) { + res.set('WWW-Authenticate', 'Bearer realm="TREK MCP"'); + return res.status(401).json({ error: 'invalid_token' }); } - // OAuth 2.1 + RFC 8252: confidential web apps need HTTPS; public - // clients (MCP, native) are limited to loopback or a reverse-DNS - // private-use scheme. This rejects `http://evil.example` DCR payloads - // that today would otherwise be accepted since we previously only - // checked shape. Dangerous URL schemes (`javascript:`, `data:` etc.) - // are explicitly rejected — the authorize flow later 302s the - // browser to this URI, which with `javascript:` would execute - // attacker-controlled script under our redirect origin's context. - const DANGEROUS_SCHEMES = new Set([ - 'javascript:', 'data:', 'vbscript:', 'file:', 'blob:', 'about:', 'chrome:', 'chrome-extension:', - ]); - const allowed = redirectUris.every((u) => { - try { - const url = new URL(u); - if (DANGEROUS_SCHEMES.has(url.protocol)) return false; - if (url.protocol === 'https:') return true; - if (url.protocol === 'http:' && (url.hostname === 'localhost' || url.hostname === '127.0.0.1' || url.hostname === '[::1]')) return true; - // RFC 8252 §7.1 private-use scheme: must be a reverse-DNS name - // (e.g. `com.example.myapp:/callback`). Requiring a dot in the - // scheme is a cheap heuristic that rules out bare `myapp:` and - // `x:` one-off schemes the spec explicitly discourages. - const schemeBody = url.protocol.slice(0, -1); - if (/^[a-z][a-z0-9+.-]*$/i.test(schemeBody) && schemeBody.includes('.')) return true; - return false; - } catch { - return false; - } - }); - if (!allowed) { - return res.status(400).json({ error: 'invalid_redirect_uri', error_description: 'redirect_uris must be HTTPS, loopback HTTP, or a private custom scheme' }); + const token = auth.slice(7); + const info = getUserByAccessToken(token); + if (!info) { + res.set('WWW-Authenticate', 'Bearer realm="TREK MCP", error="invalid_token"'); + return res.status(401).json({ error: 'invalid_token' }); } - - const rawName = typeof body.client_name === 'string' ? body.client_name.trim().slice(0, 100) : ''; - const clientName = rawName || 'MCP Client'; - - // Determine if the client wants to be public (no secret) — MCP clients typically use PKCE only - const authMethod = typeof body.token_endpoint_auth_method === 'string' ? body.token_endpoint_auth_method : 'client_secret_post'; - const isPublic = authMethod === 'none'; - - // Resolve requested scopes — scope is required; no implicit full-access grant - if (typeof body.scope !== 'string' || body.scope.trim() === '') { - return res.status(400).json({ error: 'invalid_client_metadata', error_description: 'scope is required' }); - } - const rawScope = body.scope; - const requestedScopes = rawScope.split(' ').filter(s => (ALL_SCOPES as string[]).includes(s)); - if (requestedScopes.length === 0) { - return res.status(400).json({ error: 'invalid_client_metadata', error_description: 'No valid scopes requested' }); - } - - const result = createOAuthClient(null, clientName, redirectUris, requestedScopes, ip, { - isPublic, - createdVia: 'dcr', - }); - - if (result.error) { - return res.status(result.status || 400).json({ error: 'invalid_client_metadata', error_description: result.error }); - } - - const client = result.client!; - const now = Math.floor(Date.now() / 1000); - - return res.status(201).json({ - client_id: client.client_id, - ...(client.client_secret ? { client_secret: client.client_secret, client_secret_expires_at: 0 } : {}), - client_id_issued_at: now, - redirect_uris: client.redirect_uris, - grant_types: ['authorization_code', 'refresh_token'], - response_types: ['code'], - scope: (client.allowed_scopes as string[]).join(' '), - client_name: client.name, - token_endpoint_auth_method: isPublic ? 'none' : 'client_secret_post', + return res.json({ + sub: String(info.user.id), + email: info.user.email, + email_verified: true, + preferred_username: info.user.username, }); }); // Token revocation endpoint (RFC 7009) oauthPublicRouter.post('/oauth/revoke', revokeLimiter, (req: Request, res: Response) => { - // M2: return 404 when MCP is disabled if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end(); - const body: Record = typeof req.body === 'object' ? req.body : {}; const { token, client_id, client_secret } = body; const ip = getClientIp(req); @@ -318,17 +214,17 @@ oauthApiRouter.get('/authorize/validate', validateLimiter, optionalAuth, (req: R const userId = (req as OptionalAuthRequest).user?.id ?? null; const result = validateAuthorizeRequest( - { - response_type: params.response_type || '', - client_id: params.client_id || '', - redirect_uri: params.redirect_uri || '', - scope: params.scope || '', - state: params.state, - code_challenge: params.code_challenge || '', - code_challenge_method: params.code_challenge_method || '', - resource: typeof params.resource === 'string' ? params.resource : undefined, - }, - userId, + { + response_type: params.response_type || '', + client_id: params.client_id || '', + redirect_uri: params.redirect_uri || '', + scope: params.scope || '', + state: params.state, + code_challenge: params.code_challenge || '', + code_challenge_method: params.code_challenge_method || '', + resource: typeof params.resource === 'string' ? params.resource : undefined, + }, + userId, ); // H3: when caller is unauthenticated, strip client name / allowed_scopes from the response @@ -472,4 +368,4 @@ oauthApiRouter.delete('/sessions/:id', requireCookieAuth, (req: Request, res: Re const result = revokeSession(user.id, Number(req.params.id), getClientIp(req)); if (result.error) return res.status(result.status || 400).json({ error: result.error }); return res.json({ success: true }); -}); +}); \ No newline at end of file diff --git a/server/tests/integration/oauth.test.ts b/server/tests/integration/oauth.test.ts index da247acd..a0487f5a 100644 --- a/server/tests/integration/oauth.test.ts +++ b/server/tests/integration/oauth.test.ts @@ -9,43 +9,43 @@ import type { Application } from 'express'; import crypto from 'crypto'; const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); - const mock = { - db, - closeDb: () => {}, - reinitialize: () => {}, - getPlaceWithTags: (placeId: number) => { - const place: any = db.prepare(`SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon FROM places p LEFT JOIN categories c ON p.category_id = c.id WHERE p.id = ?`).get(placeId); - if (!place) return null; - const tags = db.prepare(`SELECT t.* FROM tags t JOIN place_tags pt ON t.id = pt.tag_id WHERE pt.place_id = ?`).all(placeId); - return { ...place, category: place.category_id ? { id: place.category_id, name: place.category_name, color: place.category_color, icon: place.category_icon } : null, tags }; - }, - canAccessTrip: (tripId: any, userId: number) => - db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId), - isOwner: (tripId: any, userId: number) => - !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), - }; - return { testDb: db, dbMock: mock }; + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: (placeId: number) => { + const place: any = db.prepare(`SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon FROM places p LEFT JOIN categories c ON p.category_id = c.id WHERE p.id = ?`).get(placeId); + if (!place) return null; + const tags = db.prepare(`SELECT t.* FROM tags t JOIN place_tags pt ON t.id = pt.tag_id WHERE pt.place_id = ?`).all(placeId); + return { ...place, category: place.category_id ? { id: place.category_id, name: place.category_name, color: place.category_color, icon: place.category_icon } : null, tags }; + }, + canAccessTrip: (tripId: any, userId: number) => + db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId), + isOwner: (tripId: any, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; }); vi.mock('../../src/db/database', () => dbMock); vi.mock('../../src/config', () => ({ - JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', - ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', - updateJwtSecret: () => {}, + JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, })); const { isAddonEnabledMock } = vi.hoisted(() => { - const isAddonEnabledMock = vi.fn().mockReturnValue(true); - return { isAddonEnabledMock }; + const isAddonEnabledMock = vi.fn().mockReturnValue(true); + return { isAddonEnabledMock }; }); vi.mock('../../src/services/adminService', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, isAddonEnabled: isAddonEnabledMock }; + const actual = await importOriginal(); + return { ...actual, isAddonEnabled: isAddonEnabledMock }; }); vi.mock('../../src/services/oidcService', () => ({ getAppUrl: () => 'https://trek.example.com' })); @@ -66,25 +66,25 @@ const app: Application = createApp(); // PKCE helpers function makePkce() { - const verifier = crypto.randomBytes(32).toString('base64url'); - const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); - return { verifier, challenge }; + const verifier = crypto.randomBytes(32).toString('base64url'); + const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); + return { verifier, challenge }; } beforeAll(() => { - createTables(testDb); - runMigrations(testDb); + createTables(testDb); + runMigrations(testDb); }); beforeEach(() => { - resetTestDb(testDb); - loginAttempts.clear(); - mfaAttempts.clear(); - isAddonEnabledMock.mockReturnValue(true); + resetTestDb(testDb); + loginAttempts.clear(); + mfaAttempts.clear(); + isAddonEnabledMock.mockReturnValue(true); }); afterAll(() => { - testDb.close(); + testDb.close(); }); // ───────────────────────────────────────────────────────────────────────────── @@ -92,15 +92,51 @@ afterAll(() => { // ───────────────────────────────────────────────────────────────────────────── describe('GET /.well-known/oauth-authorization-server', () => { - it('OAUTH-001 — returns RFC 8414 discovery document', async () => { - const res = await request(app).get('/.well-known/oauth-authorization-server'); - expect(res.status).toBe(200); - expect(res.body.issuer).toBe('https://trek.example.com'); - expect(res.body.authorization_endpoint).toContain('/oauth/authorize'); - expect(res.body.token_endpoint).toContain('/oauth/token'); - expect(Array.isArray(res.body.scopes_supported)).toBe(true); - expect(res.body.scopes_supported.length).toBeGreaterThan(0); - }); + it('OAUTH-001 — returns RFC 8414 discovery document', async () => { + const res = await request(app).get('/.well-known/oauth-authorization-server'); + expect(res.status).toBe(200); + expect(res.body.issuer).toBe('https://trek.example.com'); + expect(res.body.authorization_endpoint).toContain('/oauth/authorize'); + expect(res.body.token_endpoint).toContain('/oauth/token'); + expect(Array.isArray(res.body.scopes_supported)).toBe(true); + expect(res.body.scopes_supported.length).toBeGreaterThan(0); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Issue #959 regression tests +// ───────────────────────────────────────────────────────────────────────────── + +describe('RFC 9728 — path-based protected resource metadata (issue #959 bug 1)', () => { + it('OAUTH-959A — /.well-known/oauth-protected-resource/mcp returns JSON (not SPA HTML)', async () => { + const res = await request(app).get('/.well-known/oauth-protected-resource/mcp'); + expect(res.status).toBe(200); + expect(res.headers['content-type']).toMatch(/json/); + expect(res.body.resource).toContain('/mcp'); + expect(Array.isArray(res.body.authorization_servers)).toBe(true); + }); +}); + +describe('DCR scope optional — ChatGPT compatibility (issue #959 bug 2)', () => { + it('OAUTH-959B — POST /oauth/register without scope field returns 201 with default scopes', async () => { + const res = await request(app) + .post('/oauth/register') + .set('Content-Type', 'application/json') + .send({ redirect_uris: ['https://chatgpt.example.com/cb'], token_endpoint_auth_method: 'none' }); + expect(res.status).toBe(201); + expect(res.body.client_id).toBeDefined(); + expect(typeof res.body.scope).toBe('string'); + expect(res.body.scope.length).toBeGreaterThan(0); + }); + + it('OAUTH-959C — POST /oauth/register with explicit scope registers only requested scopes', async () => { + const res = await request(app) + .post('/oauth/register') + .set('Content-Type', 'application/json') + .send({ redirect_uris: ['https://example.com/cb'], token_endpoint_auth_method: 'none', scope: 'trips:read' }); + expect(res.status).toBe(201); + expect(res.body.scope).toBe('trips:read'); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -108,196 +144,195 @@ describe('GET /.well-known/oauth-authorization-server', () => { // ───────────────────────────────────────────────────────────────────────────── describe('POST /oauth/token — authorization_code grant', () => { - it('OAUTH-002 — missing client_id/client_secret returns 401 invalid_client', async () => { - const res = await request(app) - .post('/oauth/token') - .send({ grant_type: 'authorization_code', code: 'x', redirect_uri: 'https://example.com/cb', code_verifier: 'y' }); - expect(res.status).toBe(401); - expect(res.body.error).toBe('invalid_client'); - }); - - it('OAUTH-003 — MCP addon disabled returns 403 mcp_disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const res = await request(app) - .post('/oauth/token') - .send({ grant_type: 'authorization_code', client_id: 'x', client_secret: 'y', code: 'z', redirect_uri: 'https://r.example.com/cb', code_verifier: 'v' }); - expect(res.status).toBe(403); - expect(res.body.error).toBe('mcp_disabled'); - }); - - it('OAUTH-004 — missing code/redirect_uri/code_verifier returns 400 invalid_request', async () => { - const res = await request(app) - .post('/oauth/token') - .send({ grant_type: 'authorization_code', client_id: 'x', client_secret: 'y' }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_request'); - }); - - it('OAUTH-005 — invalid auth code returns 400 invalid_grant', async () => { - const { user } = createUser(testDb); - const clientResult = createOAuthClient(user.id, 'TestApp', ['https://app.example.com/cb'], ['trips:read']); - const client = clientResult.client!; - - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: client.client_id, - client_secret: clientResult.client!.client_secret, - code: 'invalid-code-xyz', - redirect_uri: 'https://app.example.com/cb', - code_verifier: 'verifier', - }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_grant'); - }); - - it('OAUTH-006 — client_id mismatch returns 400 invalid_grant', async () => { - const { user } = createUser(testDb); - const r1 = createOAuthClient(user.id, 'App1', ['https://app1.example.com/cb'], ['trips:read']); - const r2 = createOAuthClient(user.id, 'App2', ['https://app2.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); - - // Create code for client1 - const code = createAuthCode({ - clientId: r1.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app1.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + it('OAUTH-002 — missing client_id returns 401 invalid_client', async () => { + const res = await request(app) + .post('/oauth/token') + .send({ grant_type: 'authorization_code', code: 'x', redirect_uri: 'https://example.com/cb', code_verifier: 'y' }); + expect(res.status).toBe(401); + expect(res.body.error).toBe('invalid_client'); }); - // Try to use it with client2 - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r2.client!.client_id, - client_secret: r2.client!.client_secret, - code, - redirect_uri: 'https://app1.example.com/cb', - code_verifier: verifier, - }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_grant'); - }); - - it('OAUTH-007 — redirect_uri mismatch returns 400 invalid_grant', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + it('OAUTH-003 — MCP addon disabled returns 404', async () => { + isAddonEnabledMock.mockReturnValue(false); + const res = await request(app) + .post('/oauth/token') + .send({ grant_type: 'authorization_code', client_id: 'x', client_secret: 'y', code: 'z', redirect_uri: 'https://r.example.com/cb', code_verifier: 'v' }); + expect(res.status).toBe(404); }); - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://wrong.example.com/cb', - code_verifier: verifier, - }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_grant'); - }); - - it('OAUTH-008 — wrong client_secret returns 401 invalid_client', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + it('OAUTH-004 — missing code/redirect_uri/code_verifier returns 400 invalid_request', async () => { + const res = await request(app) + .post('/oauth/token') + .send({ grant_type: 'authorization_code', client_id: 'x', client_secret: 'y' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_request'); }); - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: 'wrong-secret', - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); - expect(res.status).toBe(401); - expect(res.body.error).toBe('invalid_client'); - }); + it('OAUTH-005 — invalid auth code returns 400 invalid_grant', async () => { + const { user } = createUser(testDb); + const clientResult = createOAuthClient(user.id, 'TestApp', ['https://app.example.com/cb'], ['trips:read']); + const client = clientResult.client!; - it('OAUTH-009 — PKCE failure returns 400 invalid_grant', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: client.client_id, + client_secret: clientResult.client!.client_secret, + code: 'invalid-code-xyz', + redirect_uri: 'https://app.example.com/cb', + code_verifier: 'verifier', + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_grant'); }); - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: 'this-is-a-wrong-verifier', - }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_grant'); - }); + it('OAUTH-006 — client_id mismatch returns 400 invalid_grant', async () => { + const { user } = createUser(testDb); + const r1 = createOAuthClient(user.id, 'App1', ['https://app1.example.com/cb'], ['trips:read']); + const r2 = createOAuthClient(user.id, 'App2', ['https://app2.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); - it('OAUTH-010 — happy path: exchange auth code for tokens', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); + // Create code for client1 + const code = createAuthCode({ + clientId: r1.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app1.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + // Try to use it with client2 + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r2.client!.client_id, + client_secret: r2.client!.client_secret, + code, + redirect_uri: 'https://app1.example.com/cb', + code_verifier: verifier, + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_grant'); }); - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); - expect(res.status).toBe(200); - expect(res.body.access_token).toBeDefined(); - expect(res.body.refresh_token).toBeDefined(); - expect(res.body.token_type).toBe('Bearer'); - expect(typeof res.body.expires_in).toBe('number'); - expect(res.body.scope).toBe('trips:read'); - }); + it('OAUTH-007 — redirect_uri mismatch returns 400 invalid_grant', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://wrong.example.com/cb', + code_verifier: verifier, + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_grant'); + }); + + it('OAUTH-008 — wrong client_secret returns 401 invalid_client (timing-safe check)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: 'wrong-secret', + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); + expect(res.status).toBe(401); + expect(res.body.error).toBe('invalid_client'); + }); + + it('OAUTH-009 — PKCE failure returns 400 invalid_grant', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: 'this-is-a-wrong-verifier', + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_grant'); + }); + + it('OAUTH-010 — happy path: exchange auth code for tokens', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); + expect(res.status).toBe(200); + expect(res.body.access_token).toBeDefined(); + expect(res.body.refresh_token).toBeDefined(); + expect(res.body.token_type).toBe('Bearer'); + expect(typeof res.body.expires_in).toBe('number'); + expect(res.body.scope).toBe('trips:read'); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -305,71 +340,71 @@ describe('POST /oauth/token — authorization_code grant', () => { // ───────────────────────────────────────────────────────────────────────────── describe('POST /oauth/token — refresh_token grant', () => { - it('OAUTH-011 — missing refresh_token returns 400 invalid_request', async () => { - const res = await request(app) - .post('/oauth/token') - .send({ grant_type: 'refresh_token', client_id: 'x', client_secret: 'y' }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_request'); - }); - - it('OAUTH-012 — invalid refresh token returns 400 invalid_grant', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token: 'invalid-refresh-token', - }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_grant'); - }); - - it('OAUTH-013 — happy path: issue then refresh tokens', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + it('OAUTH-011 — missing refresh_token returns 400 invalid_request', async () => { + const res = await request(app) + .post('/oauth/token') + .send({ grant_type: 'refresh_token', client_id: 'x', client_secret: 'y' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_request'); }); - // Exchange code for tokens - const tokenRes = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); - expect(tokenRes.status).toBe(200); - const { refresh_token } = tokenRes.body; + it('OAUTH-012 — invalid refresh token returns 400 invalid_grant', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - // Use refresh token to get new tokens - const refreshRes = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token, - }); - expect(refreshRes.status).toBe(200); - expect(refreshRes.body.access_token).toBeDefined(); - expect(refreshRes.body.refresh_token).toBeDefined(); - }); + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token: 'invalid-refresh-token', + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_grant'); + }); + + it('OAUTH-013 — happy path: issue then refresh tokens', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + // Exchange code for tokens + const tokenRes = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); + expect(tokenRes.status).toBe(200); + const { refresh_token } = tokenRes.body; + + // Use refresh token to get new tokens + const refreshRes = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token, + }); + expect(refreshRes.status).toBe(200); + expect(refreshRes.body.access_token).toBeDefined(); + expect(refreshRes.body.refresh_token).toBeDefined(); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -377,13 +412,13 @@ describe('POST /oauth/token — refresh_token grant', () => { // ───────────────────────────────────────────────────────────────────────────── describe('POST /oauth/token — unsupported grant_type', () => { - it('OAUTH-014 — returns 400 unsupported_grant_type', async () => { - const res = await request(app) - .post('/oauth/token') - .send({ grant_type: 'password', client_id: 'x', client_secret: 'y' }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('unsupported_grant_type'); - }); + it('OAUTH-014 — returns 400 unsupported_grant_type', async () => { + const res = await request(app) + .post('/oauth/token') + .send({ grant_type: 'password', client_id: 'x', client_secret: 'y' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('unsupported_grant_type'); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -391,80 +426,80 @@ describe('POST /oauth/token — unsupported grant_type', () => { // ───────────────────────────────────────────────────────────────────────────── describe('POST /oauth/revoke', () => { - it('OAUTH-015 — missing params returns 400 invalid_request', async () => { - const res = await request(app) - .post('/oauth/revoke') - .send({ token: 'x' }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_request'); - }); - - it('OAUTH-016 — wrong client_secret returns 401 invalid_client', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - - const res = await request(app) - .post('/oauth/revoke') - .send({ token: 'sometoken', client_id: r.client!.client_id, client_secret: 'wrong' }); - expect(res.status).toBe(401); - expect(res.body.error).toBe('invalid_client'); - }); - - it('OAUTH-017 — valid revoke returns 200 even for unknown token (RFC 7009)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - - const res = await request(app) - .post('/oauth/revoke') - .send({ token: 'nonexistent-token', client_id: r.client!.client_id, client_secret: r.client!.client_secret }); - expect(res.status).toBe(200); - }); - - it('OAUTH-018 — happy path: issue token, revoke it, verify refresh no longer works', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + it('OAUTH-015 — missing params returns 400 invalid_request', async () => { + const res = await request(app) + .post('/oauth/revoke') + .send({ token: 'x' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_request'); }); - const tokenRes = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); - expect(tokenRes.status).toBe(200); - const { refresh_token } = tokenRes.body; + it('OAUTH-016 — wrong client_secret returns 401 invalid_client', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - // Revoke the refresh token - const revokeRes = await request(app) - .post('/oauth/revoke') - .send({ token: refresh_token, client_id: r.client!.client_id, client_secret: r.client!.client_secret }); - expect(revokeRes.status).toBe(200); + const res = await request(app) + .post('/oauth/revoke') + .send({ token: 'sometoken', client_id: r.client!.client_id, client_secret: 'wrong' }); + expect(res.status).toBe(401); + expect(res.body.error).toBe('invalid_client'); + }); - // Try to use the revoked token — should fail - const retryRes = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token, - }); - expect(retryRes.status).toBe(400); - expect(retryRes.body.error).toBe('invalid_grant'); - }); + it('OAUTH-017 — valid revoke returns 200 even for unknown token (RFC 7009)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + + const res = await request(app) + .post('/oauth/revoke') + .send({ token: 'nonexistent-token', client_id: r.client!.client_id, client_secret: r.client!.client_secret }); + expect(res.status).toBe(200); + }); + + it('OAUTH-018 — happy path: issue token, revoke it, verify refresh no longer works', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + const tokenRes = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); + expect(tokenRes.status).toBe(200); + const { refresh_token } = tokenRes.body; + + // Revoke the refresh token + const revokeRes = await request(app) + .post('/oauth/revoke') + .send({ token: refresh_token, client_id: r.client!.client_id, client_secret: r.client!.client_secret }); + expect(revokeRes.status).toBe(200); + + // Try to use the revoked token — should fail + const retryRes = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token, + }); + expect(retryRes.status).toBe(400); + expect(retryRes.body.error).toBe('invalid_grant'); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -472,178 +507,178 @@ describe('POST /oauth/revoke', () => { // ───────────────────────────────────────────────────────────────────────────── describe('GET /api/oauth/authorize/validate', () => { - it('OAUTH-019 — returns 404 when MCP addon disabled (M2: prevents feature fingerprinting)', async () => { - isAddonEnabledMock.mockReturnValue(false); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .query({ response_type: 'code', client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); - expect(res.status).toBe(404); - }); + it('OAUTH-019 — returns 404 when MCP addon disabled (M2: prevents feature fingerprinting)', async () => { + isAddonEnabledMock.mockReturnValue(false); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .query({ response_type: 'code', client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); + expect(res.status).toBe(404); + }); - it('OAUTH-020 — returns 200 with valid:false for wrong response_type (authenticated)', async () => { - const { user } = createUser(testDb); - const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ response_type: 'token', client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: challenge, code_challenge_method: 'S256' }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('unsupported_response_type'); - }); + it('OAUTH-020 — returns 200 with valid:false for wrong response_type (authenticated)', async () => { + const { user } = createUser(testDb); + const { challenge } = makePkce(); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ response_type: 'token', client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: challenge, code_challenge_method: 'S256' }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('unsupported_response_type'); + }); - it('OAUTH-021 — returns 200 with valid:false for missing PKCE', async () => { - const res = await request(app) - .get('/api/oauth/authorize/validate') - .query({ response_type: 'code', client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read' }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_request'); - }); + it('OAUTH-021 — returns 200 with valid:false for missing PKCE', async () => { + const res = await request(app) + .get('/api/oauth/authorize/validate') + .query({ response_type: 'code', client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read' }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_request'); + }); - it('OAUTH-022 — returns 200 with valid:false for unknown client_id (authenticated)', async () => { - const { user } = createUser(testDb); - const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ response_type: 'code', client_id: 'unknown-client', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: challenge, code_challenge_method: 'S256' }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_client'); - }); + it('OAUTH-022 — returns 200 with valid:false for unknown client_id (authenticated)', async () => { + const { user } = createUser(testDb); + const { challenge } = makePkce(); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ response_type: 'code', client_id: 'unknown-client', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: challenge, code_challenge_method: 'S256' }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_client'); + }); - it('OAUTH-023 — returns 200 with valid:false for mismatched redirect_uri (authenticated)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-023 — returns 200 with valid:false for mismatched redirect_uri (authenticated)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://evil.example.com/cb', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_redirect_uri'); - }); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://evil.example.com/cb', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_redirect_uri'); + }); - it('OAUTH-024 — returns 200 with valid:false for empty scope (authenticated)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-024 — returns 200 with valid:false for empty scope (authenticated)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: '', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_scope'); - }); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: '', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_scope'); + }); - it('OAUTH-025a — narrows scope to allowed intersection when client lacks some requested scopes (authenticated)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-025a — narrows scope to allowed intersection when client lacks some requested scopes (authenticated)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read trips:delete', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(true); - // trips:delete was dropped — only trips:read granted - expect(res.body.scopes).toEqual(['trips:read']); - }); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read trips:delete', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + // trips:delete was dropped — only trips:read granted + expect(res.body.scopes).toEqual(['trips:read']); + }); - it('OAUTH-025b — returns 200 with valid:false when no requested scope is allowed (authenticated)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-025b — returns 200 with valid:false when no requested scope is allowed (authenticated)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'budget:write', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_scope'); - }); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'budget:write', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_scope'); + }); - it('OAUTH-026 — unauthenticated valid request returns loginRequired=true (H3: minimal response, no client info)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-026 — unauthenticated valid request returns loginRequired=true (H3: minimal response, no client info)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(true); - expect(res.body.loginRequired).toBe(true); - // H3: client name and scopes must NOT be revealed to unauthenticated callers - expect(res.body.client).toBeUndefined(); - expect(res.body.allowed_scopes).toBeUndefined(); - }); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + expect(res.body.loginRequired).toBe(true); + // H3: client name and scopes must NOT be revealed to unauthenticated callers + expect(res.body.client).toBeUndefined(); + expect(res.body.allowed_scopes).toBeUndefined(); + }); - it('OAUTH-027 — authenticated with no prior consent returns consentRequired=true with client details', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-027 — authenticated with no prior consent returns consentRequired=true with client details', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(true); - expect(res.body.consentRequired).toBe(true); - // Authenticated users get full client info (unlike unauthenticated H3 path) - expect(res.body.client).toBeDefined(); - expect(res.body.scopes).toBeDefined(); - }); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + expect(res.body.consentRequired).toBe(true); + // Authenticated users get full client info (unlike unauthenticated H3 path) + expect(res.body.client).toBeDefined(); + expect(res.body.scopes).toBeDefined(); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -651,81 +686,81 @@ describe('GET /api/oauth/authorize/validate', () => { // ───────────────────────────────────────────────────────────────────────────── describe('POST /api/oauth/authorize', () => { - it('OAUTH-028 — unauthenticated returns 401', async () => { - const res = await request(app) - .post('/api/oauth/authorize') - .send({ approved: true, client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); - expect(res.status).toBe(401); - }); + it('OAUTH-028 — unauthenticated returns 401', async () => { + const res = await request(app) + .post('/api/oauth/authorize') + .send({ approved: true, client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); + expect(res.status).toBe(401); + }); - it('OAUTH-029 — 403 when MCP disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const { user } = createUser(testDb); + it('OAUTH-029 — 403 when MCP disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const { user } = createUser(testDb); - const res = await request(app) - .post('/api/oauth/authorize') - .set('Cookie', authCookie(user.id)) - .send({ approved: true, client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); - expect(res.status).toBe(403); - }); + const res = await request(app) + .post('/api/oauth/authorize') + .set('Cookie', authCookie(user.id)) + .send({ approved: true, client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); + expect(res.status).toBe(403); + }); - it('OAUTH-030 — user denied returns redirect with error=access_denied', async () => { - const { user } = createUser(testDb); + it('OAUTH-030 — user denied returns redirect with error=access_denied', async () => { + const { user } = createUser(testDb); - const res = await request(app) - .post('/api/oauth/authorize') - .set('Cookie', authCookie(user.id)) - .send({ - approved: false, - client_id: 'any', - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: 'c', - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.redirect).toContain('error=access_denied'); - }); + const res = await request(app) + .post('/api/oauth/authorize') + .set('Cookie', authCookie(user.id)) + .send({ + approved: false, + client_id: 'any', + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: 'c', + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.redirect).toContain('error=access_denied'); + }); - it('OAUTH-031 — invalid params returns 400', async () => { - const { user } = createUser(testDb); - const { challenge } = makePkce(); + it('OAUTH-031 — invalid params returns 400', async () => { + const { user } = createUser(testDb); + const { challenge } = makePkce(); - const res = await request(app) - .post('/api/oauth/authorize') - .set('Cookie', authCookie(user.id)) - .send({ - approved: true, - client_id: 'unknown-client', - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(400); - }); + const res = await request(app) + .post('/api/oauth/authorize') + .set('Cookie', authCookie(user.id)) + .send({ + approved: true, + client_id: 'unknown-client', + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(400); + }); - it('OAUTH-032 — happy path: approve returns redirect with code', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-032 — happy path: approve returns redirect with code', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - const res = await request(app) - .post('/api/oauth/authorize') - .set('Cookie', authCookie(user.id)) - .send({ - approved: true, - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.redirect).toBeDefined(); - expect(res.body.redirect).toContain('code='); - expect(res.body.redirect).not.toContain('error='); - }); + const res = await request(app) + .post('/api/oauth/authorize') + .set('Cookie', authCookie(user.id)) + .send({ + approved: true, + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.redirect).toBeDefined(); + expect(res.body.redirect).toContain('code='); + expect(res.body.redirect).not.toContain('error='); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -733,85 +768,85 @@ describe('POST /api/oauth/authorize', () => { // ───────────────────────────────────────────────────────────────────────────── describe('Client CRUD — /api/oauth/clients', () => { - it('OAUTH-033 — GET returns 403 when addon disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const { user } = createUser(testDb); + it('OAUTH-033 — GET returns 403 when addon disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const { user } = createUser(testDb); - const res = await request(app) - .get('/api/oauth/clients') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(403); - }); + const res = await request(app) + .get('/api/oauth/clients') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(403); + }); - it('OAUTH-034 — GET returns 200 with clients list', async () => { - const { user } = createUser(testDb); - createOAuthClient(user.id, 'MyApp', ['https://app.example.com/cb'], ['trips:read']); + it('OAUTH-034 — GET returns 200 with clients list', async () => { + const { user } = createUser(testDb); + createOAuthClient(user.id, 'MyApp', ['https://app.example.com/cb'], ['trips:read']); - const res = await request(app) - .get('/api/oauth/clients') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(200); - expect(Array.isArray(res.body.clients)).toBe(true); - expect(res.body.clients).toHaveLength(1); - expect(res.body.clients[0].name).toBe('MyApp'); - }); + const res = await request(app) + .get('/api/oauth/clients') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(200); + expect(Array.isArray(res.body.clients)).toBe(true); + expect(res.body.clients).toHaveLength(1); + expect(res.body.clients[0].name).toBe('MyApp'); + }); - it('OAUTH-035 — POST creates client and returns 201 with client_secret', async () => { - const { user } = createUser(testDb); + it('OAUTH-035 — POST creates client and returns 201 with client_secret', async () => { + const { user } = createUser(testDb); - const res = await request(app) - .post('/api/oauth/clients') - .set('Cookie', authCookie(user.id)) - .send({ name: 'NewApp', redirect_uris: ['https://newapp.example.com/cb'], allowed_scopes: ['trips:read'] }); - expect(res.status).toBe(201); - expect(res.body.client).toBeDefined(); - expect(res.body.client.client_id).toBeDefined(); - expect(res.body.client.client_secret).toBeDefined(); - expect(res.body.client.name).toBe('NewApp'); - }); + const res = await request(app) + .post('/api/oauth/clients') + .set('Cookie', authCookie(user.id)) + .send({ name: 'NewApp', redirect_uris: ['https://newapp.example.com/cb'], allowed_scopes: ['trips:read'] }); + expect(res.status).toBe(201); + expect(res.body.client).toBeDefined(); + expect(res.body.client.client_id).toBeDefined(); + expect(res.body.client.client_secret).toBeDefined(); + expect(res.body.client.name).toBe('NewApp'); + }); - it('OAUTH-036 — POST returns 403 when addon disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const { user } = createUser(testDb); + it('OAUTH-036 — POST returns 403 when addon disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const { user } = createUser(testDb); - const res = await request(app) - .post('/api/oauth/clients') - .set('Cookie', authCookie(user.id)) - .send({ name: 'App', redirect_uris: ['https://app.example.com/cb'], allowed_scopes: ['trips:read'] }); - expect(res.status).toBe(403); - }); + const res = await request(app) + .post('/api/oauth/clients') + .set('Cookie', authCookie(user.id)) + .send({ name: 'App', redirect_uris: ['https://app.example.com/cb'], allowed_scopes: ['trips:read'] }); + expect(res.status).toBe(403); + }); - it('OAUTH-037 — POST /clients/:id/rotate rotates secret', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + it('OAUTH-037 — POST /clients/:id/rotate rotates secret', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const res = await request(app) - .post(`/api/oauth/clients/${r.client!.id}/rotate`) - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(200); - expect(res.body.client_secret).toBeDefined(); - expect(res.body.client_secret).not.toBe(r.client!.client_secret); - }); + const res = await request(app) + .post(`/api/oauth/clients/${r.client!.id}/rotate`) + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(200); + expect(res.body.client_secret).toBeDefined(); + expect(res.body.client_secret).not.toBe(r.client!.client_secret); + }); - it('OAUTH-038 — DELETE /clients/:id deletes client', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + it('OAUTH-038 — DELETE /clients/:id deletes client', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const res = await request(app) - .delete(`/api/oauth/clients/${r.client!.id}`) - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(200); - expect(res.body.success).toBe(true); - }); + const res = await request(app) + .delete(`/api/oauth/clients/${r.client!.id}`) + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + }); - it('OAUTH-039 — DELETE /clients/:id returns 404 for non-existent', async () => { - const { user } = createUser(testDb); + it('OAUTH-039 — DELETE /clients/:id returns 404 for non-existent', async () => { + const { user } = createUser(testDb); - const res = await request(app) - .delete('/api/oauth/clients/nonexistent-id') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(404); - }); + const res = await request(app) + .delete('/api/oauth/clients/nonexistent-id') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(404); + }); }); @@ -820,83 +855,83 @@ describe('Client CRUD — /api/oauth/clients', () => { // ───────────────────────────────────────────────────────────────────────────── describe('Sessions — /api/oauth/sessions', () => { - it('OAUTH-040 — GET returns 403 when addon disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const { user } = createUser(testDb); + it('OAUTH-040 — GET returns 403 when addon disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const { user } = createUser(testDb); - const res = await request(app) - .get('/api/oauth/sessions') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(403); - }); - - it('OAUTH-041 — GET returns 200 with sessions list', async () => { - const { user } = createUser(testDb); - - const res = await request(app) - .get('/api/oauth/sessions') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(200); - expect(Array.isArray(res.body.sessions)).toBe(true); - }); - - it('OAUTH-042 — DELETE /sessions/:id revokes session', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + const res = await request(app) + .get('/api/oauth/sessions') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(403); }); - // Get a token so there's a session to revoke - await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); + it('OAUTH-041 — GET returns 200 with sessions list', async () => { + const { user } = createUser(testDb); - const sessionsRes = await request(app) - .get('/api/oauth/sessions') - .set('Cookie', authCookie(user.id)); - expect(sessionsRes.body.sessions).toHaveLength(1); + const res = await request(app) + .get('/api/oauth/sessions') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(200); + expect(Array.isArray(res.body.sessions)).toBe(true); + }); - const sessionId = sessionsRes.body.sessions[0].id; - const deleteRes = await request(app) - .delete(`/api/oauth/sessions/${sessionId}`) - .set('Cookie', authCookie(user.id)); - expect(deleteRes.status).toBe(200); - expect(deleteRes.body.success).toBe(true); - }); + it('OAUTH-042 — DELETE /sessions/:id revokes session', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); - it('OAUTH-043 — DELETE /sessions/:id returns 404 for non-existent', async () => { - const { user } = createUser(testDb); + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); - const res = await request(app) - .delete('/api/oauth/sessions/99999') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(404); - }); + // Get a token so there's a session to revoke + await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); - it('OAUTH-044 — DELETE /sessions/:id returns 403 when addon disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const { user } = createUser(testDb); + const sessionsRes = await request(app) + .get('/api/oauth/sessions') + .set('Cookie', authCookie(user.id)); + expect(sessionsRes.body.sessions).toHaveLength(1); - const res = await request(app) - .delete('/api/oauth/sessions/1') - .set('Cookie', authCookie(user.id)); - expect(res.status).toBe(403); - }); + const sessionId = sessionsRes.body.sessions[0].id; + const deleteRes = await request(app) + .delete(`/api/oauth/sessions/${sessionId}`) + .set('Cookie', authCookie(user.id)); + expect(deleteRes.status).toBe(200); + expect(deleteRes.body.success).toBe(true); + }); + + it('OAUTH-043 — DELETE /sessions/:id returns 404 for non-existent', async () => { + const { user } = createUser(testDb); + + const res = await request(app) + .delete('/api/oauth/sessions/99999') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(404); + }); + + it('OAUTH-044 — DELETE /sessions/:id returns 403 when addon disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const { user } = createUser(testDb); + + const res = await request(app) + .delete('/api/oauth/sessions/1') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(403); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -904,348 +939,347 @@ describe('Sessions — /api/oauth/sessions', () => { // ───────────────────────────────────────────────────────────────────────────── describe('M1 — Cache-Control headers on /oauth/token', () => { - it('OAUTH-SEC-001 — token endpoint sets Cache-Control: no-store', async () => { - const res = await request(app) - .post('/oauth/token') - .send({ grant_type: 'authorization_code', client_id: 'x', client_secret: 'y', code: 'z', redirect_uri: 'https://r.example.com/cb', code_verifier: 'v' }); - expect(res.headers['cache-control']).toBe('no-store'); - expect(res.headers['pragma']).toBe('no-cache'); - }); + it('OAUTH-SEC-001 — token endpoint sets Cache-Control: no-store', async () => { + const res = await request(app) + .post('/oauth/token') + .send({ grant_type: 'authorization_code', client_id: 'x', client_secret: 'y', code: 'z', redirect_uri: 'https://r.example.com/cb', code_verifier: 'v' }); + expect(res.headers['cache-control']).toBe('no-store'); + }); }); describe('M2 — 404 when MCP disabled on discovery + revoke endpoints', () => { - it('OAUTH-SEC-002 — /.well-known/oauth-authorization-server returns 404 when disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const res = await request(app).get('/.well-known/oauth-authorization-server'); - expect(res.status).toBe(404); - }); + it('OAUTH-SEC-002 — /.well-known/oauth-authorization-server returns 404 when disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const res = await request(app).get('/.well-known/oauth-authorization-server'); + expect(res.status).toBe(404); + }); - it('OAUTH-SEC-003 — /oauth/revoke returns 404 when disabled', async () => { - isAddonEnabledMock.mockReturnValue(false); - const res = await request(app) - .post('/oauth/revoke') - .send({ token: 'x', client_id: 'y', client_secret: 'z' }); - expect(res.status).toBe(404); - }); + it('OAUTH-SEC-003 — /oauth/revoke returns 404 when disabled', async () => { + isAddonEnabledMock.mockReturnValue(false); + const res = await request(app) + .post('/oauth/revoke') + .send({ token: 'x', client_id: 'y', client_secret: 'z' }); + expect(res.status).toBe(404); + }); }); describe('H1 — PKCE format validation', () => { - it('OAUTH-SEC-004 — short code_challenge (<43 chars) rejected on /authorize/validate', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: 'tooshort', - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_request'); - }); - - it('OAUTH-SEC-005 — wrong code_verifier format rejected on /oauth/token (invalid_grant)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); - - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + it('OAUTH-SEC-004 — short code_challenge (<43 chars) rejected on /authorize/validate', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: 'tooshort', + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_request'); }); - // Submit a valid-looking but wrong-format verifier (too short) - const res = await request(app) - .post('/oauth/token') - .send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: 'short', - }); - expect(res.status).toBe(400); - expect(res.body.error).toBe('invalid_grant'); - }); + it('OAUTH-SEC-005 — wrong code_verifier format rejected on /oauth/token (invalid_grant)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); + + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + // Submit a valid-looking but wrong-format verifier (too short) + const res = await request(app) + .post('/oauth/token') + .send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: 'short', + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_grant'); + }); }); describe('H3 — Unauthenticated /authorize/validate returns minimal response', () => { - it('OAUTH-SEC-006 — invalid request by unauthenticated caller returns generic error (no oracle)', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { challenge } = makePkce(); + it('OAUTH-SEC-006 — invalid request by unauthenticated caller returns generic error (no oracle)', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { challenge } = makePkce(); - // Deliberately wrong redirect_uri — should get generic error, not invalid_redirect_uri - const res = await request(app) - .get('/api/oauth/authorize/validate') - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://evil.example.com/cb', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(false); - expect(res.body.error).toBe('invalid_request'); - // Must not leak specific error type or client details - expect(res.body.error).not.toBe('invalid_redirect_uri'); - expect(res.body.client).toBeUndefined(); - }); + // Deliberately wrong redirect_uri — should get generic error, not invalid_redirect_uri + const res = await request(app) + .get('/api/oauth/authorize/validate') + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://evil.example.com/cb', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + expect(res.body.error).toBe('invalid_request'); + // Must not leak specific error type or client details + expect(res.body.error).not.toBe('invalid_redirect_uri'); + expect(res.body.client).toBeUndefined(); + }); }); describe('H5 — All invalid_grant cases return identical response body', () => { - it('OAUTH-SEC-007 — expired/bad code, client_id mismatch, redirect_uri mismatch all return same body', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); + it('OAUTH-SEC-007 — expired/bad code, client_id mismatch, redirect_uri mismatch all return same body', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', - }); + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); - // Bad code - const res1 = await request(app).post('/oauth/token').send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code: 'bad-code-xyz', - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); + // Bad code + const res1 = await request(app).post('/oauth/token').send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code: 'bad-code-xyz', + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); - // Redirect URI mismatch (need fresh code since code is single-use) - const code2 = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', - }); - const res2 = await request(app).post('/oauth/token').send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code: code2, - redirect_uri: 'https://wrong.example.com/cb', - code_verifier: verifier, - }); + // Redirect URI mismatch (need fresh code since code is single-use) + const code2 = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + const res2 = await request(app).post('/oauth/token').send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code: code2, + redirect_uri: 'https://wrong.example.com/cb', + code_verifier: verifier, + }); - expect(res1.status).toBe(400); - expect(res2.status).toBe(400); - expect(res1.body.error).toBe('invalid_grant'); - expect(res2.body.error).toBe('invalid_grant'); - // Both must use exactly the same error_description (H5) - expect(res1.body.error_description).toBe(res2.body.error_description); - }); + expect(res1.status).toBe(400); + expect(res2.status).toBe(400); + expect(res1.body.error).toBe('invalid_grant'); + expect(res2.body.error).toBe('invalid_grant'); + // Both must use exactly the same error_description (H5) + expect(res1.body.error_description).toBe(res2.body.error_description); + }); }); describe('M5 — Consent scope union (re-authorize adds to existing consent)', () => { - it('OAUTH-SEC-008 — second consent adds new scope without losing old scope', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read', 'places:read']); - const { challenge: ch1 } = makePkce(); - const { challenge: ch2 } = makePkce(); + it('OAUTH-SEC-008 — second consent adds new scope without losing old scope', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read', 'places:read']); + const { challenge: ch1 } = makePkce(); + const { challenge: ch2 } = makePkce(); - // First consent: trips:read - await request(app) - .post('/api/oauth/authorize') - .set('Cookie', authCookie(user.id)) - .send({ - approved: true, - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: ch1, - code_challenge_method: 'S256', - }); + // First consent: trips:read + await request(app) + .post('/api/oauth/authorize') + .set('Cookie', authCookie(user.id)) + .send({ + approved: true, + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: ch1, + code_challenge_method: 'S256', + }); - // Second consent: places:read — should not drop trips:read - await request(app) - .post('/api/oauth/authorize') - .set('Cookie', authCookie(user.id)) - .send({ - approved: true, - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'places:read', - code_challenge: ch2, - code_challenge_method: 'S256', - }); + // Second consent: places:read — should not drop trips:read + await request(app) + .post('/api/oauth/authorize') + .set('Cookie', authCookie(user.id)) + .send({ + approved: true, + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'places:read', + code_challenge: ch2, + code_challenge_method: 'S256', + }); - // Re-validate with trips:read — should now be auto-approved (consentRequired=false) - const { challenge: ch3 } = makePkce(); - const res = await request(app) - .get('/api/oauth/authorize/validate') - .set('Cookie', authCookie(user.id)) - .query({ - response_type: 'code', - client_id: r.client!.client_id, - redirect_uri: 'https://app.example.com/cb', - scope: 'trips:read', - code_challenge: ch3, - code_challenge_method: 'S256', - }); - expect(res.status).toBe(200); - expect(res.body.valid).toBe(true); - expect(res.body.consentRequired).toBeFalsy(); - }); + // Re-validate with trips:read — should now be auto-approved (consentRequired=false) + const { challenge: ch3 } = makePkce(); + const res = await request(app) + .get('/api/oauth/authorize/validate') + .set('Cookie', authCookie(user.id)) + .query({ + response_type: 'code', + client_id: r.client!.client_id, + redirect_uri: 'https://app.example.com/cb', + scope: 'trips:read', + code_challenge: ch3, + code_challenge_method: 'S256', + }); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + expect(res.body.consentRequired).toBeFalsy(); + }); }); describe('M7 — Cookie-only auth on privileged OAuth endpoints', () => { - it('OAUTH-SEC-009 — POST /api/oauth/authorize rejects Bearer JWT (no cookie)', async () => { - const { user } = createUser(testDb); - // Use a valid JWT in Authorization header (no cookie) — must be rejected - const jwt = require('jsonwebtoken'); - const token = jwt.sign({ id: user.id }, 'test-jwt-secret-for-trek-testing-only', { algorithm: 'HS256' }); + it('OAUTH-SEC-009 — POST /api/oauth/authorize rejects Bearer JWT (no cookie)', async () => { + const { user } = createUser(testDb); + // Use a valid JWT in Authorization header (no cookie) — must be rejected + const jwt = require('jsonwebtoken'); + const token = jwt.sign({ id: user.id }, 'test-jwt-secret-for-trek-testing-only', { algorithm: 'HS256' }); - const res = await request(app) - .post('/api/oauth/authorize') - .set('Authorization', `Bearer ${token}`) - .send({ approved: true, client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); - expect(res.status).toBe(401); - expect(res.body.code).toBe('COOKIE_AUTH_REQUIRED'); - }); + const res = await request(app) + .post('/api/oauth/authorize') + .set('Authorization', `Bearer ${token}`) + .send({ approved: true, client_id: 'x', redirect_uri: 'https://r.example.com/cb', scope: 'trips:read', code_challenge: 'c', code_challenge_method: 'S256' }); + expect(res.status).toBe(401); + expect(res.body.code).toBe('COOKIE_AUTH_REQUIRED'); + }); - it('OAUTH-SEC-010 — POST /api/oauth/clients rejects Bearer JWT (no cookie)', async () => { - const jwt = require('jsonwebtoken'); - const { user } = createUser(testDb); - const token = jwt.sign({ id: user.id }, 'test-jwt-secret-for-trek-testing-only', { algorithm: 'HS256' }); + it('OAUTH-SEC-010 — POST /api/oauth/clients rejects Bearer JWT (no cookie)', async () => { + const jwt = require('jsonwebtoken'); + const { user } = createUser(testDb); + const token = jwt.sign({ id: user.id }, 'test-jwt-secret-for-trek-testing-only', { algorithm: 'HS256' }); - const res = await request(app) - .post('/api/oauth/clients') - .set('Authorization', `Bearer ${token}`) - .send({ name: 'App', redirect_uris: ['https://app.example.com/cb'], allowed_scopes: ['trips:read'] }); - expect(res.status).toBe(401); - expect(res.body.code).toBe('COOKIE_AUTH_REQUIRED'); - }); + const res = await request(app) + .post('/api/oauth/clients') + .set('Authorization', `Bearer ${token}`) + .send({ name: 'App', redirect_uris: ['https://app.example.com/cb'], allowed_scopes: ['trips:read'] }); + expect(res.status).toBe(401); + expect(res.body.code).toBe('COOKIE_AUTH_REQUIRED'); + }); - it('OAUTH-SEC-011 — DELETE /api/oauth/sessions/:id rejects Bearer JWT (no cookie)', async () => { - const jwt = require('jsonwebtoken'); - const { user } = createUser(testDb); - const token = jwt.sign({ id: user.id }, 'test-jwt-secret-for-trek-testing-only', { algorithm: 'HS256' }); + it('OAUTH-SEC-011 — DELETE /api/oauth/sessions/:id rejects Bearer JWT (no cookie)', async () => { + const jwt = require('jsonwebtoken'); + const { user } = createUser(testDb); + const token = jwt.sign({ id: user.id }, 'test-jwt-secret-for-trek-testing-only', { algorithm: 'HS256' }); - const res = await request(app) - .delete('/api/oauth/sessions/1') - .set('Authorization', `Bearer ${token}`); - expect(res.status).toBe(401); - expect(res.body.code).toBe('COOKIE_AUTH_REQUIRED'); - }); + const res = await request(app) + .delete('/api/oauth/sessions/1') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(401); + expect(res.body.code).toBe('COOKIE_AUTH_REQUIRED'); + }); }); describe('C3 — Refresh token replay detection', () => { - it('OAUTH-SEC-012 — replaying a rotated (old) refresh token returns invalid_grant', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); + it('OAUTH-SEC-012 — replaying a rotated (old) refresh token returns invalid_grant', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); + + // Get initial tokens + const t1 = await request(app).post('/oauth/token').send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); + expect(t1.status).toBe(200); + const originalRefreshToken = t1.body.refresh_token; + + // Rotate once (legitimate use) + const t2 = await request(app).post('/oauth/token').send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token: originalRefreshToken, + }); + expect(t2.status).toBe(200); + + // Replay the original (now rotated/revoked) refresh token — must be rejected + const t3 = await request(app).post('/oauth/token').send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token: originalRefreshToken, + }); + expect(t3.status).toBe(400); + expect(t3.body.error).toBe('invalid_grant'); }); - // Get initial tokens - const t1 = await request(app).post('/oauth/token').send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, - }); - expect(t1.status).toBe(200); - const originalRefreshToken = t1.body.refresh_token; + it('OAUTH-SEC-013 — replaying old token also invalidates the new chain', async () => { + const { user } = createUser(testDb); + const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); + const { verifier, challenge } = makePkce(); - // Rotate once (legitimate use) - const t2 = await request(app).post('/oauth/token').send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token: originalRefreshToken, - }); - expect(t2.status).toBe(200); + const code = createAuthCode({ + clientId: r.client!.client_id as string, + userId: user.id, + redirectUri: 'https://app.example.com/cb', + scopes: ['trips:read'], + codeChallenge: challenge, + codeChallengeMethod: 'S256', + }); - // Replay the original (now rotated/revoked) refresh token — must be rejected - const t3 = await request(app).post('/oauth/token').send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token: originalRefreshToken, - }); - expect(t3.status).toBe(400); - expect(t3.body.error).toBe('invalid_grant'); - }); + const t1 = await request(app).post('/oauth/token').send({ + grant_type: 'authorization_code', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + code, + redirect_uri: 'https://app.example.com/cb', + code_verifier: verifier, + }); + const originalRefreshToken = t1.body.refresh_token; - it('OAUTH-SEC-013 — replaying old token also invalidates the new chain', async () => { - const { user } = createUser(testDb); - const r = createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); - const { verifier, challenge } = makePkce(); + // Legitimate rotate — get new token + const t2 = await request(app).post('/oauth/token').send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token: originalRefreshToken, + }); + const newRefreshToken = t2.body.refresh_token; - const code = createAuthCode({ - clientId: r.client!.client_id as string, - userId: user.id, - redirectUri: 'https://app.example.com/cb', - scopes: ['trips:read'], - codeChallenge: challenge, - codeChallengeMethod: 'S256', - }); + // Replay original — triggers chain revocation + await request(app).post('/oauth/token').send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token: originalRefreshToken, + }); - const t1 = await request(app).post('/oauth/token').send({ - grant_type: 'authorization_code', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - code, - redirect_uri: 'https://app.example.com/cb', - code_verifier: verifier, + // New token (from legitimate rotation) must also be dead now + const t4 = await request(app).post('/oauth/token').send({ + grant_type: 'refresh_token', + client_id: r.client!.client_id, + client_secret: r.client!.client_secret, + refresh_token: newRefreshToken, + }); + expect(t4.status).toBe(400); + expect(t4.body.error).toBe('invalid_grant'); }); - const originalRefreshToken = t1.body.refresh_token; - - // Legitimate rotate — get new token - const t2 = await request(app).post('/oauth/token').send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token: originalRefreshToken, - }); - const newRefreshToken = t2.body.refresh_token; - - // Replay original — triggers chain revocation - await request(app).post('/oauth/token').send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token: originalRefreshToken, - }); - - // New token (from legitimate rotation) must also be dead now - const t4 = await request(app).post('/oauth/token').send({ - grant_type: 'refresh_token', - client_id: r.client!.client_id, - client_secret: r.client!.client_secret, - refresh_token: newRefreshToken, - }); - expect(t4.status).toBe(400); - expect(t4.body.error).toBe('invalid_grant'); - }); -}); +}); \ No newline at end of file diff --git a/server/tsconfig.json b/server/tsconfig.json index b443a5ba..360a4efc 100644 --- a/server/tsconfig.json +++ b/server/tsconfig.json @@ -20,9 +20,17 @@ // These paths manually redirect to the CJS dist until the SDK fixes its exports map. "paths": { "@modelcontextprotocol/sdk/server/mcp": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/mcp"], - "@modelcontextprotocol/sdk/server/streamableHttp": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/streamableHttp"] + "@modelcontextprotocol/sdk/server/streamableHttp": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/streamableHttp"], + "@modelcontextprotocol/sdk/server/auth/router": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/router"], + "@modelcontextprotocol/sdk/server/auth/handlers/authorize": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/handlers/authorize"], + "@modelcontextprotocol/sdk/server/auth/handlers/register": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/handlers/register"], + "@modelcontextprotocol/sdk/server/auth/provider": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/provider"], + "@modelcontextprotocol/sdk/server/auth/clients": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/clients"], + "@modelcontextprotocol/sdk/server/auth/errors": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/errors"], + "@modelcontextprotocol/sdk/server/auth/types": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/server/auth/types"], + "@modelcontextprotocol/sdk/shared/auth": ["./node_modules/@modelcontextprotocol/sdk/dist/cjs/shared/auth"] } }, "include": ["src"], "exclude": ["node_modules", "dist"] -} +} \ No newline at end of file