-
Notifications
You must be signed in to change notification settings - Fork 171
Add auth server design proposals #3236
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3236 +/- ##
=======================================
Coverage 56.96% 56.97%
=======================================
Files 351 351
Lines 34962 34962
=======================================
+ Hits 19916 19918 +2
+ Misses 13390 13388 -2
Partials 1656 1656 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| ``` | ||
|
|
||
| The JWTS are signed using RSA or ECDSA keys, and the public keys are exposed |
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 we skip RSA and only use more hardened algorithms?
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 I was trying to be too careful and went by https://openid.net/specs/openid-connect-core-1_0.html#ServerMTI which requires: "OPs MUST support signing ID Tokens with the RSA SHA-256 a
lgorithm (an alg value of RS256)". And because these are keys that sign the JWTs we hand out to clients and the clients then verify the signatures with out JWT endpoint, I wanted to support even clients that might stick to RSA.
In general, the key is user-provided in the branch I have and the algo is derived from the key. Not supporting RSA would mean throwing an error when an RSA key is provided.
What about this?
- we'd generate an ECDSA key if no key is present
- we'd warn about an RSA key if provided
- we'd recommend an ECDSA key in the docs
The alternative is to just go with ECSDA only.
|
|
||
| // New creates a new OAuth authorization server. | ||
| func New(ctx context.Context, cfg Config, stor storage.Storage) (Server, error) | ||
| ``` |
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.
these type of interface signatures are not quite necessary in RFCs IMO
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.
OK, these are in fact based on the branch where the AS is implemented. I included them to drive home the fact that the first deliverable will be a go package that we can use in different ways (embed, standalone, ...)
JAORMX
left a comment
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.
my only ask is that session management and JWT key retrieval is abstracted enough to be pluggable. When we reach scaling problems, we'll need to hook off-process mechanisms for these.
|
|
||
| The `/oauth/revoke` endpoint will accept both access and refresh tokens and invalidate them. Revoking a refresh token cascades to invalidate all related access tokens. Optionally, revocation can also cascade to delete the stored upstream IDP tokens associated with the session. | ||
|
|
||
| ### 8. Dynamic Client Registration (RFC 7591) |
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.
noob question: what clients would be using DCR with a ToolHive authorization 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.
I think most of them, MCP made the questionable choice as a standard to go with DRC which is not really supported by many commercial IDPs, hence also why everyone and their dog is implementing custom ASs with DCR support.
The latest MCP spec revision is moving to client metadata, but DCR is what most clients use these days.
| - **ID token verification**: Upstream ID tokens verified against JWKS before trusting | ||
| - **Nonce validation**: Replay protection for ID tokens | ||
| - **Secret management**: Signing keys and client secrets stored securely | ||
| - **Rate limiting**: Authentication endpoints protected against brute force |
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.
What level of sophistication in RL are we targeting?
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.
this will not be for the MVP, I was thinking something very simple like a sliding window to check number of requests per minute to /oauth/token
MCP servers need to authenticate users via OAuth/OIDC and obtain upstream identity tokens to call external APIs (GitHub, Google, Atlassian) on behalf of users. OAuth 2.0 requires pre-registered redirect URIs and client credentials at each Identity Provider, making it impractical for individual user clients to register directly. A centralized Authorization Server solves this by acting as the single registered OAuth client with upstream IDPs, enabling Dynamic Client Registration for MCP clients while managing the complexity of upstream token storage and retrieval. The solution implements an OAuth Authorization Server built on Fosite that authenticates users against upstream IDPs, stores their tokens, and issues its own JWTs with a `tsid` claim linking to the stored credentials. The server exposes standard OAuth 2.0/OIDC endpoints including authorization with PKCE, token exchange, dynamic client registration (RFC 7591), and JWKS for token validation. MCP servers and vMCP can then validate incoming JWTs and retrieve the upstream IDP tokens via the `tsid` to make authenticated API calls on behalf of users. The design documents in this commit outline the solution in detail.
fa99bc0 to
0dbb60d
Compare
|
Hey @jhrozek! Just a heads up - we've recently set up a dedicated repository for ToolHive RFCs and proposals: https://github.com/stacklok/toolhive-rfcs Going forward, we'd encourage submitting design proposals there instead of in this repository. The RFC repo has:
If you'd like to migrate this proposal to the new repo, feel free to do so. We're happy to help if you have any questions about the process! |
|
Closed in favor of stacklok/toolhive-rfcs#19 |
MCP servers need to authenticate users via OAuth/OIDC and obtain upstream identity tokens to call external APIs (GitHub, Google, Atlassian) on behalf of users. OAuth 2.0 requires pre-registered redirect URIs and client credentials at each Identity Provider, making it impractical for individual user clients to register directly. A centralized Authorization Server solves this by acting as the single registered OAuth client with upstream IDPs, enabling Dynamic Client Registration for MCP clients while managing the complexity of upstream token storage and retrieval.
The solution implements an OAuth Authorization Server built on Fosite that authenticates users against upstream IDPs, stores their tokens, and issues its own JWTs with a
tsidclaim linking to the stored credentials. The server exposes standard OAuth 2.0/OIDC endpoints including authorization with PKCE, token exchange, dynamic client registration (RFC 7591), and JWKS for token validation. MCP servers and vMCP can then validate incoming JWTs and retrieve the upstream IDP tokens via thetsidto make authenticated API calls on behalf of users.The design documents in this commit outline the solution in detail.