feat: add -jwt flag for JWT bearer auth to mcp-test-client - #5909
Conversation
|
|
oauth_configs to new mcp_oauth_flows table
#5709
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/mcps/mcp-test-client/main.go (1)
95-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd table-driven tests for the credential contract.
Cover these cases:
-jwtsetsAuthorization: Bearer <jwt>.-bearerpreserves the existing behavior.- Both flags return an error before connecting.
Extract header construction and validation into a helper that returns an error. This makes the conflict path testable without testing
os.Exit.As per coding guidelines, behavior changes require deterministic, table-driven Go coverage.
🤖 Prompt for 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. In `@examples/mcps/mcp-test-client/main.go` around lines 95 - 101, Extract the credential validation and Authorization header construction from the main flow into a helper that returns an error, preserving the existing -jwt, -bearer, and mutually exclusive behavior without calling os.Exit. Add deterministic table-driven tests covering JWT headers, bearer headers, and conflicting flags returning an error before any connection attempt.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@examples/mcps/mcp-test-client/main.go`:
- Around line 95-101: Extract the credential validation and Authorization header
construction from the main flow into a helper that returns an error, preserving
the existing -jwt, -bearer, and mutually exclusive behavior without calling
os.Exit. Add deterministic table-driven tests covering JWT headers, bearer
headers, and conflicting flags returning an error before any connection attempt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d23e6c61-8a7a-43b5-b092-a5227d9e122f
📒 Files selected for processing (2)
examples/mcps/mcp-test-client/README.mdexamples/mcps/mcp-test-client/main.go
7a8136b to
d18cb99
Compare
1832ada to
1179fbb
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
d18cb99 to
996b11f
Compare
1179fbb to
0468d13
Compare
996b11f to
3ccecd9
Compare
0468d13 to
ab1b6a5
Compare
3ccecd9 to
2e779cd
Compare
ab1b6a5 to
1ef7197
Compare
Merge activity
|
The base branch was changed.
1ef7197 to
97031fe
Compare
Summary
Adds a
-jwtflag to the MCP test client, allowing a JWT to be passed as theAuthorization: Bearercredential. This provides a distinct flag for JWT-based authentication separate from the existing-bearerflag (which is intended for virtual keys), making the client's credential options more explicit and self-documenting.Changes
-jwtCLI flag that setsAuthorization: Bearer <jwt>on outbound requests-bearerand-jwt, since both write to the sameAuthorizationheader — the client exits with an error if both are providedheadersauth modeType of change
Affected areas
How to test
Run the test client with a JWT against a Bifrost MCP server configured for
headersorbothauth mode:Verify that passing both
-bearerand-jwttogether exits with an error:Breaking changes
Related issues
Security considerations
The
-jwtflag transmits a JWT as a bearer token over HTTP. Users should ensure the MCP server endpoint is served over TLS in any non-local environment to prevent token exposure in transit.Checklist
docs/contributing/README.mdand followed the guidelines