bito-code-review[bot] commented on code in PR #44709:
URL: https://github.com/apache/superset/pull/44709#discussion_r4113902228
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>Subject dropped on tokenless path</b></div>
<div id="fix">
The carry-over at lines 269-270 only runs when `user_token` is set. On the
service-account path (`user_token=None`, `encrypted_extra` providing
`service_account_info`/`catalog`), `update_params_from_encrypted_extra` still
populates `connect_args["adapter_kwargs"]`, and the same shallow merge
described in the comment drops the URL `subject` - impersonation silently never
applies. Consider carrying `subject` whenever `connect_args` will be populated.
</div>
</div>
<small><i>Code Review Run #62eb45</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
