VladaZakharova commented on code in PR #72954:
URL: https://github.com/apache/airflow/pull/72954#discussion_r4156785305
##########
providers/google/src/airflow/providers/google/common/hooks/base_google.py:
##########
@@ -360,7 +360,11 @@ def get_credentials_and_project_id(self) ->
tuple[Credentials, str | None]:
idp_issuer_url: str | None = self._get_field("idp_issuer_url", None)
client_id: str | None = self._get_field("client_id", None)
client_secret: str | None = self._get_field("client_secret", None)
- idp_extra_params: str | None = self._get_field("idp_extra_params",
None)
+ # The connection form saves this field as ``idp_extra_parameters``;
``idp_extra_params`` is
+ # still accepted for extras that were written by hand against the
previous field name.
+ idp_extra_params: str | None = self._get_field("idp_extra_parameters",
None) or self._get_field(
Review Comment:
Are any of these extra parameter fields considered sensitive? If users pass
client secrets or assertion tokens here, we should ensure they are masked in
task execution logs and UI connection views
##########
providers/google/src/airflow/providers/google/cloud/utils/credentials_provider.py:
##########
@@ -399,7 +399,10 @@ def
_get_credentials_using_credential_config_file_and_token_supplier(self):
info =
_get_info_from_credential_configuration_file(self.credential_config_file)
info["subject_token_supplier"] =
ClientCredentialsGrantFlowTokenSupplier(
- oidc_issuer_url=self.idp_issuer_url, client_id=self.client_id,
client_secret=self.client_secret
+ oidc_issuer_url=self.idp_issuer_url,
+ client_id=self.client_id,
+ client_secret=self.client_secret,
+ **(self.idp_extra_params_dict or {}),
Review Comment:
Could we ensure we sanitize or guard against extra_params overriding
protected OAuth keys (e.g., grant_type, audience, subject_token_type)? A
collision check here would prevent accidental or unexpected payload overrides
##########
providers/google/src/airflow/providers/google/cloud/utils/credentials_provider.py:
##########
@@ -399,7 +399,10 @@ def
_get_credentials_using_credential_config_file_and_token_supplier(self):
info =
_get_info_from_credential_configuration_file(self.credential_config_file)
info["subject_token_supplier"] =
ClientCredentialsGrantFlowTokenSupplier(
- oidc_issuer_url=self.idp_issuer_url, client_id=self.client_id,
client_secret=self.client_secret
+ oidc_issuer_url=self.idp_issuer_url,
Review Comment:
If a user has implemented a custom SubjectTokenSupplier that defines
get_subject_token(self, context) without **kwargs, calling it with the new
arguments will raise a TypeError. Could we check the signature or inspect the
callable to ensure backward compatibility for existing custom implementations?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
