fix: Proper MCP / Oauth support#10965
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 WalkthroughThis PR renames the OAuth callback field from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
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 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (39.26%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10965 +/- ##
==========================================
+ Coverage 32.94% 32.97% +0.02%
==========================================
Files 1387 1387
Lines 65371 65382 +11
Branches 9679 9681 +2
==========================================
+ Hits 21539 21557 +18
+ Misses 42736 42728 -8
- Partials 1096 1097 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/backend/base/langflow/api/v1/schemas.py (1)
438-443: Consider handling empty strings in normalization.The
model_post_initmethod only checks forNonevalues, but the service layer (line 1305 in service.py) also treats empty strings as requiring fallback tooauth_callback_path. For consistency, consider normalizing empty strings as well:def model_post_init(self, __context, /) -> None: """Normalize oauth_callback_path to oauth_callback_url for backwards compatibility.""" # If oauth_callback_url is not set but oauth_callback_path is, use the path value - if self.oauth_callback_url is None and self.oauth_callback_path is not None: + if (self.oauth_callback_url is None or not self.oauth_callback_url) and self.oauth_callback_path: self.oauth_callback_url = self.oauth_callback_path - # If both are set, oauth_callback_url takes precedence (already set correctly)This ensures that both empty strings and
Nonevalues trigger the backward compatibility fallback, matching the service layer logic.docs/openapi/openapi.yaml (1)
4300-4311: AuthSettings: deprecation + newoauth_callback_urlfield look correctThe schema keeps
oauth_callback_pathwith a clear deprecation notice and introducesoauth_callback_urlwith matching optional string/nulltyping, so existing clients remain valid while new ones can migrate to the URL-based field. Consider optionally addingformat: urionoauth_callback_url(and possibly the deprecated field) to better communicate expected semantics to tooling, but this is not required.src/frontend/src/modals/authModal/index.tsx (1)
89-122: OAuth callback URL autosync works, with two small polish opportunitiesThe autosync logic for
oauthCallbackUrl(including the regex guard and new URL construction) is reasonable and avoids overwriting customized values. Two minor nits you may want to consider:
- The comment on Line 96 still says “Callback Path”; updating it to “Callback URL” would better match the new behavior.
- When autosyncing, you always generate
http://${host}:${port}/auth/idaas/callback. If users configure an HTTPS server URL, this will silently downgrade tohttp. You could optionally derive the scheme fromoauthServerUrl(falling back tohttpif absent) to better respect secure setups.UI wiring (label
htmlFor, inputid,valuebinding, andhandleAuthFieldChange("oauthCallbackUrl", ...)) all line up correctly.Also applies to: 280-297, 124-139
docs/openapi/openapi.json (2)
6971-6983: Clarify deprecation and enforce path-only semantics for legacy field.Good deprecation. To prevent ambiguous values and guide migration, constrain the legacy field to a path and add an example/reason.
"oauth_callback_path": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], + "anyOf": [ + { + "type": "string", + "pattern": "^/.*" + }, + { + "type": "null" + } + ], "title": "Oauth Callback Path", "deprecated": true, - "description": "Deprecated: Use oauth_callback_url instead" + "description": "Deprecated: Use oauth_callback_url instead. This legacy field accepts only a path (e.g., '/oauth/callback').", + "examples": ["/oauth/callback"], + "x-deprecated-reason": "Replaced by oauth_callback_url; path-only value caused ambiguity behind proxies." },
6984-6994: Validate absolute URL and state precedence.Add URI format, an example, and a brief precedence note to reduce misconfigurations.
"oauth_callback_url": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "title": "Oauth Callback Url" + "anyOf": [ + { + "type": "string", + "format": "uri", + "minLength": 1 + }, + { + "type": "null" + } + ], + "title": "Oauth Callback Url", + "description": "Absolute callback URL used as the OAuth redirect_uri. If both oauth_callback_url and oauth_callback_path are provided, oauth_callback_url takes precedence.", + "examples": ["https://app.example.com/api/oauth/callback"] },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/docs/Agents/mcp-server.mdx(1 hunks)docs/openapi/openapi.json(1 hunks)docs/openapi/openapi.yaml(1 hunks)src/backend/base/langflow/api/v1/schemas.py(1 hunks)src/frontend/src/modals/authModal/index.tsx(6 hunks)src/frontend/src/types/mcp/index.ts(1 hunks)src/lfx/src/lfx/services/mcp_composer/service.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx}: Use React 18 with TypeScript for frontend development
Use Zustand for state management
Files:
src/frontend/src/types/mcp/index.tssrc/frontend/src/modals/authModal/index.tsx
docs/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (.cursor/rules/docs_development.mdc)
docs/docs/**/*.{md,mdx}: Markdown files must include YAML frontmatter with title, description, and sidebar_position
Use Docusaurus admonitions (:::tip, :::warning, :::danger) for important information, warnings, and critical alerts
Code blocks must include a title attribute and specify the language (e.g., ```python title="filename.py")
All images must use descriptive alt text that clearly explains what the image shows
Use sentence case for headers and proper capitalization for terminology: Langflow, Component, Flow, API, JSON
Use bold formatting for UI elements, italic for emphasis, and backticks for inline code
Use second person ('you') for instructions and present tense for current features in documentation content
Tables in documentation must include columns for Input/Output name, Type, Required (if applicable), and Description
Internal links between documentation pages must be functional and properly formatted using Docusaurus link syntax
Files:
docs/docs/Agents/mcp-server.mdx
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/**/*.py: Use FastAPI async patterns withawaitfor async operations in component execution methods
Useasyncio.create_task()for background tasks and implement proper cleanup with try/except forasyncio.CancelledError
Usequeue.put_nowait()for non-blocking queue operations andasyncio.wait_for()with timeouts for controlled get operations
Files:
src/backend/base/langflow/api/v1/schemas.py
src/backend/base/langflow/api/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Backend API endpoints should be organized by version (v1/, v2/) under
src/backend/base/langflow/api/with specific modules for features (chat.py, flows.py, users.py, etc.)
Files:
src/backend/base/langflow/api/v1/schemas.py
src/frontend/src/**/*.{tsx,jsx,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
Use Tailwind CSS for styling
Files:
src/frontend/src/modals/authModal/index.tsx
src/frontend/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{tsx,jsx}: Implement dark mode support using the useDarkMode hook and dark store
Use Lucide React for icon components in the application
Files:
src/frontend/src/modals/authModal/index.tsx
⏰ 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). (22)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Lint Backend / Run Mypy (3.11)
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Lint Backend / Run Mypy (3.10)
- GitHub Check: Lint Backend / Run Mypy (3.13)
- GitHub Check: Run Frontend Unit Tests / Frontend Jest Unit Tests
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Run Backend Tests / Test CLI - Python 3.10
- GitHub Check: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
- GitHub Check: Test Docs Build / Test Docs Build
- GitHub Check: Test Starter Templates
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: build-and-deploy
- GitHub Check: Run Ruff Check and Format
- GitHub Check: Update Starter Projects
- GitHub Check: Update Component Index
🔇 Additional comments (4)
src/lfx/src/lfx/services/mcp_composer/service.py (1)
1291-1308: LGTM! Backward compatibility properly implemented.The environment variable mapping has been updated to use
oauth_callback_url, and the backward compatibility logic correctly falls back tooauth_callback_pathwhen the new field is not provided. The implementation checks for both absence and empty values, ensuring robust migration support.docs/docs/Agents/mcp-server.mdx (1)
240-240: LGTM! Clear documentation update.The field label and description have been updated to accurately reflect that a full callback URL is required, which must match exactly what is registered with the OAuth provider. This clarifies the migration from path to URL.
src/frontend/src/types/mcp/index.ts (1)
6-7: LGTM! Proper deprecation and new field addition.The type definition correctly adds the new
oauth_callback_urlfield and marksoauth_callback_pathas deprecated with a clear inline comment guiding developers to use the new field.src/frontend/src/modals/authModal/index.tsx (1)
33-59: Init/useEffect fallback fromoauth_callback_path→oauth_callback_urlis soundState initialization and the
useEffectcorrectly preferoauth_callback_urland fall back tooauth_callback_path, so existing projects migrate seamlessly while new ones use the canonical field. No functional issues here.Also applies to: 61-81
This comment has been minimized.
This comment has been minimized.
|
Build successful! ✅ |
This pull request refactors the OAuth callback configuration by replacing the legacy
oauth_callback_pathfield with a new, more descriptiveoauth_callback_urlfield across the backend, frontend, documentation, and API schema. It also introduces backwards compatibility handling to ensure existing configurations remain functional while encouraging migration to the new field.OAuth Callback Field Migration
oauth_callback_urlfield to backend models (AuthSettings), frontend types (AuthSettingsType), and API schemas (openapi.yaml,openapi.json), markingoauth_callback_pathas deprecated and updating documentation to reflect the change. [1] [2] [3] [4] [5]authModal/index.tsxto useoauth_callback_url, with fallback tooauth_callback_pathfor backwards compatibility, and revised UI labels from "Callback Path" to "Callback URL". [1] [2] [3] [4] [5] [6]service.py) and environment variable mapping to useoauth_callback_url, with logic to fallback tooauth_callback_pathif necessary. [1] [2]These changes ensure a smoother transition for users and developers, improve clarity, and maintain compatibility with existing configurations.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.