-
Notifications
You must be signed in to change notification settings - Fork 9
Add Support for Separate Authorization Server Mode #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Updated config.ts with AUTH_MODE, AUTH_SERVER_URL, AUTH_SERVER_PORT - Added validation for port consistency between URL and PORT variables - Added comprehensive JSDoc comments for all config variables - Created .env.integrated and .env.separate config files - Updated redis.ts to use REDIS_URL from config - Added .claude/ to .gitignore
- Created shared/types.ts with well-documented type definitions - Created shared/auth-core.ts with core auth functions (PKCE, tokens, encryption) - Created shared/redis-auth.ts with Redis operations for auth data - Updated src/services/auth.ts to use shared modules - Added TokenIntrospectionResponse interface for Mode 2
- Created ExternalAuthVerifier for token validation with external auth server - Added mode switching logic to src/index.ts - Integrated mode uses mcpAuthRouter (current behavior) - Separate mode uses mcpAuthMetadataRouter and external verifier - Fake upstream auth routes only available in integrated mode - Added auth server metadata fetching with error handling
- Corrected imports in external-verifier.ts (OAuthTokenVerifier from provider.js) - Fixed InvalidTokenError import from errors.js - Changed logger.warn to logger.info (warn method doesn't exist) - Removed unused OAuthTokens import from services/auth.ts
- Implemented retry logic (5 attempts, 3 second delay) for auth server connection - Added token validation caching with 60-second default TTL - Cache respects token expiration time if provided - Cache invalidation on 401/403 responses - Automatic cache cleanup every minute - Debug logging for cache hits and misses
- Created auth-server/index.ts with full OAuth endpoint support - Reuses EverythingAuthProvider from main server - Added custom /oauth/introspect endpoint for token validation - Includes fake upstream auth routes for user authentication - Added TypeScript configuration for auth server - Added comprehensive npm scripts for all modes: - dev:integrated, dev:separate, dev:auth-server - dev:with-separate-auth (runs both servers) - build:auth-server, build:all - Added concurrently dependency for running multiple servers - Added detailed README for auth server - Successfully serves OAuth metadata and health endpoints
- Updated Prerequisites with Redis setup (Docker Compose + OrbStack) - Added comprehensive Authentication Modes section - Added Testing with MCP Inspector section with step-by-step guides - Updated Features to highlight dual mode support - Updated Configuration with new environment variables - Updated Development Commands with all new npm scripts - Added architecture diagrams for both modes - Referenced auth-server/README.md for detailed auth server info All phases now completed - Mode 2 implementation ready for use!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, few small tweaks
* fix: add /mcp to endpoints in misc README.mds * improve architecture interaction diagrams in README.md * add "OAuth Flow Analysis" section in README.md
- Remove /oauth prefix from introspect endpoint to match SDK pattern - Fix auth-server endpoints: /introspect not /oauth/introspect - Update external-verifier to call correct introspect endpoint - Add 30-day expiry to client registration (REDIS_EXPIRY_TIMES.CLIENT_REGISTRATION) - Remove inaccurate scratch/ directory references from README - Add backlinks from auth-server/README.md to main README sections - Verified all implementation details against running servers and Redis data
- Fix server hanging issue: incomplete HTTP responses in shttp.ts - Add proper JSON-RPC error responses per MCP specification - Fix PKCE challenge encoding: use base64url not standard base64 - Add comprehensive e2e scripts with OAuth flow and feature testing: - scripts/test-integrated-e2e.sh: Tests integrated mode - scripts/test-separate-e2e.sh: Tests separate mode - Scripts verify all README claims: 7 tools, 100 resources (paginated), 3 prompts - Add environment variable support and prerequisite checking - Add TODO documentation for Streamable HTTP implementation choices - Update README with table of contents and scripts documentation - Update project structure to include scripts directory Scripts successfully verify: ✅ Complete OAuth 2.0 + PKCE flows in both modes ✅ Token introspection and cross-server session management ✅ All MCP features working correctly ✅ README accuracy (tool/resource counts verified programmatically)
- Update streamable-http-design.md with correct file references - Reflect current dual transport architecture (SSE + Streamable HTTP) - Update file paths to match actual implementation structure - Clarify stateful session management approach - Remove outdated research content, focus on current implementation
- Add express-rate-limit to custom endpoints for security: - /introspect: 100 requests per 5 minutes - /fakeupstreamauth/*: 20 requests per minute - Static files: 25 requests per 10 minutes - Add automated e2e testing npm scripts using concurrently: - npm run test:e2e:integrated: Auto-starts server and runs test - npm run test:e2e:separate: Auto-starts both servers and runs test - Update README with automated testing approach documentation - Update docs/streamable-http-design.md with current file structure - Remove redundancy in README e2e testing documentation - Verified: All tests, linting, and e2e scripts pass with rate limiting
- Apply staticFileRateLimit to /mcp-logo.png route in auth server - Addresses remaining check failure about missing rate limiting - Matches rate limiting pattern used in main server
- Add missing staticFileRateLimit definition in auth-server/index.ts - Resolves compilation error in auth server build - Addresses PR check failure about missing rate limiting on line 123
- Make 'npm run build' build both main server and auth server - Remove redundant build:auth-server and build:all scripts - Update README to reflect simplified build command - Ensures compilation errors in auth server are caught by default build
The package-lock.json contained references to Anthropic's internal artifactory registry which GitHub Actions cannot access, causing npm ci to fail. Regenerated with public registry.npmjs.org. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
||
// Serve resource metadata only (not auth endpoints) | ||
app.use(mcpAuthMetadataRouter({ | ||
oauthMetadata: authMetadata, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thnk we should only be serving the .well-known/oauth-authorization-server
endpoint on integrated mode, but I think the code here is about populating for separate mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be addressed now, but also worth validating with @pcarleton
Remove incorrect mcpAuthMetadataRouter call when running in separate mode. The resource server should not serve OAuth metadata endpoints - only the auth server should provide these endpoints.
Use single tsconfig.json for both main server and auth-server to avoid nested directory issues during build. Update e2e test scripts to run directly instead of using concurrently.
Add logging to track authorization flow steps and token introspection to help diagnose issues with separate mode authentication.
Add state parameter round-trip validation to prevent CSRF attacks. Document each OAuth step including PKCE generation and token exchange. Kill existing servers before starting new ones to avoid conflicts.
Fixes MCP Inspector connection by providing OAuth discovery endpoint that points to the external auth server at http://localhost:3001
Keep serving .well-known/oauth-authorization-server from MCP server in separate mode as some clients may expect to find it there
Enhance token validation in separate mode to fully comply with MCP specification: - Validate audience (aud) claim to ensure tokens are issued for this specific MCP server - Validate temporal claims (nbf, iat) with appropriate clock skew tolerance - Add configurable canonical URI for audience validation - Improve logging for validation failures These changes prevent token passthrough attacks and ensure tokens are properly scoped to the intended resource server, as required by the MCP OAuth 2.0 specification. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
from irl: we'll deploy integrated mode, we could do separate in the future.
one thing to note is that b/c we do the oauth metadata endpoint for backwards compatibility, the two modes are somewhat difficult to distinguish.
updated and reviewed w/@paulc
Hi! I know is already approved, so I'm happy to move this feedback to a new ticket! I've been using this branch for the last few days with
Huge thank you for the work you've done on this project! You've saved me countless hours and helped me understand oauth again! |
package.json
Outdated
"lint": "eslint src/", | ||
"test": "NODE_OPTIONS=--experimental-vm-modules jest" | ||
"copy-static": "mkdir -p dist/src/static && cp -r src/static/* dist/src/static/", | ||
"lint": "eslint src/ auth-server/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's also a shared
directory now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah good catch, thanks!
@dylang super appreciate you trying this out! Will go through all your feedback shortly - I'd rather fix now than kick the can down the road, but if I run into something that's hard to address I'll call it out so you can raise it separately. |
The auth server's /introspect endpoint now correctly sets the 'aud' field to the resource server URL (BASE_URI) instead of the client ID. This ensures proper audience validation when the MCP server verifies tokens in separate mode. - Import BASE_URI in auth server - Set aud to BASE_URI in introspection response - Fixes "Token was not issued for this resource server" error 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
@dylang I've gone ahead and pushed this with a couple of minor fixes/tweaks rather than churning it, so most of your callouts are still pending - some responses and followup requests:
Any chance you have a repro? It works for me using Inspector and the new e2e test, but more datapoints definitely welcome.
Yeah, only for internal - separate-mode stuff should only be in
Agree that this could be confusing - something like "demo" would probably be better. A production server acting as its own auth server would probably need to swap this out for something more industrial-grade.
Yes, this would be helpful. If you want to start a new section in the README and add the verifications you've done with a little detail, that would be a great start! And re failing with Inspector, if you can put together a repro in a new issue, that would be super helpful. Thanks again for kicking the tires so thoroughly on this! |
Summary
Implements support for external authorization servers, enabling the MCP server to delegate authentication to a separate OAuth provider. This demonstrates both authorization patterns specified in the MCP specification: integrated (existing) and separate (new).
Key Changes
Architecture
Testing
Backward Compatibility
Implementation Highlights
mcpAuthMetadataRouter
The PR provides a flexible authentication system that supports both integrated and separate authorization server modes.
PR Reviewer's Guide: Dual Authentication Mode Support
Overview
This PR introduces dual authentication mode support to the MCP Example Remote Server, allowing it to operate in two distinct modes:
This architectural change enables the example server to demonstrate both simple standalone deployments and enterprise integration patterns where organizations use existing OAuth infrastructure (Auth0, Okta, AWS Cognito, etc.).
High-Level Changes
1. Dual Mode Architecture
2. Shared Authentication Logic
3. Standalone Authorization Server
4. Infrastructure Improvements
5. Testing & Documentation
6. Security Enhancements
File-by-File Breakdown
Configuration & Environment Files
.env.integrated
,.env.separate
(new)src/config.ts
(modified)AUTH_MODE
,AUTH_SERVER_PORT
,AUTH_SERVER_URL
configurationShared Authentication Modules
shared/auth-core.ts
(new)shared/redis-auth.ts
(new)auth:
prefix for isolationshared/types.ts
(new)TokenIntrospectionResponse
src/services/auth.ts
(modified)Standalone Authorization Server
auth-server/index.ts
(new)mcpAuthRouter
/introspect
endpoint for token validation (RFC 7662)auth-server/README.md
(new)auth-server/tsconfig.json
(new)MCP Server Modifications
src/index.ts
(heavily modified)src/auth/external-verifier.ts
(new)src/auth/provider.ts
(modified)src/handlers/fakeauth.ts
(modified)Infrastructure & Build
tsconfig.json
(modified)shared/
andauth-server/
directoriespackage.json
(modified)dev:integrated
,dev:separate
,dev:auth-server
,dev:with-separate-auth
concurrently
for running multiple serversexpress-rate-limit
for securitydocker-compose.yml
(new)package-lock.json
(modified)Testing
scripts/test-integrated-e2e.sh
(new)scripts/test-separate-e2e.sh
(new)src/services/auth.test.ts
(modified)auth:
)Documentation
README.md
(extensively modified)docs/user-id-system.md
(modified)docs/streamable-http-design.md
(modified)Key Design Decisions
Redis Namespace Isolation: All auth-related keys use
auth:
prefix to prevent collisions with MCP session keys, enabling both modes to work consistently.Backwards Compatibility: In separate mode, the MCP server still serves OAuth metadata for clients that expect it on the resource server, even though it delegates to an external auth server.
Token Introspection: Chose RFC 7662 token introspection over JWT validation to demonstrate stateful token validation patterns common in enterprise environments.
Shared Modules: Extracted common logic to
shared/
directory to maintain DRY principle while supporting two distinct deployment modes.Comprehensive Validation: Added audience validation and temporal claim checks to meet MCP specification requirements for secure token handling.
Testing Recommendations
npm run test:e2e:integrated
to verify the complete flownpm run test:e2e:separate
to test delegationnpm test
to verify all componentsMigration Impact