-
Notifications
You must be signed in to change notification settings - Fork 14
fix: Add multi-admin domain support for cross-domain OAuth #919
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
Merged
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
When OAuth callback fails (e.g., due to mismatching state), redirect to tenant-specific login page instead of generic login. This prevents redirect loops that occur when: 1. OAuth fails on root domain callback 2. Generic login doesn't detect tenant context 3. Login redirects back to OAuth, which fails again Also add logged_out=1 param to prevent auto-redirect back to OAuth. Additionally fix alembic migration head to chain from merge migration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The default nginx server was returning a 302 redirect to /admin which generated an invalid URL (using internal hostname instead of the external custom domain). Now the root location passes the request to admin_ui which properly handles Apx-Incoming-Host header to look up tenants by virtual_host and render their landing page. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Revert the incorrect change to cdaa4d359774's parent and create a proper merge migration to combine the orphan branch merge (g1h2i3j4k5l6) with the placement_ids migration (cdaa4d359774). This fixes the deploy failure caused by migration overlap error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The CustomProxyFix middleware was setting script_root=/admin for ALL production requests, including custom domain requests via Approximated. This caused index() to delegate to admin_index() which redirects to login. Now CustomProxyFix checks for Apx-Incoming-Host header and leaves script_root empty for custom domain requests, allowing them to reach the landing page logic in index(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The fix for custom domain landing pages is working. Removed the temporary debug logging that was added to trace the issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When accessing a secondary admin domain (like sales-agent.scope3.com), users were getting redirected to /admin/login?logged_out=1 due to OAuth session state being lost across domains. Root cause: - OAuth callback is configured for admin.sales-agent.adcontextprotocol.org - Session state stored on sales-agent.scope3.com is not available on the callback domain, causing state mismatch and OAuth failure Solution: - Add ADMIN_DOMAINS env var for configuring multiple admin domains - Add get_admin_domains() function to return all configured admin domains - Update is_admin_domain() to check against all configured admin domains - Redirect from secondary admin domains to primary for OAuth flow - Configure sales-agent.scope3.com as a secondary admin domain in fly.toml 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The fly.toml had hardcoded values for ADMIN_UI_URL, GOOGLE_OAUTH_REDIRECT_URI, BASE_DOMAIN, SALES_AGENT_DOMAIN, etc. that were overriding the correct values set as Fly secrets. This caused OAuth to fail because the redirect URI pointed to a non-existent domain (admin.sales-agent.adcontextprotocol.org). These values should only be set via `fly secrets set`, not in fly.toml. Also reverts the multi-admin domain changes from the previous commit as they were based on incorrect assumptions about the domain configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When OAuth is configured, internal pages redirect to the OAuth provider (external URL). The Flask test client doesn't support following external redirects, causing "Following external redirects is not supported" errors. Fix by: 1. Not following redirects automatically in validate_link() 2. Treating redirects to external URLs (OAuth) as valid links 3. Following internal redirects recursively 4. Checking for external redirects in validate_page() before parsing HTML 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When OAuth is configured (which CI does with test credentials), the /login endpoint redirects to the OAuth provider instead of rendering the login page. Update tests to accept both 200 (login page) and 302 (OAuth redirect) as valid responses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <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
admin.sales-agent.adcontextprotocol.orgwhich is not a real domainfly secrets setRoot Cause
PR #886 added hardcoded values to fly.toml for:
ADMIN_UI_URL = "https://admin.sales-agent.adcontextprotocol.org"GOOGLE_OAUTH_REDIRECT_URI = "https://admin.sales-agent.adcontextprotocol.org/auth/google/callback"BASE_DOMAIN,SALES_AGENT_DOMAIN,GCP_PROJECT_ID,ADCP_MULTI_TENANTThese fly.toml values take precedence over secrets, causing OAuth to fail because the redirect URI pointed to a non-existent domain.
Solution
Remove the hardcoded values from fly.toml. All domain-specific configuration should be set via
fly secrets set, which already has the correct values.Test plan
sales-agent.scope3.com/admin/works🤖 Generated with Claude Code