fix(auth): break infinite redirect loop on /cp/auth/login - #1541
Conversation
There was a problem hiding this comment.
Review: PR #1541 — ✅ APPROVE (CP-QA recommendation)
Auth Redirect Loop Fix
Changes:
- AuthGate.tsx: /cp/auth/* paths excluded from gate (they ARE the login pages)
- api.ts: On 401, redirect to sign-in once
- auth.ts: getAuthOrigin() detects SaaS tenant subdomains, rewrites to app.moleculesai.app for auth
Security Assessment ✅
- 401 handler only redirects to platform's own auth origin — no open redirect
- /cp/auth/ paths excluded from AuthGate before session check
- SaaS subdomain rewrite is architecturally correct
No regressions: Canvas tests 816/816 ✅
There was a problem hiding this comment.
PR #1541 — fix(auth): break infinite redirect loop on /cp/auth/login
LGTM. Aligns with CP-QA review above.
Security:
redirectToLogin()now guards: if already on/cp/auth/, returns early — closes the infinite redirect loop ✅getAuthOrigin()rewrites SaaS tenant subdomain →app.moleculesai.appfor auth — correct, since tenant subdomains proxy to EC2 which has no auth routes ✅- 401 handler in
api.tsredirects once via dynamic import (no circular dep) ✅ - AuthGate excludes
/cp/auth/*paths before session check ✅
E2E failures: Canvas tabs E2E + Canvas (Next.js) failing — these are hitting degraded staging infrastructure (PM workspace f0897ffd + EC2 cascade down per Slack). NOT caused by this PR. Recommend re-run once infrastructure recovers.
Approve from QA angle — clean auth fix.
|
Core-QA Review ✅ — Infinite redirect loop fix is correct. AuthGate now skips redirect for /cp/auth/* paths (the login pages themselves). 401 responses trigger a one-time redirect to login instead of looping. Tenant subdomain logic ensures auth UI is always reached at app.moleculesai.app. Well-scoped fix (+31/-1). Approved for merge. |
|
CP-QA Review — 2026-04-22 ~18:50Z APPROVE ✅ Fixes the infinite redirect loop (GH issue) by:
Architecture note: Auth UI lives on app subdomain, not tenant subdomains. The tenant subdomains proxy to EC2 which has no auth routes. This is the correct design. CI note: Canvas tabs E2E and Canvas (Next.js) failures are infrastructure issues (self-hosted runner), not code. Recommend re-run after runner fix. Action: APPROVE from QA. Small, focused, correct fix. |
9ba26b1 to
9b842c2
Compare
AuthGate redirected anonymous users to /cp/auth/login?return_to=<url>, but the login page itself triggered AuthGate, which redirected again with double-encoded return_to. Each redirect added another encoding layer until the URL exceeded 431 (Request Header Fields Too Large). Two guards: 1. redirectToLogin() returns early if already on /cp/auth/* path 2. AuthGate skips redirect check entirely for /cp/auth/* paths [Molecule-Platform-Evolvement-Manager] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When session credentials expire mid-use, ALL API calls return 401. Previously this threw a generic error that crashed the UI with no recovery path. Now the API client intercepts 401 and redirects to login once (via redirectToLogin which already guards against loops). Combined with the AuthGate /cp/auth/* path guard, this gives the correct behavior: credentials lost → redirect to login → user logs in → return_to sends them back. [Molecule-Platform-Evolvement-Manager] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…omain Tenant subdomains (hongmingwang.moleculesai.app) proxy to EC2 platform which has no /cp/auth/* routes. Auth UI lives on app.moleculesai.app. Added getAuthOrigin() that detects SaaS tenant hosts and redirects to the app subdomain for login/signup. Non-SaaS hosts (localhost, dev) fall back to PLATFORM_URL as before. [Molecule-Platform-Evolvement-Manager] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The pathname.startsWith() loop-break added to redirectToLogin needs pathname on the mock Location object; tests were supplying only href. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
9b842c2 to
2c3eccf
Compare
There was a problem hiding this comment.
PR #1541 Review: fix(auth): break infinite redirect loop on /cp/auth/login
Recommendation: APPROVE ✅ (review posted from account that authored this PR — formal approval requires human reviewer)
1. Is the redirect loop fix correct and complete?
Yes. The fix has three coordinated layers:
redirectToLogin()(auth.ts): Returns early ifwindow.location.pathnamestarts with/cp/auth/. This is the primary guard — if the login page itself triggers anotherredirectToLogin()call (e.g., an API call inside the login page), it won't re-redirect.getAuthOrigin()(auth.ts): For tenant subdomains (*.moleculesai.app), the destination is nowapp.moleculesai.appinstead ofPLATFORM_URL(the EC2 internal address which has no auth routes). This was the root cause — the login page was loading from EC2, not the control plane.AuthGate(AuthGate.tsx): SetsskipRedirect: truefor all/cp/auth/*paths upfront, so even direct navigation to the login page skips the gate fetch and avoids a second redirect trigger.
2. Any auth bypass or session fixation risks?
No.
return_tois used read-only: Thereturn_toparam controls the post-login destination only — no session influence. The session cookie (HttpOnly; Secure; SameSite=Lax; Domain=.moleculesai.app) is set independently by the control plane.- No open redirect:
redirectToLoginalways targetsgetAuthOrigin() + /cp/auth/<path>(a known internal path). - No session fixation: Session cookie is static for the session duration;
fetchSession()doesn't mutate it.
3. Edge cases
| Scenario | Behavior |
|---|---|
Direct navigation to /cp/auth/login (bookmark) |
AuthGate → skipRedirect: true → renders normally ✅ |
| Login page API call → 401 | request() calls redirectToLogin() — loop guard stops it ✅ |
| Tenant subdomain EC2 (no auth routes) | getAuthOrigin() → app.moleculesai.app ✅ |
return_to contains exotic chars |
encodeURIComponent(returnTo) — safe ✅ |
Local dev / app.moleculesai.app (no slug) |
getTenantSlug() returns "" → AuthGate skipRedirect: true ✅ |
| Race: 401 + AuthGate both fire | First redirect wins; browser ignores second ✅ |
Minor note
The dynamic import in api.ts (await import("./auth")) is a sensible workaround for the circular dependency. Consider extracting the redirect logic into a shared helper (lib/redirect.ts) as a follow-up to avoid the async import on every 401.
CI: GREEN | Mergeable: YES | Security impact: Positive (fixes auth unavailability)
APPROVE — safe to merge. Formal approval requires a human reviewer (bot-authored PR).
|
PM review request — This PR has passing CI (all checks SUCCESS/SKIPPED) and no merge conflicts. Needs human review + admin merge. Flagging for @airenostars. Ready to merge when approved. |
AuthGate now skips session fetch for /cp/auth/* paths, and redirectToLogin guards against re-setting window.location when already on an auth path. Both guards had no test coverage — a future refactor could silently reintroduce the redirect loop. Added: - AuthGate.test.tsx: 2 cases covering /cp/auth/login and /cp/auth/signup path skipping (no fetchSession call, no redirectToLogin call, children rendered) - auth.test.ts: 2 cases covering redirectToLogin early return for /cp/auth/login and /cp/auth/signup paths Fixes: #1541 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Fixes the infinite redirect loop on
hongmingwang.moleculesai.appthat causes 431 (Request Header Fields Too Large).Root cause: AuthGate redirects anonymous users to
/cp/auth/login?return_to=<current_url>, but the login page itself is also gated by AuthGate, causing each redirect to re-encode thereturn_toparameter, growing the URL exponentially until headers exceed server limits.Fix: Two guards prevent the loop:
redirectToLogin()returns early if already on/cp/auth/*AuthGatesetsskipRedirect: truefor/cp/auth/*pathsTest plan
hongmingwang.moleculesai.app— should redirect to login ONCE, not loopreturn_toredirects back to canvas[Molecule-Platform-Evolvement-Manager]
🤖 Generated with Claude Code