bito-code-review[bot] commented on code in PR #44709:
URL: https://github.com/apache/superset/pull/44709#discussion_r4129940859
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -255,7 +255,19 @@ def impersonate_user(
url = url.update_query_dict({"subject": user.email})
if user_token:
- url = url.update_query_dict({"access_token": user_token})
+ # Pass the token through ``connect_args`` rather than the URL.
+ # ``update_params_from_encrypted_extra`` stores the catalog (and
any
+ # service account) in ``connect_args["adapter_kwargs"]``, and
SQLAlchemy
+ # merges ``connect_args`` over the dialect's own arguments
shallowly,
+ # so a token in the URL would be dropped and the query would run
+ # without credentials. For the same reason a ``subject`` set on
the URL
+ # above is carried over, so it isn't dropped either.
+ connect_args = engine_kwargs.setdefault("connect_args", {})
+ adapter_kwargs = connect_args.setdefault("adapter_kwargs", {})
+ gsheetsapi_kwargs = adapter_kwargs.setdefault("gsheetsapi", {})
+ gsheetsapi_kwargs["access_token"] = user_token
+ if subject := url.query.get("subject"):
+ gsheetsapi_kwargs.setdefault("subject", subject)
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to carry over the `subject` into `adapter_kwargs`
on the tokenless service-account path is a valid observation for ensuring
consistent impersonation behavior. However, as noted in the thread, this
behavior was already addressed in a previous commit (f6083492f8), which ensures
that tokenless service-account connections correctly carry the `subject` when
domain-wide delegation is enabled, supported by existing unit tests.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
