feat(mcp): map branded AIRBYTE_MCP_* env into typed build_mcp_auth - #1085
Conversation
Migrate airbyte/mcp/server.py off the retired fastmcp_extensions.resolve_mcp_auth env mapping onto the typed build_mcp_auth API. This server now owns its branded AIRBYTE_MCP_* env names and maps them into JWTAuthConfig/OIDCAuthConfig, with a PyAirbyte-owned storage-factory hook (AIRBYTE_MCP_OIDC_CLIENT_STORAGE_FACTORY) feeding OIDCAuthConfig.client_storage. HTTP transport is now always authenticated, defaulting to the Airbyte Cloud realm. Bumps fastmcp-extensions to >=0.14.0. 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/1784854608-mcp-branded-typed-auth' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1784854608-mcp-branded-typed-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. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates PyAirbyte’s MCP HTTP transport authentication to align with fastmcp-extensions>=0.14.0 by moving env-var parsing into airbyte.mcp.server and passing typed auth config objects into build_mcp_auth. It also rebrands auth-related environment variables into the AIRBYTE_MCP_* namespace and makes HTTP transport authentication default-on (Cloud realm defaults).
Changes:
- Bump
fastmcp-extensionsto>=0.14.0,<1.0.0and migrate fromresolve_mcp_authtobuild_mcp_authwith typedJWTAuthConfig/OIDCAuthConfig. - Introduce branded
AIRBYTE_MCP_*env-var mapping (including blank-as-unset behavior) and optional OIDC client storage factory injection. - Add unit tests covering the new env mapping and auth assembly defaults/precedence.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates locked dependency version for fastmcp-extensions and its transitive deps. |
pyproject.toml |
Bumps fastmcp-extensions constraint and documents the migration rationale. |
airbyte/mcp/server.py |
Implements branded env-var parsing and builds typed auth configs for build_mcp_auth. |
airbyte/mcp/http_main.py |
Updates HTTP entrypoint documentation to match the new auth behavior and env vars. |
airbyte/mcp/__init__.py |
Updates user-facing docs for always-on HTTP auth and rebranded env vars. |
tests/unit_tests/test_mcp_auth.py |
Adds coverage for env mapping, defaults, signing-key precedence, OIDC activation, and storage factory injection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
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 is now configured through server-owned ChangesMCP authentication configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Environment
participant server._create_auth
participant build_mcp_auth
participant JWTVerifier
participant OIDCProxy
Environment->>server._create_auth: AIRBYTE_MCP_* values
server._create_auth->>build_mcp_auth: JWTAuthConfig and OIDCAuthConfig
build_mcp_auth->>JWTVerifier: Verify bearer tokens
build_mcp_auth->>OIDCProxy: Configure interactive OIDC
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/__init__.py`:
- Around line 204-205: Update the defaults description near the Airbyte Cloud
realm note to state that the verifier defaults to Cloud’s JWKS, issuer,
audience, and algorithm, while AIRBYTE_MCP_AUTH_JWT_PUBLIC_KEY has no Cloud
default. Clarify that the Cloud JWKS fallback is used only when neither
key-source override is configured, without implying all headless variables are
populated.
- Line 203: Clarify the HTTP guide namespace wording in airbyte/mcp/__init__.py
at lines 203-203 by limiting the AIRBYTE_MCP_* statement to transport-auth names
or explicitly identifying MCP_SERVER_URL as the exception. In
airbyte/mcp/http_main.py at lines 12-16, retain MCP_SERVER_URL as the base-URL
variable and align the surrounding wording with that exception.
🪄 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 Plus
Run ID: f4379817-9114-43f5-8c61-281ca1c76d7c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
airbyte/mcp/__init__.pyairbyte/mcp/http_main.pyairbyte/mcp/server.pypyproject.tomltests/unit_tests/test_mcp_auth.py
devin-ai-integration
Bot
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
airbyte/mcp/server.py:229
pkgutil.resolve_name()will raise (e.g.,ImportError,AttributeError,ValueError) ifAIRBYTE_MCP_OIDC_CLIENT_STORAGE_FACTORYis malformed or points at a missing symbol, which will currently crash module import / server startup with a low-context traceback. Consider catching exceptions here and raising a clearer error mentioning the env var name and expected format.
factory_spec = os.getenv(OIDC_CLIENT_STORAGE_FACTORY_ENV, "").strip()
if not factory_spec:
return None
factory: _ClientStorageFactory = pkgutil.resolve_name(factory_spec)
return factory(encryption_source_material=encryption_source_material)
airbyte/mcp/server.py:232
_create_auth()now always builds aJWTAuthConfig(with Cloud defaults) and therefore should always return anAuthProvider. Keeping| Nonein the return type is misleading for callers and for static typing (and contradicts the docstring/tests that assert a verifier is returned by default).
def _create_auth() -> AuthProvider | None:
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Thanks @copilot. Addressing the two low-confidence findings from the overview: ☑️ 🚫 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
airbyte/mcp/server.py:180
- The storage-factory hook is described as optional/nullable (and
_resolve_client_storage()returnsAsyncKeyValue | None), but_ClientStorageFactory.__call__is typed as always returningAsyncKeyValue. This makes the typing contract inconsistent and prevents factories from intentionally returningNoneto keep the in-memory default.
class _ClientStorageFactory(Protocol):
"""Callable that builds a durable `OIDCProxy` OAuth-state backend.
A deployment names its factory via
`AIRBYTE_MCP_OIDC_CLIENT_STORAGE_FACTORY` (`"package.module:callable"`). The
callable receives the OIDC client secret as `encryption_source_material` so
it can derive an at-rest encryption key, and returns an `AsyncKeyValue`
store. Keeping the concrete backend (Firestore, Redis, ...) behind this hook
lets PyAirbyte stay generic — the infrastructure-specific factory ships in
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
airbyte/mcp/server.py:242
_resolve_client_storage()wraps failures frompkgutil.resolve_name(), but if the resolved object is not callable or doesn't accept the requiredencryption_source_material=...kwarg, theTypeErrorfromfactory(...)will bubble up without naming the env var or expected callable signature. Since this is operator-facing configuration, it’s better to re-raise a clearValueErrorin that case as well.
return factory(encryption_source_material=encryption_source_material)
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-fastThe overall coverage in commit eacae28 in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytest-no-credsThe overall coverage in commit eacae28 in the Show a code coverage summary of the most impacted files.
Python / code-coverage/pytestThe overall coverage in commit eacae28 in the Show a code coverage summary of the most impacted files.
Updated |
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
airbyte/mcp/server.py:214
_resolve_client_storage()wraps errors frompkgutil.resolve_name, but if the resolved object is not callable or has an incompatible signature, calling it will raiseTypeErrorwith a less actionable traceback. Since this is an operator-facing env var, consider catchingTypeErrorand re-raising aValueErrorthat names the env var and expected callable signature.
return factory(encryption_source_material=encryption_source_material)
…h warning Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
airbyte/mcp/server.py (1)
251-271: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail closed on partial OIDC env configuration?
_create_auth()rejects missingAIRBYTE_MCP_OIDC_CONFIG_URL, but if onlyAIRBYTE_MCP_OIDC_CLIENT_IDorAIRBYTE_MCP_OIDC_CLIENT_SECRETis set, OIDC stays unset and HTTP can still start unauthenticated. Since docs say both OIDC values are required, wdyt about raising aValueErrorfor that partial-credential case so a typo/rotator leak is not silently ignored?🤖 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 251 - 271, Update _create_auth so it raises ValueError when exactly one of OIDC_CLIENT_ID_ENV or OIDC_CLIENT_SECRET_ENV is configured, rather than leaving oidc unset and starting unauthenticated. Preserve the existing configuration-URL validation and OIDCAuthConfig construction when both credentials are present.
🧹 Nitpick comments (1)
airbyte/mcp/http_main.py (1)
104-105: 🩺 Stability & Availability | 🔵 TrivialStartup warning for unauthenticated HTTP removed — worth an info log instead?
Unauthenticated-by-default is now an intentional fallback, so a warning no longer fits — but dropping all visibility means an operator has no easy way to confirm from logs whether a deployment is actually running with auth configured. wdyt about keeping a single
logger.infoline stating whether auth is enabled (and which mode), so this is still discoverable without digging through env vars?🤖 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/http_main.py` around lines 104 - 105, Update the HTTP startup logging around logger.info to retain one informational message describing whether authentication is enabled and, when enabled, which authentication mode is configured; preserve the intentional unauthenticated fallback without emitting a warning.
🤖 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.
Outside diff comments:
In `@airbyte/mcp/server.py`:
- Around line 251-271: Update _create_auth so it raises ValueError when exactly
one of OIDC_CLIENT_ID_ENV or OIDC_CLIENT_SECRET_ENV is configured, rather than
leaving oidc unset and starting unauthenticated. Preserve the existing
configuration-URL validation and OIDCAuthConfig construction when both
credentials are present.
---
Nitpick comments:
In `@airbyte/mcp/http_main.py`:
- Around line 104-105: Update the HTTP startup logging around logger.info to
retain one informational message describing whether authentication is enabled
and, when enabled, which authentication mode is configured; preserve the
intentional unauthenticated fallback without emitting a warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17be8360-c2f5-4c2a-960c-78c5ffab593c
📒 Files selected for processing (4)
airbyte/mcp/__init__.pyairbyte/mcp/http_main.pyairbyte/mcp/server.pytests/unit_tests/test_mcp_auth.py
🚧 Files skipped from review as they are similar to previous changes (1)
- airbyte/mcp/init.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
airbyte/mcp/server.py:216
_resolve_client_storage()wraps resolve/import failures into a clearValueError, but the actual factory invocation can still raise a rawTypeError(e.g. resolved symbol is not callable, or doesn’t accept theencryption_source_materialkwarg). Since this is an operator-facing env var, it’s better to re-raise aValueErrornamingAIRBYTE_MCP_OIDC_CLIENT_STORAGE_FACTORYand the expected callable signature to avoid an opaque startup traceback.
return factory(encryption_source_material=encryption_source_material)
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Addressing CodeRabbit's two out-of-diff findings (they couldn't post inline) — both fixed in eacae28:
Ruff + pyrefly clean; 21 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
airbyte/mcp/server.py:216
_resolve_client_storage()wraps errors frompkgutil.resolve_name(), but if the resolved symbol is not callable or has an incompatible signature, the subsequentfactory(...)call will raise a rawTypeError(operator-facing traceback) rather than a clearValueErrornaming the env var and required parameter. Since this is configured via an env var, it’s worth wrapping call-timeTypeErroras well.
)
raise ValueError(msg) from exc
return factory(encryption_source_material=encryption_source_material)
|
Summary
fastmcp-extensions0.14.0 removed the library's env-var mapping (resolve_mcp_auth), so each MCP server now owns its own env names and hands the library typed config objects. This migrates PyAirbyte's HTTP-transport auth (airbyte/mcp/server.py) to the typedbuild_mcp_authAPI and rebrands its auth env vars into theAIRBYTE_MCP_*namespace (AJ: "read branded env var names — unique domain of env vars is added security layer").Names here, values in the deployment. PyAirbyte declares only the env var names and maps their values into typed configs. It embeds no provider-specific configuration values — no
cloud.airbyte.comrealm URLs, issuer, JWKS URI, audience, or algorithm. Those concrete values are supplied at deploy time by the deployment's own repo (the hosted Cloud MCP image inairbyte-ops-mcp), keeping infra config out of this generic library.Behavior:
JWTVerifieractivates once a signing-key source (AIRBYTE_MCP_AUTH_JWKS_URIorAIRBYTE_MCP_AUTH_JWT_PUBLIC_KEY) is set; the interactiveOIDCProxyactivates onceAIRBYTE_MCP_OIDC_CLIENT_ID+AIRBYTE_MCP_OIDC_CLIENT_SECRETare set. With no auth env,_create_auth()returnsNone(unauthenticated local behavior).AIRBYTE_MCP_OIDC_CONFIG_URLis not,_create_auth()raises a clearValueErrornaming the missing var rather than falling back to a baked-in realm.AIRBYTE_MCP_*), and theMCP_AUTH_AIRBYTE_CLOUDopt-in toggle is removed.MCP_SERVER_URLstays unbranded (it's a deployment URL, not an auth var).New
_create_auth()shape:PyAirbyte stays backend-agnostic. Durable OAuth-state storage for the interactive
OIDCProxyis injected via a PyAirbyte-owned factory hook rather than any concrete store:The concrete Firestore/Fernet factory (and its infra config) ships in the deployment's own package — the hosted Cloud MCP image in
airbyte-ops-mcp— keeping PyAirbyte free of infra-specific params.Docs in
airbyte/mcp/__init__.pyandairbyte/mcp/http_main.pyupdated to the branded names and the "names here, values at deploy time" model. Bumpsfastmcp-extensionsto>=0.14.0. Addstests/unit_tests/test_mcp_auth.pycovering the branded names, blank-as-unset handling, per-path activation, JWT/OIDC env mapping, the missing-discovery-URL error, and storage-factory resolution/injection.Requested by AJ Steers.
Link to Devin session: https://app.devin.ai/sessions/a5b9501ef92c412aad0408b7c74ef9c8
Requested by: Aaron ("AJ") Steers (@aaronsteers)
Summary by CodeRabbit
New Features
AIRBYTE_MCP_*settings for interactive OIDC and headless bearer-token (JWT) verification, including optional durable OIDC client-state storage.AIRBYTE_MCP_*values, while stdio remains unauthenticated.Documentation
AIRBYTE_MCP_*namespace and clarified blank-as-unset fallback to unauthenticated local behavior when unset.Tests
Chores
fastmcp-extensionsdependency constraint.