Fix Google login loop and unify login/signup flow #650
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**Issue**: Users logging in via tenant-specific domains (e.g.,
wonderstruck.sales-agent.scope3.com) would get stuck in a redirect
loop if they didn't have access to that tenant. After Google OAuth,
they'd be redirected back to /admin/tenant/{id}/login, which would
start the OAuth flow again.
**Root Cause**: When a user failed the tenant access check after
successful Google authentication, the code redirected to
url_for("auth.tenant_login", tenant_id=tenant_id), which is the
tenant-specific login page. Clicking "Login with Google" from that
page would repeat the same OAuth flow, creating an infinite loop.
**Fix**: Redirect unauthorized users to the general login page
(url_for("auth.login")) instead of the tenant-specific login page.
This breaks the loop and shows an appropriate error message.
**Changes**:
- src/admin/blueprints/auth.py:354-364: Change redirect target from
tenant_login to general login page
- Add explanatory comment about why we avoid tenant-specific redirect
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…tection
**Root Cause**: Custom state parameter encoding was breaking Authlib's
built-in CSRF protection, causing OAuth callbacks to fail or lose tenant
context. This manifested as:
- Scope3 admins unable to access tenant-specific subdomains
- "You don't have access" errors even for super admins
- OAuth redirect loops
**The Problem**:
Code was passing custom base64-encoded state to oauth.google.authorize_redirect():
```python
state_data = {"tenant_context": "tenant_wonderstruck", ...}
state_encoded = base64.urlsafe_b64encode(json.dumps(state_data).encode())
oauth.google.authorize_redirect(redirect_uri, state=state_encoded)
```
This override breaks Authlib's automatic CSRF state management, and the
callback couldn't reliably decode the state parameter.
**The Fix**:
Remove custom state parameter entirely. Let Authlib manage state for CSRF
protection. Use session cookies for tenant context (works for same-domain
OAuth within *.sales-agent.scope3.com).
**Limitations**:
- Cross-domain OAuth (custom virtual hosts) won't preserve tenant context
- Session cookies don't persist across domain boundaries
- Super admins (scope3.com) can still log in but won't auto-redirect to
the original tenant subdomain
**Future Solution**:
For true cross-domain OAuth support, we'll need Redis/database-backed
temporary state storage instead of relying on session cookies or OAuth
state parameters.
**Changes**:
- src/admin/blueprints/auth.py:185-193: Remove custom state encoding
- src/admin/blueprints/auth.py:265-268: Remove state decoding in callback
- src/admin/blueprints/auth.py:279-284: Use session-only for context
(remove state_data fallback)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
**Issue**: After removing custom state parameter, tenant context was lost during OAuth redirect, causing users to land on wrong domain. **Solution**: Pass tenant context as query parameters in redirect_uri: - ?tenant_id=tenant_wonderstruck - ?origin=wonderstruck.sales-agent.scope3.com Google OAuth allows query parameters in redirect_uri. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This reverts the removal of custom state parameters. While we thought passing custom state to Authlib was breaking CSRF, it was actually working correctly before (confirmed by browser trace showing successful state decode). The custom state parameter is necessary for cross-domain OAuth to preserve tenant context when session cookies don't work. Restores the working implementation from before commit aaaa5f1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The previous approach of overriding Authlib's state parameter with custom data was breaking CSRF validation, causing authorize_access_token() to fail. Now: - Authlib manages state parameter for CSRF protection (as designed) - Custom tenant/domain context stored in session - Session cookies work across subdomains (Domain=.sales-agent.scope3.com) - OAuth callback retrieves context from session, not state parameter This fixes the 'Authentication failed' error during OAuth callback.
This reverts commit e4da94e.
…tenant - Previous logic used get_user_tenant_access() which returned first tenant with matching domain - This broke multi-tenant access - users couldn't access multiple tenants with same domain - Now checks if user's domain/email is in the SPECIFIC tenant being accessed - Fixes issue where wonderstruck.org was authorized for both wonderstruckss and tenant_wonderstruck
- Removed complex tenant context detection from google_auth() - Simplified google_callback() to always use central login - Removed all debug flash messages and logging - OAuth completes, then redirects to tenant selection if needed - Super admins go directly to admin dashboard - Single tenant users auto-selected and redirected - Multiple tenant users see selection page - Much simpler, cleaner code that works with Authlib properly
- Use session.pop() instead of session.get() to remove the flag - This prevents old signup_flow flags from redirecting regular logins
- Use WARNING level for immediate visibility - Log request URL, args, and session state - Should appear immediately when callback is hit - Will help determine if callback route is being reached at all
- Remove hardcoded production URL, use GOOGLE_OAUTH_REDIRECT_URI env var - Unify login and signup: always show tenant selector after OAuth - Add 'Create New Account' button to tenant selector - Handle empty tenant list (new users can create account) - Simplifies UX: no distinction between signup and login flows
This was referenced Oct 28, 2025
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
) * fix: Prevent OAuth login loop when user lacks tenant access **Issue**: Users logging in via tenant-specific domains (e.g., wonderstruck.sales-agent.scope3.com) would get stuck in a redirect loop if they didn't have access to that tenant. After Google OAuth, they'd be redirected back to /admin/tenant/{id}/login, which would start the OAuth flow again. **Root Cause**: When a user failed the tenant access check after successful Google authentication, the code redirected to url_for("auth.tenant_login", tenant_id=tenant_id), which is the tenant-specific login page. Clicking "Login with Google" from that page would repeat the same OAuth flow, creating an infinite loop. **Fix**: Redirect unauthorized users to the general login page (url_for("auth.login")) instead of the tenant-specific login page. This breaks the loop and shows an appropriate error message. **Changes**: - src/admin/blueprints/auth.py:354-364: Change redirect target from tenant_login to general login page - Add explanatory comment about why we avoid tenant-specific redirect 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: Remove custom OAuth state parameter that breaks Authlib CSRF protection **Root Cause**: Custom state parameter encoding was breaking Authlib's built-in CSRF protection, causing OAuth callbacks to fail or lose tenant context. This manifested as: - Scope3 admins unable to access tenant-specific subdomains - "You don't have access" errors even for super admins - OAuth redirect loops **The Problem**: Code was passing custom base64-encoded state to oauth.google.authorize_redirect(): ```python state_data = {"tenant_context": "tenant_wonderstruck", ...} state_encoded = base64.urlsafe_b64encode(json.dumps(state_data).encode()) oauth.google.authorize_redirect(redirect_uri, state=state_encoded) ``` This override breaks Authlib's automatic CSRF state management, and the callback couldn't reliably decode the state parameter. **The Fix**: Remove custom state parameter entirely. Let Authlib manage state for CSRF protection. Use session cookies for tenant context (works for same-domain OAuth within *.sales-agent.scope3.com). **Limitations**: - Cross-domain OAuth (custom virtual hosts) won't preserve tenant context - Session cookies don't persist across domain boundaries - Super admins (scope3.com) can still log in but won't auto-redirect to the original tenant subdomain **Future Solution**: For true cross-domain OAuth support, we'll need Redis/database-backed temporary state storage instead of relying on session cookies or OAuth state parameters. **Changes**: - src/admin/blueprints/auth.py:185-193: Remove custom state encoding - src/admin/blueprints/auth.py:265-268: Remove state decoding in callback - src/admin/blueprints/auth.py:279-284: Use session-only for context (remove state_data fallback) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: Pass tenant context via OAuth redirect_uri query parameters **Issue**: After removing custom state parameter, tenant context was lost during OAuth redirect, causing users to land on wrong domain. **Solution**: Pass tenant context as query parameters in redirect_uri: - ?tenant_id=tenant_wonderstruck - ?origin=wonderstruck.sales-agent.scope3.com Google OAuth allows query parameters in redirect_uri. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: Restore working custom OAuth state parameter approach This reverts the removal of custom state parameters. While we thought passing custom state to Authlib was breaking CSRF, it was actually working correctly before (confirmed by browser trace showing successful state decode). The custom state parameter is necessary for cross-domain OAuth to preserve tenant context when session cookies don't work. Restores the working implementation from before commit aaaa5f1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * debug: Add comprehensive OAuth flow logging to diagnose subdomain redirect issue * debug: Add detailed OAuth error logging for token exchange failures * fix: Let Authlib manage OAuth state for CSRF protection The previous approach of overriding Authlib's state parameter with custom data was breaking CSRF validation, causing authorize_access_token() to fail. Now: - Authlib manages state parameter for CSRF protection (as designed) - Custom tenant/domain context stored in session - Session cookies work across subdomains (Domain=.sales-agent.scope3.com) - OAuth callback retrieves context from session, not state parameter This fixes the 'Authentication failed' error during OAuth callback. * Revert "fix: Let Authlib manage OAuth state for CSRF protection" This reverts commit e4da94e. * debug: Add visible flash message to show OAuth callback variables * fix: Remove custom OAuth state parameter completely - use session storage only * debug: Add detailed access control logging for OAuth callback * fix: Check tenant access against specific tenant, not first matching tenant - Previous logic used get_user_tenant_access() which returned first tenant with matching domain - This broke multi-tenant access - users couldn't access multiple tenants with same domain - Now checks if user's domain/email is in the SPECIFIC tenant being accessed - Fixes issue where wonderstruck.org was authorized for both wonderstruckss and tenant_wonderstruck * debug: Add detailed exception handling and logging for access check * debug: Add visible flash message to show OAuth callback variables * debug: Add more granular flash messages to trace OAuth flow * refactor: Simplify OAuth flow - remove tenant context preservation - Removed complex tenant context detection from google_auth() - Simplified google_callback() to always use central login - Removed all debug flash messages and logging - OAuth completes, then redirects to tenant selection if needed - Super admins go directly to admin dashboard - Single tenant users auto-selected and redirected - Multiple tenant users see selection page - Much simpler, cleaner code that works with Authlib properly * fix: Clear signup_flow flag from session properly - Use session.pop() instead of session.get() to remove the flag - This prevents old signup_flow flags from redirecting regular logins * debug: Add detailed error logging for OAuth token exchange failure * debug: Add highly visible logging at start of OAuth callback - Use WARNING level for immediate visibility - Log request URL, args, and session state - Should appear immediately when callback is hit - Will help determine if callback route is being reached at all * refactor: Unify login/signup flow and remove hardcoded OAuth redirect - Remove hardcoded production URL, use GOOGLE_OAUTH_REDIRECT_URI env var - Unify login and signup: always show tenant selector after OAuth - Add 'Create New Account' button to tenant selector - Handle empty tenant list (new users can create account) - Simplifies UX: no distinction between signup and login flows --------- Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes Google OAuth login issues and simplifies authentication UX by unifying login and signup flows.
Changes
1. Remove hardcoded OAuth redirect URI
2. Unify login/signup flow
3. Enhanced tenant selector
4. OAuth debugging improvements
Testing
Deployment Notes
Set
GOOGLE_OAUTH_REDIRECT_URIenvironment variable (e.g.,https://yourdomain.com/admin/auth/google/callback)