-
Notifications
You must be signed in to change notification settings - Fork 69
feat(mcp): Add HTTP header-based authentication support for cloud operations #867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…rations - Add ContextVar-based request context management for per-request auth values - Implement ASGI middleware to extract auth from HTTP headers - Support both X-Airbyte-Cloud-* and Airbyte-Cloud-* header prefixes - Update cloud operations to use header values when available, fallback to env vars - Middleware only active in HTTP/SSE modes, STDIO mode unchanged Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1763055706-mcp-http-header-auth' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1763055706-mcp-http-header-auth'Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
Community SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
- Remove CLOUD_API_URL_CVAR and get_effective_cloud_api_url() - Remove X-Airbyte-Cloud-Api-Url header extraction from middleware - Remove api_root parameter from CloudWorkspace construction - Simplify to only support client ID, client secret, and workspace ID headers Co-Authored-By: AJ Steers <[email protected]>
📝 WalkthroughWalkthroughAdds per-request header-based Cloud authentication: a ContextVar-backed request context, an ASGI HeaderAuthMiddleware that populates it from HTTP headers for MCP HTTP/SSE requests, and wiring so cloud_ops and the FastMCP app use those per-request values with environment fallbacks. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware as HeaderAuthMiddleware
participant Context as RequestContext (ContextVars)
participant App as FastMCP / cloud_ops
participant Env as EnvironmentResolvers
Client->>Middleware: HTTP request with auth headers
activate Middleware
Middleware->>Middleware: Normalize headers & extract auth values
Middleware->>Context: Set CLOUD_*_CVAR values (request-scoped)
Middleware->>App: Forward request
activate App
App->>Context: Call get_effective_cloud_*()
Context-->>App: Return header value if set
alt header not set
App->>Env: Call resolve_cloud_*() fallbacks
Env-->>App: Return env value
end
App-->>Client: Response
deactivate App
Middleware->>Context: Reset CLOUD_*_CVAR values (finally)
deactivate Middleware
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Would you like me to draft unit tests for Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
airbyte/mcp/_request_context.py (1)
36-38: Potential double-wrapping of SecretString values.If
header_valueis already aSecretString(e.g., manually set in tests), line 38 wraps it again:SecretString(SecretString(...)). While this works due to inheritance, it's redundant.Wdyt about checking
if not isinstance(header_value, SecretString)before wrapping? Same applies to the other getter functions below.airbyte/mcp/_middleware.py (1)
60-61: Header decoding could raise UnicodeDecodeError.Line 61 decodes headers without error handling. If a malformed header contains invalid UTF-8, this would raise
UnicodeDecodeErrorand crash the request. While rare (most ASGI servers validate headers), wdyt about adding a try-except to gracefully handle this edge case?For example:
- headers = scope.get("headers", []) - header_dict = {name.decode().lower(): value.decode() for name, value in headers} + headers = scope.get("headers", []) + try: + header_dict = {name.decode().lower(): value.decode() for name, value in headers} + except UnicodeDecodeError: + logger.warning("Failed to decode headers, proceeding without auth headers") + header_dict = {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airbyte/mcp/_middleware.py(1 hunks)airbyte/mcp/_request_context.py(1 hunks)airbyte/mcp/cloud_ops.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
airbyte/mcp/cloud_ops.py (2)
airbyte/mcp/_request_context.py (3)
get_effective_cloud_client_id(30-39)get_effective_cloud_client_secret(42-51)get_effective_cloud_workspace_id(54-63)airbyte/cloud/workspaces.py (1)
CloudWorkspace(64-612)
airbyte/mcp/_request_context.py (2)
airbyte/cloud/auth.py (3)
resolve_cloud_client_id(17-22)resolve_cloud_client_secret(9-14)resolve_cloud_workspace_id(36-41)airbyte/secrets/base.py (1)
SecretString(38-143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (No Creds)
🔇 Additional comments (6)
airbyte/mcp/cloud_ops.py (2)
15-19: Clean import migration to request context pattern.The switch from
resolve_*toget_effective_*functions properly reflects the new header-aware authentication approach. Looks good!
46-50: CloudWorkspace construction looks good with current implementation.The usage of the new
get_effective_*getters is correct. Once the API URL support is added (per my earlier comment), this would need to passapi_root=get_effective_cloud_api_url()to the constructor, wdyt?airbyte/mcp/_middleware.py (3)
56-58: Correct scope type filtering.Only processing HTTP requests and bypassing other scope types (like websocket) is the right approach for this middleware. STDIO mode will remain unaffected. 👍
73-94: Excellent ContextVar cleanup pattern.The token tracking and
finallyblock ensure CVARs are properly reset even if the app raises an exception. This is the correct concurrency-safe pattern for ContextVar usage in ASGI middleware. The debug logs also avoid exposing sensitive values. Well done!
96-109: Clean helper for header name fallback.The implementation correctly handles trying multiple header name variants (with/without X- prefix) and returns the first match. Simple and effective!
airbyte/mcp/_request_context.py (1)
13-17: Missing API URL support - inconsistent with PR objectives.The PR objectives state that
X-Airbyte-Cloud-Api-Url/Airbyte-Cloud-Api-Url(optional) should be supported, but there's noresolve_cloud_api_urlimport, noCLOUD_API_URL_CVAR, and noget_effective_cloud_api_url()function in this file. The middleware also doesn't extract this header.Was the api_url support intentionally deferred, or is this an oversight? If it's intentional, the PR objectives should be updated. Otherwise, wdyt about adding the missing pieces?
⛔ Skipped due to learnings
Learnt from: yohannj Repo: airbytehq/PyAirbyte PR: 716 File: airbyte/logs.py:384-402 Timestamp: 2025-07-11T19:53:44.427Z Learning: In the PyAirbyte project, when reviewing PRs, maintain clear separation of concerns. Don't suggest changes that are outside the scope of the PR's main objective, even if they would improve consistency or fix other issues. This helps with reviewing changes and potential reverts.
feat(mcp): Add HTTP header-based authentication support for cloud operations
Summary
Implements HTTP header-based authentication for the PyAirbyte MCP server when running in HTTP/SSE modes. This allows clients to pass Airbyte Cloud credentials via HTTP headers instead of relying solely on environment variables.
Key Changes:
_request_context.pywith ContextVars for storing per-request auth values_middleware.pywith ASGI middleware to extract auth from HTTP headerscloud_ops.pyto use header values when available, falling back to env varsserver.pyto inject middleware into FastMCP appSupported Headers (case-insensitive, both prefixes supported):
X-Airbyte-Cloud-Client-IdorAirbyte-Cloud-Client-IdX-Airbyte-Cloud-Client-SecretorAirbyte-Cloud-Client-SecretX-Airbyte-Cloud-Workspace-IdorAirbyte-Cloud-Workspace-IdX-Airbyte-Cloud-Api-UrlorAirbyte-Cloud-Api-Url(optional)Design:
Review & Testing Checklist for Human
X-Airbyte-Cloud-*andAirbyte-Cloud-*headers work# type: ignore[list-item]- verify this is acceptable or if there's a better approachTest Plan
python -m airbyte.mcp.server(may need to add HTTP mode support to main())Notes
airbyte.mcpmodule as requiredSummary by CodeRabbit