feat: migrate OAuth public endpoints to MCP SDK auth handlers

Fixes issue #959 — two bugs causing ChatGPT's custom MCP connector to fail: 1. RFC 9728 path-based PRM: ChatGPT requests /.well-known/oauth-protected-resource/mcp (path-aware URL per RFC 9728 §5). The old TREK handler only registered the base path; requests for the path variant fell through to the SPA catch-all and returned HTML. mcpAuthMetadataRouter registers the path-aware URL automatically. 2. DCR without scope: ChatGPT never sends scope during Dynamic Client Registration (RFC 7591 makes it optional). The old handler returned 400 for missing scope. clientRegistrationHandler accepts it; trekClientsStore.registerClient defaults to ALL_SCOPES when absent, and the user still grants only what they approve at the consent UI (scopeSelectable=true for DCR clients is unchanged). Hybrid approach: SDK handles /.well-known, /oauth/authorize (redirect to consent SPA), and /oauth/register. TREK keeps its own /oauth/token and /oauth/revoke because SDK clientAuth does plain-text secret comparison while TREK uses SHA-256 hashing — incompatible without a full clientAuth rewrite. SPA consent page renamed /oauth/authorize → /oauth/consent to avoid routing conflict with the SDK's backend authorize handler now mounted at that path. Existing URL paths (/oauth/token etc.) are unchanged so active Claude.ai connections are unaffected. Other: lazy-init SDK metadata router so getAppUrl() (DB query) is not called at createApp() time; path-aware mcpAddonGate so only /.well-known returns 404 when MCP is disabled (previously a blanket middleware blocked all routes including static files); /api/oauth mounted before the SDK middleware chain so SPA-facing routes with their own 403 gates are reached correctly.
2026-06-21 06:11:45 +00:00 · 2026-05-05 13:01:32 +02:00
parent 69620e7276
commit 86129bbfbc
10 changed files with 380 additions and 153 deletions
@@ -218,7 +218,7 @@ export default function App() {
        <Route path="/forgot-password" element={<ForgotPasswordPage />} />
        <Route path="/reset-password" element={<ResetPasswordPage />} />
        {/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */}
-        <Route path="/oauth/authorize" element={<OAuthAuthorizePage />} />
+        <Route path="/oauth/consent" element={<OAuthAuthorizePage />} />
        <Route
          path="/dashboard"
          element={
@@ -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(<LoginPage />);

      await waitFor(() => {
-        expect(sessionStorage.getItem('oidc_redirect')).toBe('/oauth/authorize?client_id=foo');
+        expect(sessionStorage.getItem('oidc_redirect')).toBe('/oauth/consent?client_id=foo');
      });
    });

@@ -67,13 +67,13 @@ describe('LoginPage — OIDC redirect preservation', () => {
    });

    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(<LoginPage />);

      await waitFor(() => {
        expect(mockNavigate).toHaveBeenCalledWith(
-          '/oauth/authorize?client_id=foo&state=xyz',
+          '/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(<LoginPage />);

@@ -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 = {
@@ -124,7 +124,7 @@ export default function OAuthAuthorizePage(): React.ReactElement {
  }

  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)
  }

@@ -57,7 +57,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,
+      },
    }
  }
 })
