feat(auth): add bound token support for access and JWT id tokens for Cloud Run#17698
feat(auth): add bound token support for access and JWT id tokens for Cloud Run#17698nbayati wants to merge 1 commit into
Conversation
Switch the MDS token acquisition from a GET to a POST request when the agentic cert is detected. * Add `get_agent_identity_certificate_and_bytes()` utility to read the raw certificate bytes alongside the parsed cert. * Update `_metadata.get_service_account_token()` (for access tokens) and `IDTokenCredentials.refresh()` (for ID tokens) to send a POST request with the `certificate_chain` payload instead of a GET request when bound tokens are supported. * Update `_metadata.get()` helper to support `method` and `body` params. * Add and update unit tests to verify the new POST request flows.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Google Auth library to request bound tokens from the Compute Engine metadata server using a POST request with the certificate chain in the body, rather than passing a fingerprint in the URL. To support this, get_agent_identity_certificate_and_bytes was introduced to retrieve both the parsed certificate and its raw bytes, and the metadata get helper was updated to support POST requests and bodies. Feedback on the changes suggests simplifying a redundant tuple check in credentials.py by directly unpacking the returned value from get_agent_identity_certificate_and_bytes.
| cert_and_bytes = ( | ||
| _agent_identity_utils.get_agent_identity_certificate_and_bytes() | ||
| ) | ||
| if cert_and_bytes: | ||
| cert, cert_bytes = cert_and_bytes | ||
| if cert and _agent_identity_utils.should_request_bound_token(cert): | ||
| method = "POST" | ||
| body = json.dumps( | ||
| {"certificate_chain": cert_bytes.decode("utf-8")} | ||
| ).encode("utf-8") | ||
| metrics_header["Content-Type"] = "application/json" |
There was a problem hiding this comment.
Since get_agent_identity_certificate_and_bytes() always returns a 2-tuple (either (cert, cert_bytes) or (None, None)), it is never None or empty. Therefore, the check if cert_and_bytes: is redundant. We can simplify this by directly unpacking the tuple and checking if cert:, which is also consistent with the implementation in _metadata.py.
cert, cert_bytes = (
_agent_identity_utils.get_agent_identity_certificate_and_bytes()
)
if cert and _agent_identity_utils.should_request_bound_token(cert):
method = "POST"
body = json.dumps(
{"certificate_chain": cert_bytes.decode("utf-8")}
).encode("utf-8")
metrics_header["Content-Type"] = "application/json"
| headers (Optional[Mapping[str, str]]): Headers for the request. | ||
| return_none_for_not_found_error (Optional[bool]): If True, returns None | ||
| for 404 error instead of throwing an exception. | ||
| timeout (int): How long to wait, in seconds for the metadata server to respond. |
There was a problem hiding this comment.
Please also update the doc string
method (str): The HTTP method to use for the request. Defaults to "GET".
body (Optional[bytes]): The HTTP request body payload to send. Defaults to None.
| method = "GET" | ||
| body = None | ||
|
|
||
| cert, cert_bytes = _agent_identity_utils.get_agent_identity_certificate_and_bytes() |
There was a problem hiding this comment.
From AI code review,
Could we implement the fallback to search the well-known credential path (
/var/run/secrets/workload-spiffe-credentials/) when theGOOGLE_API_CERTIFICATE_CONFIGenvironment variable is not set?
Currently, without this fallback, the feature will only function on platforms that explicitly inject that environment variable (like Cloud Run), but it will not work out-of-the-box on GKE or GCE environments where the variable is typically unset. Supporting both the config file and the well-known SPIFFE path fallback is essential for cross-platform compatibility.
Otherwise, please update the docstring to highlight the limitation on GKE or GCE environments when the GOOGLE_API_CERTIFICATE_CONFIG variable is not set
| # look up the certificate. | ||
| is_opted_out = ( | ||
| os.environ.get( | ||
| environment_vars.GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES, |
There was a problem hiding this comment.
From AI code review
To align with the cross-SDK standardization for certificate-bound tokens, we should support both the standard environment variable
GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES(without the_AGENTinfix) as well as the legacyGOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICESflag.
Could we update this check to look for both env vars so we maintain backward compatibility while adopting the unified standard?
| cert_and_bytes = ( | ||
| _agent_identity_utils.get_agent_identity_certificate_and_bytes() | ||
| ) | ||
| if cert_and_bytes: | ||
| cert, cert_bytes = cert_and_bytes | ||
| if cert and _agent_identity_utils.should_request_bound_token(cert): | ||
| method = "POST" | ||
| body = json.dumps( | ||
| {"certificate_chain": cert_bytes.decode("utf-8")} | ||
| ).encode("utf-8") | ||
| metrics_header["Content-Type"] = "application/json" |
| method = "GET" | ||
| body = None |
There was a problem hiding this comment.
nit: add comment
| method = "GET" | |
| body = None | |
| # Default to standard GET. We conditionally upgrade to POST (bound token) | |
| # if an agent identity certificate is present and active. | |
| method = "GET" | |
| body = None |
| method = "GET" | ||
| body = None |
There was a problem hiding this comment.
nit: add comment
| method = "GET" | |
| body = None | |
| # Default to standard GET. We conditionally upgrade to POST (bound token) | |
| # if an agent identity certificate is present and active. | |
| method = "GET" | |
| body = None |
3 participants
This PR adds the following:
Switch the MDS token acquisition from a GET to a POST request when the agentic cert is detected.
Add
get_agent_identity_certificate_and_bytes()utility to read the raw certificate bytes alongside the parsed cert.Update
_metadata.get_service_account_token()(for access tokens) andIDTokenCredentials.refresh()(for ID tokens) to send a POST request with thecertificate_chainpayload instead of a GET request when bound tokens are supported.Update
_metadata.get()helper to supportmethodandbodyparams.Add and update unit tests to verify the new POST request flows.
design: go/sdk-mds-bound-token
id token verification:
Note:
This PR relies on the existing pattern of locating the certificates using the path provided by the config file available at GOOGLE_API_CERTIFICATE_CONFIG. It does not currently fallback on checking the well known location if the env var is not set, which would limit the scope to CR, as GKE and GCE don't set this env var.
It uses the same condition to decide if a bound token should be requested for both access token and id token. We might decide to add a separate env var to opt out.
it still uses the existing
GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICESflag to opt out. We might update it and add a new env var, but will keep the old one for backward compatibility.