-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(auth): add bound token support for access and JWT id tokens for Cloud Run #17698
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,6 +253,8 @@ def get( | |
| headers=None, | ||
| return_none_for_not_found_error=False, | ||
| timeout=_METADATA_DEFAULT_TIMEOUT, | ||
| method="GET", | ||
| body=None, | ||
| ): | ||
| """Fetch a resource from the metadata server. | ||
|
|
||
|
|
@@ -317,9 +319,15 @@ def get( | |
| last_exception = None | ||
| for attempt in backoff: | ||
| try: | ||
| response = request( | ||
| url=url, method="GET", headers=headers_to_use, timeout=timeout | ||
| ) | ||
| kwargs = { | ||
| "url": url, | ||
| "method": method, | ||
| "headers": headers_to_use, | ||
| "timeout": timeout, | ||
| } | ||
| if body is not None: | ||
| kwargs["body"] = body | ||
| response = request(**kwargs) | ||
| if response.status in transport.DEFAULT_RETRYABLE_STATUS_CODES: | ||
| _LOGGER.warning( | ||
| "Compute Engine Metadata server unavailable on " | ||
|
|
@@ -489,18 +497,27 @@ def get_service_account_token(request, service_account="default", scopes=None): | |
| scopes = ",".join(scopes) | ||
| params["scopes"] = scopes | ||
|
|
||
| cert = _agent_identity_utils.get_and_parse_agent_identity_certificate() | ||
| method = "GET" | ||
| body = None | ||
|
|
||
| cert, cert_bytes = _agent_identity_utils.get_agent_identity_certificate_and_bytes() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From AI code review,
Otherwise, please update the docstring to highlight the limitation on GKE or GCE environments when the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GCE and GKE Metadata servers are not ready in prod and their launches are coming later. If we add the fallback now, it's going to break GKE and GCE. |
||
| if cert: | ||
| if _agent_identity_utils.should_request_bound_token(cert): | ||
| fingerprint = _agent_identity_utils.calculate_certificate_fingerprint(cert) | ||
| params["bindCertificateFingerprint"] = fingerprint | ||
| method = "POST" | ||
| body = json.dumps({"certificate_chain": cert_bytes.decode("utf-8")}).encode( | ||
| "utf-8" | ||
| ) | ||
|
|
||
| metrics_header = { | ||
| metrics.API_CLIENT_HEADER: metrics.token_request_access_token_mds() | ||
| } | ||
| if method == "POST": | ||
| metrics_header["Content-Type"] = "application/json" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This seems to make a semantics change here - this is no longer just a "metrics" header |
||
|
|
||
| path = "instance/service-accounts/{0}/token".format(service_account) | ||
| token_json = get(request, path, params=params, headers=metrics_header) | ||
| token_json = get( | ||
| request, path, params=params, headers=metrics_header, method=method, body=body | ||
| ) | ||
| token_expiry = _helpers.utcnow() + datetime.timedelta( | ||
| seconds=token_json["expires_in"] | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |||||||||||||
| """ | ||||||||||||||
|
|
||||||||||||||
| import datetime | ||||||||||||||
| import json | ||||||||||||||
| import logging | ||||||||||||||
| from typing import Optional, TYPE_CHECKING | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -526,13 +527,36 @@ def _call_metadata_identity_endpoint(self, request): | |||||||||||||
| ValueError: If extracting expiry from the obtained ID token fails. | ||||||||||||||
| """ | ||||||||||||||
| try: | ||||||||||||||
| from google.auth import _agent_identity_utils | ||||||||||||||
|
|
||||||||||||||
| path = "instance/service-accounts/default/identity" | ||||||||||||||
| params = {"audience": self._target_audience, "format": "full"} | ||||||||||||||
| metrics_header = { | ||||||||||||||
| metrics.API_CLIENT_HEADER: metrics.token_request_id_token_mds() | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| method = "GET" | ||||||||||||||
| body = None | ||||||||||||||
|
Comment on lines
+538
to
+539
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add comment
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| 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" | ||||||||||||||
|
Comment on lines
+541
to
+551
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since 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"
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please take a look |
||||||||||||||
|
|
||||||||||||||
| id_token = _metadata.get( | ||||||||||||||
| request, path, params=params, headers=metrics_header | ||||||||||||||
| request, | ||||||||||||||
| path, | ||||||||||||||
| params=params, | ||||||||||||||
| headers=metrics_header, | ||||||||||||||
| method=method, | ||||||||||||||
| body=body, | ||||||||||||||
| ) | ||||||||||||||
| except exceptions.TransportError as caught_exc: | ||||||||||||||
| new_exc = exceptions.RefreshError(caught_exc) | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: add comment