fix: FastMCP compatibility by joar · Pull Request #2266 · IBM/mcp-context-forge

joar · 2026-01-21T12:14:04Z

🐛 Bug-fix PR

Before opening this PR please:

make lint - passes ruff, mypy, pylint
make test - all unit + integration tests green
make coverage - ≥ 90 %
make docker docker-run-ssl or make podman podman-run-ssl
Update relevant documentation.
Tested with sqlite and postgres + redis.
Manual regression no longer fails. Ensure the UI and /version work correctly.

📌 Summary

What problem does this PR fix and why?

🔁 Reproduction Steps

Link the issue and minimal steps to reproduce the bug.

🐞 Root Cause

What was wrong and where?

💡 Fix Description

How did you solve it? Key design points.

🧪 Verification

Check	Command	Status
Lint suite	`make lint`
Unit tests	`make test`
Coverage ≥ 90 %	`make coverage`
Manual regression no longer fails	steps / screenshots

📐 MCP Compliance (if relevant)

Matches current MCP spec
No breaking change to MCP clients

✅ Checklist

Code formatted (make black isort pre-commit)
No secrets/credentials committed

crivetimihai · 2026-01-24T15:17:26Z

Hi @joar - thanks for filing this PR. What issue does it resolve, and can you share how to reproduce please? Please change to "Ready for review" as well when we should review & merge this PR. Thank you!

crivetimihai · 2026-01-25T00:45:04Z

Thanks for this PR @joar! Here's my understanding of what this fixes:

Summary

This PR addresses two OAuth/DCR compatibility issues when MCP Gateway acts as an OAuth client registering with upstream authorization servers:

1. Trailing Slash Normalization for OAuth Discovery

Problem: The MCP Python SDK (used by FastMCP) has a bug where Pydantic's AnyHttpUrl automatically appends trailing slashes to bare hostnames. For example, http://localhost:8000 becomes http://localhost:8000/.

Per RFC 8414 §3.3 and RFC 9728 §3, the issuer URL in metadata responses must exactly match the URL used for discovery. This mismatch causes OAuth discovery to fail in clients like Google's ADK and this gateway.

Fix: Strip trailing slashes before constructing .well-known discovery URLs, following RFC 8414 Section 3.1:

discovery_base_url = issuer.rstrip("/")
rfc8414_url = f"{discovery_base_url}/.well-known/oauth-authorization-server"

This works around the upstream SDK issue: modelcontextprotocol/python-sdk#1919

2. Refresh Token Grant Type

Adds "refresh_token" to the list of grant types during DCR, enabling registered OAuth clients to obtain refresh tokens for long-lived sessions.

I'll rebase, and address the rest of the findings, leave it with me as this requires alembic migration.

The original trailing slash fix introduced a bug where the issuer validation would fail when the server returned an issuer without trailing slash but the client passed one (or vice versa). Changes: - Normalize both the input issuer and metadata issuer for comparison - Use normalized issuer as cache key for consistent cache lookup - Add tests for trailing slash normalization scenarios - Update test to expect refresh_token in grant_types Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

Address review feedback: 1. Normalize issuer consistently across the entire DCR flow: - Allowlist validation uses normalized comparison - Storage uses normalized issuer - Lookup uses normalized issuer 2. Make refresh_token conditional on AS support: - Check grant_types_supported in AS metadata - Only request refresh_token if AS advertises support 3. Fix grant_types fallback: - Use requested grant_types as fallback when AS response omits them - Previously hardcoded to ["authorization_code"] which dropped refresh_token 4. Add comprehensive tests: - Test refresh_token inclusion when AS supports it - Test grant_types fallback behavior - Test allowlist trailing slash normalization Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

crivetimihai · 2026-01-25T01:42:50Z

Rebased and Extended

Thanks for the initial fix @joar! I've rebased your branch onto main and extended it to address additional edge cases discovered during review.

Commits Added

Commit	Description
`387b1c6`	Your original fix (preserved)
`ac85029`	Fix issuer validation bug - compare normalized URLs on both sides
`488b288`	Complete issuer normalization across DCR flow
`342586c`	Handle null `grant_types_supported` + add migration

Changes Summary

1. Issuer Normalization (Complete)

Discovery URLs: ✅ (your fix)
Cache keys: ✅ normalized for consistent lookup
Issuer validation: ✅ normalize both input and metadata issuer
Allowlist comparison: ✅ normalize both sides
Database storage: ✅ store normalized issuer
Database lookup: ✅ query with normalized issuer

2. Refresh Token Handling (Improved)

Changed from unconditional to conditional based on grant_types_supported
Prevents DCR failures with strict AS servers that reject unsupported grant types
Added config option MCPGATEWAY_DCR_REQUEST_REFRESH_TOKEN_WHEN_UNSUPPORTED=true for permissive mode

3. Bug Fixes

Fixed TypeError when AS returns {"grant_types_supported": null} (explicit null vs missing key)
Fixed grant_types fallback to use requested values instead of hardcoded ["authorization_code"]

4. Database Migration

Added Alembic migration to normalize existing issuer values in registered_oauth_clients
Prevents duplicate registrations from legacy rows with trailing slashes
Idempotent, works with SQLite and PostgreSQL

New Tests Added (7 total)

Trailing slash normalization in discovery
Cache key normalization
Allowlist trailing slash matching
refresh_token inclusion when AS supports it
Grant types fallback behavior
Null grant_types_supported handling
Permissive refresh token mode

Test Results

30 DCR tests pass
300 OAuth/DCR related tests pass

Ready for final review!

