fix(mcp): Preserve existing auth in mTLS transport#6201
Conversation
HTTP header names are case-insensitive, but the mTLS auth bridge only checked for an exact Authorization key before adding ADC credentials. httpx can pass the existing header as lowercase authorization, causing the user token to be missed. Fixes google#6200
564cbe6 to
c637b5f
Compare
|
Hi @h-tsuboi918 , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Can you please fix the failing ci test before we can proceed with a review. |
|
Hi @rohityan ,Thanks for checking this. I looked into the failed CI job: https://github.com/google/adk-python/actions/runs/28122333977/job/83277473901?pr=6201 The failing step is The relevant workflow logic is here:
CHANGED_FILES=$(git diff --diff-filter=ACMR --name-only origin/${GITHUB_BASE_REF}...HEAD | grep -E '\.py$' || true)
FILES_WITH_ENDPOINTS=$(grep -lE 'https?://[a-zA-Z0-9.-]+\.googleapis\.com' $CHANGED_FILES)
FILES_MISSING_MTLS=$(grep -L '.mtls.googleapis.com' $FILES_WITH_ENDPOINTS)The job log reports this file: This PR does touch that file, but the matched
scopes = ['https://www.googleapis.com/auth/cloud-platform']That value is an OAuth scope, not a service endpoint that can or should have a corresponding If we want to fix this, I think the right place is the CI check rather than adding a dummy For example, before treating a changed file as a file containing a FILES_WITH_ENDPOINTS=$(
grep -HEo 'https?://[a-zA-Z0-9.-]+\.googleapis\.com[^"'\''[:space:]]*' $CHANGED_FILES \
| grep -vE 'https://www\.googleapis\.com/auth(/|$)' \
| cut -d: -f1 \
| sort -u || true
)Then the existing The intended behavior would be:
The actual change in this PR is limited to preserving an existing
For that reason, I do not think adding a dummy |
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
N/A
Problem:
The MCP mTLS auth bridge checked only for an exact
Authorizationheader key before adding ADC credentials.httpx.Request.headerscan be converted into a plain dict with lowercaseauthorization, so the existing user / 3LO Authorization header could be missed.When that happens, ADK may inject ADC / runtime Service Account credentials into the same MCP request, causing the MCP server to observe the runtime principal instead of the intended user principal.
Solution:
Make
_RefreshableAsyncCredentials.before_request()check for an existing Authorization header case-insensitively before refreshing and injecting credentials.This matches HTTP header semantics and preserves existing behavior for correctly cased
Authorizationheaders while also handling lowercaseauthorizationfrom thehttpxrequest path.Testing Plan
Unit Tests:
Passed locally:
Additional checks:
Manual End-to-End (E2E) Tests:
Not run. This is covered by a focused unit test for the credential injection guard. The changed behavior is isolated to preserving an existing lowercase
authorizationheader before ADC refresh/injection.Checklist
Additional context
The issue is specific to the MCP mTLS transport path where ADK bridges
httpxrequests intogoogle-auth-aioAsyncAuthorizedSession. The existing Authorization header may originate from MCP auth credential handling or a header provider, including user / 3LO token propagation.No new code comments were added because the code change is limited to the existing Authorization header guard. There are no dependent downstream changes.