feat: encrypt oauth auth settings at rest #9490
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughImplements encryption for MCP auth_settings: adds an Alembic migration to encrypt/decrypt existing folder.auth_settings, introduces a new encryption utility module, updates API endpoints to encrypt on write and decrypt on read (plus adjusted mcp-proxy args/env), and adds unit tests for both the API behavior and encryption utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as MCP Projects API
participant Enc as mcp_encryption
participant DB
rect rgba(230,240,255,0.5)
note right of Client: Update project MCP settings
Client->>API: PATCH /projects/{id}/mcp (auth_settings)
API->>Enc: encrypt_auth_settings(auth_settings)
Enc-->>API: encrypted_auth_settings
API->>DB: UPDATE project.auth_settings = encrypted_auth_settings
DB-->>API: OK
API-->>Client: 200
end
rect rgba(230,255,230,0.5)
note right of Client: List project tools / Get project
Client->>API: GET /projects/{id}
API->>DB: SELECT project.auth_settings
DB-->>API: encrypted_auth_settings
API->>Enc: decrypt_auth_settings(encrypted_auth_settings)
Enc-->>API: plaintext_auth_settings
API-->>Client: 200 (auth_settings decrypted)
end
sequenceDiagram
autonumber
participant API as MCP Projects API
participant DB
participant Enc as mcp_encryption
participant Proc as mcp-proxy (subprocess)
API->>DB: SELECT project.auth_settings
DB-->>API: encrypted_auth_settings
alt has auth_settings
API->>Enc: decrypt_auth_settings(...)
Enc-->>API: auth_settings (includes auth_type, OAuth fields)
API->>Proc: spawn mcp-proxy<br/>args: ["mcp-proxy","--sse-url",sse_url,"--auth_type",auth_type]<br/>env: OAUTH_* derived from settings
else no auth_settings
API->>Proc: spawn mcp-proxy<br/>args: ["mcp-proxy","--sse-url",sse_url]
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
enhancement
jordanrfrazier
force-pushed
the
mcp-composer-oauth
branch
from
d1ccde9 to
5a0b9bb
Compare
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
![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
♻️ Duplicate comments (1)
src/backend/tests/unit/services/auth/test_mcp_encryption.py (1)
152-167: is_encrypted coverage is helpful; signature tweak recommendedGiven the test passes None, updating the function type hints (see prior comment) will align code and tests.
🧹 Nitpick comments (6)
src/backend/tests/unit/api/v1/test_mcp_projects.py (1)
447-513: Solid end-to-end coverage for at-rest encryption; add a couple of quick wins
- Nice: validates encryption at write and decryption at read against the real DB.
- Nit: URL usage is mixed across this module (some calls start with "/", others not). Consider standardizing for consistency.
- Optional: Strengthen the DB assertion by also verifying the ciphertext looks like something we produced (e.g., using is_encrypted) in addition to the length check.
You could add this assertion (imports at top as needed):
@@ - stored_secret = updated_project.auth_settings.get("oauth_client_secret") + stored_secret = updated_project.auth_settings.get("oauth_client_secret") assert stored_secret is not None assert stored_secret != "super-secret-password-123" # Should be encrypted - # The encrypted value should be a base64-like string (Fernet token) + # The encrypted value should be a base64-like string (Fernet token) assert len(stored_secret) > 50 # Encrypted values are longer + # And it should be decryptable with our key (i.e., recognized as "encrypted") + from langflow.services.auth.mcp_encryption import is_encrypted + assert is_encrypted(stored_secret)src/backend/base/langflow/api/v1/mcp_projects.py (1)
123-131: Decryption on read works; consider not returning secrets by defaultDecrypting before building the AuthSettings model is correct. However, returning plaintext secrets in a GET response can be risky. Consider an opt-in flag (e.g., include_secrets=true for owners) or redaction by default for highly sensitive fields.
src/backend/base/langflow/alembic/versions/0882f9657f22_encrypt_existing_mcp_auth_settings_.py (2)
8-16: Remove unused imports to satisfy linterssqlmodel, Inspector, and migration are imported but unused; get_settings_service is imported in upgrade/downgrade but unused as well. Clean these up to keep make lint green.
@@ -import sqlmodel -from sqlalchemy.engine.reflection import Inspector -from langflow.utils import migration + # (removed unused imports)And inside upgrade/downgrade:
- from langflow.services.deps import get_settings_service + # get_settings_service is not needed here; encrypt/decrypt functions handle it internally
25-73: Migration logic is reasonable; watch JSON handling across backendsWriting json.dumps(...) into a JSON/JSONB column via text() works on SQLite but may double-encode on PostgreSQL unless casted. Consider using a Table-bound UPDATE with typed parameters, or CAST on PG.
Outside this hunk, an example approach:
folder = sa.Table("folder", sa.MetaData(), autoload_with=conn) stmt = folder.update().where(folder.c.id == folder_id).values(auth_settings=encrypted_settings) conn.execute(stmt)This lets SQLAlchemy handle JSON serialization for the target dialect.
src/backend/base/langflow/services/auth/mcp_encryption.py (1)
88-108: Broaden type and short-circuit in is_encryptedTests call is_encrypted(None) and the function already guards, but the type hints say str only. Make it Optional[str | bytes] and keep the early return.
-def is_encrypted(value: str) -> bool: +from typing import Optional, Union + +def is_encrypted(value: Optional[Union[str, bytes]]) -> bool: @@ - if not value: + if not value: return False @@ - auth_utils.decrypt_api_key(value, settings_service) + # decrypt_api_key expects str; normalize + to_check = value.decode() if isinstance(value, (bytes, bytearray)) else value + auth_utils.decrypt_api_key(to_check, settings_service) return Truesrc/backend/tests/unit/services/auth/test_mcp_encryption.py (1)
82-93: Minor: avoid unnecessary patchesThese tests don’t exercise settings access (they return early for None), so patching get_settings_service isn’t needed.
- @patch("langflow.services.auth.mcp_encryption.get_settings_service") - def test_encrypt_none_returns_none(self, mock_get_settings): + def test_encrypt_none_returns_none(self): @@ - @patch("langflow.services.auth.mcp_encryption.get_settings_service") - def test_decrypt_none_returns_none(self, mock_get_settings): + def test_decrypt_none_returns_none(self):
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
src/backend/base/langflow/alembic/versions/0882f9657f22_encrypt_existing_mcp_auth_settings_.py(1 hunks)src/backend/base/langflow/api/v1/mcp_projects.py(4 hunks)src/backend/base/langflow/services/auth/mcp_encryption.py(1 hunks)src/backend/tests/unit/api/v1/test_mcp_projects.py(1 hunks)src/backend/tests/unit/services/auth/test_mcp_encryption.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/tests/unit/services/auth/test_mcp_encryption.pysrc/backend/base/langflow/services/auth/mcp_encryption.pysrc/backend/tests/unit/api/v1/test_mcp_projects.pysrc/backend/base/langflow/alembic/versions/0882f9657f22_encrypt_existing_mcp_auth_settings_.pysrc/backend/base/langflow/api/v1/mcp_projects.py
src/backend/tests/unit/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Test component integration within flows using create_flow, build_flow, and get_build_events utilities
Files:
src/backend/tests/unit/services/auth/test_mcp_encryption.pysrc/backend/tests/unit/api/v1/test_mcp_projects.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/unit/services/auth/test_mcp_encryption.pysrc/backend/tests/unit/api/v1/test_mcp_projects.py
🧬 Code graph analysis (5)
src/backend/tests/unit/services/auth/test_mcp_encryption.py (2)
src/backend/base/langflow/services/auth/mcp_encryption.py (3)
decrypt_auth_settings(58-85)encrypt_auth_settings(22-55)is_encrypted(88-107)src/backend/base/langflow/services/auth/utils.py (1)
encrypt_api_key(456-460)
src/backend/base/langflow/services/auth/mcp_encryption.py (2)
src/backend/base/langflow/services/deps.py (1)
get_settings_service(117-130)src/backend/base/langflow/services/auth/utils.py (2)
decrypt_api_key(463-489)encrypt_api_key(456-460)
src/backend/tests/unit/api/v1/test_mcp_projects.py (3)
src/backend/tests/conftest.py (1)
logged_in_headers(497-503)src/backend/base/langflow/services/deps.py (1)
session_scope(157-179)src/backend/base/langflow/services/database/models/folder/model.py (1)
Folder(21-36)
src/backend/base/langflow/alembic/versions/0882f9657f22_encrypt_existing_mcp_auth_settings_.py (2)
src/backend/base/langflow/services/auth/mcp_encryption.py (2)
encrypt_auth_settings(22-55)decrypt_auth_settings(58-85)src/backend/base/langflow/services/deps.py (1)
get_settings_service(117-130)
src/backend/base/langflow/api/v1/mcp_projects.py (2)
src/backend/base/langflow/services/auth/mcp_encryption.py (2)
decrypt_auth_settings(58-85)encrypt_auth_settings(22-55)src/backend/base/langflow/api/v1/schemas.py (1)
AuthSettings(444-458)
⏰ 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). (4)
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Update Starter Projects
- GitHub Check: Run Ruff Check and Format
- GitHub Check: Ruff Style Check (3.13)
🔇 Additional comments (7)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
40-41: Importing encryption helpers where they’re used is appropriateThe new imports are scoped correctly to this module and keep responsibilities clear.
src/backend/base/langflow/alembic/versions/0882f9657f22_encrypt_existing_mcp_auth_settings_.py (1)
75-123: Rollback path mirrors upgrade; ensure key stability between envsDecrypting on downgrade is symmetric—good. Operationally, ensure the same SECRET_KEY material is present during both upgrade and runtime; otherwise, records may become undecryptable.
Provide upgrade/runbook notes to ops that SECRET_KEY must be stable and present when running Alembic migrations for this revision.
src/backend/base/langflow/services/auth/mcp_encryption.py (3)
10-19: SENSITIVE_FIELDS set looks good and future-proofCovers typical token/secret fields. Keeping it centralized simplifies future additions.
22-56: Encryption path is correct and idempotent
- Attempts decrypt-first to avoid double-encryption—good.
- Copies input before mutation, avoids side effects.
- Logs at debug without leaking values.
58-86: Decryption path handles mixed statesGracefully tolerates plaintext legacy values—good for backward compatibility.
src/backend/tests/unit/services/auth/test_mcp_encryption.py (2)
14-28: Mock settings service is realistic and lightweightUsing a generated Fernet key via SecretStr-like mock ensures encrypt/decrypt paths are exercised faithfully.
48-67: Happy path encryption assertions hit the right thingsChecks only sensitive fields are transformed.
jordanrfrazier
force-pushed
the
mcp-composer-oauth
branch
from
8c7e860 to
f66f222
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
github-actions
bot
added
enhancement
|
Adds encryption at rest for sensitive fields in auth settings, and decrypts on retrieval.
Ignore everything but the first few files; the rest are just import re-orderings.
To test manually:
LANGFLOW_FEATURE_MCP_COMPOSER=true~/Documents/langflow/langflow mcp-composer-oauth ❯ sqlite3 src/backend/base/langflow/langflow.db "SELECT name, auth_settings FROM folder WHERE name='TestProject1';"# update to your project nameYou should see an encrypted value where your secret key lives (instead of plaintext)
Summary by CodeRabbit
New Features
Tests
Chores