Restructure backend auth docs for balanced pattern coverage#554
Restructure backend auth docs for balanced pattern coverage#554
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Restructures ToolHive’s backend authentication documentation to give equal coverage to three MCP-server-to-backend auth patterns, while keeping client-to-MCP-server authentication focused in auth-framework.mdx.
Changes:
- Reorganized
backend-auth.mdxinto three primary backend auth patterns (static credentials, token exchange, embedded authorization server) with expanded diagrams and guidance. - Moved/condensed embedded authorization server details in
auth-framework.mdx, linking tobackend-auth.mdxfor backend-token storage/forwarding specifics. - Updated “choosing the right model” guidance into a scenario→pattern table and refreshed related links.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/toolhive/concepts/backend-auth.mdx | Reframes page around three backend auth patterns and adds detailed embedded authorization server mechanics. |
| docs/toolhive/concepts/auth-framework.mdx | Tightens embedded authorization server explanation from the client perspective and cross-links to backend-auth for backend-facing details. |
| ToolHive's backend authentication patterns, including token exchange, | ||
| federated identity, and the embedded authorization server. |
There was a problem hiding this comment.
The frontmatter description lists “token exchange, federated identity, and the embedded authorization server” but omits the “static credentials” pattern that the page now presents as one of the three backend auth patterns. Consider updating the description to reflect the three patterns consistently (static credentials, token exchange, embedded authorization server) so the summary matches the page content.
| ToolHive's backend authentication patterns, including token exchange, | |
| federated identity, and the embedded authorization server. | |
| ToolHive's backend authentication patterns, including static credentials, | |
| token exchange, and the embedded authorization server. |
jhrozek
force-pushed
the
backend-auth-authserver
branch
from
e3f89ef to
43ef10d
Compare
|
To my suprise, I agree with Copilot's suggestions and ended up taking them all |
Editorial reviewThe restructuring achieves its goal: all three patterns now get equal coverage with their own sections, diagrams, and guidance. The decision table and the new "Token storage and forwarding" section are clear improvements. A few issues to address before merge. Primary issues1. "Security and operational benefits" is now inaccurate The first bullet says: "MCP servers receive short-lived, properly scoped access tokens instead of embedding long-lived secrets." But the page now documents static credentials as a first-class pattern—those are long-lived secrets. This is only accurate for the token-based patterns. Suggest scoping the bullet explicitly, or replacing it with something like: "Token-based patterns provide short-lived, properly scoped credentials—see Choosing the right backend authentication model to select the most appropriate level of security for your scenario." 2. Terminology inconsistency: "model" vs "pattern" The document uses "pattern" consistently throughout (intro, 3. "Token exchange in depth" sub-headings don't parallel the overview The overview uses:
The in-depth section uses different names for the same sub-patterns:
A reader who wants to dig deeper into "Same IdP with token exchange" won't find a matching heading. Suggest renaming the in-depth sub-sections to match, e.g. Secondary issues
Inline suggestions
|
jhrozek
force-pushed
the
backend-auth-authserver
branch
from
43ef10d to
dcc62b5
Compare
|
@danbarr thanks for the review and sorry for the offsite-induced delay. I pushed the fixes in a separate patch for easier re-review. |
danbarr
left a comment
danbarr
left a comment
There was a problem hiding this comment.
One minor note and one question, but LGTM!
| ```mermaid | ||
| flowchart LR | ||
| User[User] | ||
| Proxy[ToolHive Proxy] | ||
| ExtProvider[External Provider] | ||
|
|
||
| User -->|connect| Proxy | ||
| Proxy -->|redirect to login| ExtProvider | ||
| User -->|authenticate| ExtProvider | ||
| ExtProvider -->|authorization code| Proxy | ||
| Proxy -->|exchange code for token| ExtProvider | ||
| ExtProvider -->|upstream tokens| Proxy | ||
| Proxy -->|issue JWT| User | ||
| ``` |
There was a problem hiding this comment.
This chart has a lot of lines connecting just a few nodes, so it's a bit unclear what the order of operations is without bouncing up and down between it and the list below. Numbering the connector labels might help this?
There was a problem hiding this comment.
I changed it to a flow chart. Now this is different from the other diagams on this page, so maybe I should change them all to flow charts?
The reason I changed to a flow chart was that the text was rendering white-on-white for me with the usual labels for some reason..
There was a problem hiding this comment.
Mean to say you changed it to a sequence diagram? (it was a flowchart previously)
In any case, I think this style does work better for these ordered processes, thanks.
There was a problem hiding this comment.
BTW if you have a code snippet you can send me that was rendering wrong, I'll investigate if we have a CSS issue somewhere.
| In Kubernetes deployments, session storage is currently in-memory only. Upstream | ||
| tokens are lost when pods restart, requiring users to re-authenticate. |
There was a problem hiding this comment.
Are we planning to address this? If so, worth linking to the GitHub issue (if there is one)?
There was a problem hiding this comment.
ah yeah, we merged redis recently but don't have a good guide for it. I've linked to the CRD definitions as a stopgap and will work on the redis amend in parallel
The backend-auth.mdx page was heavily weighted toward token exchange while the embedded authorization server and static credentials got minimal coverage. Restructure so all three backend auth patterns get equal treatment with their own sections, diagrams, and guidance. Maintain the page separation: auth-framework.mdx focuses on client-to-MCP-server authentication, backend-auth.mdx focuses on MCP-server-to-backend authentication. The embedded authorization server spans both concerns, so each page covers its respective perspective. Key changes to backend-auth.mdx: - Add static credentials section with Vault cross-reference - Add embedded authorization server section with OAuth flow diagram, key characteristics, and tsid-based token storage/forwarding detail - Reorganize token exchange content under "Token exchange in depth" - Replace verbose choosing-the-right-model bullets with decision table - Broaden front matter, intro, and benefits to cover all patterns Key changes to auth-framework.mdx: - Trim embedded auth server section to focus on client-facing flow - Add cross-reference to backend-auth.mdx for backend token details - Fix "Why ToolHive centralizes" to attribute per-server cost to centralization and registration/federation gaps to the auth server
Address PR review feedback: - Add static credentials to frontmatter description - Clarify credential delivery covers headers and env vars - Scope DCR descriptions to ToolHive registration
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jhrozek
force-pushed
the
backend-auth-authserver
branch
from
dcc62b5 to
bafa620
Compare
Description
The backend-auth.mdx page was heavily weighted toward token exchange while the embedded authorization server and static credentials got minimal coverage. Restructure so all three backend auth patterns get equal treatment with their own sections, diagrams, and guidance.
The restructuring also helps maintain the page separation: auth-framework.mdx focuses on client-to-MCP-server authentication, backend-auth.mdx focuses on MCP-server-to-backend authentication. The embedded authorization server spans both concerns, so each page covers its respective perspective.
Type of change
Related issues/PRs
N/A this was just me reading the docs and getting confused
Screenshots
Submitter checklist
Content and formatting
Reviewer checklist
Content