feat(mcp): always-on HTTP auth via typed build_mcp_auth with Airbyte Cloud defaults - #1084
Conversation
Co-Authored-By: AJ Steers <aj@airbyte.io>
🤖 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. 💡 Show Tips and TricksTesting 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/1784611568-cloud-mcp-airbyte-branded-auth' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1784611568-cloud-mcp-airbyte-branded-auth'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
There was a problem hiding this comment.
Pull request overview
This PR changes the MCP HTTP transport to be authenticated by default (targeting Airbyte Cloud out-of-the-box), and updates the server-side auth wiring so Airbyte-branded environment variables are translated into the generic names consumed by fastmcp_extensions.resolve_mcp_auth.
Changes:
- Default HTTP auth to Airbyte Cloud Keycloak realms (JWT verification always configured; interactive OIDC activates once client creds are provided).
- Replace the old
MCP_AUTH_AIRBYTE_CLOUDopt-in with baked-in Cloud defaults + anAIRBYTE_MCP_*env-var surface mapped into genericOIDC_*/MCP_AUTH_*. - Update HTTP entrypoint and package docs to reflect the new defaults and env vars.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
airbyte/mcp/server.py |
Introduces baked Airbyte Cloud auth defaults and maps AIRBYTE_MCP_* env vars into resolve_mcp_auth. |
airbyte/mcp/http_main.py |
Updates HTTP entrypoint documentation and adjusts the “no auth” warning messaging. |
airbyte/mcp/__init__.py |
Updates user-facing docs for HTTP auth behavior and server env-var configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHTTP MCP authentication now resolves Airbyte-specific settings into ChangesMCP HTTP authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant HTTPEntrypoint
participant ClientCredentialsExchangeMiddleware
participant AirbyteTokenEndpoint
participant FastMCPAuth
MCPClient->>HTTPEntrypoint: HTTP request
HTTPEntrypoint->>ClientCredentialsExchangeMiddleware: Wrapped ASGI request
ClientCredentialsExchangeMiddleware->>AirbyteTokenEndpoint: Exchange Basic credentials when enabled
AirbyteTokenEndpoint-->>ClientCredentialsExchangeMiddleware: Short-lived access_token
ClientCredentialsExchangeMiddleware->>FastMCPAuth: Forward Basic or rewritten Bearer request
FastMCPAuth-->>MCPClient: Authenticated MCP response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (1)
airbyte/mcp/server.py (1)
144-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider warning on legacy bare
OIDC_CLIENT_ID/OIDC_CLIENT_SECRET.Per the PR objectives, deployments using the old bare
OIDC_CLIENT_ID/OIDC_CLIENT_SECRETmust rename toAIRBYTE_MCP_OIDC_CLIENT_ID/AIRBYTE_MCP_OIDC_CLIENT_SECRET. Since_create_auth()only reads the new prefixed names, an operator who forgets to rename gets silent no-OIDC behavior rather than a signal. A one-line check (e.g. warn if the bare names are set inos.environbut the prefixed ones aren't) would catch this migration foot-gun cheaply, wdyt?🤖 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 `@airbyte/mcp/server.py` around lines 144 - 152, Update the authentication setup around _AUTH_ENV_MAP and _create_auth() to warn when legacy bare OIDC_CLIENT_ID or OIDC_CLIENT_SECRET is present in os.environ while its corresponding AIRBYTE_MCP-prefixed variable is unset. Keep _create_auth() reading only the new prefixed names, and emit a clear migration warning for each detected legacy variable.
🤖 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.
Nitpick comments:
In `@airbyte/mcp/server.py`:
- Around line 144-152: Update the authentication setup around _AUTH_ENV_MAP and
_create_auth() to warn when legacy bare OIDC_CLIENT_ID or OIDC_CLIENT_SECRET is
present in os.environ while its corresponding AIRBYTE_MCP-prefixed variable is
unset. Keep _create_auth() reading only the new prefixed names, and emit a clear
migration warning for each detected legacy variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0278a2c4-9cde-4e8c-a815-ec773aa41d0f
📒 Files selected for processing (3)
airbyte/mcp/__init__.pyairbyte/mcp/http_main.pyairbyte/mcp/server.py
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-fastThe overall coverage in commit f14290d in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytest-no-credsThe overall coverage in commit f14290d in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytestThe overall coverage in commit f14290d in the Show a code coverage summary of the most impacted files.
Updated |
… shadowed by the baked JWKS default Co-Authored-By: AJ Steers <aj@airbyte.io>
Add AIRBYTE_MCP_AUTH_ALLOW_CLIENT_CREDENTIALS (off by default). When set, the server accepts Authorization: Basic base64(client_id:client_secret), exchanges it for a short-lived bearer token server-side (cached until shortly before expiry, guarded by an async lock), and rewrites the request to Bearer so the existing JWTVerifier validates it. Runs as the outermost ASGI layer, ahead of FastMCP's auth middleware. Bearer/OIDC paths and fail-closed startup unchanged. Co-Authored-By: AJ Steers <aj@airbyte.io>
Resolve http_main.py: keep the outermost client-credentials ASGI wrap (uvicorn) while adding main's assert_http_trusted_execution_disabled startup guard. Co-Authored-By: AJ Steers <aj@airbyte.io>
The HTTP client-credentials exchange imports httpx and the HTTP entrypoint runs uvicorn directly; both were previously transitive (deptry DEP003). Co-Authored-By: AJ Steers <aj@airbyte.io>
Avoids a CodeQL clear-text-logging false positive: the migration warning only logs env var *names*, but reading OIDC_CLIENT_SECRET via os.getenv created a sensitive-data taint source. Membership checks never read the value. Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@airbyte/mcp/_client_credentials.py`:
- Around line 82-95: Replace the global self._lock in the credential caching
flow with locks keyed by the per-credential cache key, and use the corresponding
lock in the request/token exchange path around cache access and minting. Ensure
different credential pairs proceed independently while concurrent requests for
the same credential remain serialized.
- Around line 137-170: Update _mint_token to catch httpx.HTTPError from the
token request and ValueError from response.json(), log a warning without
exposing credentials or tokens, and return None for either failure so token
verification continues through the existing fail-closed path.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5e197e3c-37a4-4214-aa6f-bf98e142237e
📒 Files selected for processing (4)
airbyte/mcp/__init__.pyairbyte/mcp/_client_credentials.pyairbyte/mcp/http_main.pyairbyte/mcp/server.py
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte/mcp/init.py
- Wrap the client-credentials token request in try/except for httpx.HTTPError and response.json() ValueError so an unreachable/non-JSON token endpoint falls through to the verifier (401) instead of surfacing a 500. - Replace the single asyncio.Lock with a per-credential lock so a slow token endpoint stalls only the affected credential, not all Basic-auth traffic. Addresses CodeRabbit review on #1084. Co-Authored-By: AJ Steers <aj@airbyte.io>
…view - Drop the branded (secret-named) replacement var from the legacy-env warning so no secret-named symbol reaches the log sink (CodeQL false positive on the OIDC_CLIENT_SECRET_ENV constant); still names the legacy vars. - Use public Mapping[str, str] instead of private os._Environ for the client_credentials_enabled env param. - Add unit tests for the opt-in Basic client-credentials middleware (enable flag, Basic decode, cache key, bearer rewrite, routing, caching, and fail-through). Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Save and restore the ambient AIRBYTE_MCP_ENV_FILE around the module-import-time pop so importing airbyte.mcp.server for these tests no longer permanently mutates the process environment for other test modules. Co-Authored-By: AJ Steers <aj@airbyte.io>
Match the merged ops-mcp server (airbytehq/airbyte-ops-mcp#1135): the cloud-mcp server reads only the branded AIRBYTE_MCP_OIDC_* / AIRBYTE_MCP_AUTH_* names, so the pre-rename generic names are simply unrecognized. Remove the _LEGACY_AUTH_ENV map and _warn_on_legacy_auth_env migration warning per the no-backwards-compatibility directive. Co-Authored-By: AJ Steers <aj@airbyte.io>
fastmcp-extensions 0.14.0 drops the library's env-var auth mapping (resolve_mcp_auth). Map the server's AIRBYTE_MCP_* env vars directly into JWTAuthConfig / OIDCAuthConfig and call build_mcp_auth. _resolve_signing_key now returns a (jwks_uri, public_key) tuple. Behavior preserved: Airbyte Cloud realm defaults with zero config, static-key-not-shadowed precedence, and interactive OIDC only when both client credentials are present. Co-Authored-By: AJ Steers <aj@airbyte.io>
…sing in lib) Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Summary
Running the Airbyte MCP server over HTTP now always authenticates, targeting Airbyte Cloud out of the box with zero auth config. This replaces the previous opt-in
MCP_AUTH_AIRBYTE_CLOUD=trueboolean, which meant a hosted deployment that set onlyOIDC_*silently accepted no headless bearer tokens (the bug that let a valid minted Airbyte Cloud token get401ed on prod).This server owns the Airbyte Cloud realm literals (public, non-secret) as defaults and maps its own Airbyte-branded env vars into the typed config objects
fastmcp_extensions.build_mcp_authconsumes, so the extensions library stays provider-agnostic (it reads no environment variables of its own).fastmcp-extensions0.14.0 migration (breaking upstream)fastmcp-extensions0.14.0removed the library-side env-var mapping (resolve_mcp_authand its genericOIDC_*/MCP_AUTH_*parsing). This PR bumps the floor to>=0.14.0,<1.0.0and moves the env→config translation into this server:The operator-facing env-var surface is unchanged — only the internal hop to generic names is gone.
_resolve_signing_key()now returns a(jwks_uri, public_key)tuple instead of a generic-name dict.Behavior (unchanged by the migration)
JWTVerifieris always built against Airbyte Cloud's_airbyte-application-clientsrealm (headless bearer works by default).airbyterealm).MultiAuth; HTTP startup fails fast if no auth provider resolves.AIRBYTE_MCP_*env vars for self-hosted deployments.Env var surface (server-owned branded names)
MCP_SERVER_URLunchanged (per-deploy public base URL / OIDC redirect base); defaults tohttp://localhost:8080.Opt-in: static client credentials via HTTP Basic
Behind
AIRBYTE_MCP_AUTH_ALLOW_CLIENT_CREDENTIALS(off by default), the server accepts long-lived credentials via standard HTTP Basic (client_secret_basic), exchanges them for a short-lived token server-side (async-locked, cached until shortly before expiry), rewrites toAuthorization: Bearer <token>, and lets the existing JWT verifier handle it.Bearer, unauthenticated, and malformed-Basic requests pass through unchanged. No custom headers, no URL credentials, no secrets/tokens logged.Test plan
poe check(ruff, pyrefly, collect-only) clean locally.tests/unit_tests/test_mcp_server_auth.pyupdated for the typed API:_resolve_signing_keytuple return, plus newbuild_mcp_auth-spy tests asserting the Cloud JWT defaults, branded-env claim overrides, OIDC activation only when both credentials present, and no OIDC on the bearer-only path.test_mcp_server_auth.py+test_mcp_client_credentials.py: 31 passed locally.Requested by AJ.
Link to Devin session: https://app.devin.ai/sessions/0b0d7690d5354d5c95e12ccf1b2e620e
Requested by: Aaron ("AJ") Steers (@aaronsteers)