This is an automated email from the ASF dual-hosted git repository. FreeOnePlus pushed a commit to branch agent/sec-008-external-oauth-challenge in repository https://gitbox.apache.org/repos/asf/doris-mcp-server.git
commit f83315467cb4b26ed3b1b6b87f32cc37a957944e Author: FreeOnePlus <[email protected]> AuthorDate: Wed Jul 29 21:16:25 2026 +0800 fix: return external OAuth bearer challenges --- CHANGELOG.md | 3 + README.md | 10 ++ doris_mcp_server/auth/mcp_auth_middleware.py | 23 +++ doris_mcp_server/auth/oauth_handlers.py | 21 ++- doris_mcp_server/auth/oauth_provider.py | 25 ++- doris_mcp_server/auth/oauth_resource.py | 143 ++++++++++++++++ doris_mcp_server/auth/oauth_token_validation.py | 14 +- doris_mcp_server/main.py | 10 ++ doris_mcp_server/multiworker_app.py | 22 ++- doris_mcp_server/utils/config.py | 20 +++ doris_mcp_server/utils/security.py | 6 + test/auth/test_external_oauth_resource.py | 199 ++++++++++++++++++++++ test/auth/test_external_oauth_validation.py | 14 +- test/security/test_bearer_credentials.py | 32 ++++ test/security/test_mcp_auth_middleware.py | 216 +++++++++++++++++++++++- 15 files changed, 743 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b37fd1..7817758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,9 @@ under **Unreleased** until a new version is selected and published. - Required external OAuth access tokens to pass trusted RFC 7662 issuer/resource/audience, lifetime, and scope validation before userinfo is used. +- Preserved external OAuth token failures through the authentication boundary + and returned RFC 6750/RFC 9728 Bearer challenges, protected-resource + metadata, and operation-specific insufficient-scope responses. - Released Doris connections on SQL profile, data freshness, and access analysis paths. - Improved Doris 4 role metadata compatibility and query recovery behavior. diff --git a/README.md b/README.md index b034dcd..d671925 100644 --- a/README.md +++ b/README.md @@ -663,6 +663,16 @@ Dedicated `OAUTH_INTROSPECTION_CLIENT_ID` and regular OAuth client credentials are used. Remote issuer, discovery, introspection, and userinfo URLs must use HTTPS. +For HTTP deployments, the server publishes RFC 9728 metadata at +`/.well-known/oauth-protected-resource`. Missing or invalid credentials receive +an HTTP 401 Bearer challenge containing `resource_metadata` and the minimum +configured scopes. A valid token that lacks an operation scope receives HTTP +403 with `error="insufficient_scope"` and the exact scope required for +step-up authorization. Access tokens and provider-internal error details are +not copied into these responses. These HTTP OAuth challenges do not apply to +stdio transport, where credentials are supplied through the local process +environment. + ### Doris-Backed OAuth Authentication Doris-backed OAuth is a separate OAuth mode where Doris itself is the authorization backend. The MCP client discovers this server's OAuth metadata, the user signs in with a Doris username and password, the server validates those credentials by creating a per-user Doris connection pool, and issued `doa_` access tokens route tool calls through that Doris user's pool. MCP scopes control which MCP operations can be called; Doris RBAC controls which catalogs, databases, tables, and metadata t [...] diff --git a/doris_mcp_server/auth/mcp_auth_middleware.py b/doris_mcp_server/auth/mcp_auth_middleware.py index fcf5140..631480a 100644 --- a/doris_mcp_server/auth/mcp_auth_middleware.py +++ b/doris_mcp_server/auth/mcp_auth_middleware.py @@ -35,6 +35,10 @@ from .doris_oauth_handlers import ( insufficient_scope_response, protected_resource_error_response, ) +from .oauth_resource import ( + external_oauth_error_response, + external_oauth_insufficient_scope_response, +) from .operation_policy import OperationAuthorizationError ASGIApp = Callable[[dict[str, Any], Callable[..., Awaitable[Any]], Callable[..., Awaitable[Any]]], Awaitable[Any]] @@ -66,6 +70,7 @@ class MCPAuthASGIMiddleware: self.effective_auth = effective_auth async def __call__(self, scope, receive, send): + credentials = None try: credentials = await extract_bearer_credentials_from_scope(scope) auth_context = await self.security_manager.authenticate_request(credentials) @@ -75,6 +80,14 @@ class MCPAuthASGIMiddleware: exc, self.effective_auth.doris_oauth_base_url, ) + elif self.effective_auth.oauth_discovery_mode == "external_oauth": + response = external_oauth_error_response( + exc, + self.effective_auth, + credentials_present=bool( + credentials and credentials.is_bearer + ), + ) else: response = JSONResponse( {"error": "Authentication required", "message": str(exc)}, @@ -119,6 +132,16 @@ class MCPAuthASGIMiddleware: exc.required_scope, body, ) + elif ( + self.effective_auth.oauth_discovery_mode == "external_oauth" + and exc.required_scope + and exc.error_code == "PERMISSION_DENIED" + ): + response = external_oauth_insufficient_scope_response( + self.effective_auth, + exc.required_scope, + body, + ) else: response = JSONResponse(body, status_code=exc.status_code) await response(scope, receive, send) diff --git a/doris_mcp_server/auth/oauth_handlers.py b/doris_mcp_server/auth/oauth_handlers.py index 4b8a46d..57ce38c 100644 --- a/doris_mcp_server/auth/oauth_handlers.py +++ b/doris_mcp_server/auth/oauth_handlers.py @@ -27,7 +27,9 @@ import json from starlette.responses import JSONResponse, RedirectResponse, HTMLResponse from starlette.requests import Request +from ..utils.config import get_effective_auth_config from ..utils.logger import get_logger +from .oauth_resource import external_oauth_protected_resource_metadata logger = get_logger(__name__) @@ -143,6 +145,23 @@ class OAuthHandlers: {"error": f"Failed to get provider info: {str(e)}"}, status_code=500 ) + + async def handle_protected_resource_metadata( + self, + request: Request, + ) -> JSONResponse: + """Return RFC 9728 metadata for external OAuth deployments.""" + effective_auth = get_effective_auth_config( + self.security_manager.config + ) + if not effective_auth.enable_external_oauth_auth: + return JSONResponse( + {"error": "external_oauth_disabled"}, + status_code=404, + ) + return JSONResponse( + external_oauth_protected_resource_metadata(effective_auth) + ) async def handle_demo_page(self, request: Request) -> HTMLResponse: """Handle OAuth demo page @@ -309,4 +328,4 @@ class OAuthHandlers: </html> """ - return HTMLResponse(html_content) \ No newline at end of file + return HTMLResponse(html_content) diff --git a/doris_mcp_server/auth/oauth_provider.py b/doris_mcp_server/auth/oauth_provider.py index 16b69d4..d1971ef 100644 --- a/doris_mcp_server/auth/oauth_provider.py +++ b/doris_mcp_server/auth/oauth_provider.py @@ -24,7 +24,10 @@ from typing import Dict, Any, Optional, Tuple from datetime import datetime from .oauth_client import OAuthClient -from .oauth_token_validation import OAuthAccessTokenContext +from .oauth_token_validation import ( + OAuthAccessTokenContext, + OAuthAccessTokenValidationError, +) from .oauth_types import OAuthTokens, OAuthUserInfo, OAuthState from ..utils.security import AuthContext, SecurityLevel from ..utils.logger import get_logger @@ -119,9 +122,12 @@ class OAuthAuthenticationProvider: logger.info(f"OAuth authentication successful for user: {auth_context.user_id}") return auth_context + except OAuthAccessTokenValidationError: + logger.warning("OAuth callback access token was rejected") + raise except Exception as e: logger.error(f"OAuth callback handling failed: {e}") - raise ValueError(f"OAuth authentication failed: {str(e)}") + raise ValueError(f"OAuth authentication failed: {str(e)}") from e async def authenticate_with_token(self, access_token: str) -> AuthContext: """Authenticate using OAuth access token @@ -157,9 +163,12 @@ class OAuthAuthenticationProvider: logger.info(f"OAuth token authentication successful for user: {auth_context.user_id}") return auth_context + except OAuthAccessTokenValidationError: + logger.warning("OAuth bearer access token was rejected") + raise except Exception as e: logger.error(f"OAuth token authentication failed: {e}") - raise ValueError(f"OAuth token authentication failed: {str(e)}") + raise ValueError(f"OAuth token authentication failed: {str(e)}") from e async def refresh_authentication(self, refresh_token: str) -> Tuple[AuthContext, str]: """Refresh OAuth authentication @@ -195,9 +204,12 @@ class OAuthAuthenticationProvider: logger.info(f"OAuth refresh successful for user: {auth_context.user_id}") return auth_context, tokens.access_token + except OAuthAccessTokenValidationError: + logger.warning("OAuth refreshed access token was rejected") + raise except Exception as e: logger.error(f"OAuth refresh failed: {e}") - raise ValueError(f"OAuth refresh failed: {str(e)}") + raise ValueError(f"OAuth refresh failed: {str(e)}") from e async def _create_auth_context( self, @@ -216,8 +228,9 @@ class OAuthAuthenticationProvider: AuthContext for the user """ if user_info.sub != token_context.subject: - raise ValueError( - "OAuth userinfo subject does not match the access token" + raise OAuthAccessTokenValidationError( + "invalid_token", + "OAuth userinfo subject does not match the access token", ) # Determine security level based on roles or email domain diff --git a/doris_mcp_server/auth/oauth_resource.py b/doris_mcp_server/auth/oauth_resource.py new file mode 100644 index 0000000..4e2170f --- /dev/null +++ b/doris_mcp_server/auth/oauth_resource.py @@ -0,0 +1,143 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""OAuth protected-resource metadata and Bearer challenge helpers.""" + +from urllib.parse import urlsplit, urlunsplit + +from starlette.responses import JSONResponse + +from ..utils.config import EffectiveAuthConfig +from .oauth_token_validation import OAuthAccessTokenValidationError + + +def external_oauth_resource_metadata_url(resource: str) -> str: + """Return the root RFC 9728 metadata URL served by this MCP server.""" + parsed = urlsplit(resource) + if not parsed.scheme or not parsed.netloc: + raise ValueError("External OAuth resource must be an absolute URI") + return urlunsplit( + ( + parsed.scheme, + parsed.netloc, + "/.well-known/oauth-protected-resource", + "", + "", + ) + ) + + +def _quote_challenge_value(value: str) -> str: + """Escape one RFC 7235 quoted-string value and reject header injection.""" + sanitized = str(value).replace("\r", "").replace("\n", "") + return sanitized.replace("\\", "\\\\").replace('"', '\\"') + + +def bearer_challenge_header( + resource_metadata_url: str, + *, + error: str | None = None, + scopes: tuple[str, ...] = (), +) -> str: + """Build an RFC 6750 Bearer challenge with RFC 9728 discovery metadata.""" + parameters = [ + f'resource_metadata="{_quote_challenge_value(resource_metadata_url)}"' + ] + if error: + parameters.append(f'error="{_quote_challenge_value(error)}"') + if scopes: + scope = " ".join(dict.fromkeys(scopes)) + parameters.append(f'scope="{_quote_challenge_value(scope)}"') + return f"Bearer {', '.join(parameters)}" + + +def external_oauth_protected_resource_metadata( + effective_auth: EffectiveAuthConfig, +) -> dict[str, object]: + """Build the RFC 9728 document for an external authorization server.""" + return { + "resource": effective_auth.external_oauth_resource, + "authorization_servers": [effective_auth.external_oauth_issuer], + "scopes_supported": list(effective_auth.external_oauth_scopes), + "bearer_methods_supported": ["header"], + } + + +def external_oauth_error_response( + error: Exception, + effective_auth: EffectiveAuthConfig, + *, + credentials_present: bool, +) -> JSONResponse: + """Map external OAuth authentication failures to OAuth resource errors.""" + default_scopes = ( + effective_auth.external_oauth_required_scopes + or effective_auth.external_oauth_scopes + ) + challenge_error = "invalid_token" if credentials_present else None + status_code = 401 + body = { + "error": "invalid_token" if credentials_present else "authentication_required", + "error_description": ( + "Invalid access token" if credentials_present else "Authentication required" + ), + } + + if isinstance(error, OAuthAccessTokenValidationError): + challenge_error = error.error + status_code = error.status_code + body = { + "error": error.error, + "error_description": error.description, + } + default_scopes = error.required_scopes or default_scopes + + metadata_url = external_oauth_resource_metadata_url( + effective_auth.external_oauth_resource + ) + return JSONResponse( + body, + status_code=status_code, + headers={ + "WWW-Authenticate": bearer_challenge_header( + metadata_url, + error=challenge_error, + scopes=default_scopes, + ) + }, + ) + + +def external_oauth_insufficient_scope_response( + effective_auth: EffectiveAuthConfig, + required_scope: str, + body: dict[str, object], +) -> JSONResponse: + """Return a standards-compliant runtime step-up challenge.""" + metadata_url = external_oauth_resource_metadata_url( + effective_auth.external_oauth_resource + ) + return JSONResponse( + body, + status_code=403, + headers={ + "WWW-Authenticate": bearer_challenge_header( + metadata_url, + error="insufficient_scope", + scopes=(required_scope,), + ) + }, + ) diff --git a/doris_mcp_server/auth/oauth_token_validation.py b/doris_mcp_server/auth/oauth_token_validation.py index e83e1af..9fec578 100644 --- a/doris_mcp_server/auth/oauth_token_validation.py +++ b/doris_mcp_server/auth/oauth_token_validation.py @@ -27,10 +27,18 @@ from typing import Any class OAuthAccessTokenValidationError(ValueError): """An external access token failed resource-server validation.""" - def __init__(self, error: str, description: str): + def __init__( + self, + error: str, + description: str, + *, + required_scopes: Sequence[str] = (), + ): super().__init__(description) self.error = error self.description = description + self.required_scopes = tuple(dict.fromkeys(required_scopes)) + self.status_code = 403 if error == "insufficient_scope" else 401 @dataclass(frozen=True, slots=True) @@ -153,12 +161,16 @@ class ExternalOAuthTokenValidator: raise OAuthAccessTokenValidationError( "insufficient_scope", "OAuth access token is missing a required scope", + required_scopes=tuple(sorted(self.required_scopes)), ) granted_scopes = tuple(sorted(token_scopes & self.allowed_scopes)) if not granted_scopes: raise OAuthAccessTokenValidationError( "insufficient_scope", "OAuth access token has no allowed scope", + required_scopes=tuple( + sorted(self.required_scopes or self.allowed_scopes) + ), ) subject = str(claims.get("sub") or "") diff --git a/doris_mcp_server/main.py b/doris_mcp_server/main.py index 2d8beaf..f53aca1 100644 --- a/doris_mcp_server/main.py +++ b/doris_mcp_server/main.py @@ -244,6 +244,11 @@ class DorisServer: async def oauth_provider_info(request): return await oauth_handlers.handle_provider_info(request) + + async def oauth_protected_resource_metadata(request): + return await oauth_handlers.handle_protected_resource_metadata( + request + ) async def oauth_demo(request): return await oauth_handlers.handle_demo_page(request) @@ -291,6 +296,11 @@ class DorisServer: routes = [Route("/health", health_check, methods=["GET"])] if effective_auth.enable_external_oauth_auth: routes.extend([ + Route( + "/.well-known/oauth-protected-resource", + oauth_protected_resource_metadata, + methods=["GET"], + ), Route("/auth/login", oauth_login, methods=["GET"]), Route("/auth/callback", oauth_callback, methods=["GET"]), Route("/auth/provider", oauth_provider_info, methods=["GET"]), diff --git a/doris_mcp_server/multiworker_app.py b/doris_mcp_server/multiworker_app.py index 54b7418..89b924c 100644 --- a/doris_mcp_server/multiworker_app.py +++ b/doris_mcp_server/multiworker_app.py @@ -231,10 +231,20 @@ async def doris_oauth_unavailable(request): return JSONResponse({"error": "doris_oauth_not_initialized"}, status_code=503) -async def doris_oauth_protected_resource_metadata(request): - if not _doris_oauth_handlers: +async def oauth_protected_resource_metadata(request): + if _worker_effective_auth is None: return await doris_oauth_unavailable(request) - return await _doris_oauth_handlers.protected_resource_metadata(request) + if _worker_effective_auth and _worker_effective_auth.enable_doris_oauth_auth: + if not _doris_oauth_handlers: + return await doris_oauth_unavailable(request) + return await _doris_oauth_handlers.protected_resource_metadata(request) + if ( + _worker_effective_auth + and _worker_effective_auth.enable_external_oauth_auth + and _oauth_handlers + ): + return await _oauth_handlers.handle_protected_resource_metadata(request) + return JSONResponse({"error": "oauth_disabled"}, status_code=404) async def doris_oauth_authorization_server_metadata(request): @@ -395,7 +405,11 @@ basic_app = Starlette( Route("/auth/provider", oauth_provider_info, methods=["GET"]), Route("/auth/demo", oauth_demo, methods=["GET"]), # Doris OAuth endpoints are explicit to avoid top-level silent 404s. - Route("/.well-known/oauth-protected-resource", doris_oauth_protected_resource_metadata, methods=["GET"]), + Route( + "/.well-known/oauth-protected-resource", + oauth_protected_resource_metadata, + methods=["GET"], + ), Route("/.well-known/oauth-authorization-server", doris_oauth_authorization_server_metadata, methods=["GET"]), Route("/oauth/register", doris_oauth_register, methods=["POST"]), Route("/oauth/authorize", doris_oauth_authorize, methods=["GET"]), diff --git a/doris_mcp_server/utils/config.py b/doris_mcp_server/utils/config.py index 07dba2d..2b66249 100644 --- a/doris_mcp_server/utils/config.py +++ b/doris_mcp_server/utils/config.py @@ -101,6 +101,10 @@ class EffectiveAuthConfig: legacy_auth_type: str auth_config_warnings: tuple[str, ...] = () doris_oauth_base_url: str = "" + external_oauth_issuer: str = "" + external_oauth_resource: str = "" + external_oauth_scopes: tuple[str, ...] = () + external_oauth_required_scopes: tuple[str, ...] = () source_summary: dict[str, str] = field(default_factory=dict) @@ -1800,6 +1804,22 @@ def normalize_effective_auth_config( auth_methods=tuple(methods), oauth_discovery_mode=discovery_mode, doris_oauth_base_url=config.security.doris_oauth_base_url.rstrip("/"), + external_oauth_issuer=( + config.security.oauth_issuer if enable_external_oauth_auth else "" + ), + external_oauth_resource=( + config.security.oauth_resource if enable_external_oauth_auth else "" + ), + external_oauth_scopes=( + tuple(config.security.oauth_scopes) + if enable_external_oauth_auth + else () + ), + external_oauth_required_scopes=( + tuple(config.security.oauth_required_scopes) + if enable_external_oauth_auth + else () + ), transport=transport, requested_workers=requested, effective_workers=effective_workers, diff --git a/doris_mcp_server/utils/security.py b/doris_mcp_server/utils/security.py index 3ffe093..d6acb27 100644 --- a/doris_mcp_server/utils/security.py +++ b/doris_mcp_server/utils/security.py @@ -319,6 +319,12 @@ class DorisSecurityManager: raise # All enabled authentication methods failed + from ..auth.oauth_token_validation import ( + OAuthAccessTokenValidationError, + ) + + if isinstance(last_error, OAuthAccessTokenValidationError): + raise last_error error_message = f"Authentication failed: {str(last_error)}" if last_error else "No authentication method succeeded" self.logger.warning( f"Authentication failed for client {credentials.client_ip}: {error_message}" diff --git a/test/auth/test_external_oauth_resource.py b/test/auth/test_external_oauth_resource.py new file mode 100644 index 0000000..e899d05 --- /dev/null +++ b/test/auth/test_external_oauth_resource.py @@ -0,0 +1,199 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from types import SimpleNamespace + +import httpx +import pytest +from starlette.requests import Request + +import doris_mcp_server.multiworker_app as multiworker_app +from doris_mcp_server.auth.oauth_handlers import OAuthHandlers +from doris_mcp_server.auth.oauth_resource import ( + bearer_challenge_header, + external_oauth_protected_resource_metadata, + external_oauth_resource_metadata_url, +) +from doris_mcp_server.auth.oauth_token_validation import ( + OAuthAccessTokenValidationError, +) +from doris_mcp_server.utils.config import ( + DorisConfig, + _mark_source, + normalize_effective_auth_config, +) + +ISSUER = "https://issuer.example.test/tenant"; +RESOURCE = "https://mcp.example.test/mcp"; + + +def _external_oauth_config() -> DorisConfig: + config = DorisConfig() + config.security.enable_oauth_auth = True + _mark_source(config, "enable_oauth_auth", "test") + config.security.oauth_provider = "custom" + config.security.oauth_client_id = "oauth-client" + config.security.oauth_client_secret = "oauth-secret" + config.security.oauth_redirect_uri = "http://localhost:3000/auth/callback"; + config.security.oauth_authorization_endpoint = f"{ISSUER}/authorize" + config.security.oauth_token_endpoint = f"{ISSUER}/token" + config.security.oauth_introspection_endpoint = f"{ISSUER}/introspect" + config.security.oauth_userinfo_endpoint = f"{ISSUER}/userinfo" + config.security.oauth_issuer = ISSUER + config.security.oauth_resource = RESOURCE + config.security.oauth_audience = RESOURCE + config.security.oauth_scopes = [ + "tool:list", + "resource:list", + "resource:read", + ] + config.security.oauth_required_scopes = ["tool:list"] + normalize_effective_auth_config(config) + return config + + +def test_external_oauth_metadata_matches_normalized_resource_context(): + effective = _external_oauth_config().effective_auth + + assert effective.external_oauth_issuer == ISSUER + assert effective.external_oauth_resource == RESOURCE + assert effective.external_oauth_scopes == ( + "tool:list", + "resource:list", + "resource:read", + ) + assert effective.external_oauth_required_scopes == ("tool:list",) + assert external_oauth_protected_resource_metadata(effective) == { + "resource": RESOURCE, + "authorization_servers": [ISSUER], + "scopes_supported": [ + "tool:list", + "resource:list", + "resource:read", + ], + "bearer_methods_supported": ["header"], + } + + +def test_external_oauth_metadata_url_uses_served_root_well_known_path(): + assert external_oauth_resource_metadata_url(RESOURCE) == ( + "https://mcp.example.test/.well-known/oauth-protected-resource"; + ) + + +def test_bearer_challenge_escapes_quoted_values_and_header_injection(): + challenge = bearer_challenge_header( + 'https://mcp.example.test/.well-known/"metadata"\r\nInjected: yes', + error="invalid_token", + scopes=("tool:list", "tool:list", "resource:read"), + ) + + assert "\r" not in challenge + assert "\n" not in challenge + assert '\\"metadata\\"' in challenge + assert challenge.count("tool:list") == 1 + assert 'scope="tool:list resource:read"' in challenge + + [email protected] +async def test_external_oauth_handler_serves_protected_resource_metadata(): + config = _external_oauth_config() + handlers = OAuthHandlers(SimpleNamespace(config=config)) + + response = await handlers.handle_protected_resource_metadata( + Request({"type": "http", "method": "GET", "path": "/"}) + ) + + assert response.status_code == 200 + assert b'"resource":"https://mcp.example.test/mcp";' in response.body + assert b'"authorization_servers":["https://issuer.example.test/tenant";]' in ( + response.body + ) + + [email protected] +async def test_multiworker_dispatches_external_oauth_resource_metadata( + monkeypatch, +): + config = _external_oauth_config() + handlers = OAuthHandlers(SimpleNamespace(config=config)) + monkeypatch.setattr( + multiworker_app, + "_worker_effective_auth", + config.effective_auth, + ) + monkeypatch.setattr(multiworker_app, "_oauth_handlers", handlers) + monkeypatch.setattr(multiworker_app, "_doris_oauth_handlers", None) + + response = await multiworker_app.oauth_protected_resource_metadata( + Request({"type": "http", "method": "GET", "path": "/"}) + ) + + assert response.status_code == 200 + assert b'"resource":"https://mcp.example.test/mcp";' in response.body + + [email protected] +async def test_multiworker_http_routes_external_oauth_metadata_and_challenge( + monkeypatch, +): + config = _external_oauth_config() + handlers = OAuthHandlers(SimpleNamespace(config=config)) + + class RejectingSecurityManager: + async def authenticate_request(self, credentials): + raise OAuthAccessTokenValidationError( + "invalid_token", + "OAuth access token has expired", + ) + + monkeypatch.setattr(multiworker_app, "_worker_initialized", True) + monkeypatch.setattr( + multiworker_app, + "_worker_effective_auth", + config.effective_auth, + ) + monkeypatch.setattr( + multiworker_app, + "_worker_security_manager", + RejectingSecurityManager(), + ) + monkeypatch.setattr(multiworker_app, "_oauth_handlers", handlers) + monkeypatch.setattr(multiworker_app, "_doris_oauth_handlers", None) + + transport = httpx.ASGITransport(app=multiworker_app.app) + async with httpx.AsyncClient( + transport=transport, + base_url="https://mcp.example.test";, + ) as client: + metadata = await client.get("/.well-known/oauth-protected-resource") + rejected = await client.post( + "/mcp", + headers={"Authorization": "Bearer expired-token"}, + json={"jsonrpc": "2.0", "id": 1, "method": "server/discover"}, + ) + + assert metadata.status_code == 200 + assert metadata.json()["resource"] == RESOURCE + assert rejected.status_code == 401 + assert rejected.json()["error"] == "invalid_token" + challenge = rejected.headers["www-authenticate"] + assert 'error="invalid_token"' in challenge + assert 'scope="tool:list"' in challenge + assert "oauth-protected-resource" in challenge + assert "expired-token" not in rejected.text + assert "expired-token" not in challenge diff --git a/test/auth/test_external_oauth_validation.py b/test/auth/test_external_oauth_validation.py index 2b97a15..7af778e 100644 --- a/test/auth/test_external_oauth_validation.py +++ b/test/auth/test_external_oauth_validation.py @@ -134,9 +134,14 @@ async def test_failed_introspection_never_calls_userinfo(): client = RejectingOAuthClient() provider = _provider(client) - with pytest.raises(ValueError, match="inactive"): + with pytest.raises( + OAuthAccessTokenValidationError, + match="inactive", + ) as exc_info: await provider.authenticate_with_token("rejected-access") + assert exc_info.value.error == "invalid_token" + assert exc_info.value.status_code == 401 assert client.events == [("introspect", "rejected-access")] @@ -148,5 +153,10 @@ async def test_userinfo_subject_must_match_introspected_subject(): ) provider = _provider(client) - with pytest.raises(ValueError, match="userinfo subject does not match"): + with pytest.raises( + OAuthAccessTokenValidationError, + match="userinfo subject does not match", + ) as exc_info: await provider.authenticate_with_token("access-1") + + assert exc_info.value.error == "invalid_token" diff --git a/test/security/test_bearer_credentials.py b/test/security/test_bearer_credentials.py index d22ff2d..0dbda9b 100644 --- a/test/security/test_bearer_credentials.py +++ b/test/security/test_bearer_credentials.py @@ -21,6 +21,9 @@ from types import SimpleNamespace import pytest +from doris_mcp_server.auth.oauth_token_validation import ( + OAuthAccessTokenValidationError, +) from doris_mcp_server.utils.auth_credentials import ( BearerCredentials, normalize_bearer_credentials, @@ -138,6 +141,35 @@ async def test_security_manager_passes_the_same_dto_to_every_provider( assert received[0][1] is credentials [email protected] +async def test_security_manager_preserves_external_oauth_challenge_error(): + expected = OAuthAccessTokenValidationError( + "insufficient_scope", + "A required scope is missing", + required_scopes=("tool:call:exec_query",), + ) + + class Provider: + async def authenticate_oauth(self, credentials): + raise expected + + manager = object.__new__(DorisSecurityManager) + manager.auth_provider = Provider() + manager.logger = logging.getLogger(__name__) + manager._get_effective_auth_config = lambda: SimpleNamespace( + auth_methods=("external_oauth",) + ) + + with pytest.raises(OAuthAccessTokenValidationError) as exc_info: + await manager.authenticate_request( + BearerCredentials(scheme="bearer", token="external-token") + ) + + assert exc_info.value is expected + assert exc_info.value.status_code == 403 + assert exc_info.value.required_scopes == ("tool:call:exec_query",) + + @pytest.mark.asyncio async def test_static_token_provider_uses_canonical_credentials(): token_info = SimpleNamespace( diff --git a/test/security/test_mcp_auth_middleware.py b/test/security/test_mcp_auth_middleware.py index c601b2d..45a8dc4 100644 --- a/test/security/test_mcp_auth_middleware.py +++ b/test/security/test_mcp_auth_middleware.py @@ -4,13 +4,25 @@ import pytest import doris_mcp_server.auth.mcp_auth_middleware as middleware_module from doris_mcp_server.auth.mcp_auth_middleware import MCPAuthASGIMiddleware +from doris_mcp_server.auth.oauth_token_validation import ( + OAuthAccessTokenValidationError, +) from doris_mcp_server.auth.operation_policy import OperationAuthorizationError from doris_mcp_server.utils.auth_credentials import BearerCredentials from doris_mcp_server.utils.config import EffectiveAuthConfig from doris_mcp_server.utils.security import AuthContext, get_current_auth_context -def _effective(auth_methods=("token",), discovery_mode="none", base_url=""): +def _effective( + auth_methods=("token",), + discovery_mode="none", + base_url="", + *, + external_issuer="https://issuer.example.test";, + external_resource="https://mcp.example.test/mcp";, + external_scopes=("tool:list", "resource:read"), + external_required_scopes=("tool:list",), +): return EffectiveAuthConfig( enable_token_auth="token" in auth_methods, enable_jwt_auth="jwt" in auth_methods, @@ -23,6 +35,10 @@ def _effective(auth_methods=("token",), discovery_mode="none", base_url=""): requested_workers=1, effective_workers=1, legacy_auth_type="", + external_oauth_issuer=external_issuer, + external_oauth_resource=external_resource, + external_oauth_scopes=external_scopes, + external_oauth_required_scopes=external_required_scopes, ) @@ -135,6 +151,155 @@ async def test_mcp_auth_middleware_returns_doris_oauth_challenge_on_401(): assert body["error"] == "authentication_required" [email protected] +async def test_external_oauth_missing_token_returns_discovery_challenge(): + class SecurityManager: + async def authenticate_request(self, credentials): + assert credentials.is_bearer is False + raise ValueError("Missing external OAuth access token") + + async def downstream(scope, receive, send): + raise AssertionError("downstream must not be called") + + messages = [] + middleware = MCPAuthASGIMiddleware( + SecurityManager(), + downstream, + _effective( + auth_methods=("external_oauth",), + discovery_mode="external_oauth", + ), + ) + await middleware( + { + "type": "http", + "path": "/mcp", + "headers": [], + "client": ("127.0.0.1", 1), + }, + _receive, + _send_collector(messages), + ) + + assert messages[0]["status"] == 401 + challenge = dict(messages[0]["headers"])[b"www-authenticate"] + assert challenge.startswith(b"Bearer ") + assert ( + b'resource_metadata="https://mcp.example.test/' + b'.well-known/oauth-protected-resource"' in challenge + ) + assert b'scope="tool:list"' in challenge + assert b"error=" not in challenge + body = json.loads(messages[1]["body"]) + assert body == { + "error": "authentication_required", + "error_description": "Authentication required", + } + + [email protected] [email protected]( + ("oauth_error", "status_code", "required_scopes"), + [ + ("invalid_token", 401, ()), + ("insufficient_scope", 403, ("tool:call:exec_query",)), + ], +) +async def test_external_oauth_validation_error_preserves_bearer_challenge( + oauth_error, + status_code, + required_scopes, +): + secret = "must-not-leak" + + class SecurityManager: + async def authenticate_request(self, credentials): + assert credentials.token == secret + raise OAuthAccessTokenValidationError( + oauth_error, + f"Controlled {oauth_error} description", + required_scopes=required_scopes, + ) + + async def downstream(scope, receive, send): + raise AssertionError("downstream must not be called") + + messages = [] + middleware = MCPAuthASGIMiddleware( + SecurityManager(), + downstream, + _effective( + auth_methods=("external_oauth",), + discovery_mode="external_oauth", + ), + ) + await middleware( + { + "type": "http", + "path": "/mcp", + "headers": [ + (b"authorization", f"Bearer {secret}".encode()) + ], + "client": ("127.0.0.1", 1), + }, + _receive, + _send_collector(messages), + ) + + assert messages[0]["status"] == status_code + challenge = dict(messages[0]["headers"])[b"www-authenticate"] + assert f'error="{oauth_error}"'.encode() in challenge + assert b"oauth-protected-resource" in challenge + if required_scopes: + assert b'scope="tool:call:exec_query"' in challenge + assert secret.encode() not in challenge + assert secret.encode() not in messages[1]["body"] + body = json.loads(messages[1]["body"]) + assert body["error"] == oauth_error + + [email protected] +async def test_external_oauth_generic_failure_does_not_expose_provider_detail(): + secret = "provider-reflected-secret" + + class SecurityManager: + async def authenticate_request(self, credentials): + raise RuntimeError(f"introspection backend echoed {secret}") + + async def downstream(scope, receive, send): + raise AssertionError("downstream must not be called") + + messages = [] + middleware = MCPAuthASGIMiddleware( + SecurityManager(), + downstream, + _effective( + auth_methods=("external_oauth",), + discovery_mode="external_oauth", + ), + ) + await middleware( + { + "type": "http", + "path": "/mcp", + "headers": [(b"authorization", f"Bearer {secret}".encode())], + "client": ("127.0.0.1", 1), + }, + _receive, + _send_collector(messages), + ) + + assert messages[0]["status"] == 401 + challenge = dict(messages[0]["headers"])[b"www-authenticate"] + assert b'error="invalid_token"' in challenge + assert secret.encode() not in challenge + assert secret.encode() not in messages[1]["body"] + assert json.loads(messages[1]["body"]) == { + "error": "invalid_token", + "error_description": "Invalid access token", + } + + @pytest.mark.asyncio async def test_mcp_auth_middleware_returns_insufficient_scope_challenge_on_operation_denial(): auth_context = AuthContext( @@ -182,6 +347,55 @@ async def test_mcp_auth_middleware_returns_insufficient_scope_challenge_on_opera assert body["error"] == "PERMISSION_DENIED" [email protected] +async def test_external_oauth_operation_denial_returns_step_up_challenge(): + auth_context = AuthContext( + token_id="t1", + user_id="alice", + auth_method="external_oauth", + oauth_scopes=["tool:list"], + ) + + class SecurityManager: + async def authenticate_request(self, credentials): + return auth_context + + async def downstream(scope, receive, send): + raise OperationAuthorizationError( + "Missing required scope", + status_code=403, + error_code="PERMISSION_DENIED", + required_scope="tool:call:exec_query", + operation="tool:exec_query", + ) + + messages = [] + middleware = MCPAuthASGIMiddleware( + SecurityManager(), + downstream, + _effective( + auth_methods=("external_oauth",), + discovery_mode="external_oauth", + ), + ) + await middleware( + { + "type": "http", + "path": "/mcp", + "headers": [(b"authorization", b"Bearer external-token")], + "client": ("127.0.0.1", 1), + }, + _receive, + _send_collector(messages), + ) + + assert messages[0]["status"] == 403 + challenge = dict(messages[0]["headers"])[b"www-authenticate"] + assert b'error="insufficient_scope"' in challenge + assert b'scope="tool:call:exec_query"' in challenge + assert b"oauth-protected-resource" in challenge + + @pytest.mark.asyncio async def test_mcp_auth_middleware_resets_context_on_verify_failure(monkeypatch): auth_context = AuthContext(token_id="t1", user_id="u1", auth_method="token") --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
