-
Notifications
You must be signed in to change notification settings - Fork 808
feat: Add support for separate Authorization Server / Resource server in server flow (spec: DRAFT-2025-v2) #503
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
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
/** | ||
* Simple in-memory implementation of OAuth clients store for demo purposes. | ||
* In production, this should be backed by a persistent database. | ||
*/ |
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.
Maybe emphasising a bit more this is a demo
/** | |
* Simple in-memory implementation of OAuth clients store for demo purposes. | |
* In production, this should be backed by a persistent database. | |
*/ | |
/** | |
* 🚨 DEMO ONLY - NOT FOR PRODUCTION | |
* | |
* This example demonstrates MCP OAuth flow but lacks some of the features required for production use: | |
* - Separate auth/resource servers | |
* - PKCE validation | |
* - Persistent token storage | |
* - Rate limiting |
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.
sg, although this does support PKCE (see below) and separate AS / RS. This is intended to be a standalone AS.
provider, | ||
issuerUrl: authServerUrl, | ||
scopesSupported: ['mcp:tools'], | ||
// This endpoint is set up on the Authorization server, but really shouldn't be. |
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.
maybe add a link to the spec and explanation this is only for demo
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.
yea let me see about re-working this... we should make it easier to provide just the backwards-compat endpoint
import { DemoInMemoryAuthProvider } from './demoInMemoryOAuthProvider.js'; | ||
|
||
// Check for OAuth flag | ||
const useOAuth = process.argv.includes('--oauth'); |
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.
worth adding to README in src/examples/README.md as I just copy-pasted command and forgot to add it 🙈
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.
Okay, had to do a bit of an overhaul, and then refactored to hopefully make things simpler.
Changes:
- There's now a single metadata router, that the "all-in-one" flow (
mcpAuthRouter
) uses, and can be used by a separate-as-rs flow viamcpAuthMetadataRouter
. - This router now takes an OAuthMetadata instance as would be returned by an Authorization Server. This matches the expected flow of having a stanadlone authorization server, getting its metadata, and then advertising it on the server. For backwards compatibility, it will just re-advertise the AS's metadata on the well-known endpoint.
- The bearer middleware now takes a
OAuthTokenVerifier
which requires only 1 function instead of a full provider, since that's all you need on the MCP server in a "separate-as-rs" implementation.
/** | ||
* Simple in-memory implementation of OAuth clients store for demo purposes. | ||
* In production, this should be backed by a persistent database. | ||
*/ |
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.
sg, although this does support PKCE (see below) and separate AS / RS. This is intended to be a standalone AS.
provider, | ||
issuerUrl: authServerUrl, | ||
scopesSupported: ['mcp:tools'], | ||
// This endpoint is set up on the Authorization server, but really shouldn't be. |
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.
yea let me see about re-working this... we should make it easier to provide just the backwards-compat endpoint
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
Motivation and Context
This adds support for RFC 9728 (Protected Resource Metadata) in line with the latest draft spec https://modelcontextprotocol.io/specification/draft/basic/authorization#2-3-1-authorization-server-location
How Has This Been Tested?
Breaking Changes
This change should be strictly additive, adding an additional metadata endpoint, and extra information to the
WWW-Authenticate
headers.Types of changes
Checklist
Additional context
there is another PR that implements the client side of this here:
#416