docs: clarify OAuth revocation behavior, disable_vk_identity flag, and token lifetime semantics - #4807
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ChangesOAuth Config Field and Documentation Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
|
|
f0cc9f8 to
fce7b65
Compare
38b8dd1 to
4065be9
Compare
Merge activity
|
Confidence Score: 5/5Safe to merge — documentation and comment corrections only, with no changes to runtime logic. All changes are documentation, OpenAPI schema descriptions, and Go source comments. The factual claims were verified against the implementation: the DELETE handler in framework/oauth2/main.go confirms no upstream revocation endpoint is called, and the DisableVKIdentity handler logic in mcpserver.go and config.go matches the documented behavior. The only gap found is that disable_vk_identity is missing from the config field reference table in gateway-auth.mdx, which is a documentation completeness issue rather than an accuracy problem. docs/mcp/gateway-auth.mdx — the Configuration section's field table does not yet list disable_vk_identity alongside the other oauth2_server_config fields. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Client holds access token] --> B{Grant revoked?}
B -- No --> C{Identity active?}
B -- Yes --> D[Refresh denied]
D --> E[Token usable until TTL expires]
C -- Deleted or deactivated --> G[Identity check fails]
G --> H[Token rejected immediately]
C -- Active --> I[Request succeeds]
subgraph Bulk cutoffs
J[disable_vk_identity] --> L[vk-mode grants rejected at /mcp]
N[enforce_auth_on_inference] --> O[Session tokens blocked at /mcp]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Client holds access token] --> B{Grant revoked?}
B -- No --> C{Identity active?}
B -- Yes --> D[Refresh denied]
D --> E[Token usable until TTL expires]
C -- Deleted or deactivated --> G[Identity check fails]
G --> H[Token rejected immediately]
C -- Active --> I[Request succeeds]
subgraph Bulk cutoffs
J[disable_vk_identity] --> L[vk-mode grants rejected at /mcp]
N[enforce_auth_on_inference] --> O[Session tokens blocked at /mcp]
end
Reviews (2): Last reviewed commit: "fix: doc fixes" | Re-trigger Greptile |
fce7b65 to
145e756
Compare
…and token lifetime semantics (#4807) ## Summary This PR clarifies and corrects documentation around OAuth token lifecycle, revocation behavior, and the new `disable_vk_identity` configuration option. It also fixes inaccurate descriptions of how Bifrost handles upstream OAuth revocation, access token validation, and the distinction between revoking a grant versus deleting the bound identity. ## Changes - Added `revoked` as a documented OAuth config status value, clarifying that the config row is retained with no live token after a DELETE call. - Corrected the DELETE endpoint description: Bifrost does **not** call the upstream provider's revocation endpoint — it only deletes the stored token locally and marks the config `revoked`. Users must revoke at the provider's dashboard if upstream invalidation is needed. - Clarified that access token refresh happens lazily on next use after expiration, not proactively before expiration. - Updated the per-user OAuth note to clearly separate upstream per-user OAuth (tokens held for upstream MCP services) from Bifrost acting as an OAuth 2.1 Authorization Server for inbound `/mcp` clients, with a cross-reference to the Gateway Authentication page. - Expanded the Token Lifetime & Revocation section to document: - The in-memory identity cache used during access token validation and its fallback behavior. - The distinction between revoking a grant (stops refresh, but existing access token lives out its TTL) versus deleting the bound virtual key or user (also rejects the already-issued access token on its next request). - The behavior of `disable_vk_identity`: rejects all virtual-key-mode grants at `/mcp` and on refresh, forcing re-authentication as a user. - Added `disable_vk_identity` to the OpenAPI config schema with a description of its scope and conditions. - Updated `RevokeOAuth2RefreshTokensByMode` comment to be more precise about its purpose as a bulk-revoke utility. - Corrected internal comments in `OAuth2ServerConfig` to reference the OAuth Grants UI (not "Connected Clients UI") and `DisableVKIdentity` (not `EnforceAuthOnInference`) as the vk-mode revocation path. - Added a note explaining why revoked grants leave short-lived access tokens usable until expiry, and why this is an intentional trade-off. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Review the updated documentation pages for accuracy against the actual Bifrost OAuth implementation: - `docs/mcp/auth/oauth.mdx` — verify DELETE behavior and status values match implementation. - `docs/mcp/auth/per-user-oauth.mdx` — verify the note correctly distinguishes upstream vs. inbound OAuth. - `docs/mcp/gateway-auth.mdx` — verify token lifetime, revocation, and `disable_vk_identity` descriptions match behavior. - `docs/openapi/schemas/management/config.yaml` — verify `disable_vk_identity` schema matches the Go struct definition. ## Breaking changes - [ ] Yes - [x] No ## Security considerations The documentation now explicitly states that Bifrost does **not** call upstream OAuth provider revocation endpoints on DELETE. Operators who assumed upstream tokens were being invalidated must be made aware they need to revoke tokens directly at the provider's dashboard. The updated revocation lifecycle documentation also clarifies the security window left by short-lived access tokens after a grant revoke, and recommends lowering `access_token_ttl` for tighter control. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
This PR clarifies and corrects documentation around OAuth token lifecycle, revocation behavior, and the new
disable_vk_identityconfiguration option. It also fixes inaccurate descriptions of how Bifrost handles upstream OAuth revocation, access token validation, and the distinction between revoking a grant versus deleting the bound identity.Changes
revokedas a documented OAuth config status value, clarifying that the config row is retained with no live token after a DELETE call.revoked. Users must revoke at the provider's dashboard if upstream invalidation is needed./mcpclients, with a cross-reference to the Gateway Authentication page.disable_vk_identity: rejects all virtual-key-mode grants at/mcpand on refresh, forcing re-authentication as a user.disable_vk_identityto the OpenAPI config schema with a description of its scope and conditions.RevokeOAuth2RefreshTokensByModecomment to be more precise about its purpose as a bulk-revoke utility.OAuth2ServerConfigto reference the OAuth Grants UI (not "Connected Clients UI") andDisableVKIdentity(notEnforceAuthOnInference) as the vk-mode revocation path.Type of change
Affected areas
How to test
Review the updated documentation pages for accuracy against the actual Bifrost OAuth implementation:
docs/mcp/auth/oauth.mdx— verify DELETE behavior and status values match implementation.docs/mcp/auth/per-user-oauth.mdx— verify the note correctly distinguishes upstream vs. inbound OAuth.docs/mcp/gateway-auth.mdx— verify token lifetime, revocation, anddisable_vk_identitydescriptions match behavior.docs/openapi/schemas/management/config.yaml— verifydisable_vk_identityschema matches the Go struct definition.Breaking changes
Security considerations
The documentation now explicitly states that Bifrost does not call upstream OAuth provider revocation endpoints on DELETE. Operators who assumed upstream tokens were being invalidated must be made aware they need to revoke tokens directly at the provider's dashboard. The updated revocation lifecycle documentation also clarifies the security window left by short-lived access tokens after a grant revoke, and recommends lowering
access_token_ttlfor tighter control.Checklist
docs/contributing/README.mdand followed the guidelines