feat(mcp): reuse caller's verified token for Airbyte Cloud API passthrough - #1078
feat(mcp): reuse caller's verified token for Airbyte Cloud API passthrough#1078Aaron ("AJ") Steers (aaronsteers) wants to merge 2 commits into
Conversation
…rough 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/1784059452-cloud-mcp-token-passthrough' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1784059452-cloud-mcp-token-passthrough'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. |
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMCP cloud client creation now gives bearer tokens precedence over client credentials, while empty tokens trigger credential fallback. Unit tests cover cloud-client selection and transport bearer-token resolution across missing, empty, and verified token cases. ChangesMCP bearer authentication
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-fastThe overall coverage in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytest-no-credsThe overall coverage in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytestThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
| if bearer_token: | ||
| client_id = None | ||
| client_secret = None | ||
|
|
There was a problem hiding this comment.
Before we throw away the client ID and client secret, should we check if the bearer token is even valid? If it's not valid, then using the client ID and secret instead to get a new token would be the preferred approach, rather than failing hard when all three are provided
There was a problem hiding this comment.
🙋 Human Input Needed: which fallback behavior do you want? Below is why I think the passthrough path is already safe, and the options for the config-injected-bearer edge case.
The passthrough path is already covered by transport verification. In hosted mode (MCP_AUTH_AIRBYTE_CLOUD=true), the JWTVerifier cryptographically validates the bearer (signature, issuer, audience, exp) before any tool runs. An expired/tampered/invalid token is rejected with a 401 at the transport layer, so _get_cloud_client never executes — by the time we drop the client creds, the token is guaranteed valid. This is the same guarantee Ops MCP relies on (its AGENTS.md matrix: no token → 401, valid → 200, tampered → 401), and its _get_access_token returns the bearer directly with no re-validation or fallback.
So the "invalid bearer + creds both present" case only arises off the passthrough path — i.e. when a bearer is injected via the AIRBYTE_CLOUD_BEARER_TOKEN env var / Authorization header on a server that isn't doing realm verification. That's an operator-config path, not the delegated-caller flow this PR targets.
Options if we want to harden that edge case:
- Local expiry check — decode the JWT
expand fall back to client creds if it's expired. Cheap and no network, but only catches expiry (not revocation/wrong-signature) and assumes the bearer is a decodable JWT rather than an opaque token. - Runtime fallback — try the bearer, and on
401/403retry with client creds. Most robust, butCloudClient/api_utilhas no such dual-credential retry today, so it's meaningfully more machinery and spans the whole cloud client — larger than this focused auth PR. - Keep as-is — bearer wins, matching the Ops MCP reference exactly; track graceful fallback separately if/when we want to support the static-bearer-plus-creds config.
My recommendation: keep this PR at option 3 (scoped precedence fix that mirrors Ops MCP) and open a follow-up for option 1 or 2 if you want the config path hardened. But happy to fold option 1 in here if you'd prefer it in one go — it's the only self-contained one. Which way do you want to go?
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes Cloud MCP authentication passthrough so that, when a verified transport bearer token is available, downstream Airbyte Cloud API calls are made as the caller (using that token) rather than failing due to mutually-exclusive auth inputs.
Changes:
- Update
_get_cloud_clientto preferbearer_tokenand dropclient_id/client_secretwhen a bearer is present. - Add unit tests covering
_get_cloud_clientbearer-vs-client-credentials precedence. - Add unit tests for
_resolve_transport_bearer_tokenbehavior in stdio vs verified-token contexts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
airbyte/mcp/cloud.py |
Prefer bearer-token auth for CloudClient construction to avoid mutual-exclusivity errors and enable delegated Cloud API calls. |
tests/unit_tests/test_mcp_cloud.py |
Adds tests asserting bearer-token precedence and fallback to client credentials when bearer token is empty. |
tests/unit_tests/test_mcp_tool_utils.py |
Adds tests for transport bearer-token resolution behavior (None/empty/verified token). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ecedence Co-Authored-By: AJ Steers <aj@airbyte.io>
| return CloudClient( | ||
| bearer_token=bearer_token, | ||
| public_api_root=api_url, | ||
| config_api_root=config_api_url, | ||
| organization_id=organization_id, | ||
| ) |
There was a problem hiding this comment.
I don't understand why we shouldn't keep the prior implementation and make CloudClient handle this for us.
return CloudClient(
client_id=client_id,
client_secret=client_secret,
bearer_token=bearer_token,
public_api_root=api_url,
config_api_root=config_api_url,
organization_id=organization_id,
)
I also don't understand the claim that the CloudClient class would fail auth or fail to auth in the prior implementaton. Everything appears to have been correct before this PR.
|
Closing per discussion with AJ (Devin session: https://app.devin.ai/sessions/a8d355a69ca34409bd3b13d2a8122123). This MCP-layer precedence fix was misguided: the pre-existing code already handled pure-passthrough and pure-stdio correctly. The real issue is that |
Summary
The Cloud MCP (hosted at
cloud-mcp) should call the Airbyte Cloud API as the caller, not solely as a statically-configured application. When an agent authenticates the MCP transport with an Airbyte-Cloud-issued bearer token (verified against the_airbyte-application-clientsrealm whenMCP_AUTH_AIRBYTE_CLOUD=true), that same JWT is itself a valid Cloud API bearer — so it should flow through to downstream Cloud calls, delegating them to the caller's identity. Configuredclient_id/client_secretremain the fallback for stdio/local mode where there is no caller token.The transport-side wiring for this already landed in #1073 (
_resolve_transport_bearer_token→BEARER_TOKEN_CONFIG_ARGdefaulting to the verified transport token, plus the Airbyte Cloud realm defaults). The missing piece was the "bearer wins over client creds" resolution at the point the MCP tools build aCloudClient._get_cloud_clientread all three ofbearer_token,client_id,client_secretfrom MCP config and passed them together toCloudClient. ButCloudClient(via_AirbyteCredentials.from_auth) rejects receiving both a bearer token and client credentials at once. In hosted mode — transport token present andAIRBYTE_CLOUD_CLIENT_ID/SECRETset in the environment — this raisedCannot use both client credentials and bearer token authentication, breaking the passthrough.This PR makes the MCP client-construction helper prefer the bearer token, mirroring Ops MCP's
_get_access_token("ifbearer_tokenis provided, use it directly; otherwise exchange client credentials"):How it mirrors Ops MCP
BEARER_TOKENconfig arg defaulting to the transport token — already present in PyAirbyte from feat(mcp): support headless bearer-token transport auth #1073 (server.py::_create_auth,_tool_utils.py::_resolve_transport_bearer_token/BEARER_TOKEN_CONFIG_ARG), identical in shape toairbyte_ops_mcp/mcp/server.py._get_access_token-style "bearer wins over client creds" — Ops MCP centralizes this incloud_admin/api_client.py::_get_access_token. In PyAirbyte the low-levelapi_util.get_airbyte_server_instanceintentionally requires exactly one of bearer vs. client creds (a documentedCloudClient/CloudClientConfiginvariant), so the analogous, scoped place to apply "bearer wins" is the MCP client-construction helper_get_cloud_client. This keeps the change confined to the MCP auth layer without altering the public Cloud client's mutual-exclusivity contract.Expected behavior
cloud-mcp): caller's verified Airbyte Cloud token → used directly as the Cloud API bearer (delegated identity). Env client creds present but ignored while a token is passed._resolve_transport_bearer_tokenreturns"") → falls back toAIRBYTE_CLOUD_CLIENT_ID/AIRBYTE_CLOUD_CLIENT_SECRET.Requested by AJ Steers (Aaron ("AJ") Steers (@aaronsteers)).
Test Plan
Added focused unit tests:
tests/unit_tests/test_mcp_cloud.py::test_get_cloud_client_prefers_bearer_token_over_client_credentials— parametrized: bearer token present →CloudClientgets the bearer andclient_id/client_secretare dropped (no mutual-exclusivity error); empty bearer (stdio) → falls back to client creds.tests/unit_tests/test_mcp_tool_utils.py::test_resolve_transport_bearer_token— parametrized:get_access_token()returnsNone(stdio) or an empty token →""; a verified token → returned as-is.Local checks (all green):
uv run ruff format --check .(changed files)uv run ruff check .(changed files)uv run pyrefly check(0 errors; the 3 warnings are pre-existingredundant-castincloud.py)uv run pytest tests/unit_tests/test_mcp_cloud.py tests/unit_tests/test_mcp_tool_utils.py→ 18 passedLink to Devin session: https://app.devin.ai/sessions/a8d355a69ca34409bd3b13d2a8122123
Summary by CodeRabbit
Bug Fixes
Tests