-
Notifications
You must be signed in to change notification settings - Fork 16
Fix authentication and tenant detection issues #573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🐛 Root Cause: Nginx was stripping the Authorization header before proxying A2A requests to the backend server, causing all authenticated requests to fail with "Missing authentication token - Bearer token required in Authorization header" 🔧 Fix: Added `proxy_set_header Authorization $http_authorization;` to all /a2a location blocks in both config/nginx/nginx.conf and fly/nginx.conf 📍 Affected endpoints: - *.adcontextprotocol.org/a2a (agent routing) - *.sales-agent.scope3.com/a2a (tenant routing) - sales-agent.scope3.com/a2a (base domain) - External domains/a2a (default server) ✅ Impact: Buyers can now successfully authenticate to sales agent A2A endpoints by sending `Authorization: Bearer <token>` header 🔗 Related: - Tenant detection already working (from 2025-10-23 fix) - Python middleware correctly extracts Bearer token when present - Issue was in nginx proxy layer, not application code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🎯 Goal: Make authentication flexible across both protocols Previously: - MCP: Only accepted x-adcp-auth header - A2A: Only accepted Authorization: Bearer header - Inconsistent and fragile Now: - Both MCP and A2A accept BOTH headers - Prefer protocol-standard header (x-adcp-auth for MCP, Authorization for A2A) - Fallback to alternate header if primary not present 🔧 Changes: Application Code: - src/core/auth_utils.py: MCP now accepts both x-adcp-auth (preferred) and Authorization - src/a2a_server/adcp_a2a_server.py: A2A now accepts both Authorization (preferred) and x-adcp-auth Nginx Configuration: - config/nginx/nginx.conf: All /mcp and /a2a locations forward BOTH headers - fly/nginx.conf: All /mcp and /a2a locations forward BOTH headers ✅ Benefits: 1. Cross-protocol compatibility (MCP clients can use A2A endpoints and vice versa) 2. More resilient to header variations 3. Easier client implementation (use either header convention) 4. Forward compatible with future auth changes 📍 Preference Order: - MCP: x-adcp-auth → Authorization - A2A: Authorization → x-adcp-auth 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds detailed logging to diagnose tenant detection failures: - Logs number of headers returned by get_http_headers() - Logs all header keys when headers are available - Logs which fallback method is used - Logs critical failure when no headers available - Helps diagnose why tenant detection is failing in production This will help identify if: 1. Headers are not being passed through FastMCP 2. Headers are being stripped by nginx/proxy 3. Fallback methods are working correctly Related to production issue where test-agent.adcontextprotocol.org returns 'No tenant context set' error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The generated schemas were updated and now use BrandManifest10 instead of BrandManifest8, but schema_helpers.py wasn't updated to match. Changes: - Update import from BrandManifest8 to BrandManifest10 - Update all references in function signatures - Update __all__ export list Fixes import error preventing MCP server startup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Oct 24, 2025
bokelley
added a commit
that referenced
this pull request
Oct 24, 2025
* Fix schema generator to use union operator for consistency The schema generation script was missing the --use-union-operator flag, causing it to regenerate all files with Optional[X] instead of X | None syntax whenever it ran. Changes: - Add --use-union-operator flag to datamodel-codegen command - Ensures generated schemas use modern Python 3.10+ syntax (X | None) - Prevents 70+ file changes every time schema generation runs - Fixes line wrapping inconsistencies in 14 schema files - Remove broken check-generated-schemas hook (script never existed) Root cause: PR #572 checked in schemas with X | None syntax, but the generator script still produced Optional[X] syntax, creating constant drift between committed files and regenerated files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Apply formatting consistency to schemas from PR #573 merge --------- Co-authored-by: Claude <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Oct 24, 2025
When list_authorized_properties is called via A2A without an auth token, it was failing with "No tenant context set" error because no ToolContext was being created to pass headers for tenant detection. Problem: - A2A handler only created ToolContext when auth_token present - Without ToolContext, headers (Apx-Incoming-Host, etc.) not passed through - get_principal_from_context() couldn't detect tenant from headers - get_current_tenant() raised error Solution: - Always create ToolContext for list_authorized_properties - Pass headers even when auth_token is None - Allows tenant detection via Apx-Incoming-Host, Host, or x-adcp-tenant - ToolContext has principal_id=None for unauthenticated calls Impact: - Fixes test-agent.adcontextprotocol.org A2A calls - Public discovery endpoints now work without authentication - Tenant properly detected from virtual host headers Testing: - Verified tenant config exists: tenant_id=default, virtual_host=test-agent.adcontextprotocol.org - Debug script shows configuration is correct - Issue was in A2A header passing, not tenant setup Related: - Similar to MCP fix in PR #573 (tenant detection from headers) - test-agent calls now match wonderstruck behavior
bokelley
added a commit
that referenced
this pull request
Oct 24, 2025
When list_authorized_properties is called via A2A without an auth token, it was failing with "No tenant context set" error because no ToolContext was being created to pass headers for tenant detection. Problem: - A2A handler only created ToolContext when auth_token present - Without ToolContext, headers (Apx-Incoming-Host, etc.) not passed through - get_principal_from_context() couldn't detect tenant from headers - get_current_tenant() raised error Solution: - Always create ToolContext for list_authorized_properties - Pass headers even when auth_token is None - Allows tenant detection via Apx-Incoming-Host, Host, or x-adcp-tenant - ToolContext has principal_id=None for unauthenticated calls Impact: - Fixes test-agent.adcontextprotocol.org A2A calls - Public discovery endpoints now work without authentication - Tenant properly detected from virtual host headers Testing: - Verified tenant config exists: tenant_id=default, virtual_host=test-agent.adcontextprotocol.org - Debug script shows configuration is correct - Issue was in A2A header passing, not tenant setup Related: - Similar to MCP fix in PR #573 (tenant detection from headers) - test-agent calls now match wonderstruck behavior
bokelley
added a commit
that referenced
this pull request
Oct 24, 2025
When list_authorized_properties is called via A2A without an auth token, it was failing with "No tenant context set" error because no ToolContext was being created to pass headers for tenant detection. Problem: - A2A handler only created ToolContext when auth_token present - Without ToolContext, headers (Apx-Incoming-Host, etc.) not passed through - get_principal_from_context() couldn't detect tenant from headers - get_current_tenant() raised error Solution: - Always create ToolContext for list_authorized_properties - Pass headers even when auth_token is None - Allows tenant detection via Apx-Incoming-Host, Host, or x-adcp-tenant - ToolContext has principal_id=None for unauthenticated calls Impact: - Fixes test-agent.adcontextprotocol.org A2A calls - Public discovery endpoints now work without authentication - Tenant properly detected from virtual host headers Testing: - Verified tenant config exists: tenant_id=default, virtual_host=test-agent.adcontextprotocol.org - Debug script shows configuration is correct - Issue was in A2A header passing, not tenant setup Related: - Similar to MCP fix in PR #573 (tenant detection from headers) - test-agent calls now match wonderstruck behavior
EmmaLouise2018
pushed a commit
that referenced
this pull request
Oct 24, 2025
* Fix A2A authentication: Forward Authorization header through nginx 🐛 Root Cause: Nginx was stripping the Authorization header before proxying A2A requests to the backend server, causing all authenticated requests to fail with "Missing authentication token - Bearer token required in Authorization header" 🔧 Fix: Added `proxy_set_header Authorization $http_authorization;` to all /a2a location blocks in both config/nginx/nginx.conf and fly/nginx.conf 📍 Affected endpoints: - *.adcontextprotocol.org/a2a (agent routing) - *.sales-agent.scope3.com/a2a (tenant routing) - sales-agent.scope3.com/a2a (base domain) - External domains/a2a (default server) ✅ Impact: Buyers can now successfully authenticate to sales agent A2A endpoints by sending `Authorization: Bearer <token>` header 🔗 Related: - Tenant detection already working (from 2025-10-23 fix) - Python middleware correctly extracts Bearer token when present - Issue was in nginx proxy layer, not application code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Support both Authorization and x-adcp-auth headers for MCP and A2A 🎯 Goal: Make authentication flexible across both protocols Previously: - MCP: Only accepted x-adcp-auth header - A2A: Only accepted Authorization: Bearer header - Inconsistent and fragile Now: - Both MCP and A2A accept BOTH headers - Prefer protocol-standard header (x-adcp-auth for MCP, Authorization for A2A) - Fallback to alternate header if primary not present 🔧 Changes: Application Code: - src/core/auth_utils.py: MCP now accepts both x-adcp-auth (preferred) and Authorization - src/a2a_server/adcp_a2a_server.py: A2A now accepts both Authorization (preferred) and x-adcp-auth Nginx Configuration: - config/nginx/nginx.conf: All /mcp and /a2a locations forward BOTH headers - fly/nginx.conf: All /mcp and /a2a locations forward BOTH headers ✅ Benefits: 1. Cross-protocol compatibility (MCP clients can use A2A endpoints and vice versa) 2. More resilient to header variations 3. Easier client implementation (use either header convention) 4. Forward compatible with future auth changes 📍 Preference Order: - MCP: x-adcp-auth → Authorization - A2A: Authorization → x-adcp-auth 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add debug logging for header extraction in tenant detection Adds detailed logging to diagnose tenant detection failures: - Logs number of headers returned by get_http_headers() - Logs all header keys when headers are available - Logs which fallback method is used - Logs critical failure when no headers available - Helps diagnose why tenant detection is failing in production This will help identify if: 1. Headers are not being passed through FastMCP 2. Headers are being stripped by nginx/proxy 3. Fallback methods are working correctly Related to production issue where test-agent.adcontextprotocol.org returns 'No tenant context set' error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix BrandManifest8 → BrandManifest10 import in schema_helpers The generated schemas were updated and now use BrandManifest10 instead of BrandManifest8, but schema_helpers.py wasn't updated to match. Changes: - Update import from BrandManifest8 to BrandManifest10 - Update all references in function signatures - Update __all__ export list Fixes import error preventing MCP server startup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
EmmaLouise2018
pushed a commit
that referenced
this pull request
Oct 24, 2025
* Fix schema generator to use union operator for consistency The schema generation script was missing the --use-union-operator flag, causing it to regenerate all files with Optional[X] instead of X | None syntax whenever it ran. Changes: - Add --use-union-operator flag to datamodel-codegen command - Ensures generated schemas use modern Python 3.10+ syntax (X | None) - Prevents 70+ file changes every time schema generation runs - Fixes line wrapping inconsistencies in 14 schema files - Remove broken check-generated-schemas hook (script never existed) Root cause: PR #572 checked in schemas with X | None syntax, but the generator script still produced Optional[X] syntax, creating constant drift between committed files and regenerated files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Apply formatting consistency to schemas from PR #573 merge --------- Co-authored-by: Claude <noreply@anthropic.com>
EmmaLouise2018
pushed a commit
that referenced
this pull request
Oct 24, 2025
When list_authorized_properties is called via A2A without an auth token, it was failing with "No tenant context set" error because no ToolContext was being created to pass headers for tenant detection. Problem: - A2A handler only created ToolContext when auth_token present - Without ToolContext, headers (Apx-Incoming-Host, etc.) not passed through - get_principal_from_context() couldn't detect tenant from headers - get_current_tenant() raised error Solution: - Always create ToolContext for list_authorized_properties - Pass headers even when auth_token is None - Allows tenant detection via Apx-Incoming-Host, Host, or x-adcp-tenant - ToolContext has principal_id=None for unauthenticated calls Impact: - Fixes test-agent.adcontextprotocol.org A2A calls - Public discovery endpoints now work without authentication - Tenant properly detected from virtual host headers Testing: - Verified tenant config exists: tenant_id=default, virtual_host=test-agent.adcontextprotocol.org - Debug script shows configuration is correct - Issue was in A2A header passing, not tenant setup Related: - Similar to MCP fix in PR #573 (tenant detection from headers) - test-agent calls now match wonderstruck behavior
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
* Fix A2A authentication: Forward Authorization header through nginx 🐛 Root Cause: Nginx was stripping the Authorization header before proxying A2A requests to the backend server, causing all authenticated requests to fail with "Missing authentication token - Bearer token required in Authorization header" 🔧 Fix: Added `proxy_set_header Authorization $http_authorization;` to all /a2a location blocks in both config/nginx/nginx.conf and fly/nginx.conf 📍 Affected endpoints: - *.adcontextprotocol.org/a2a (agent routing) - *.sales-agent.scope3.com/a2a (tenant routing) - sales-agent.scope3.com/a2a (base domain) - External domains/a2a (default server) ✅ Impact: Buyers can now successfully authenticate to sales agent A2A endpoints by sending `Authorization: Bearer <token>` header 🔗 Related: - Tenant detection already working (from 2025-10-23 fix) - Python middleware correctly extracts Bearer token when present - Issue was in nginx proxy layer, not application code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Support both Authorization and x-adcp-auth headers for MCP and A2A 🎯 Goal: Make authentication flexible across both protocols Previously: - MCP: Only accepted x-adcp-auth header - A2A: Only accepted Authorization: Bearer header - Inconsistent and fragile Now: - Both MCP and A2A accept BOTH headers - Prefer protocol-standard header (x-adcp-auth for MCP, Authorization for A2A) - Fallback to alternate header if primary not present 🔧 Changes: Application Code: - src/core/auth_utils.py: MCP now accepts both x-adcp-auth (preferred) and Authorization - src/a2a_server/adcp_a2a_server.py: A2A now accepts both Authorization (preferred) and x-adcp-auth Nginx Configuration: - config/nginx/nginx.conf: All /mcp and /a2a locations forward BOTH headers - fly/nginx.conf: All /mcp and /a2a locations forward BOTH headers ✅ Benefits: 1. Cross-protocol compatibility (MCP clients can use A2A endpoints and vice versa) 2. More resilient to header variations 3. Easier client implementation (use either header convention) 4. Forward compatible with future auth changes 📍 Preference Order: - MCP: x-adcp-auth → Authorization - A2A: Authorization → x-adcp-auth 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add debug logging for header extraction in tenant detection Adds detailed logging to diagnose tenant detection failures: - Logs number of headers returned by get_http_headers() - Logs all header keys when headers are available - Logs which fallback method is used - Logs critical failure when no headers available - Helps diagnose why tenant detection is failing in production This will help identify if: 1. Headers are not being passed through FastMCP 2. Headers are being stripped by nginx/proxy 3. Fallback methods are working correctly Related to production issue where test-agent.adcontextprotocol.org returns 'No tenant context set' error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix BrandManifest8 → BrandManifest10 import in schema_helpers The generated schemas were updated and now use BrandManifest10 instead of BrandManifest8, but schema_helpers.py wasn't updated to match. Changes: - Update import from BrandManifest8 to BrandManifest10 - Update all references in function signatures - Update __all__ export list Fixes import error preventing MCP server startup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
* Fix schema generator to use union operator for consistency The schema generation script was missing the --use-union-operator flag, causing it to regenerate all files with Optional[X] instead of X | None syntax whenever it ran. Changes: - Add --use-union-operator flag to datamodel-codegen command - Ensures generated schemas use modern Python 3.10+ syntax (X | None) - Prevents 70+ file changes every time schema generation runs - Fixes line wrapping inconsistencies in 14 schema files - Remove broken check-generated-schemas hook (script never existed) Root cause: PR prebid#572 checked in schemas with X | None syntax, but the generator script still produced Optional[X] syntax, creating constant drift between committed files and regenerated files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Apply formatting consistency to schemas from PR prebid#573 merge --------- Co-authored-by: Claude <noreply@anthropic.com>
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
…bid#577) When list_authorized_properties is called via A2A without an auth token, it was failing with "No tenant context set" error because no ToolContext was being created to pass headers for tenant detection. Problem: - A2A handler only created ToolContext when auth_token present - Without ToolContext, headers (Apx-Incoming-Host, etc.) not passed through - get_principal_from_context() couldn't detect tenant from headers - get_current_tenant() raised error Solution: - Always create ToolContext for list_authorized_properties - Pass headers even when auth_token is None - Allows tenant detection via Apx-Incoming-Host, Host, or x-adcp-tenant - ToolContext has principal_id=None for unauthenticated calls Impact: - Fixes test-agent.adcontextprotocol.org A2A calls - Public discovery endpoints now work without authentication - Tenant properly detected from virtual host headers Testing: - Verified tenant config exists: tenant_id=default, virtual_host=test-agent.adcontextprotocol.org - Debug script shows configuration is correct - Issue was in A2A header passing, not tenant setup Related: - Similar to MCP fix in PR prebid#573 (tenant detection from headers) - test-agent calls now match wonderstruck behavior
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes multiple authentication and tenant detection issues discovered in production.
Issues Fixed
1. ✅ Nginx Stripping Authorization Header (A2A)
Problem: Nginx was not forwarding the
Authorizationheader to A2A endpoints, causing all authenticated A2A requests to fail with "Missing authentication token" error.Root Cause: The nginx configuration used
proxy_set_headerdirectives but didn't explicitly forward theAuthorizationheader. When nginx usesproxy_set_header, it stops auto-forwarding headers, so ALL needed headers must be explicitly listed.Fix: Added
proxy_set_header Authorization $http_authorization;to all 6/a2alocation blocks in:config/nginx/nginx.conf(4 locations)fly/nginx.conf(2 locations)Impact: A2A authentication now works correctly with
Authorization: Bearer <token>header.2. ✅ Inconsistent Auth Header Support
Problem: MCP only accepted
x-adcp-authheader, A2A only acceptedAuthorization: Bearerheader - inconsistent and fragile.Fix: Both protocols now accept BOTH headers with smart preference order:
x-adcp-auth(AdCP convention) → falls back toAuthorizationAuthorization(HTTP standard) → falls back tox-adcp-authChanges:
src/core/auth_utils.py: MCP'sget_principal_from_context()accepts both headerssrc/a2a_server/adcp_a2a_server.py: A2A middleware accepts both headersBenefits:
3. 🔍 Debug Logging for Tenant Detection
Problem: Production MCP requests to
test-agent.adcontextprotocol.orgfailing with "No tenant context set" error.Investigation: Added comprehensive debug logging to diagnose:
Status: Requires production testing to diagnose root cause.
Changes:
src/core/main.py- detailed logging inget_principal_from_context()Testing
Deployment Notes
IMPORTANT: After merge, deploy to production and test with:
Check logs for DEBUG output showing:
Related
docs/testing/postmortems/2025-10-25-nginx-authorization-header-bug.md🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com