…igration Address additional review findings: 1. Fix TypeError when grant_types_supported is explicit null: - Use `metadata.get("grant_types_supported") or []` instead of `metadata.get("grant_types_supported", [])` - The latter returns None when key exists with null value 2. Add configurable permissive refresh_token mode: - New setting: dcr_request_refresh_token_when_unsupported - Default: False (strict mode - only request if AS advertises support) - When True: request refresh_token if AS omits grant_types_supported 3. Add Alembic migration to normalize legacy issuer values: - Strips trailing slashes from registered_oauth_clients.issuer - Idempotent and works with SQLite and PostgreSQL - Prevents duplicate registrations from legacy rows 4. Add comprehensive tests: - Test explicit null grant_types_supported handling - Test permissive refresh_token mode Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

Update documentation for new DCR refresh token configuration option: - README.md: Add to DCR settings table - charts/mcp-stack/values.yaml: Add with comment - charts/mcp-stack/README.md: Regenerated via helm-docs - docs/docs/manage/dcr.md: Add env var and behavior note - docs/docs/config.schema.json: Add schema definition Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix: FastMCP compatibility * fix: normalize issuer URL for metadata validation and caching The original trailing slash fix introduced a bug where the issuer validation would fail when the server returned an issuer without trailing slash but the client passed one (or vice versa). Changes: - Normalize both the input issuer and metadata issuer for comparison - Use normalized issuer as cache key for consistent cache lookup - Add tests for trailing slash normalization scenarios - Update test to expect refresh_token in grant_types Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: complete issuer normalization and conditional refresh_token Address review feedback: 1. Normalize issuer consistently across the entire DCR flow: - Allowlist validation uses normalized comparison - Storage uses normalized issuer - Lookup uses normalized issuer 2. Make refresh_token conditional on AS support: - Check grant_types_supported in AS metadata - Only request refresh_token if AS advertises support 3. Fix grant_types fallback: - Use requested grant_types as fallback when AS response omits them - Previously hardcoded to ["authorization_code"] which dropped refresh_token 4. Add comprehensive tests: - Test refresh_token inclusion when AS supports it - Test grant_types fallback behavior - Test allowlist trailing slash normalization Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: handle null grant_types_supported and add issuer normalization migration Address additional review findings: 1. Fix TypeError when grant_types_supported is explicit null: - Use `metadata.get("grant_types_supported") or []` instead of `metadata.get("grant_types_supported", [])` - The latter returns None when key exists with null value 2. Add configurable permissive refresh_token mode: - New setting: dcr_request_refresh_token_when_unsupported - Default: False (strict mode - only request if AS advertises support) - When True: request refresh_token if AS omits grant_types_supported 3. Add Alembic migration to normalize legacy issuer values: - Strips trailing slashes from registered_oauth_clients.issuer - Idempotent and works with SQLite and PostgreSQL - Prevents duplicate registrations from legacy rows 4. Add comprehensive tests: - Test explicit null grant_types_supported handling - Test permissive refresh_token mode Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docs: add DCR_REQUEST_REFRESH_TOKEN_WHEN_UNSUPPORTED to documentation Update documentation for new DCR refresh token configuration option: - README.md: Add to DCR settings table - charts/mcp-stack/values.yaml: Add with comment - charts/mcp-stack/README.md: Regenerated via helm-docs - docs/docs/manage/dcr.md: Add env var and behavior note - docs/docs/config.schema.json: Add schema definition Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>

crivetimihai added this to the Release 1.0.0-RC1 milestone Jan 22, 2026

crivetimihai added the blocked Blocked by some other predecessor issue or PR. See notes. label Jan 24, 2026

joar and others added 3 commits January 25, 2026 00:49

fix: FastMCP compatibility

387b1c6

crivetimihai self-assigned this Jan 25, 2026

crivetimihai force-pushed the fastmcp-compatibility branch from 8126f30 to 342586c Compare January 25, 2026 01:42

crivetimihai marked this pull request as ready for review January 25, 2026 01:43

crivetimihai requested review from crivetimihai, kevalmahajan and madhav165 as code owners January 25, 2026 01:43

crivetimihai force-pushed the fastmcp-compatibility branch from 342586c to 0549e0e Compare January 25, 2026 01:48

crivetimihai approved these changes Jan 25, 2026

View reviewed changes

crivetimihai merged commit 3db08a3 into IBM:main Jan 25, 2026
52 checks passed

crivetimihai removed the blocked Blocked by some other predecessor issue or PR. See notes. label Jan 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: FastMCP compatibility#2266

fix: FastMCP compatibility#2266
crivetimihai merged 5 commits intoIBM:mainfrom
joar:fastmcp-compatibility

joar commented Jan 21, 2026

Uh oh!

crivetimihai commented Jan 24, 2026

Uh oh!

crivetimihai commented Jan 25, 2026 •

edited

Loading

Uh oh!

crivetimihai commented Jan 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

joar commented Jan 21, 2026

🐛 Bug-fix PR

📌 Summary

🔁 Reproduction Steps

🐞 Root Cause

💡 Fix Description

🧪 Verification

📐 MCP Compliance (if relevant)

✅ Checklist

Uh oh!

crivetimihai commented Jan 24, 2026

Uh oh!

crivetimihai commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Trailing Slash Normalization for OAuth Discovery

2. Refresh Token Grant Type

Uh oh!

crivetimihai commented Jan 25, 2026

Rebased and Extended

Commits Added

Changes Summary

New Tests Added (7 total)

Test Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

crivetimihai commented Jan 25, 2026 •

edited

Loading