feat: add token-exchange demo MCP server for testing against real identity providers - #5730
Conversation
oauth_configs to new mcp_oauth_flows table
#5709
|
|
4cec69f to
768ee94
Compare
4379093 to
33dcddc
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. |
768ee94 to
d3c936f
Compare
33dcddc to
0f63fe2
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. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@examples/mcps/token-exchange-demo-server/go.mod`:
- Line 7: Update the dependency pins in the module configuration: raise
github.com/go-jose/go-jose/v4 to v4.1.4 or later and github.com/buger/jsonparser
to v1.1.2 or later, preserving the rest of the module graph.
In `@examples/mcps/token-exchange-demo-server/main_test.go`:
- Around line 29-34: Remove the unused signKey field from the testIdP struct,
leaving the server, key, and issuer fields unchanged.
- Line 204: Update the request setup in the relevant test to use
httptest.NewRequestWithContext with context.Background() instead of
httptest.NewRequest, preserving the existing POST method, URL, and body
arguments.
In `@examples/mcps/token-exchange-demo-server/main.go`:
- Around line 171-180: Update the Authorization handling around rawToken
extraction to match the Bearer scheme case-insensitively while preserving token
trimming and unauthorized responses for other schemes. Add a regression case
covering a lowercase “bearer <token>” header.
🪄 Autofix
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: da7bb648-990b-4749-946e-1d81a417d90b
⛔ Files ignored due to path filters (2)
core/go.sumis excluded by!**/*.sumexamples/mcps/token-exchange-demo-server/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
docs/mcp/auth/token-exchange.mdxexamples/mcps/token-exchange-demo-server/README.mdexamples/mcps/token-exchange-demo-server/go.modexamples/mcps/token-exchange-demo-server/main.goexamples/mcps/token-exchange-demo-server/main_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/mcps/token-exchange-demo-server/README.md
- docs/mcp/auth/token-exchange.mdx
|
|
||
| require ( | ||
| github.com/coreos/go-oidc/v3 v3.16.0 | ||
| github.com/go-jose/go-jose/v4 v4.1.3 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
cd examples/mcps/token-exchange-demo-server
for module in \
github.com/go-jose/go-jose/v4 \
github.com/buger/jsonparser
do
echo "=== $module ==="
go list -m -u -json "$module"
go mod why -m "$module"
done
govulncheck ./...Repository: maximhq/bifrost
Length of output: 2125
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '=== repository state ==='
git status --short
git diff --stat
git branch --all --no-color | head -50
printf '%s\n' '=== module files ==='
cat -n examples/mcps/token-exchange-demo-server/go.mod
printf '%s\n' '=== dependency references ==='
rg -n --hidden --glob '!vendor/**' \
'go-jose|jsonparser|mcp-go|jsonschema|go-oidc|Bearer|bearer|JWT|JWE|token' \
examples/mcps/token-exchange-demo-server
printf '%s\n' '=== stack metadata ==='
find .. -maxdepth 3 -type f \( -iname '*stack*' -o -name '.gitmodules' \) -printRepository: maximhq/bifrost
Length of output: 14415
🌐 Web query:
OSV github.com/go-jose/go-jose/v4 v4.1.3 JWE decryption panic fixed version
💡 Result:
The fixed version for the JWE decryption panic in github.com/go-jose/go-jose/v4 is v4.1.4 [1][2]. This vulnerability, tracked as GHSA-78h2-9frx-2jm8 and CVE-2026-34986, occurs when decrypting a JSON Web Encryption (JWE) object if the alg field specifies a key wrapping algorithm (excluding A128GCMKW, A192GCMKW, and A256GCMKW) and the encrypted_key field is empty [1][3][4]. The issue causes a panic during the key unwrap process, which can lead to a denial-of-service [1][4]. If updating to version 4.1.4 is not immediately possible, you can mitigate the risk by ensuring your application does not accept JWE tokens containing key wrapping algorithms if they are not required, or by pre-validating JWE objects to ensure the encrypted_key field is non-empty [1][3].
Citations:
- 1: GHSA-78h2-9frx-2jm8
- 2: https://github.com/go-jose/go-jose/releases/tag/v4.1.4
- 3: https://osv.dev/vulnerability/GHSA-78h2-9frx-2jm8
- 4: https://pkg.go.dev/vuln/GO-2026-4945
🌐 Web query:
OSV github.com/buger/jsonparser v1.1.1 denial of service fixed version
💡 Result:
The denial of service vulnerability in github.com/buger/jsonparser version 1.1.1, identified as CVE-2026-32285 (also tracked as GO-2026-4514 and GHSA-6g7g-w4f8-9c9x), is fixed in version 1.1.2 [1][2][3]. This vulnerability occurs because the Delete function fails to properly validate offsets when processing malformed JSON input, which can result in a negative slice index, leading to a runtime panic and denial of service [1][2][4]. Users are advised to upgrade to version 1.1.2 or later to address this issue [1][2][3].
Citations:
- 1: https://osv.dev/vulnerability/CVE-2026-32285
- 2: GHSA-6g7g-w4f8-9c9x
- 3: fix: prevent panic on negative slice index in Delete with malformed JSON (GO-2026-4514) buger/jsonparser#276
- 4: Panic in Delete() via slice bounds out of range [-1:] on malformed input (v1.1.1) buger/jsonparser#275
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
modroot="$(go env GOPATH)/pkg/mod"
printf '%s\n' '=== owning module requirements ==='
for f in \
"$modroot/github.com/coreos/go-oidc/v3@v3.16.0/go.mod" \
"$modroot/github.com/mark3labs/mcp-go@v0.43.2/go.mod" \
"$modroot/github.com/invopop/jsonschema@v0.13.0/go.mod" \
"$modroot/github.com/wk8/go-ordered-map/v2@v2.1.8/go.mod"
do
if test -f "$f"; then
echo "--- $f"
cat "$f"
fi
done
printf '%s\n' '=== vulnerable API usage in cached modules ==='
rg -n --glob '*.go' \
'jsonparser\.(Delete|Get|ArrayEach|ObjectEach|Parse|Set)|go-jose|jose\.(Parse|Decrypt|DecryptMulti|ParseEncrypted)|ParseEncrypted|Decrypt' \
"$modroot/github.com/coreos/go-oidc" \
"$modroot/github.com/mark3labs/mcp-go" \
"$modroot/github.com/invopop/jsonschema" \
"$modroot/github.com/wk8/go-ordered-map" 2>/dev/null || true
printf '%s\n' '=== vulnerable implementation excerpts ==='
rg -n -A35 -B10 \
'func Delete|func \(.*\) Decrypt|func \(.*\) DecryptMulti|func ParseEncrypted' \
"$modroot/github.com/buger/jsonparser@v1.1.1" \
"$modroot/github.com/go-jose/go-jose/v4@v4.1.3" 2>/dev/null || true
printf '%s\n' '=== available versions ==='
go list -m -versions github.com/buger/jsonparser
go list -m -versions github.com/go-jose/go-jose/v4Repository: maximhq/bifrost
Length of output: 50373
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
modroot="$(go env GOPATH)/pkg/mod"
printf '%s\n' '=== application production imports and handlers ==='
sed -n '40,235p' examples/mcps/token-exchange-demo-server/main.go
printf '%s\n' '=== go-oidc token parsing path ==='
sed -n '190,245p' "$modroot/github.com/coreos/go-oidc/v3@v3.16.0/oidc/verify.go"
sed -n '1,55p' "$modroot/github.com/coreos/go-oidc/v3@v3.16.0/oidc/jose.go"
printf '%s\n' '=== jsonparser call sites in non-test cached code ==='
rg -n --glob '*.go' --glob '!**/*_test.go' \
'github.com/buger/jsonparser|jsonparser\.' \
"$modroot/github.com/mark3labs/mcp-go@v0.43.2" \
"$modroot/github.com/invopop/jsonschema@v0.13.0" \
"$modroot/github.com/wk8/go-ordered-map/v2@v2.1.8" \
"$modroot/github.com/mailru/easyjson@v0.7.7" 2>/dev/null || true
printf '%s\n' '=== all Delete call sites in the dependency graph ==='
rg -n --glob '*.go' --glob '!**/*_test.go' \
'jsonparser\.Delete|jsonparser\.Set|jsonparser\.ObjectEach|jsonparser\.ArrayEach' \
"$modroot/github.com/mark3labs/mcp-go@v0.43.2" \
"$modroot/github.com/invopop/jsonschema@v0.13.0" \
"$modroot/github.com/wk8/go-ordered-map/v2@v2.1.8" \
"$modroot/github.com/mailru/easyjson@v0.7.7" 2>/dev/null || true
printf '%s\n' '=== direct go-jose imports in application ==='
rg -n --glob '*.go' \
'github.com/go-jose/go-jose/v4|josejwt|ParseEncrypted|Decrypt' \
examples/mcps/token-exchange-demo-serverRepository: maximhq/bifrost
Length of output: 12349
Update the vulnerable dependency pins before merge.
Upgrade github.com/go-jose/go-jose/v4 to v4.1.4 or later and github.com/buger/jsonparser to v1.1.2 or later. These versions remain in the module graph, although this server does not reach the vulnerable JWE decryption or jsonparser.Delete paths.
🤖 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/token-exchange-demo-server/go.mod` at line 7, Update the
dependency pins in the module configuration: raise github.com/go-jose/go-jose/v4
to v4.1.4 or later and github.com/buger/jsonparser to v1.1.2 or later,
preserving the rest of the module graph.
Source: Linters/SAST tools
| type testIdP struct { | ||
| server *httptest.Server | ||
| key *rsa.PrivateKey | ||
| issuer string | ||
| signKey *josejwt.SigningKey | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused signKey field.
testIdP.signKey is never assigned or read. The supplied golangci-lint result reports this as an error.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 33-33: field signKey is unused
(unused)
🤖 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/token-exchange-demo-server/main_test.go` around lines 29 - 34,
Remove the unused signKey field from the testIdP struct, leaving the server,
key, and issuer fields unchanged.
Source: Linters/SAST tools
| }) | ||
| handler := bearerVerifyMiddleware(verifier)(downstream) | ||
|
|
||
| req := httptest.NewRequest(http.MethodPost, "/", nil) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- stack and worktree ---'
git status --short
git branch --show-current
git log --oneline -5
printf '%s\n' '--- target file context ---'
sed -n '180,220p' examples/mcps/token-exchange-demo-server/main_test.go
printf '%s\n' '--- module files ---'
find examples/mcps/token-exchange-demo-server -maxdepth 2 -name go.mod -o -name go.work -print
find . -maxdepth 3 -name go.mod -print
printf '%s\n' '--- httptest request usages in target module ---'
rg -n 'httptest\.NewRequest|NewRequestWithContext|noctx' examples/mcps/token-exchange-demo-server
printf '%s\n' '--- context key usage in target module ---'
rg -n 'context\.WithValue|\.Value\(' examples/mcps/token-exchange-demo-server || true
printf '%s\n' '--- noctx configuration/rule references ---'
rg -n 'noctx|NewRequestWithContext|httptest\.NewRequest' .github .golangci* Makefile examples 2>/dev/null | head -200 || trueRepository: maximhq/bifrost
Length of output: 3735
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- MCP example files and module boundaries ---'
find examples/mcps -maxdepth 3 -type f -print | sort
printf '%s\n' '--- all Go module/workspace files ---'
find . -type f \( -name go.mod -o -name go.work \) -print | sort
printf '%s\n' '--- target imports and context-key declaration ---'
sed -n '1,80p' examples/mcps/token-exchange-demo-server/main_test.go
sed -n '1,70p' examples/mcps/token-exchange-demo-server/main.go
sed -n '190,250p' examples/mcps/token-exchange-demo-server/main.go
printf '%s\n' '--- current commit and parent changes for target file ---'
git show --stat --oneline HEAD
git diff HEAD^ HEAD -- examples/mcps/token-exchange-demo-server/main_test.go
git diff HEAD^ HEAD -- examples/mcps/token-exchange-demo-server/main.go
printf '%s\n' '--- lint configuration references ---'
find . -maxdepth 3 -type f \( -iname '*golangci*' -o -iname '*lint*' -o -name '.go-version' \) -print | sort
rg -n --hidden --glob '!/.git/**' '\bnoctx\b|golangci-lint|linters:' . | head -250 || trueRepository: maximhq/bifrost
Length of output: 34573
🌐 Web query:
noctx analyzer httptest.NewRequest golangci-lint documentation
💡 Result:
The noctx linter in golangci-lint is designed to detect function calls that perform operations—such as HTTP requests—without providing a context.Context [1][2][3]. When you encounter a noctx warning for httptest.NewRequest, it is because the linter expects you to use httptest.NewRequestWithContext instead [4][5]. This requirement applies to net/http functions as well, such as http.NewRequest, which the linter mandates be replaced with http.NewRequestWithContext [4][6]. If you believe these warnings are inappropriate—for example, when using httptest.NewRequest in test files where context.Background() is acceptable—you can address them using the following methods: 1. Exclude test files: You can configure golangci-lint to exclude noctx checks for test files in your .golangci.yml file [7]: issues: exclude-rules: - path: _test.go linters: - noctx 2. Filter by warning message: You can also exclude specific warnings using regex matching [6]: issues: exclude-rules: - path: (.+).go$ text: net/http/httptest.NewRequest must not be called. use net/http/httptest.NewRequestWithContext These configurations allow you to keep noctx enabled for production code while suppressing it where it may cause unnecessary friction in your test suite [7][6].
Citations:
- 1: https://github.com/sonatard/noctx
- 2: Add Noctx golangci/golangci-lint#1179
- 3: https://golangci-lint.run/docs/linters/
- 4: https://github.com/sonatard/noctx/blob/master/noctx.go
- 5: https://github.com/rancher/fleet/actions/runs/23037400304
- 6: Fix for noctx lint issues kubermatic/dashboard#7544
- 7: Ignore requests in tests sonatard/noctx#6
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target module metadata ---'
cat examples/mcps/token-exchange-demo-server/go.mod
printf '%s\n' '--- pre-commit Go lint hook ---'
sed -n '1,30p' .pre-commit-config.yaml
printf '%s\n' '--- target module tracked files and dependency references ---'
git ls-files examples/mcps/token-exchange-demo-server
rg -n 'github\.com/(coreos/go-oidc|go-jose/go-jose|mark3labs/mcp-go)' examples/mcps/token-exchange-demo-server
printf '%s\n' '--- read-only tidy diff from the target module ---'
if command -v go >/dev/null 2>&1; then
(cd examples/mcps/token-exchange-demo-server && go mod tidy -diff)
else
printf '%s\n' 'go tool unavailable'
fiRepository: maximhq/bifrost
Length of output: 3972
Use an explicit request context.
The noctx linter rejects httptest.NewRequest. Replace it with httptest.NewRequestWithContext(context.Background(), ...).
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 204-204: net/http/httptest.NewRequest must not be called. use net/http/httptest.NewRequestWithContext
(noctx)
🤖 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/token-exchange-demo-server/main_test.go` at line 204, Update
the request setup in the relevant test to use httptest.NewRequestWithContext
with context.Background() instead of httptest.NewRequest, preserving the
existing POST method, URL, and body arguments.
Sources: Coding guidelines, Linters/SAST tools
| h := r.Header.Get("Authorization") | ||
| if h == "" { | ||
| unauthorized("missing Authorization header") | ||
| return | ||
| } | ||
| if !strings.HasPrefix(h, "Bearer ") { | ||
| unauthorized("Authorization scheme must be Bearer") | ||
| return | ||
| } | ||
| rawToken := strings.TrimSpace(strings.TrimPrefix(h, "Bearer ")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file="examples/mcps/token-exchange-demo-server/main.go"
sed -n '145,205p' "$file"
printf '\n--- related tests and references ---\n'
rg -n --glob '*.go' 'token-exchange-demo-server|Authorization scheme must be Bearer|HasPrefix\(h, "Bearer "|rawToken|EqualFold\(.*Bearer' .Repository: maximhq/bifrost
Length of output: 4113
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- repository state ---'
git status --short
git branch --show-current
printf '%s\n' '--- recent commit summary ---'
git log --oneline -8
printf '%s\n' '--- imports and middleware references ---'
sed -n '1,45p' examples/mcps/token-exchange-demo-server/main.go
rg -n --glob '*.go' 'bearerVerifyMiddleware\(' examples/mcps
printf '%s\n' '--- parser behavior for representative headers ---'
python3 - <<'PY'
headers = [
"Bearer token",
"bearer token",
"BEARER token",
"Bearer token",
"Bearer\ttoken",
"Bearer token extra",
"Bearer ",
" Bearer token ",
]
for h in headers:
parts = h.split()
accepted = len(parts) == 2 and parts[0].lower() == "bearer"
print(repr(h), "=>", parts, "accepted_by_proposed_parser=", accepted)
PYRepository: maximhq/bifrost
Length of output: 4211
🌐 Web query:
RFC 9110 Authorization header authentication scheme case-insensitive whitespace grammar Bearer
💡 Result:
In the context of the Authorization header, the authentication scheme (such as "Bearer") is case-insensitive, while the handling of whitespace is governed by specific HTTP grammar rules [1][2][3]. Authentication Scheme Case Sensitivity According to RFC 9110, the authentication scheme is defined as a token, and authentication schemes are matched in a case-insensitive manner [4][1]. Although IANA and some specifications may present schemes like "Bearer" in title case, implementations must treat these identifiers as case-insensitive to ensure compliance [2][3]. Therefore, "Bearer", "bearer", and "BEARER" are equivalent and valid [3]. Whitespace Grammar The syntax for the Authorization header is defined as follows [5]: Authorization = credentials credentials = auth-scheme 1SP ( token68 / #auth-param) The grammar uses 1SP to specify the separator between the authentication scheme and the credentials [5]. In RFC 9110 terminology, this corresponds to RWS (Required Whitespace), which is defined as one or more spaces (SP) or horizontal tabs (HTAB) [6]. Summary of Key Points: - Case Sensitivity: The auth-scheme token is case-insensitive [1][2]. Servers and clients must handle variations in casing (e.g., "Bearer" vs "bearer") interchangeably [3]. - Whitespace: The specification requires at least one space (or tab) as a separator between the scheme and the credentials [6][5]. While RWS allows for multiple spaces or tabs, common practice and interoperability recommendations suggest using a single space (SP) for readability and consistency [6][7].
Citations:
- 1: https://httpwg.org/specs/rfc7235.html
- 2: case sensitivity of Bearer http authentication scheme oauth-wg/oauth-v2-1#166
- 3: https://auth0.com/blog/the-bearer-token-case/
- 4: https://www.rfc-editor.org/rfc/rfc9110.html
- 5: https://grammars.wiki/catalog/rfc9110-http.html
- 6: https://datatracker.ietf.org/doc/html/rfc9110
- 7: https://www.mail-archive.com/oauth@ietf.org/msg24655.html
🏁 Script executed:
#!/bin/bash
set -e
file="examples/mcps/token-exchange-demo-server/main_test.go"
printf '%s\n' '--- test structure ---'
sed -n '1,280p' "$file"
printf '%s\n' '--- all Authorization test cases ---'
rg -n -C 4 'Authorization|Bearer|unauthorized|StatusUnauthorized|bearerVerifyMiddleware' "$file"Repository: maximhq/bifrost
Length of output: 13382
Accept case-insensitive Bearer schemes.
HTTP authentication scheme names are case-insensitive. Replace the case-sensitive prefix check with case-insensitive scheme matching. Add a bearer <token> regression case.
🤖 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/token-exchange-demo-server/main.go` around lines 171 - 180,
Update the Authorization handling around rawToken extraction to match the Bearer
scheme case-insensitively while preserving token trimming and unauthorized
responses for other schemes. Add a regression case covering a lowercase “bearer
<token>” header.
d3c936f to
70ce9d2
Compare
0f63fe2 to
e141ca5
Compare
Merge activity
|
…r token-exchange demo server
e141ca5 to
c0c792f
Compare
Summary
Briefly explain the purpose of this PR and the problem it solves.
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines