aminghadersohi commented on code in PR #44709:
URL: https://github.com/apache/superset/pull/44709#discussion_r4129939778
##########
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:
Verified at head f566396d4a: `impersonate_user` carries the subject into
`connect_args["adapter_kwargs"]["gsheetsapi"]` on the tokenless service-account
path when `domain_wide_delegation` is enabled, which was fixed in f6083492f8.
That path is covered by
`test_impersonate_user_service_account_with_delegation`,
`..._without_delegation` and `..._delegation_needs_an_email`.
--
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]
