feat: add JWT Bearer authentication path for /mcp with session validation and context injection#4508
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds RS256 JWT authentication for the ChangesMCP JWT Authentication
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPServerHandler
participant getMCPServerForRequest
participant verifyMCPJWT
participant injectJWTContext
participant CredStore
Client->>MCPServerHandler: POST/GET /mcp (Authorization: Bearer eyJ...)
MCPServerHandler->>getMCPServerForRequest: fasthttp.RequestCtx
getMCPServerForRequest->>verifyMCPJWT: rawToken + Config store
verifyMCPJWT-->>getMCPServerForRequest: jwtMCPClaims (bf_mode, scope, aud)
getMCPServerForRequest-->>MCPServerHandler: mcpAuthResult{server, claims, vk}
MCPServerHandler->>injectJWTContext: bifrostCtx + claims + vk
injectJWTContext-->>MCPServerHandler: sets UserID/VK/SessionID + JWTAuthenticated=true
MCPServerHandler->>CredStore: ConnectionHeaders(ctx)
CredStore->>CredStore: check JWTAuthenticated + SessionValidated
alt Session not validated
CredStore-->>MCPServerHandler: error (identity not verified)
MCPServerHandler-->>Client: 401 Unauthorized
else Session validated
CredStore-->>MCPServerHandler: upstream auth headers
MCPServerHandler-->>Client: MCP response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/bifrost-http/handlers/mcpserver.go`:
- Around line 598-607: The issue is that the code unconditionally rejects
user-mode JWT calls lacking a browser session by returning an error when
sessionUserID is empty, which prevents valid JWTs from reaching per-user
resolvers. Instead, restructure the MCPAuthModeUser check to only validate the
session when one exists: remove the early return error when sessionUserID is
empty, and only perform the session matching validation and set the
BifrostContextKeyOAuth2JWTSessionValidated flag if sessionUserID is not empty.
This allows valid user-mode JWTs without a dashboard session to proceed and lets
upstream OAuth/header resolvers apply their own authentication guards.
- Around line 667-684: The ensureVKMCPServerByValue and ensureVKMCPServer
methods lack a security check for inactive virtual keys that exists in the JWT
path. After calling GetVirtualKeyByValue in the ensureVKMCPServer method (around
line 698), add a validation check using vk.IsActiveValue() to ensure the virtual
key is active before caching or returning the server. If the virtual key is not
active, return an error with an appropriate message to fail closed on inactive
keys, matching the pattern used in other handlers like oauth2_consent.go and
governance/resolver.go.
In `@transports/bifrost-http/handlers/oauth2_jwt.go`:
- Around line 64-72: In the ParseWithClaims call with the key validation
callback function, replace the broad RSA family check (currently checking
*jwt.SigningMethodRSA) with a specific RS256 algorithm check by verifying
t.Method.Alg() equals "RS256" or using the specific RS256 signing method type.
Additionally, remove the jwt.WithIssuedAt() option and instead add explicit
validation within the callback function to ensure the iat (issued at) claim is
present and non-zero in the claims struct before returning the public key,
rejecting tokens that lack this required claim.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bed1a51b-929f-4dda-94db-038b2f0c1b73
📒 Files selected for processing (5)
core/mcp/credstore/per_user_headers.gocore/mcp/credstore/per_user_oauth.gocore/schemas/bifrost.gotransports/bifrost-http/handlers/mcpserver.gotransports/bifrost-http/handlers/oauth2_jwt.go
d6a46f9 to
5d1f810
Compare
df31ae6 to
6ebebaf
Compare
5d1f810 to
bc90e18
Compare
6ebebaf to
7a09a9d
Compare
bc90e18 to
39907be
Compare
2be7d78 to
4f9b35b
Compare
1f9653e to
b156095
Compare
4f9b35b to
76885d3
Compare
76885d3 to
d82df7b
Compare
b156095 to
8a84174
Compare

Summary
This PR adds JWT Bearer token authentication to the
/mcpendpoint, enabling Bifrost-issued JWTs to be used as a first-class authentication mechanism alongside existing VK/header credentials. It also introduces a session validation gate that prevents per-user upstream flows (OAuth and header submission) from being initiated unless the caller's identity has been actively verified for the current request.Changes
extractBearerJWTto detect and extract JWT Bearer tokens from theAuthorizationheader, distinguishing them from VK credentials by theeyJprefix.verifyMCPJWTto validate RS256-signed JWTs against the active signing key, enforcing expiry, issued-at, key ID, and RFC 8707 audience checks against the/mcpresource URL.injectJWTContextto translate verified JWT claims (bf_mode=user|vk|session) into the sameBifrostContextkeys that header-based auth sets, ensuring downstream resolvers (governance, per-user OAuth, tool-group filtering) work without modification.mcpAuthResultto carry the authenticated MCP server, JWT claims, and resolved VK out ofgetMCPServerForRequest, replacing the previous single return value.getMCPServerForRequestto implement a prioritized auth flow: JWT → oauth-strict rejection → VK/header → anonymous dev mode. Inoauthstrict mode, non-JWT requests are rejected with aWWW-Authenticateheader per RFC 9728.BifrostContextKeyOAuth2JWTAuthenticatedandBifrostContextKeyOAuth2JWTSessionValidatedcontext keys. The session-validated key is set only when a user-mode JWT'ssubis confirmed against an active dashboard session.per_user_oauth.goandper_user_headers.go: when a request is JWT-authenticated in user mode, upstream flows cannot be initiated unlessBifrostContextKeyOAuth2JWTSessionValidatedis present.ensureVKMCPServerByValueto allow VK server lookup by value from both the header path and the JWT VK path.Type of change
Affected areas
How to test
go version go test ./.../mcpwith a valid Bifrost-issued JWT inAuthorization: Bearer <token>and confirm the appropriate context keys are set and the correct scoped server is used.bf_mode=userJWT and no active dashboard session; confirm a401is returned with a message indicating no active session.bf_mode=userJWT and a mismatched session user; confirm rejection.MCPServerAuthMode=oauthand send a request with a VK header; confirm rejection with aWWW-Authenticateheader.EnforceAuthOnInference=false, no credentials) still works.Breaking changes
getMCPServerForRequestnow returns*mcpAuthResultinstead of*server.MCPServer. Any code calling this method directly must be updated. WhenMCPServerAuthModeis set tooauth, header-based credentials are no longer accepted at the/mcpendpoint.Security considerations
/mcpresource URL, preventing token reuse across resources.bf_sub, preventing replay of valid tokens after session expiry.WWW-Authenticateheaders are emitted on auth failures in discovery-enabled modes, per RFC 9728.Checklist
docs/contributing/README.mdand followed the guidelines