Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds end-user documentation for ToolHive’s new embedded authorization server feature (RFC THV-0031), explaining how it works conceptually and how to configure it in Kubernetes via the Operator.
Changes:
- Adds an “Embedded authorization server” concept section, including an OAuth flow sequence diagram and model comparison.
- Extends the Kubernetes authentication guide with a new “Approach 4” and a full embedded-auth setup walkthrough (Secrets,
MCPExternalAuthConfig, andMCPServerexamples). - Adds testing/verification steps and troubleshooting guidance specific to embedded auth.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/toolhive/guides-k8s/auth-k8s.mdx | Documents embedded auth server setup in Kubernetes, including configuration examples and troubleshooting. |
| docs/toolhive/concepts/auth-framework.mdx | Explains embedded auth server behavior, when to use it, and how it compares to external token validation. |
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
I haven't really dove into the technical accuracy of the examples - I trust they are correct and will review them properly, but I think we should discuss explaining the benefits from a different angle
danbarr
left a comment
danbarr
left a comment
There was a problem hiding this comment.
Editorial review (remaining items)
Generated with the help of Claude using the docs-review skill. The suggestions below and inline suggestions above are a mix of human and AI review.
Primary issues
"Choosing the right backend authentication model" crosses the page's stated scope
The #### Choosing the right backend authentication model section in auth-framework.mdx discusses backend authentication patterns (static credentials, token exchange, embedded auth server). This crosses into backend-auth territory — the page's own scope note says it covers "client-to-MCP-server authentication" only, and backend auth is explicitly deferred to backend-auth.mdx.
The section is useful as a decision tree, but its placement under ### Embedded authorization server makes it seem like a sub-topic of the embedded auth server rather than a cross-cutting concern.
Suggestion: Consider one of:
- Move this section to
backend-auth.mdxas a new decision-guide section (adding the embedded auth server as a third pattern alongside token exchange and federation), and link to it from here. - Keep it in this file but promote it to its own
###heading at the same level as### Embedded authorization server, and note that it bridges client and backend auth concerns.
Important limitation buried in a long bullet point
In the #### Key characteristics list, the "Direct upstream redirect" bullet buries a significant limitation:
Chained authentication—where a client authenticates with a corporate IdP like Okta, which then federates to an external provider like GitHub—is not yet supported.
This is a key "gotcha" that will determine whether the feature works for a given deployment. A user skimming the bullet list could easily miss it.
Suggestion: Extract the chained-auth limitation into a :::warning or :::info admonition immediately after the bullet list so it stands out visually.
Dual-purpose of oidcConfig in embedded auth MCPServer needs clearer explanation
In the K8s guide's Step 5, the MCPServer resource uses both externalAuthConfigRef and oidcConfig. The YAML comments and the :::note below explain the relationship, but the oidcConfig is being used to validate JWTs issued by the embedded auth server itself — a different mental model from external IdP validation in Approaches 1–3. This could easily trip up users.
Suggestion: Add a brief explanatory sentence before the YAML block, e.g.:
The MCPServer needs two configuration references:
externalAuthConfigRefto enable the embedded authorization server, andoidcConfigto validate the JWTs that the embedded authorization server issues. TheoidcConfigissuer must match theissuerin yourMCPExternalAuthConfig.
The YAML comments and :::note can then reinforce rather than carry the explanation.
Secondary issues
| Issue | Recommendation |
|---|---|
| DCR mentioned but not elaborated | Dynamic Client Registration (RFC 7591) is listed in Key characteristics but never discussed in the K8s guide setup. Add a brief note saying it works automatically / requires no configuration, or add a setup note in the guide. |
| Approach 4 intro is slightly repetitive | "This approach is ideal when you need ToolHive to retrieve OAuth tokens from an upstream provider for MCP servers that accept Authorization: Bearer tokens" largely restates the preceding sentence. Consider tightening to a single sentence. |
| Test section step 4 is vague | "Connect with an MCP client that supports the MCP OAuth specification" — is there a recommended client or a way to test with curl? If not, a cross-reference to the client compatibility page would help. |
| Guide page length | The file grows from ~470 to ~790 lines. Not a problem today, but worth noting — if a 5th auth approach is added, consider extracting each approach into its own page. |
What works well
- Clean Diataxis separation between concepts and how-to guide
- The sequence diagram clearly illustrates the full OAuth flow
- Step-by-step structure follows the established pattern from the token exchange guide
- Good use of admonitions for key rotation tips and ephemeral key warnings
- Troubleshooting section is specific and actionable
- Bidirectional cross-references between concepts and guide pages
- The OAuth 2.0 provider variant (GitHub example) is a practical addition
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
This is looking great! I left a couple of comments but I really like these docs
| spec: | ||
| type: embeddedAuthServer | ||
| embeddedAuthServer: | ||
| issuer: 'https://mcp.example.com' |
There was a problem hiding this comment.
does the issuer need to match the resourceUrl in the MCPServer? Might be worth documenting what these values are or should be
There was a problem hiding this comment.
Does it need to match the resourceUrl? It needs to match the issuer on oidcConfig, but I don't think there is such a requirement for the resourceUrl (though they might be the same)
3 participants
Description
Adds documentation for the embedded authorization server, a new ToolHive
feature (RFC THV-0031) that runs an OAuth authorization server in-process
within the ToolHive proxy. This enables MCP servers to authenticate users
via upstream identity providers (such as Okta or Microsoft Entra ID) without
requiring clients to obtain tokens independently.
auth-framework.mdx): New section explaining how theembedded authorization server works, when to use it, and how it compares
to external token validation. Includes a sequence diagram of the full
OAuth flow and notes that this feature is Kubernetes-only.
auth-k8s.mdx): Step-by-step setup instructionscovering Secrets for signing keys and HMAC keys, the MCPExternalAuthConfig
CRD, and the MCPServer resource. Includes examples for both OIDC and
OAuth 2.0 upstream providers, a verification/testing section, and
troubleshooting guidance.
Type of change
Related issues/PRs
Embedded Authorization Server in Proxy Runner
RFC: THV-0031-auth-server-integration
Screenshots
N/A
Submitter checklist
Content and formatting
Reviewer checklist
Content