fix: MCP OAuth 2.1 token exchange and multi-node propagation (#20076)

* sequential

* zero default

* fix

* fix: preserve absolute paths in sqlite+sqlcipher URLs

Previously, the connection logic incorrectly stripped the leading slash
from `sqlite+sqlcipher` paths, forcibly converting absolute paths
(e.g., `sqlite+sqlcipher:////app/data.db`) into relative paths
(which became `app/data.db`). This caused database initialization failures
when using absolute paths, such as with Docker volume mounts.
This change removes the slash-stripping logic, ensuring that absolute
path conventions (starting with `/`) are respected while maintaining
support for relative paths (which do not start with `/`).

* fix: MCP OAuth 2.1 token exchange and multi-node propagation

Fix two MCP OAuth 2.1 bugs affecting tool server authentication:

1. Token exchange failing with duplicate credentials (#19823)
   - Removed explicit client_id/client_secret passing in handle_callback()
   - Authlib already has credentials configured during add_client(),
     passing them again caused concatenation (e.g., "ID1,ID1") and 401 errors
   - Added token validation to detect missing access_token and provide
     clear error messages instead of cryptic database constraint errors

2. OAuth clients not propagating across multi-node setups (#19901)
   - Updated get_client() and get_client_info() to auto-lazy-load
     OAuth clients from the Redis-synced TOOL_SERVER_CONNECTIONS config
   - Clients are now instantiated on-demand on any node that needs them

Fixes #19823, #19901

* Update db.py

* Update wrappers.py
This commit is contained in:
Classic298 2025-12-21 16:51:52 +01:00 committed by GitHub
parent f826d3ed75
commit ef43e81f9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -577,10 +577,14 @@ class OAuthClientManager:
return True
def get_client(self, client_id):
if client_id not in self.clients:
self.ensure_client_from_config(client_id)
client = self.clients.get(client_id)
return client["client"] if client else None
def get_client_info(self, client_id):
if client_id not in self.clients:
self.ensure_client_from_config(client_id)
client = self.clients.get(client_id)
return client["client_info"] if client else None
@ -786,16 +790,20 @@ class OAuthClientManager:
try:
client_info = self.get_client_info(client_id)
auth_params = {}
if (
client_info
and hasattr(client_info, "client_id")
and hasattr(client_info, "client_secret")
):
auth_params["client_id"] = client_info.client_id
auth_params["client_secret"] = client_info.client_secret
# Note: Do NOT pass client_id/client_secret explicitly here.
# The Authlib client already has these configured during add_client().
# Passing them again causes Authlib to concatenate them (e.g., "ID1,ID1"),
# which results in 401 errors from the token endpoint. (Fix for #19823)
token = await client.authorize_access_token(request)
# Validate that we received a proper token response
# If token exchange failed (e.g., 401), we may get an error response instead
if token and not token.get("access_token"):
error_desc = token.get("error_description", token.get("error", "Unknown error"))
error_message = f"Token exchange failed: {error_desc}"
log.error(f"Invalid token response for client_id {client_id}: {token}")
token = None
token = await client.authorize_access_token(request, **auth_params)
if token:
try:
# Add timestamp for tracking
@ -825,6 +833,7 @@ class OAuthClientManager:
error_message = "Failed to store OAuth session server-side"
log.error(f"Failed to store OAuth session server-side: {e}")
else:
if not error_message:
error_message = "Failed to obtain OAuth token"
log.warning(error_message)
except Exception as e: