-
Notifications
You must be signed in to change notification settings - Fork 8.4k
fix: clicking MCP Server button logs users out automatically #11448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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
WalkthroughThese changes simplify authentication error handling by removing manual logout and redirect logic from the auto-login hook, while introducing session readiness state management to the app initialization page to better track overall application startup status. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 5❌ Failed checks (1 error, 4 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11448 +/- ##
==========================================
- Coverage 34.84% 34.84% -0.01%
==========================================
Files 1420 1420
Lines 68188 68184 -4
Branches 9977 9976 -1
==========================================
- Hits 23762 23757 -5
- Misses 43202 43203 +1
Partials 1224 1224
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Cristhianzl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Problem
When
LANGFLOW_AUTO_LOGIN=false, clicking links that open in a new tab (e.g., MCP Server) would incorrectly show the login page even with a valid session. Refreshing the page would then correctly show authenticated content.Root Cause
A race condition between auto-login validation and session validation:
isAuthenticatedfrom Zustand (defaults tofalse)isAuthenticated=true, the error handler navigates to/loginChanges
1. src/frontend/src/pages/AppInitPage/index.tsx
Added:
isSessionReadycheck that delays rendering protected routes until session validation completes when auto-login is disabled.Reasoning: Ensures ProtectedRoute doesn't make redirect decisions based on the default
isAuthenticated=falsestate before session validation has a chance to confirm the user's actual authentication status.2. src/frontend/src/controllers/API/queries/auth/use-get-autologin.ts
Removed: The
manualLoginNotAuthenticatedbranch that navigated to/loginand calledmutationLogout().Reasoning:
isAuthenticatedwhich isfalseby default, before session validation could set it totruemutationLogout()was clearing valid session cookies before they could be validatedResult
When
LANGFLOW_AUTO_LOGIN=false:autoLogin=falseisAuthenticated=true→ authenticated content rendersisAuthenticated=false→ ProtectedRoute redirects to loginTesting Strategy
Setup: Ensure
.envhasLANGFLOW_AUTO_LOGIN=falseTest 1: New Tab Authentication (the fix)
Test 2: Unauthenticated User Still Redirects
/mcp/folder/{folderId}?redirect=paramTest 3: Login Flow Still Works
/login, enter credentialsTest 4: Session Expiry
Test 5: Auto-Login Mode (regression check)
LANGFLOW_AUTO_LOGIN=truein.env