@@ -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();
@@ -89,9 +96,15 @@ export function createApp(): express.Application {
  const hstsIncludeSubdomains = process.env.HSTS_INCLUDE_SUBDOMAINS === 'true';

  // RFC 8414 / RFC 9728: discovery docs are world-readable — open CORS regardless of deployment config
+  // Covers both the base path and the RFC 9728 path-based variant (/.well-known/oauth-protected-resource/mcp)
  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/oauth-')) {
+        cors({ origin: '*', credentials: false })(req, _res, next);
+      } else {
+        next();
+      }
+    },
  );
  app.use(cors({ origin: corsOrigin, credentials: true }));
  app.use(helmet({
@@ -340,11 +353,68 @@ 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 _sdkMetaRouter: express.Router | null = null;
+  function getMetaRouter(): express.Router {
+    if (_sdkMetaRouter) return _sdkMetaRouter;
+    const base = (getAppUrl() || 'http://localhost:3001').replace(/\/+$/, '');
+    const oauthMetadata: 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,
+    };
+    _sdkMetaRouter = mcpAuthMetadataRouter({
+      oauthMetadata,
+      resourceServerUrl: new URL(`${base}/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.startsWith('/.well-known/oauth-protected-resource');
+    if (isMetadataPath && !isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end();
+    getMetaRouter()(req, res, next);
+  });
+
+  // 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);
@@ -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<OAuthClientInformationFull | undefined> {
+    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<OAuthClientInformationFull, 'client_id' | 'client_id_issued_at'>,
+  ): Promise<OAuthClientInformationFull> {
+    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<void> {
+    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<string> {
+    throw new ServerError('PKCE validation is handled by the provider directly');
+  },
+
+  async exchangeAuthorizationCode(
+    client: OAuthClientInformationFull,
+    code: string,
+    codeVerifier?: string,
+    redirectUri?: string,
+    resource?: URL,
+  ): Promise<OAuthTokens> {
+    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<OAuthTokens> {
+    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<AuthInfo> {
+    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<void> {
+    serviceRevokeToken(request.token, client.client_id, undefined, null);
+  },
+};
@@ -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,7 +14,6 @@ import {
  revokeToken,
  verifyPKCE,
  authenticateClient,
-  isValidRedirectUri,
  listOAuthClients,
  createOAuthClient,
  deleteOAuthClient,
@@ -23,7 +22,6 @@ import {
  revokeSession,
  AuthorizeParams,
 } from '../services/oauthService';
-import { getAppUrl } from '../services/oidcService';
 import { writeAudit, getClientIp, logWarn } from '../services/auditLog';

 // ---------------------------------------------------------------------------
@@ -59,53 +57,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 +78,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 +153,9 @@ 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) => {
-  if (!isAddonEnabled(ADDON_IDS.MCP)) return res.status(404).end();
-
-  const body: Record<string, unknown> = 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' });
-  }
-  // 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 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',
-  });
-});
-
 // 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<string, string> = typeof req.body === 'object' ? req.body : {};
  const { token, client_id, client_secret } = body;
  const ip = getClientIp(req);
@@ -103,12 +103,48 @@ describe('GET /.well-known/oauth-authorization-server', () => {
  });
 });

+// ─────────────────────────────────────────────────────────────────────────────
+// 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');
+  });
+});
+
 // ─────────────────────────────────────────────────────────────────────────────
 // POST /oauth/token — authorization_code grant
 // ─────────────────────────────────────────────────────────────────────────────

 describe('POST /oauth/token — authorization_code grant', () => {
-  it('OAUTH-002 — missing client_id/client_secret returns 401 invalid_client', async () => {
+  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' });
@@ -116,13 +152,12 @@ describe('POST /oauth/token — authorization_code grant', () => {
    expect(res.body.error).toBe('invalid_client');
  });

-  it('OAUTH-003 — MCP addon disabled returns 403 mcp_disabled', async () => {
+  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(403);
-    expect(res.body.error).toBe('mcp_disabled');
+    expect(res.status).toBe(404);
  });

  it('OAUTH-004 — missing code/redirect_uri/code_verifier returns 400 invalid_request', async () => {
@@ -211,7 +246,7 @@ describe('POST /oauth/token — authorization_code grant', () => {
    expect(res.body.error).toBe('invalid_grant');
  });

-  it('OAUTH-008 — wrong client_secret returns 401 invalid_client', async () => {
+  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();
@@ -909,7 +944,6 @@ describe('M1 — Cache-Control headers on /oauth/token', () => {
      .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');
  });
 });

@@ -20,7 +20,15 @@
    // 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"],