fix: Only set SESSION_COOKIE_DOMAIN in multi-tenant mode #886
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
Fixes login issues in single-tenant deployments where session cookies were being rejected due to domain mismatch. In single-tenant mode (ADCP_MULTI_TENANT=false), Flask should use the actual request domain for session cookies rather than a hardcoded multi-tenant domain. This affects all single-tenant deployments including Fly.io, Cloud Run, and Docker deployments. Fixes authentication for single-tenant publishers deploying their own sales agent instances. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes #885. In single-machine deployments (Fly.io, Cloud Run), all services run on localhost instead of separate Docker containers. The nginx configs use docker-compose service names (adcp-server, admin-ui) which don't resolve in these environments. The fix patches the nginx config at startup to replace service names with localhost, allowing nginx to start successfully. Also updates Fly.io documentation to: - Mark persistent volume as optional - Add test mode instructions (ADCP_AUTH_TEST_MODE) - Add /login as first-time access URL - Note single-tenant mode is the default 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Separates nginx configs by deployment mode, not tenant mode: Production (single-container via run_all_services.py): - nginx-simple.conf: Uses localhost, path-based routing - nginx-multi-tenant.conf: Uses localhost, subdomain routing Development (docker-compose multi-container): - nginx-compose.conf: Uses Docker service names (adcp-server, admin-ui) This removes the runtime patching in run_all_services.py since the production configs now use localhost directly. Docker Compose mounts its own config file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- nginx-simple.conf → nginx-single-tenant.conf - nginx-compose.conf → nginx-development.conf - nginx-multi-tenant.conf (unchanged) Clear naming by purpose: - nginx-single-tenant.conf: Production, path-based routing - nginx-multi-tenant.conf: Production, subdomain routing - nginx-development.conf: Docker Compose dev environment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused [mounts] section from fly.toml (data stored in PostgreSQL) - Remove volume creation step from walkthrough - Clarify /admin redirects to login when unauthenticated - Renumber steps after removing volume step - Clean up domain configuration comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove BASE_DOMAIN - SALES_AGENT_DOMAIN is now the primary config
- All domain functions return str | None (None when not configured)
- ADMIN_DOMAIN defaults to admin.{SALES_AGENT_DOMAIN} if not set
- SUPER_ADMIN_DOMAIN is standalone (no default)
- Add validate_multi_tenant_config() to config_loader.py
- Add startup validation: multi-tenant mode requires SALES_AGENT_DOMAIN
- Update example domains from scope3.com to example.com in docs/tests
- Fix mypy errors in callers to handle None return values
Behavior:
- Single-tenant mode: Works without any domain config
- Multi-tenant mode: Fails at startup with clear error if missing config
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make docker-compose.dev.yml standalone (not an overlay) - Use CONDUCTOR_PORT for nginx proxy to avoid port conflicts between worktrees - Remove postgres port exposure (use docker exec if needed) - Remove port assignment logic from setup_conductor_workspace.sh - Remove docker-compose.override.yml generation Development: docker compose -f docker-compose.dev.yml up Production: docker compose up -d 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove .env.secrets in favor of just .env - Update docker-compose files to only reference .env - Update setup script to copy .env from root (no longer generates it) - Delete .env.secrets.template (redundant with .env.template) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Skip SUPER_ADMIN_EMAILS validation when ADCP_AUTH_TEST_MODE=true - Remove GEMINI_API_KEY, SUPER_ADMIN_EMAILS, GOOGLE_* from docker-compose - Default to ADCP_AUTH_TEST_MODE=true for out-of-box experience Users can now run `docker compose up` with no .env file and login with test accounts (test_super_admin@example.com / test123). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When mounting local code over /app, the image's entrypoint script doesn't exist. Add entrypoint: [] to override and use command directly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove legacy config files: - config/fly/*.toml (moved template to docs) - config/nginx/nginx.conf (hardcoded domains) - data/foundational_creative_formats.json (unused) - Add comprehensive environment variable documentation: - docs/deployment/environment-variables.md - docs/deployment/walkthroughs/fly.toml.template - Update authentication docs for generic OIDC support: - Support Okta, Auth0, Azure AD, Keycloak via OAUTH_DISCOVERY_URL - Keep Google OAuth as simpler alternative - Simplify .env.template for zero-config quickstart: - All values commented out by default - Test mode works out of the box - Clean up fly.toml and docker-compose: - Remove unused port env vars (hardcoded in nginx) - Move non-sensitive config from secrets to env - Add audit_logs/ to .gitignore 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add SUPER_ADMIN_DOMAINS next to SUPER_ADMIN_EMAILS in admin section - Consolidate OAuth to single generic OIDC section (removed Google-specific) - Improve ENVIRONMENT description to explain schema validation behavior - Remove duplicate SUPER_ADMIN_DOMAINS from advanced section Created #889 to track making schema validation a runtime config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolved conflicts: - .env.template: Keep improved CREATE_DEMO_TENANT description from main - docs/deployment/single-tenant.md: Keep our session cookies note, remove duplicate env var tables (now in environment-variables.md) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The production machine has a volume mounted that was missing from the config after cleanup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…x config When using conductor workspaces, the .env file contains custom port assignments (e.g., ADCP_SALES_PORT=8125) that conflict with the nginx-development.conf which expects port 8080. This fix explicitly sets ADCP_SALES_PORT=8080 in the docker-compose.dev.yml environment section, overriding any .env file setting to ensure the MCP server binds to the port nginx expects. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Two fixes for CI failures: 1. Agent card URL: When SALES_AGENT_DOMAIN is not configured, the fallback URL was "http://localhost:8091" but should be "http://localhost:8091/a2a" to include the A2A endpoint path. This matches what get_a2a_server_url() returns when the domain IS configured. 2. Publisher sync tests: After commit 9b0086b added validation for get_tenant_url() returning None, the tests started failing because they didn't mock this function. Added patches for get_tenant_url to return a valid test URL. 🤖 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
This PR fixes single-tenant deployments and simplifies the nginx configuration architecture.
1. Session Cookie Domain Fix (from PR #884)
Problem: Login fails in single-tenant deployments (Fly.io, Cloud Run, Docker) because session cookies are rejected due to domain mismatch.
Fix: Only set
SESSION_COOKIE_DOMAINwhenADCP_MULTI_TENANT=true. In single-tenant mode (default), Flask uses the actual request domain.2. Nginx Config Simplification (Fixes #885)
Problem: Nginx configs mixed deployment mode (single-container vs docker-compose) with tenant mode (single vs multi-tenant), causing confusion and requiring runtime patching.
Fix: Reorganized nginx configs by deployment mode:
nginx-simple.confnginx-multi-tenant.confnginx-compose.confThis removes the runtime patching hack in
run_all_services.pysince production configs now uselocalhostdirectly.Documentation Updates
Impact
Testing
ADCP_MULTI_TENANT=false(default)/loginwithADCP_AUTH_TEST_MODE=trueSupersedes #884
Fixes #885
🤖 Generated with Claude Code