From b38cc3a637cb39c2118e2fbc8dd5fadc531446be Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 14:30:24 -0400 Subject: [PATCH 01/20] fix: Prevent OAuth login loop when user lacks tenant access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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 --- src/admin/blueprints/auth.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index b637495bf..0471946fe 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -352,9 +352,16 @@ def google_callback(): else: return redirect(url_for("tenants.dashboard", tenant_id=tenant_id)) else: - flash("You don't have access to this tenant", "error") + # User doesn't have access to this tenant + flash( + "You don't have access to this tenant. Please contact your administrator to request access.", + "error", + ) session.clear() - return redirect(url_for("auth.tenant_login", tenant_id=tenant_id)) + + # Redirect to general login page to avoid loop + # Don't redirect back to tenant-specific login as that creates an infinite loop + return redirect(url_for("auth.login")) # Domain-based access control using email domain extraction # (ensure_user_in_tenant and get_user_tenant_access already imported above) From aaaa5f1e8b17ec3b5c57eea7913110a0f0cc5881 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 14:38:03 -0400 Subject: [PATCH 02/20] fix: Remove custom OAuth state parameter that breaks Authlib CSRF protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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 --- src/admin/blueprints/auth.py | 51 +++++++++++++----------------------- 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 0471946fe..e514a89a3 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -182,21 +182,15 @@ def google_auth(): if tenant_context: session["oauth_tenant_context"] = tenant_context - # Build custom state parameter to handle cross-domain scenarios - # For cross-domain OAuth (virtual hosts), session cookies won't work - import base64 - - state_data = { - "signup_flow": session.get("signup_flow", False), - "external_domain": approximated_host - or (host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None), - "tenant_context": tenant_context, - } - state_json = json.dumps(state_data) - state_encoded = base64.urlsafe_b64encode(state_json.encode()).decode() - - # Let Authlib manage the state parameter for CSRF protection, but pass our custom data - return oauth.google.authorize_redirect(redirect_uri, state=state_encoded) + # NOTE: We do NOT pass a custom state parameter to authorize_redirect() + # because it breaks Authlib's built-in CSRF protection. Instead, we rely + # on session cookies (which work for same-domain OAuth) and will need + # an alternative solution for cross-domain OAuth scenarios. + # + # For cross-domain OAuth, session cookies won't persist across the redirect, + # so tenant context will be lost. Super admins (scope3.com) will still work + # but won't be redirected back to the original tenant subdomain. + return oauth.google.authorize_redirect(redirect_uri) @auth_bp.route("/tenant//auth/google") @@ -268,21 +262,10 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Try to decode state parameter for cross-domain OAuth context - import base64 - - state_param = request.args.get("state") - state_data = {} - if state_param: - try: - state_json = base64.urlsafe_b64decode(state_param.encode()).decode() - state_data = json.loads(state_json) - logger.info(f"OAuth callback - decoded state: {state_data}") - except Exception as e: - logger.warning(f"Could not decode OAuth state parameter: {e}") - - # Check if this is a signup flow (from state or session) - is_signup_flow = state_data.get("signup_flow") or session.get("signup_flow") + # Check if this is a signup flow (from session only) + # NOTE: We no longer use custom state parameters because they break + # Authlib's CSRF protection. We rely on session cookies instead. + is_signup_flow = session.get("signup_flow") if is_signup_flow: logger.info(f"OAuth callback - signup flow detected for {email}") # Set signup flow in session for onboarding @@ -293,10 +276,12 @@ def google_callback(): # Debug session state before popping values logger.info(f"OAuth callback - full session: {dict(session)}") - # Get originating host and tenant context from state parameter (preferred) or session (fallback) + # Get originating host and tenant context from session + # NOTE: Session cookies only work for same-domain OAuth (*.sales-agent.scope3.com) + # For cross-domain OAuth, these values will be None. originating_host = session.pop("oauth_originating_host", None) - external_domain = state_data.get("external_domain") or session.pop("oauth_external_domain", None) - tenant_id = state_data.get("tenant_context") or session.pop("oauth_tenant_context", None) + external_domain = session.pop("oauth_external_domain", None) + tenant_id = session.pop("oauth_tenant_context", None) # Debug logging for OAuth redirect logger.info(f"OAuth callback debug - originating_host: {originating_host}") From 87923cf689adade24229aae05573cad5c3860928 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 14:47:56 -0400 Subject: [PATCH 03/20] fix: Pass tenant context via OAuth redirect_uri query parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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 --- src/admin/blueprints/auth.py | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index e514a89a3..c51f08f67 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -160,15 +160,32 @@ def google_auth(): tenant_context = tenant.tenant_id logger.info(f"Detected tenant context from Host header: {tenant_subdomain} -> {tenant_context}") - # Always use the registered OAuth redirect URI for Google (no modifications allowed) + # Build redirect URI with tenant context as query parameter + # Google OAuth allows query parameters in redirect_uri as long as base URI is registered if os.environ.get("PRODUCTION") == "true": - # For production, always use the exact registered redirect URI - redirect_uri = "https://sales-agent.scope3.com/admin/auth/google/callback" + # For production, use the registered redirect URI with query params for context + base_redirect_uri = "https://sales-agent.scope3.com/admin/auth/google/callback" + # Add tenant context and originating host as query parameters + import urllib.parse + + params = {} + if tenant_context: + params["tenant_id"] = tenant_context + if approximated_host: + params["origin"] = approximated_host + elif host and host != "sales-agent.scope3.com" and not host.startswith("admin."): + params["origin"] = host + + if params: + redirect_uri = f"{base_redirect_uri}?{urllib.parse.urlencode(params)}" + else: + redirect_uri = base_redirect_uri else: # Development fallback redirect_uri = url_for("auth.google_callback", _external=True) # Store originating host and tenant context in session for OAuth callback + # (as backup - query params are now primary) session["oauth_originating_host"] = host # Store external domain and tenant context in session for OAuth callback @@ -276,12 +293,11 @@ def google_callback(): # Debug session state before popping values logger.info(f"OAuth callback - full session: {dict(session)}") - # Get originating host and tenant context from session - # NOTE: Session cookies only work for same-domain OAuth (*.sales-agent.scope3.com) - # For cross-domain OAuth, these values will be None. - originating_host = session.pop("oauth_originating_host", None) - external_domain = session.pop("oauth_external_domain", None) - tenant_id = session.pop("oauth_tenant_context", None) + # Get originating host and tenant context from query params (primary) or session (fallback) + # Query params survive the OAuth redirect, session cookies may not for cross-domain + tenant_id = request.args.get("tenant_id") or session.pop("oauth_tenant_context", None) + external_domain = request.args.get("origin") or session.pop("oauth_external_domain", None) + originating_host = external_domain or session.pop("oauth_originating_host", None) # Debug logging for OAuth redirect logger.info(f"OAuth callback debug - originating_host: {originating_host}") From 6e3766e4072775468b6e8e5ea6ed1090432c5455 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 15:25:49 -0400 Subject: [PATCH 04/20] fix: Restore working custom OAuth state parameter approach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 aaaa5f1e. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/admin/blueprints/auth.py | 75 ++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index c51f08f67..0471946fe 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -160,32 +160,15 @@ def google_auth(): tenant_context = tenant.tenant_id logger.info(f"Detected tenant context from Host header: {tenant_subdomain} -> {tenant_context}") - # Build redirect URI with tenant context as query parameter - # Google OAuth allows query parameters in redirect_uri as long as base URI is registered + # Always use the registered OAuth redirect URI for Google (no modifications allowed) if os.environ.get("PRODUCTION") == "true": - # For production, use the registered redirect URI with query params for context - base_redirect_uri = "https://sales-agent.scope3.com/admin/auth/google/callback" - # Add tenant context and originating host as query parameters - import urllib.parse - - params = {} - if tenant_context: - params["tenant_id"] = tenant_context - if approximated_host: - params["origin"] = approximated_host - elif host and host != "sales-agent.scope3.com" and not host.startswith("admin."): - params["origin"] = host - - if params: - redirect_uri = f"{base_redirect_uri}?{urllib.parse.urlencode(params)}" - else: - redirect_uri = base_redirect_uri + # For production, always use the exact registered redirect URI + redirect_uri = "https://sales-agent.scope3.com/admin/auth/google/callback" else: # Development fallback redirect_uri = url_for("auth.google_callback", _external=True) # Store originating host and tenant context in session for OAuth callback - # (as backup - query params are now primary) session["oauth_originating_host"] = host # Store external domain and tenant context in session for OAuth callback @@ -199,15 +182,21 @@ def google_auth(): if tenant_context: session["oauth_tenant_context"] = tenant_context - # NOTE: We do NOT pass a custom state parameter to authorize_redirect() - # because it breaks Authlib's built-in CSRF protection. Instead, we rely - # on session cookies (which work for same-domain OAuth) and will need - # an alternative solution for cross-domain OAuth scenarios. - # - # For cross-domain OAuth, session cookies won't persist across the redirect, - # so tenant context will be lost. Super admins (scope3.com) will still work - # but won't be redirected back to the original tenant subdomain. - return oauth.google.authorize_redirect(redirect_uri) + # Build custom state parameter to handle cross-domain scenarios + # For cross-domain OAuth (virtual hosts), session cookies won't work + import base64 + + state_data = { + "signup_flow": session.get("signup_flow", False), + "external_domain": approximated_host + or (host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None), + "tenant_context": tenant_context, + } + state_json = json.dumps(state_data) + state_encoded = base64.urlsafe_b64encode(state_json.encode()).decode() + + # Let Authlib manage the state parameter for CSRF protection, but pass our custom data + return oauth.google.authorize_redirect(redirect_uri, state=state_encoded) @auth_bp.route("/tenant//auth/google") @@ -279,10 +268,21 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Check if this is a signup flow (from session only) - # NOTE: We no longer use custom state parameters because they break - # Authlib's CSRF protection. We rely on session cookies instead. - is_signup_flow = session.get("signup_flow") + # Try to decode state parameter for cross-domain OAuth context + import base64 + + state_param = request.args.get("state") + state_data = {} + if state_param: + try: + state_json = base64.urlsafe_b64decode(state_param.encode()).decode() + state_data = json.loads(state_json) + logger.info(f"OAuth callback - decoded state: {state_data}") + except Exception as e: + logger.warning(f"Could not decode OAuth state parameter: {e}") + + # Check if this is a signup flow (from state or session) + is_signup_flow = state_data.get("signup_flow") or session.get("signup_flow") if is_signup_flow: logger.info(f"OAuth callback - signup flow detected for {email}") # Set signup flow in session for onboarding @@ -293,11 +293,10 @@ def google_callback(): # Debug session state before popping values logger.info(f"OAuth callback - full session: {dict(session)}") - # Get originating host and tenant context from query params (primary) or session (fallback) - # Query params survive the OAuth redirect, session cookies may not for cross-domain - tenant_id = request.args.get("tenant_id") or session.pop("oauth_tenant_context", None) - external_domain = request.args.get("origin") or session.pop("oauth_external_domain", None) - originating_host = external_domain or session.pop("oauth_originating_host", None) + # Get originating host and tenant context from state parameter (preferred) or session (fallback) + originating_host = session.pop("oauth_originating_host", None) + external_domain = state_data.get("external_domain") or session.pop("oauth_external_domain", None) + tenant_id = state_data.get("tenant_context") or session.pop("oauth_tenant_context", None) # Debug logging for OAuth redirect logger.info(f"OAuth callback debug - originating_host: {originating_host}") From 7dfc6ca50dcfd8e65a05bb2f209dcb7b121cbfe2 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 15:36:59 -0400 Subject: [PATCH 05/20] debug: Add comprehensive OAuth flow logging to diagnose subdomain redirect issue --- ..._schemas_v1_core_webhook-payload_json.json | 203 ++++++++++++++++++ ...mas_v1_core_webhook-payload_json.json.meta | 6 + .../v1/_schemas_v1_enums_task-type_json.json | 27 +++ ..._schemas_v1_enums_task-type_json.json.meta | 6 + src/admin/blueprints/auth.py | 21 +- .../_schemas_v1_core_webhook_payload_json.py | 133 ++++++++++++ .../_schemas_v1_enums_task_type_json.py | 16 ++ 7 files changed, 411 insertions(+), 1 deletion(-) create mode 100644 schemas/v1/_schemas_v1_core_webhook-payload_json.json create mode 100644 schemas/v1/_schemas_v1_core_webhook-payload_json.json.meta create mode 100644 schemas/v1/_schemas_v1_enums_task-type_json.json create mode 100644 schemas/v1/_schemas_v1_enums_task-type_json.json.meta create mode 100644 src/core/schemas_generated/_schemas_v1_core_webhook_payload_json.py create mode 100644 src/core/schemas_generated/_schemas_v1_enums_task_type_json.py diff --git a/schemas/v1/_schemas_v1_core_webhook-payload_json.json b/schemas/v1/_schemas_v1_core_webhook-payload_json.json new file mode 100644 index 000000000..7b64cde31 --- /dev/null +++ b/schemas/v1/_schemas_v1_core_webhook-payload_json.json @@ -0,0 +1,203 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "/schemas/v1/core/webhook-payload.json", + "title": "Webhook Payload", + "description": "Payload structure sent to webhook endpoints when async task status changes. Combines protocol-level task metadata with domain-specific response data. This schema represents what your webhook handler will receive when a task transitions from 'submitted' to a terminal or intermediate state.", + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": "Unique identifier for this task. Use this to correlate webhook notifications with the original task submission." + }, + "task_type": { + "$ref": "/schemas/v1/enums/task-type.json", + "description": "Type of AdCP operation that triggered this webhook. Enables webhook handlers to route to appropriate processing logic." + }, + "domain": { + "type": "string", + "description": "AdCP domain this task belongs to. Helps classify the operation type at a high level.", + "enum": [ + "media-buy", + "signals" + ] + }, + "status": { + "$ref": "/schemas/v1/enums/task-status.json", + "description": "Current task status. Webhooks are only triggered for status changes after initial submission (e.g., submitted \u2192 input-required, submitted \u2192 completed, submitted \u2192 failed)." + }, + "created_at": { + "type": "string", + "format": "date-time", + "description": "ISO 8601 timestamp when the task was initially created. Useful for tracking operation duration." + }, + "updated_at": { + "type": "string", + "format": "date-time", + "description": "ISO 8601 timestamp when the task status was last updated. This matches the timestamp when the webhook was triggered." + }, + "completed_at": { + "type": "string", + "format": "date-time", + "description": "ISO 8601 timestamp when the task reached a terminal state (completed, failed, or canceled). Only present for terminal states." + }, + "message": { + "type": "string", + "description": "Human-readable summary of the current task state. Provides context about what happened and what action may be needed." + }, + "context_id": { + "type": "string", + "description": "Session/conversation identifier. Use this to continue the conversation if input-required status needs clarification or additional parameters." + }, + "progress": { + "type": "object", + "description": "Progress information for tasks still in 'working' state. Rarely seen in webhooks since 'working' tasks typically complete synchronously, but may appear if a task transitions from 'submitted' to 'working'.", + "properties": { + "percentage": { + "type": "number", + "minimum": 0, + "maximum": 100, + "description": "Completion percentage (0-100)" + }, + "current_step": { + "type": "string", + "description": "Current step or phase of the operation" + }, + "total_steps": { + "type": "integer", + "minimum": 1, + "description": "Total number of steps in the operation" + }, + "step_number": { + "type": "integer", + "minimum": 1, + "description": "Current step number" + } + }, + "additionalProperties": false + }, + "error": { + "type": "object", + "description": "Error details for failed tasks. Only present when status is 'failed'.", + "properties": { + "code": { + "type": "string", + "description": "Error code for programmatic handling" + }, + "message": { + "type": "string", + "description": "Detailed error message" + }, + "details": { + "type": "object", + "description": "Additional error context", + "properties": { + "domain": { + "type": "string", + "description": "AdCP domain where error occurred", + "enum": [ + "media-buy", + "signals" + ] + }, + "operation": { + "type": "string", + "description": "Specific operation that failed" + }, + "specific_context": { + "type": "object", + "description": "Domain-specific error context", + "additionalProperties": true + } + }, + "additionalProperties": true + } + }, + "required": [ + "code", + "message" + ], + "additionalProperties": false + } + }, + "required": [ + "task_id", + "task_type", + "domain", + "status", + "created_at", + "updated_at" + ], + "additionalProperties": true, + "notes": [ + "Webhooks are ONLY triggered when the initial response status is 'submitted' (long-running operations)", + "Webhook payloads include protocol-level fields (task_id, task_type, domain, status, timestamps) PLUS the full task-specific response data", + "The task-specific response data is merged at the top level of the webhook payload (not nested in a 'payload' field)", + "For example, a create_media_buy webhook will include task_id, task_type, domain, status, AND media_buy_id, packages, creative_deadline, etc.", + "Your webhook handler receives the complete information needed to process the result without making additional API calls" + ], + "examples": [ + { + "description": "Webhook for input-required status (human approval needed)", + "data": { + "task_id": "task_456", + "task_type": "create_media_buy", + "domain": "media-buy", + "status": "input-required", + "created_at": "2025-01-22T10:00:00Z", + "updated_at": "2025-01-22T10:15:00Z", + "context_id": "ctx_abc123", + "message": "Campaign budget $150K requires VP approval to proceed", + "buyer_ref": "nike_q1_campaign_2024" + } + }, + { + "description": "Webhook for completed create_media_buy", + "data": { + "task_id": "task_456", + "task_type": "create_media_buy", + "domain": "media-buy", + "status": "completed", + "created_at": "2025-01-22T10:00:00Z", + "updated_at": "2025-01-22T10:30:00Z", + "completed_at": "2025-01-22T10:30:00Z", + "message": "Media buy created successfully with 2 packages ready for creative assignment", + "media_buy_id": "mb_12345", + "buyer_ref": "nike_q1_campaign_2024", + "creative_deadline": "2024-01-30T23:59:59Z", + "packages": [ + { + "package_id": "pkg_12345_001", + "buyer_ref": "nike_ctv_package" + } + ] + } + }, + { + "description": "Webhook for failed sync_creatives", + "data": { + "task_id": "task_789", + "task_type": "sync_creatives", + "domain": "media-buy", + "status": "failed", + "created_at": "2025-01-22T10:45:00Z", + "updated_at": "2025-01-22T10:46:00Z", + "completed_at": "2025-01-22T10:46:00Z", + "message": "Creative sync failed due to invalid asset URLs", + "error": { + "code": "invalid_assets", + "message": "One or more creative assets could not be accessed", + "details": { + "domain": "media-buy", + "operation": "sync_creatives", + "specific_context": { + "failed_creatives": [ + "creative_001", + "creative_003" + ] + } + } + } + } + } + ] +} diff --git a/schemas/v1/_schemas_v1_core_webhook-payload_json.json.meta b/schemas/v1/_schemas_v1_core_webhook-payload_json.json.meta new file mode 100644 index 000000000..8c2294cb7 --- /dev/null +++ b/schemas/v1/_schemas_v1_core_webhook-payload_json.json.meta @@ -0,0 +1,6 @@ +{ + "etag": "W/\"68ffaa97-1ce0\"", + "last-modified": "Mon, 27 Oct 2025 17:23:35 GMT", + "downloaded_at": "2025-10-27T14:25:39.422607", + "schema_ref": "/schemas/v1/core/webhook-payload.json" +} diff --git a/schemas/v1/_schemas_v1_enums_task-type_json.json b/schemas/v1/_schemas_v1_enums_task-type_json.json new file mode 100644 index 000000000..eb895d56a --- /dev/null +++ b/schemas/v1/_schemas_v1_enums_task-type_json.json @@ -0,0 +1,27 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "/schemas/v1/enums/task-type.json", + "title": "Task Type", + "description": "Valid AdCP task types across all domains. These represent the complete set of operations that can be tracked via the task management system.", + "type": "string", + "enum": [ + "create_media_buy", + "update_media_buy", + "sync_creatives", + "activate_signal", + "get_signals" + ], + "enumDescriptions": { + "create_media_buy": "Media-buy domain: Create a new advertising campaign with one or more packages", + "update_media_buy": "Media-buy domain: Update campaign settings, package configuration, or delivery parameters", + "sync_creatives": "Media-buy domain: Sync creative assets to publisher's library with upsert semantics", + "activate_signal": "Signals domain: Activate an audience signal on a specific platform or account", + "get_signals": "Signals domain: Discover available audience signals based on natural language description" + }, + "notes": [ + "Task types map to specific AdCP task operations", + "Each task type belongs to either the 'media-buy' or 'signals' domain", + "This enum is used in task management APIs (tasks/list, tasks/get) and webhook payloads", + "New task types require a minor version bump per semantic versioning" + ] +} diff --git a/schemas/v1/_schemas_v1_enums_task-type_json.json.meta b/schemas/v1/_schemas_v1_enums_task-type_json.json.meta new file mode 100644 index 000000000..af1503d40 --- /dev/null +++ b/schemas/v1/_schemas_v1_enums_task-type_json.json.meta @@ -0,0 +1,6 @@ +{ + "etag": "W/\"68ffaa97-531\"", + "last-modified": "Mon, 27 Oct 2025 17:23:35 GMT", + "downloaded_at": "2025-10-27T14:25:41.349350", + "schema_ref": "/schemas/v1/enums/task-type.json" +} diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 0471946fe..abe379159 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -128,13 +128,20 @@ def tenant_login(tenant_id): @auth_bp.route("/auth/google") def google_auth(): """Initiate Google OAuth flow with tenant context detection.""" + # Log entry point + host = request.headers.get("Host", "") + apx_host = request.headers.get("Apx-Incoming-Host") + logger.info( + f"[OAUTH_DEBUG] google_auth() called - Host: {host}, Apx-Incoming-Host: {apx_host}, args: {dict(request.args)}" + ) + oauth = current_app.oauth if hasattr(current_app, "oauth") else None if not oauth: flash("OAuth not configured", "error") + logger.warning("[OAUTH_DEBUG] OAuth not configured, redirecting to login") return redirect(url_for("auth.login")) # Capture tenant context from headers or form data - host = request.headers.get("Host", "") tenant_context = request.args.get("tenant_context") # From login form # Check for Approximated routing headers first @@ -195,6 +202,8 @@ def google_auth(): state_json = json.dumps(state_data) state_encoded = base64.urlsafe_b64encode(state_json.encode()).decode() + logger.info(f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, state_data: {state_data}") + # Let Authlib manage the state parameter for CSRF protection, but pass our custom data return oauth.google.authorize_redirect(redirect_uri, state=state_encoded) @@ -237,15 +246,25 @@ def tenant_google_auth(tenant_id): @auth_bp.route("/auth/google/callback") def google_callback(): """Handle Google OAuth callback.""" + # Log entry point + host = request.headers.get("Host", "") + apx_host = request.headers.get("Apx-Incoming-Host") + state_param = request.args.get("state") + logger.info( + f"[OAUTH_DEBUG] google_callback() called - Host: {host}, Apx-Incoming-Host: {apx_host}, state: {state_param[:50] if state_param else None}..." + ) + oauth = current_app.oauth if hasattr(current_app, "oauth") else None if not oauth: flash("OAuth not configured", "error") + logger.warning("[OAUTH_DEBUG] OAuth not configured in callback, redirecting to login") return redirect(url_for("auth.login")) try: token = oauth.google.authorize_access_token() if not token: flash("Authentication failed", "error") + logger.warning("[OAUTH_DEBUG] No OAuth token received, redirecting to login") return redirect(url_for("auth.login")) # Get user info diff --git a/src/core/schemas_generated/_schemas_v1_core_webhook_payload_json.py b/src/core/schemas_generated/_schemas_v1_core_webhook_payload_json.py new file mode 100644 index 000000000..a8a636b57 --- /dev/null +++ b/src/core/schemas_generated/_schemas_v1_core_webhook_payload_json.py @@ -0,0 +1,133 @@ +# generated by datamodel-codegen: +# filename: _schemas_v1_core_webhook-payload_json.json +# source_etag: W/"68ffaa97-1ce0" +# source_last_modified: Mon, 27 Oct 2025 17:23:35 GMT + +from __future__ import annotations + +from enum import Enum +from typing import Annotated, Any + +from pydantic import AwareDatetime, BaseModel, ConfigDict, Field + + +class TaskType(Enum): + create_media_buy = "create_media_buy" + update_media_buy = "update_media_buy" + sync_creatives = "sync_creatives" + activate_signal = "activate_signal" + get_signals = "get_signals" + + +class Domain(Enum): + media_buy = "media-buy" + signals = "signals" + + +class Status(Enum): + submitted = "submitted" + working = "working" + input_required = "input-required" + completed = "completed" + canceled = "canceled" + failed = "failed" + rejected = "rejected" + auth_required = "auth-required" + unknown = "unknown" + + +class Progress(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + percentage: Annotated[float | None, Field(description="Completion percentage (0-100)", ge=0.0, le=100.0)] = None + current_step: Annotated[str | None, Field(description="Current step or phase of the operation")] = None + total_steps: Annotated[int | None, Field(description="Total number of steps in the operation", ge=1)] = None + step_number: Annotated[int | None, Field(description="Current step number", ge=1)] = None + + +class Details(BaseModel): + model_config = ConfigDict( + extra="allow", + ) + domain: Annotated[Domain | None, Field(description="AdCP domain where error occurred")] = None + operation: Annotated[str | None, Field(description="Specific operation that failed")] = None + specific_context: Annotated[dict[str, Any] | None, Field(description="Domain-specific error context")] = None + + +class Error(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + code: Annotated[str, Field(description="Error code for programmatic handling")] + message: Annotated[str, Field(description="Detailed error message")] + details: Annotated[Details | None, Field(description="Additional error context")] = None + + +class WebhookPayload(BaseModel): + model_config = ConfigDict( + extra="allow", + ) + task_id: Annotated[ + str, + Field( + description="Unique identifier for this task. Use this to correlate webhook notifications with the original task submission." + ), + ] + task_type: Annotated[ + TaskType, + Field( + description="Valid AdCP task types across all domains. These represent the complete set of operations that can be tracked via the task management system.", + title="Task Type", + ), + ] + domain: Annotated[ + Domain, + Field(description="AdCP domain this task belongs to. Helps classify the operation type at a high level."), + ] + status: Annotated[ + Status, + Field( + description="Standardized task status values based on A2A TaskState enum. Indicates the current state of any AdCP operation.", + title="Task Status", + ), + ] + created_at: Annotated[ + AwareDatetime, + Field( + description="ISO 8601 timestamp when the task was initially created. Useful for tracking operation duration." + ), + ] + updated_at: Annotated[ + AwareDatetime, + Field( + description="ISO 8601 timestamp when the task status was last updated. This matches the timestamp when the webhook was triggered." + ), + ] + completed_at: Annotated[ + AwareDatetime | None, + Field( + description="ISO 8601 timestamp when the task reached a terminal state (completed, failed, or canceled). Only present for terminal states." + ), + ] = None + message: Annotated[ + str | None, + Field( + description="Human-readable summary of the current task state. Provides context about what happened and what action may be needed." + ), + ] = None + context_id: Annotated[ + str | None, + Field( + description="Session/conversation identifier. Use this to continue the conversation if input-required status needs clarification or additional parameters." + ), + ] = None + progress: Annotated[ + Progress | None, + Field( + description="Progress information for tasks still in 'working' state. Rarely seen in webhooks since 'working' tasks typically complete synchronously, but may appear if a task transitions from 'submitted' to 'working'." + ), + ] = None + error: Annotated[ + Error | None, Field(description="Error details for failed tasks. Only present when status is 'failed'.") + ] = None diff --git a/src/core/schemas_generated/_schemas_v1_enums_task_type_json.py b/src/core/schemas_generated/_schemas_v1_enums_task_type_json.py new file mode 100644 index 000000000..25f696fe4 --- /dev/null +++ b/src/core/schemas_generated/_schemas_v1_enums_task_type_json.py @@ -0,0 +1,16 @@ +# generated by datamodel-codegen: +# filename: _schemas_v1_enums_task-type_json.json +# source_etag: W/"68ffaa97-531" +# source_last_modified: Mon, 27 Oct 2025 17:23:35 GMT + +from __future__ import annotations + +from enum import Enum + + +class TaskType(Enum): + create_media_buy = "create_media_buy" + update_media_buy = "update_media_buy" + sync_creatives = "sync_creatives" + activate_signal = "activate_signal" + get_signals = "get_signals" From 2ddb3381356789b83276b3e22ec163ca7479b94c Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 15:41:45 -0400 Subject: [PATCH 06/20] debug: Add detailed OAuth error logging for token exchange failures --- src/admin/blueprints/auth.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index abe379159..e87d815c8 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -262,8 +262,11 @@ def google_callback(): try: token = oauth.google.authorize_access_token() + logger.info(f"[OAUTH_DEBUG] authorize_access_token() returned: {token is not None}") + if token: + logger.info(f"[OAUTH_DEBUG] Token keys: {list(token.keys())}") if not token: - flash("Authentication failed", "error") + flash("Authentication failed. Please try again.", "error") logger.warning("[OAUTH_DEBUG] No OAuth token received, redirecting to login") return redirect(url_for("auth.login")) @@ -484,7 +487,9 @@ def google_callback(): return redirect(url_for("auth.select_tenant")) except Exception as e: - logger.error(f"OAuth callback error: {e}", exc_info=True) + logger.error(f"[OAUTH_DEBUG] OAuth callback error: {type(e).__name__}: {e}", exc_info=True) + logger.error(f"[OAUTH_DEBUG] Request args: {dict(request.args)}") + logger.error(f"[OAUTH_DEBUG] Session keys: {list(session.keys())}") flash("Authentication failed. Please try again.", "error") return redirect(url_for("auth.login")) From e4da94eb94ee8317486d039e82f2fde6ce19259c Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 15:44:39 -0400 Subject: [PATCH 07/20] 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. --- src/admin/blueprints/auth.py | 50 +++++++++++------------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index e87d815c8..d7ee074bb 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -189,23 +189,15 @@ def google_auth(): if tenant_context: session["oauth_tenant_context"] = tenant_context - # Build custom state parameter to handle cross-domain scenarios - # For cross-domain OAuth (virtual hosts), session cookies won't work - import base64 - - state_data = { - "signup_flow": session.get("signup_flow", False), - "external_domain": approximated_host - or (host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None), - "tenant_context": tenant_context, - } - state_json = json.dumps(state_data) - state_encoded = base64.urlsafe_b64encode(state_json.encode()).decode() - - logger.info(f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, state_data: {state_data}") + logger.info( + f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, " + f"tenant_context: {tenant_context}, external_domain: {approximated_host}" + ) - # Let Authlib manage the state parameter for CSRF protection, but pass our custom data - return oauth.google.authorize_redirect(redirect_uri, state=state_encoded) + # Let Authlib manage the state parameter for CSRF protection + # Custom data is stored in session (above) and will be available in callback + # Note: Session cookies work across subdomains due to Domain=.sales-agent.scope3.com + return oauth.google.authorize_redirect(redirect_uri) @auth_bp.route("/tenant//auth/google") @@ -290,21 +282,8 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Try to decode state parameter for cross-domain OAuth context - import base64 - - state_param = request.args.get("state") - state_data = {} - if state_param: - try: - state_json = base64.urlsafe_b64decode(state_param.encode()).decode() - state_data = json.loads(state_json) - logger.info(f"OAuth callback - decoded state: {state_data}") - except Exception as e: - logger.warning(f"Could not decode OAuth state parameter: {e}") - - # Check if this is a signup flow (from state or session) - is_signup_flow = state_data.get("signup_flow") or session.get("signup_flow") + # Check if this is a signup flow (from session) + is_signup_flow = session.get("signup_flow") if is_signup_flow: logger.info(f"OAuth callback - signup flow detected for {email}") # Set signup flow in session for onboarding @@ -313,12 +292,13 @@ def google_callback(): return redirect(url_for("public.signup_onboarding")) # Debug session state before popping values - logger.info(f"OAuth callback - full session: {dict(session)}") + logger.info(f"[OAUTH_DEBUG] OAuth callback - full session: {dict(session)}") - # Get originating host and tenant context from state parameter (preferred) or session (fallback) + # Get originating host and tenant context from session + # Session cookies work across subdomains due to Domain=.sales-agent.scope3.com originating_host = session.pop("oauth_originating_host", None) - external_domain = state_data.get("external_domain") or session.pop("oauth_external_domain", None) - tenant_id = state_data.get("tenant_context") or session.pop("oauth_tenant_context", None) + external_domain = session.pop("oauth_external_domain", None) + tenant_id = session.pop("oauth_tenant_context", None) # Debug logging for OAuth redirect logger.info(f"OAuth callback debug - originating_host: {originating_host}") From 5c4a4d72f6bc5710f7bbe54fc89daafe27514a20 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 17:25:49 -0400 Subject: [PATCH 08/20] Revert "fix: Let Authlib manage OAuth state for CSRF protection" This reverts commit e4da94eb94ee8317486d039e82f2fde6ce19259c. --- src/admin/blueprints/auth.py | 50 +++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index d7ee074bb..e87d815c8 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -189,15 +189,23 @@ def google_auth(): if tenant_context: session["oauth_tenant_context"] = tenant_context - logger.info( - f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, " - f"tenant_context: {tenant_context}, external_domain: {approximated_host}" - ) + # Build custom state parameter to handle cross-domain scenarios + # For cross-domain OAuth (virtual hosts), session cookies won't work + import base64 + + state_data = { + "signup_flow": session.get("signup_flow", False), + "external_domain": approximated_host + or (host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None), + "tenant_context": tenant_context, + } + state_json = json.dumps(state_data) + state_encoded = base64.urlsafe_b64encode(state_json.encode()).decode() - # Let Authlib manage the state parameter for CSRF protection - # Custom data is stored in session (above) and will be available in callback - # Note: Session cookies work across subdomains due to Domain=.sales-agent.scope3.com - return oauth.google.authorize_redirect(redirect_uri) + logger.info(f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, state_data: {state_data}") + + # Let Authlib manage the state parameter for CSRF protection, but pass our custom data + return oauth.google.authorize_redirect(redirect_uri, state=state_encoded) @auth_bp.route("/tenant//auth/google") @@ -282,8 +290,21 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Check if this is a signup flow (from session) - is_signup_flow = session.get("signup_flow") + # Try to decode state parameter for cross-domain OAuth context + import base64 + + state_param = request.args.get("state") + state_data = {} + if state_param: + try: + state_json = base64.urlsafe_b64decode(state_param.encode()).decode() + state_data = json.loads(state_json) + logger.info(f"OAuth callback - decoded state: {state_data}") + except Exception as e: + logger.warning(f"Could not decode OAuth state parameter: {e}") + + # Check if this is a signup flow (from state or session) + is_signup_flow = state_data.get("signup_flow") or session.get("signup_flow") if is_signup_flow: logger.info(f"OAuth callback - signup flow detected for {email}") # Set signup flow in session for onboarding @@ -292,13 +313,12 @@ def google_callback(): return redirect(url_for("public.signup_onboarding")) # Debug session state before popping values - logger.info(f"[OAUTH_DEBUG] OAuth callback - full session: {dict(session)}") + logger.info(f"OAuth callback - full session: {dict(session)}") - # Get originating host and tenant context from session - # Session cookies work across subdomains due to Domain=.sales-agent.scope3.com + # Get originating host and tenant context from state parameter (preferred) or session (fallback) originating_host = session.pop("oauth_originating_host", None) - external_domain = session.pop("oauth_external_domain", None) - tenant_id = session.pop("oauth_tenant_context", None) + external_domain = state_data.get("external_domain") or session.pop("oauth_external_domain", None) + tenant_id = state_data.get("tenant_context") or session.pop("oauth_tenant_context", None) # Debug logging for OAuth redirect logger.info(f"OAuth callback debug - originating_host: {originating_host}") From 28e111a436c99f4f1fcd45d102944cbf1df02403 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 17:54:17 -0400 Subject: [PATCH 09/20] debug: Add visible flash message to show OAuth callback variables --- src/admin/blueprints/auth.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index e87d815c8..c042f91fe 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -327,6 +327,13 @@ def google_callback(): logger.info(f"OAuth callback debug - PRODUCTION env: {os.environ.get('PRODUCTION')}") logger.info(f"OAuth callback debug - user email: {email}") logger.info(f"OAuth callback debug - request headers: {dict(request.headers)}") + + # Flash debug info for troubleshooting (temporary) + flash( + f"DEBUG: external_domain={external_domain}, originating_host={originating_host}, tenant_id={tenant_id}", + "info", + ) + if tenant_id: # Verify user has access to this tenant with get_db_session() as db_session: From ff2283f9ba05d129256dfee5a843aa2f9b734223 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 18:51:25 -0400 Subject: [PATCH 10/20] fix: Remove custom OAuth state parameter completely - use session storage only --- src/admin/blueprints/auth.py | 62 ++++++++++++++---------------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index c042f91fe..3daa2037e 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -182,30 +182,27 @@ def google_auth(): # Note: This works for same-domain OAuth but has limitations for cross-domain scenarios approximated_host = request.headers.get("Apx-Incoming-Host") - if approximated_host: - session["oauth_external_domain"] = approximated_host - logger.info(f"Stored external domain for OAuth redirect: {approximated_host}") + # Store external domain and tenant context in session + # Session cookies work across subdomains (Domain=.sales-agent.scope3.com) + external_domain = approximated_host or ( + host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None + ) + + if external_domain: + session["oauth_external_domain"] = external_domain + logger.info(f"Stored external domain for OAuth redirect: {external_domain}") if tenant_context: session["oauth_tenant_context"] = tenant_context - # Build custom state parameter to handle cross-domain scenarios - # For cross-domain OAuth (virtual hosts), session cookies won't work - import base64 - - state_data = { - "signup_flow": session.get("signup_flow", False), - "external_domain": approximated_host - or (host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None), - "tenant_context": tenant_context, - } - state_json = json.dumps(state_data) - state_encoded = base64.urlsafe_b64encode(state_json.encode()).decode() - - logger.info(f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, state_data: {state_data}") + logger.info( + f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, " + f"external_domain: {external_domain}, tenant_context: {tenant_context}" + ) - # Let Authlib manage the state parameter for CSRF protection, but pass our custom data - return oauth.google.authorize_redirect(redirect_uri, state=state_encoded) + # Let Authlib manage the state parameter for CSRF protection + # Do NOT pass a custom state parameter - this breaks Authlib's CSRF validation + return oauth.google.authorize_redirect(redirect_uri) @auth_bp.route("/tenant//auth/google") @@ -290,21 +287,9 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Try to decode state parameter for cross-domain OAuth context - import base64 - - state_param = request.args.get("state") - state_data = {} - if state_param: - try: - state_json = base64.urlsafe_b64decode(state_param.encode()).decode() - state_data = json.loads(state_json) - logger.info(f"OAuth callback - decoded state: {state_data}") - except Exception as e: - logger.warning(f"Could not decode OAuth state parameter: {e}") - - # Check if this is a signup flow (from state or session) - is_signup_flow = state_data.get("signup_flow") or session.get("signup_flow") + # Check if this is a signup flow (from session only) + # State parameter is managed by Authlib for CSRF protection - we don't use it for data + is_signup_flow = session.get("signup_flow") if is_signup_flow: logger.info(f"OAuth callback - signup flow detected for {email}") # Set signup flow in session for onboarding @@ -313,12 +298,13 @@ def google_callback(): return redirect(url_for("public.signup_onboarding")) # Debug session state before popping values - logger.info(f"OAuth callback - full session: {dict(session)}") + logger.info(f"[OAUTH_DEBUG] OAuth callback - full session: {dict(session)}") - # Get originating host and tenant context from state parameter (preferred) or session (fallback) + # Get originating host and tenant context from session + # Session cookies work across subdomains (Domain=.sales-agent.scope3.com) originating_host = session.pop("oauth_originating_host", None) - external_domain = state_data.get("external_domain") or session.pop("oauth_external_domain", None) - tenant_id = state_data.get("tenant_context") or session.pop("oauth_tenant_context", None) + external_domain = session.pop("oauth_external_domain", None) + tenant_id = session.pop("oauth_tenant_context", None) # Debug logging for OAuth redirect logger.info(f"OAuth callback debug - originating_host: {originating_host}") From be17bcd66e7e47d20d63d94d3d4cdcd9527d22ee Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 19:12:25 -0400 Subject: [PATCH 11/20] debug: Add detailed access control logging for OAuth callback --- src/admin/blueprints/auth.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 3daa2037e..70eef47d8 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -346,12 +346,24 @@ def google_callback(): email_domain = email.split("@")[1] if "@" in email else "" tenant_access = get_user_tenant_access(email) + # Debug logging for tenant access check + logger.info(f"[ACCESS_CHECK] email: {email}, email_domain: {email_domain}") + logger.info(f"[ACCESS_CHECK] tenant_id from session: {tenant_id}") + logger.info(f"[ACCESS_CHECK] domain_tenant: {tenant_access.get('domain_tenant')}") + if tenant_access.get("domain_tenant"): + logger.info(f"[ACCESS_CHECK] domain_tenant.tenant_id: {tenant_access['domain_tenant'].tenant_id}") + logger.info( + f"[ACCESS_CHECK] email_tenants: {[t.tenant_id for t in tenant_access.get('email_tenants', [])]}" + ) + # Check if user has access to this specific tenant has_tenant_access = False if tenant_access["domain_tenant"] and tenant_access["domain_tenant"].tenant_id == tenant_id: has_tenant_access = True + logger.info("[ACCESS_CHECK] Access granted via domain_tenant") elif any(t.tenant_id == tenant_id for t in tenant_access["email_tenants"]): has_tenant_access = True + logger.info("[ACCESS_CHECK] Access granted via email_tenants") if has_tenant_access: # Ensure user record exists (auto-create if needed) @@ -368,6 +380,11 @@ def google_callback(): return redirect(url_for("tenants.dashboard", tenant_id=tenant_id)) else: # User doesn't have access to this tenant + logger.warning( + f"[ACCESS_DENIED] User {email} denied access to tenant {tenant_id}. " + f"domain_tenant: {tenant_access.get('domain_tenant')}, " + f"email_tenants: {[t.tenant_id for t in tenant_access.get('email_tenants', [])]}" + ) flash( "You don't have access to this tenant. Please contact your administrator to request access.", "error", From 3083963ed51fd52748ee080e1bd2598d9e81e7c9 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 19:49:05 -0400 Subject: [PATCH 12/20] 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 --- src/admin/blueprints/auth.py | 56 ++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 70eef47d8..1b55351e6 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -341,29 +341,49 @@ def google_callback(): return redirect(url_for("tenants.dashboard", tenant_id=tenant_id)) # Check if user is authorized (via email list or domain list) - from src.admin.domain_access import ensure_user_in_tenant, get_user_tenant_access + from src.admin.domain_access import ensure_user_in_tenant email_domain = email.split("@")[1] if "@" in email else "" - tenant_access = get_user_tenant_access(email) # Debug logging for tenant access check logger.info(f"[ACCESS_CHECK] email: {email}, email_domain: {email_domain}") logger.info(f"[ACCESS_CHECK] tenant_id from session: {tenant_id}") - logger.info(f"[ACCESS_CHECK] domain_tenant: {tenant_access.get('domain_tenant')}") - if tenant_access.get("domain_tenant"): - logger.info(f"[ACCESS_CHECK] domain_tenant.tenant_id: {tenant_access['domain_tenant'].tenant_id}") - logger.info( - f"[ACCESS_CHECK] email_tenants: {[t.tenant_id for t in tenant_access.get('email_tenants', [])]}" - ) + logger.info(f"[ACCESS_CHECK] tenant.authorized_domains: {tenant.authorized_domains}") + logger.info(f"[ACCESS_CHECK] tenant.authorized_emails: {tenant.authorized_emails}") - # Check if user has access to this specific tenant + # Check if user has access to THIS SPECIFIC tenant + # Check domain-based access has_tenant_access = False - if tenant_access["domain_tenant"] and tenant_access["domain_tenant"].tenant_id == tenant_id: - has_tenant_access = True - logger.info("[ACCESS_CHECK] Access granted via domain_tenant") - elif any(t.tenant_id == tenant_id for t in tenant_access["email_tenants"]): - has_tenant_access = True - logger.info("[ACCESS_CHECK] Access granted via email_tenants") + if email_domain and tenant.authorized_domains: + try: + import json + + domains = ( + tenant.authorized_domains + if isinstance(tenant.authorized_domains, list) + else json.loads(tenant.authorized_domains) + ) + if email_domain in domains: + has_tenant_access = True + logger.info(f"[ACCESS_CHECK] Access granted via domain: {email_domain} in {domains}") + except (json.JSONDecodeError, TypeError) as e: + logger.warning(f"[ACCESS_CHECK] Invalid authorized_domains JSON: {e}") + + # Check email-based access + if not has_tenant_access and tenant.authorized_emails: + try: + import json + + emails = ( + tenant.authorized_emails + if isinstance(tenant.authorized_emails, list) + else json.loads(tenant.authorized_emails) + ) + if email.lower() in [e.lower() for e in emails]: + has_tenant_access = True + logger.info(f"[ACCESS_CHECK] Access granted via email: {email} in {emails}") + except (json.JSONDecodeError, TypeError) as e: + logger.warning(f"[ACCESS_CHECK] Invalid authorized_emails JSON: {e}") if has_tenant_access: # Ensure user record exists (auto-create if needed) @@ -381,9 +401,9 @@ def google_callback(): else: # User doesn't have access to this tenant logger.warning( - f"[ACCESS_DENIED] User {email} denied access to tenant {tenant_id}. " - f"domain_tenant: {tenant_access.get('domain_tenant')}, " - f"email_tenants: {[t.tenant_id for t in tenant_access.get('email_tenants', [])]}" + f"[ACCESS_DENIED] User {email} (domain: {email_domain}) denied access to tenant {tenant_id}. " + f"tenant.authorized_domains: {tenant.authorized_domains}, " + f"tenant.authorized_emails: {tenant.authorized_emails}" ) flash( "You don't have access to this tenant. Please contact your administrator to request access.", From 260d5b670b27aaf0b7921137b19e319d03056a4e Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 19:56:01 -0400 Subject: [PATCH 13/20] debug: Add detailed exception handling and logging for access check --- src/admin/blueprints/auth.py | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 1b55351e6..f0a370177 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -354,10 +354,14 @@ def google_callback(): # Check if user has access to THIS SPECIFIC tenant # Check domain-based access has_tenant_access = False - if email_domain and tenant.authorized_domains: - try: + try: + if email_domain and tenant.authorized_domains: import json + logger.info( + f"[ACCESS_CHECK] Checking domain access - type: {type(tenant.authorized_domains)}, " + f"value: {tenant.authorized_domains}" + ) domains = ( tenant.authorized_domains if isinstance(tenant.authorized_domains, list) @@ -366,14 +370,17 @@ def google_callback(): if email_domain in domains: has_tenant_access = True logger.info(f"[ACCESS_CHECK] Access granted via domain: {email_domain} in {domains}") - except (json.JSONDecodeError, TypeError) as e: - logger.warning(f"[ACCESS_CHECK] Invalid authorized_domains JSON: {e}") + else: + logger.info(f"[ACCESS_CHECK] Domain {email_domain} not in {domains}") - # Check email-based access - if not has_tenant_access and tenant.authorized_emails: - try: + # Check email-based access + if not has_tenant_access and tenant.authorized_emails: import json + logger.info( + f"[ACCESS_CHECK] Checking email access - type: {type(tenant.authorized_emails)}, " + f"value: {tenant.authorized_emails}" + ) emails = ( tenant.authorized_emails if isinstance(tenant.authorized_emails, list) @@ -382,8 +389,15 @@ def google_callback(): if email.lower() in [e.lower() for e in emails]: has_tenant_access = True logger.info(f"[ACCESS_CHECK] Access granted via email: {email} in {emails}") - except (json.JSONDecodeError, TypeError) as e: - logger.warning(f"[ACCESS_CHECK] Invalid authorized_emails JSON: {e}") + else: + logger.info(f"[ACCESS_CHECK] Email {email} not in {emails}") + + except Exception as e: + logger.error( + f"[ACCESS_CHECK] Exception during access check: {type(e).__name__}: {e}", exc_info=True + ) + # Don't grant access if there's an error + has_tenant_access = False if has_tenant_access: # Ensure user record exists (auto-create if needed) From 9afa4195f0d0e0c23b5dfc9303d653d25bb5b446 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 20:09:43 -0400 Subject: [PATCH 14/20] debug: Add visible flash message to show OAuth callback variables --- src/admin/blueprints/auth.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index f0a370177..7fa37474d 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -350,6 +350,11 @@ def google_callback(): logger.info(f"[ACCESS_CHECK] tenant_id from session: {tenant_id}") logger.info(f"[ACCESS_CHECK] tenant.authorized_domains: {tenant.authorized_domains}") logger.info(f"[ACCESS_CHECK] tenant.authorized_emails: {tenant.authorized_emails}") + flash( + f"DEBUG ACCESS CHECK: email_domain={email_domain}, " + f"authorized_domains={tenant.authorized_domains}", + "info", + ) # Check if user has access to THIS SPECIFIC tenant # Check domain-based access From bac872b5d9231ba18bd736adc077cb29063539a1 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 20:11:38 -0400 Subject: [PATCH 15/20] debug: Add more granular flash messages to trace OAuth flow --- src/admin/blueprints/auth.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 7fa37474d..234c6ef48 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -321,12 +321,15 @@ def google_callback(): ) if tenant_id: + flash(f"DEBUG: Entering tenant_id block, tenant_id={tenant_id}", "info") # Verify user has access to this tenant with get_db_session() as db_session: tenant = db_session.scalars(select(Tenant).filter_by(tenant_id=tenant_id)).first() if not tenant: + flash(f"DEBUG: Tenant not found for tenant_id={tenant_id}", "error") flash("Invalid tenant", "error") return redirect(url_for("auth.login")) + flash(f"DEBUG: Tenant found: {tenant.name}", "info") # Check if user is super admin or has tenant access if is_super_admin(email): From 9b3bc801e9266f9a776d7d1aa1f239da6b5566bd Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 20:27:06 -0400 Subject: [PATCH 16/20] 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 --- src/admin/blueprints/auth.py | 311 +++-------------------------------- 1 file changed, 26 insertions(+), 285 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 234c6ef48..4438715d8 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -10,7 +10,7 @@ from src.admin.utils import is_super_admin # type: ignore[attr-defined] from src.core.database.database_session import get_db_session -from src.core.database.models import Tenant, User +from src.core.database.models import Tenant logger = logging.getLogger(__name__) @@ -127,81 +127,19 @@ def tenant_login(tenant_id): @auth_bp.route("/auth/google") def google_auth(): - """Initiate Google OAuth flow with tenant context detection.""" - # Log entry point - host = request.headers.get("Host", "") - apx_host = request.headers.get("Apx-Incoming-Host") - logger.info( - f"[OAUTH_DEBUG] google_auth() called - Host: {host}, Apx-Incoming-Host: {apx_host}, args: {dict(request.args)}" - ) - + """Initiate Google OAuth flow - simplified central login.""" oauth = current_app.oauth if hasattr(current_app, "oauth") else None if not oauth: flash("OAuth not configured", "error") - logger.warning("[OAUTH_DEBUG] OAuth not configured, redirecting to login") return redirect(url_for("auth.login")) - # Capture tenant context from headers or form data - tenant_context = request.args.get("tenant_context") # From login form - - # Check for Approximated routing headers first - if not tenant_context: - approximated_host = request.headers.get("Apx-Incoming-Host") - if approximated_host and not approximated_host.startswith("admin."): - # Approximated handles all external routing - look up tenant by virtual_host - with get_db_session() as db_session: - tenant = db_session.scalars(select(Tenant).filter_by(virtual_host=approximated_host)).first() - if tenant: - tenant_context = tenant.tenant_id - logger.info( - f"Detected tenant context from Approximated headers: {approximated_host} -> {tenant_context}" - ) - - # Fallback to direct domain routing - if not tenant_context and ".sales-agent.scope3.com" in host and not host.startswith("admin."): - # Extract tenant subdomain from Host header - tenant_subdomain = host.split(".")[0] - with get_db_session() as db_session: - tenant = db_session.scalars(select(Tenant).filter_by(subdomain=tenant_subdomain)).first() - if tenant: - tenant_context = tenant.tenant_id - logger.info(f"Detected tenant context from Host header: {tenant_subdomain} -> {tenant_context}") - - # Always use the registered OAuth redirect URI for Google (no modifications allowed) + # Always use the registered OAuth redirect URI for Google if os.environ.get("PRODUCTION") == "true": - # For production, always use the exact registered redirect URI redirect_uri = "https://sales-agent.scope3.com/admin/auth/google/callback" else: - # Development fallback redirect_uri = url_for("auth.google_callback", _external=True) - # Store originating host and tenant context in session for OAuth callback - session["oauth_originating_host"] = host - - # Store external domain and tenant context in session for OAuth callback - # Note: This works for same-domain OAuth but has limitations for cross-domain scenarios - approximated_host = request.headers.get("Apx-Incoming-Host") - - # Store external domain and tenant context in session - # Session cookies work across subdomains (Domain=.sales-agent.scope3.com) - external_domain = approximated_host or ( - host if host != "sales-agent.scope3.com" and not host.startswith("admin.") else None - ) - - if external_domain: - session["oauth_external_domain"] = external_domain - logger.info(f"Stored external domain for OAuth redirect: {external_domain}") - - if tenant_context: - session["oauth_tenant_context"] = tenant_context - - logger.info( - f"[OAUTH_DEBUG] Initiating OAuth redirect - redirect_uri: {redirect_uri}, " - f"external_domain: {external_domain}, tenant_context: {tenant_context}" - ) - - # Let Authlib manage the state parameter for CSRF protection - # Do NOT pass a custom state parameter - this breaks Authlib's CSRF validation + # Simple OAuth flow - no tenant context preservation needed return oauth.google.authorize_redirect(redirect_uri) @@ -242,29 +180,16 @@ def tenant_google_auth(tenant_id): @auth_bp.route("/auth/google/callback") def google_callback(): - """Handle Google OAuth callback.""" - # Log entry point - host = request.headers.get("Host", "") - apx_host = request.headers.get("Apx-Incoming-Host") - state_param = request.args.get("state") - logger.info( - f"[OAUTH_DEBUG] google_callback() called - Host: {host}, Apx-Incoming-Host: {apx_host}, state: {state_param[:50] if state_param else None}..." - ) - + """Handle Google OAuth callback - simplified version.""" oauth = current_app.oauth if hasattr(current_app, "oauth") else None if not oauth: flash("OAuth not configured", "error") - logger.warning("[OAUTH_DEBUG] OAuth not configured in callback, redirecting to login") return redirect(url_for("auth.login")) try: token = oauth.google.authorize_access_token() - logger.info(f"[OAUTH_DEBUG] authorize_access_token() returned: {token is not None}") - if token: - logger.info(f"[OAUTH_DEBUG] Token keys: {list(token.keys())}") if not token: flash("Authentication failed. Please try again.", "error") - logger.warning("[OAUTH_DEBUG] No OAuth token received, redirecting to login") return redirect(url_for("auth.login")) # Get user info @@ -275,7 +200,6 @@ def google_callback(): id_token = token.get("id_token") if id_token: - # Decode without verification since we trust Google's response user = jwt.decode(id_token, options={"verify_signature": False}) if not user or not user.get("email"): @@ -287,231 +211,52 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Check if this is a signup flow (from session only) - # State parameter is managed by Authlib for CSRF protection - we don't use it for data + # Check if this is a signup flow is_signup_flow = session.get("signup_flow") if is_signup_flow: - logger.info(f"OAuth callback - signup flow detected for {email}") - # Set signup flow in session for onboarding session["signup_flow"] = True - # Redirect to onboarding wizard return redirect(url_for("public.signup_onboarding")) - # Debug session state before popping values - logger.info(f"[OAUTH_DEBUG] OAuth callback - full session: {dict(session)}") - - # Get originating host and tenant context from session - # Session cookies work across subdomains (Domain=.sales-agent.scope3.com) - originating_host = session.pop("oauth_originating_host", None) - external_domain = session.pop("oauth_external_domain", None) - tenant_id = session.pop("oauth_tenant_context", None) - - # Debug logging for OAuth redirect - logger.info(f"OAuth callback debug - originating_host: {originating_host}") - logger.info(f"OAuth callback debug - external_domain: {external_domain}") - logger.info(f"OAuth callback debug - tenant_id: {tenant_id}") - logger.info(f"OAuth callback debug - PRODUCTION env: {os.environ.get('PRODUCTION')}") - logger.info(f"OAuth callback debug - user email: {email}") - logger.info(f"OAuth callback debug - request headers: {dict(request.headers)}") - - # Flash debug info for troubleshooting (temporary) - flash( - f"DEBUG: external_domain={external_domain}, originating_host={originating_host}, tenant_id={tenant_id}", - "info", - ) - - if tenant_id: - flash(f"DEBUG: Entering tenant_id block, tenant_id={tenant_id}", "info") - # Verify user has access to this tenant - with get_db_session() as db_session: - tenant = db_session.scalars(select(Tenant).filter_by(tenant_id=tenant_id)).first() - if not tenant: - flash(f"DEBUG: Tenant not found for tenant_id={tenant_id}", "error") - flash("Invalid tenant", "error") - return redirect(url_for("auth.login")) - flash(f"DEBUG: Tenant found: {tenant.name}", "info") - - # Check if user is super admin or has tenant access - if is_super_admin(email): - session["tenant_id"] = tenant_id - session["is_super_admin"] = True - flash(f"Welcome {user.get('name', email)}! (Super Admin)", "success") - - # Redirect to tenant-specific subdomain if accessed via subdomain - if tenant.subdomain and tenant.subdomain != "localhost": - return redirect(f"https://{tenant.subdomain}.sales-agent.scope3.com/admin/") - else: - return redirect(url_for("tenants.dashboard", tenant_id=tenant_id)) - - # Check if user is authorized (via email list or domain list) - from src.admin.domain_access import ensure_user_in_tenant - - email_domain = email.split("@")[1] if "@" in email else "" - - # Debug logging for tenant access check - logger.info(f"[ACCESS_CHECK] email: {email}, email_domain: {email_domain}") - logger.info(f"[ACCESS_CHECK] tenant_id from session: {tenant_id}") - logger.info(f"[ACCESS_CHECK] tenant.authorized_domains: {tenant.authorized_domains}") - logger.info(f"[ACCESS_CHECK] tenant.authorized_emails: {tenant.authorized_emails}") - flash( - f"DEBUG ACCESS CHECK: email_domain={email_domain}, " - f"authorized_domains={tenant.authorized_domains}", - "info", - ) - - # Check if user has access to THIS SPECIFIC tenant - # Check domain-based access - has_tenant_access = False - try: - if email_domain and tenant.authorized_domains: - import json - - logger.info( - f"[ACCESS_CHECK] Checking domain access - type: {type(tenant.authorized_domains)}, " - f"value: {tenant.authorized_domains}" - ) - domains = ( - tenant.authorized_domains - if isinstance(tenant.authorized_domains, list) - else json.loads(tenant.authorized_domains) - ) - if email_domain in domains: - has_tenant_access = True - logger.info(f"[ACCESS_CHECK] Access granted via domain: {email_domain} in {domains}") - else: - logger.info(f"[ACCESS_CHECK] Domain {email_domain} not in {domains}") - - # Check email-based access - if not has_tenant_access and tenant.authorized_emails: - import json - - logger.info( - f"[ACCESS_CHECK] Checking email access - type: {type(tenant.authorized_emails)}, " - f"value: {tenant.authorized_emails}" - ) - emails = ( - tenant.authorized_emails - if isinstance(tenant.authorized_emails, list) - else json.loads(tenant.authorized_emails) - ) - if email.lower() in [e.lower() for e in emails]: - has_tenant_access = True - logger.info(f"[ACCESS_CHECK] Access granted via email: {email} in {emails}") - else: - logger.info(f"[ACCESS_CHECK] Email {email} not in {emails}") - - except Exception as e: - logger.error( - f"[ACCESS_CHECK] Exception during access check: {type(e).__name__}: {e}", exc_info=True - ) - # Don't grant access if there's an error - has_tenant_access = False - - if has_tenant_access: - # Ensure user record exists (auto-create if needed) - user_record = ensure_user_in_tenant(email, tenant_id, role="admin", name=user.get("name")) - - session["tenant_id"] = tenant_id - session["is_tenant_admin"] = user_record.role == "admin" - flash(f"Welcome {user.get('name', email)}!", "success") - - # Redirect to tenant-specific subdomain if accessed via subdomain - if tenant.subdomain and tenant.subdomain != "localhost": - return redirect(f"https://{tenant.subdomain}.sales-agent.scope3.com/admin/") - else: - return redirect(url_for("tenants.dashboard", tenant_id=tenant_id)) - else: - # User doesn't have access to this tenant - logger.warning( - f"[ACCESS_DENIED] User {email} (domain: {email_domain}) denied access to tenant {tenant_id}. " - f"tenant.authorized_domains: {tenant.authorized_domains}, " - f"tenant.authorized_emails: {tenant.authorized_emails}" - ) - flash( - "You don't have access to this tenant. Please contact your administrator to request access.", - "error", - ) - session.clear() - - # Redirect to general login page to avoid loop - # Don't redirect back to tenant-specific login as that creates an infinite loop - return redirect(url_for("auth.login")) - - # Domain-based access control using email domain extraction - # (ensure_user_in_tenant and get_user_tenant_access already imported above) + # Query all tenants user has access to + from src.admin.domain_access import get_user_tenant_access email_domain = email.split("@")[1] if "@" in email else "" - # 1. Scope3 super admin check + # Check if user is super admin if email_domain == "scope3.com" or is_super_admin(email): session["is_super_admin"] = True - session["role"] = "super_admin" - session["authenticated"] = True - session["email"] = email flash(f"Welcome {user.get('name', email)}! (Super Admin)", "success") + return redirect(url_for("core.index")) - # Check where the OAuth flow originated from - if external_domain and os.environ.get("PRODUCTION") == "true": - # OAuth was initiated from external domain routed through Approximated - # Important: External domains handle routing via Approximated - just use /admin/ - redirect_url = f"https://{external_domain}/admin/" - logger.info(f"Redirecting super admin back to external domain: {external_domain} -> {redirect_url}") - return redirect(redirect_url) - elif originating_host and originating_host.startswith("admin.") and os.environ.get("PRODUCTION") == "true": - return redirect("https://admin.sales-agent.scope3.com/admin/") - elif originating_host and os.environ.get("PRODUCTION") == "true": - # Preserve tenant-specific domains for super admins - return redirect(f"https://{originating_host}/admin/") - elif os.environ.get("PRODUCTION") == "true": - return redirect("https://admin.sales-agent.scope3.com/admin/") - else: - return redirect(url_for("core.index")) - - # 2. Check domain-based and email-based tenant access + # Get accessible tenants tenant_access = get_user_tenant_access(email) + # No access if tenant_access["total_access"] == 0: - # No access - check if this was from an external domain (should trigger signup) - if external_domain and not external_domain.endswith(".sales-agent.scope3.com"): - logger.info( - f"User {email} has no access but came from external domain {external_domain}, redirecting to signup" - ) - session["signup_flow"] = True - return redirect(url_for("public.signup_onboarding")) - - # Regular flow - no access flash("You don't have access to any tenants. Please contact your administrator.", "error") session.clear() return redirect(url_for("auth.login")) + # Single tenant - auto-select and redirect elif tenant_access["total_access"] == 1: - # Single tenant - direct access + from src.admin.domain_access import ensure_user_in_tenant + if tenant_access["domain_tenant"]: tenant = tenant_access["domain_tenant"] - access_type = "domain" else: tenant = tenant_access["email_tenants"][0] - access_type = "email" - # Ensure user record exists (auto-create if needed) + # Ensure user record exists user_record = ensure_user_in_tenant(email, tenant.tenant_id, role="admin", name=user.get("name")) session["tenant_id"] = tenant.tenant_id session["is_tenant_admin"] = user_record.role == "admin" - flash(f"Welcome {user.get('name', email)}! ({access_type.title()} Access)", "success") - - # Redirect to external domain if OAuth was initiated from external domain - if external_domain and os.environ.get("PRODUCTION") == "true": - logger.info(f"Redirecting tenant user back to external domain: {external_domain}") - return redirect(f"https://{external_domain}/admin/") - # Redirect to tenant-specific subdomain if accessed via subdomain - elif tenant.subdomain and tenant.subdomain != "localhost" and os.environ.get("PRODUCTION") == "true": - return redirect(f"https://{tenant.subdomain}.sales-agent.scope3.com/admin/") - else: - return redirect(url_for("tenants.dashboard", tenant_id=tenant.tenant_id)) + flash(f"Welcome {user.get('name', email)}!", "success") + return redirect(url_for("tenants.dashboard", tenant_id=tenant.tenant_id)) + # Multiple tenants - show selection page else: - # Multiple tenants - let user choose + session["available_tenants"] = [] if tenant_access["domain_tenant"]: @@ -519,21 +264,17 @@ def google_callback(): { "tenant_id": tenant_access["domain_tenant"].tenant_id, "name": tenant_access["domain_tenant"].name, - "access_type": "domain", - "is_admin": True, # Domain users get admin access + "subdomain": tenant_access["domain_tenant"].subdomain, } ) for tenant in tenant_access["email_tenants"]: - # Check existing user record for role, default to admin - with get_db_session() as db_session: - existing_user = db_session.scalars( - select(User).filter_by(email=email, tenant_id=tenant.tenant_id) - ).first() - is_admin = existing_user.role == "admin" if existing_user else True - session["available_tenants"].append( - {"tenant_id": tenant.tenant_id, "name": tenant.name, "access_type": "email", "is_admin": is_admin} + { + "tenant_id": tenant.tenant_id, + "name": tenant.name, + "subdomain": tenant.subdomain, + } ) return redirect(url_for("auth.select_tenant")) From 097b3bfbec7615896a1e23ba9a698d4e2b0aa89b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 20:36:29 -0400 Subject: [PATCH 17/20] 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 --- src/admin/blueprints/auth.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 4438715d8..0cbc62760 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -211,10 +211,9 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Check if this is a signup flow - is_signup_flow = session.get("signup_flow") + # Check if this is a signup flow (only if explicitly set, not just present in session) + is_signup_flow = session.pop("signup_flow", False) if is_signup_flow: - session["signup_flow"] = True return redirect(url_for("public.signup_onboarding")) # Query all tenants user has access to From 942642e7d9ed77af3fdc6dc37c7dbf7904ad12e0 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 20:45:33 -0400 Subject: [PATCH 18/20] debug: Add detailed error logging for OAuth token exchange failure --- src/admin/blueprints/auth.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 0cbc62760..0a8b6d143 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -187,8 +187,21 @@ def google_callback(): return redirect(url_for("auth.login")) try: - token = oauth.google.authorize_access_token() + logger.info("Attempting OAuth token exchange...") + try: + token = oauth.google.authorize_access_token() + logger.info(f"Token exchange result: {token is not None}") + except Exception as auth_error: + logger.error( + f"Authlib error during token exchange: {type(auth_error).__name__}: {auth_error}", exc_info=True + ) + flash(f"Authentication error: {str(auth_error)}", "error") + return redirect(url_for("auth.login")) + if not token: + logger.error("OAuth token exchange failed - authorize_access_token() returned None") + logger.error(f"Request args: {dict(request.args)}") + logger.error(f"Session keys: {list(session.keys())}") flash("Authentication failed. Please try again.", "error") return redirect(url_for("auth.login")) From 9b4044786a064c2ad286bdf31d58328d97ac1681 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 20:51:30 -0400 Subject: [PATCH 19/20] 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 --- src/admin/blueprints/auth.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 0a8b6d143..0cee5724e 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -181,8 +181,15 @@ def tenant_google_auth(tenant_id): @auth_bp.route("/auth/google/callback") def google_callback(): """Handle Google OAuth callback - simplified version.""" + # Log immediately when callback is hit + logger.warning("========== GOOGLE OAUTH CALLBACK HIT ==========") + logger.warning(f"Request URL: {request.url}") + logger.warning(f"Request args: {dict(request.args)}") + logger.warning(f"Session keys at start: {list(session.keys())}") + oauth = current_app.oauth if hasattr(current_app, "oauth") else None if not oauth: + logger.error("OAuth not configured!") flash("OAuth not configured", "error") return redirect(url_for("auth.login")) From fe9535a669cab990179a6ebbf1d0e797e6f98148 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 27 Oct 2025 21:39:25 -0400 Subject: [PATCH 20/20] 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 --- src/admin/blueprints/auth.py | 82 ++++++++++++------------------------ templates/choose_tenant.html | 20 +++++++-- 2 files changed, 44 insertions(+), 58 deletions(-) diff --git a/src/admin/blueprints/auth.py b/src/admin/blueprints/auth.py index 0cee5724e..d8489bb87 100644 --- a/src/admin/blueprints/auth.py +++ b/src/admin/blueprints/auth.py @@ -133,10 +133,9 @@ def google_auth(): flash("OAuth not configured", "error") return redirect(url_for("auth.login")) - # Always use the registered OAuth redirect URI for Google - if os.environ.get("PRODUCTION") == "true": - redirect_uri = "https://sales-agent.scope3.com/admin/auth/google/callback" - else: + # Use configured redirect URI from environment, fall back to auto-generated + redirect_uri = os.environ.get("GOOGLE_OAUTH_REDIRECT_URI") + if not redirect_uri: redirect_uri = url_for("auth.google_callback", _external=True) # Simple OAuth flow - no tenant context preservation needed @@ -231,12 +230,8 @@ def google_callback(): session["user_name"] = user.get("name", email) session["user_picture"] = user.get("picture", "") - # Check if this is a signup flow (only if explicitly set, not just present in session) - is_signup_flow = session.pop("signup_flow", False) - if is_signup_flow: - return redirect(url_for("public.signup_onboarding")) - - # Query all tenants user has access to + # Unified flow: Always show tenant selector (with option to create new tenant) + # No distinction between signup and login - keeps UX simple and consistent from src.admin.domain_access import get_user_tenant_access email_domain = email.split("@")[1] if "@" in email else "" @@ -247,56 +242,33 @@ def google_callback(): flash(f"Welcome {user.get('name', email)}! (Super Admin)", "success") return redirect(url_for("core.index")) - # Get accessible tenants + # Get all accessible tenants tenant_access = get_user_tenant_access(email) - # No access - if tenant_access["total_access"] == 0: - flash("You don't have access to any tenants. Please contact your administrator.", "error") - session.clear() - return redirect(url_for("auth.login")) - - # Single tenant - auto-select and redirect - elif tenant_access["total_access"] == 1: - from src.admin.domain_access import ensure_user_in_tenant - - if tenant_access["domain_tenant"]: - tenant = tenant_access["domain_tenant"] - else: - tenant = tenant_access["email_tenants"][0] - - # Ensure user record exists - user_record = ensure_user_in_tenant(email, tenant.tenant_id, role="admin", name=user.get("name")) + # Build tenant list for selector (empty list is fine - user can create new tenant) + session["available_tenants"] = [] - session["tenant_id"] = tenant.tenant_id - session["is_tenant_admin"] = user_record.role == "admin" - flash(f"Welcome {user.get('name', email)}!", "success") - return redirect(url_for("tenants.dashboard", tenant_id=tenant.tenant_id)) - - # Multiple tenants - show selection page - else: - - session["available_tenants"] = [] - - if tenant_access["domain_tenant"]: - session["available_tenants"].append( - { - "tenant_id": tenant_access["domain_tenant"].tenant_id, - "name": tenant_access["domain_tenant"].name, - "subdomain": tenant_access["domain_tenant"].subdomain, - } - ) + if tenant_access["domain_tenant"]: + session["available_tenants"].append( + { + "tenant_id": tenant_access["domain_tenant"].tenant_id, + "name": tenant_access["domain_tenant"].name, + "subdomain": tenant_access["domain_tenant"].subdomain, + } + ) - for tenant in tenant_access["email_tenants"]: - session["available_tenants"].append( - { - "tenant_id": tenant.tenant_id, - "name": tenant.name, - "subdomain": tenant.subdomain, - } - ) + for tenant in tenant_access["email_tenants"]: + session["available_tenants"].append( + { + "tenant_id": tenant.tenant_id, + "name": tenant.name, + "subdomain": tenant.subdomain, + } + ) - return redirect(url_for("auth.select_tenant")) + # Always show tenant selector (includes "Create New Tenant" option) + flash(f"Welcome {user.get('name', email)}!", "success") + return redirect(url_for("auth.select_tenant")) except Exception as e: logger.error(f"[OAUTH_DEBUG] OAuth callback error: {type(e).__name__}: {e}", exc_info=True) diff --git a/templates/choose_tenant.html b/templates/choose_tenant.html index 198ea33fb..e17aa7c50 100644 --- a/templates/choose_tenant.html +++ b/templates/choose_tenant.html @@ -6,23 +6,37 @@

Select Account

+ {% if tenants %}

- Your email address has access to multiple accounts. Please select which one you'd like to manage: + Your email address has access to the following accounts. Select one to continue, or create a new account:

{% for tenant_id, tenant_name in tenants %} {% endfor %}
- + Cancel
+ +
+ {% else %} +

+ You don't have access to any accounts yet. Create a new account to get started: +

+ {% endif %} + +