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
feat(trino): add support for user impersonation #14843
feat(trino): add support for user impersonation #14843
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14843 +/- ##
==========================================
- Coverage 77.62% 77.51% -0.11%
==========================================
Files 963 963
Lines 49237 49315 +78
Branches 6192 6226 +34
==========================================
+ Hits 38218 38225 +7
- Misses 10819 10889 +70
- Partials 200 201 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Thanks, @dungdm93 Looks like this is moved here : superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx . Have updated the labels here. |
superset/db_engine_specs/trino.py
Outdated
url = make_url(uri) | ||
backend_name = url.get_backend_name() | ||
|
||
# Must be Presto connection, enable impersonation, and set optional param |
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.
We want to do this only if "enable impersonation" is true, no? In that case it would be better to add it to modify_url_for_impersonation
.
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.
this is called only if impersonation is enabled
// from core.py
if self.impersonate_user:
self.db_engine_spec.update_impersonation_config(
connect_args, str(sqlalchemy_url), effective_username
)
modify_url will work only if authentication is disabled - we will have to modify connection args to pass logged In user_name based on the underlying connector - for presto, its principal_username
and for trino its user
.
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.
Ah, good point, I missed that.
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.
This is awesome, excited to see Trino being more supported in Superset.
Co-authored-by: Đặng Minh Dũng <dungdm93@live.com>
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.
LGTM, thanks for the great improvement!
* trino impersonation feature * Extra options label update * Update superset/db_engine_specs/trino.py Co-authored-by: Đặng Minh Dũng <dungdm93@live.com> Co-authored-by: rijojoseph01 <rijo.joseph@myntra.com> Co-authored-by: Đặng Minh Dũng <dungdm93@live.com>
* trino impersonation feature * Extra options label update * Update superset/db_engine_specs/trino.py Co-authored-by: Đặng Minh Dũng <dungdm93@live.com> Co-authored-by: rijojoseph01 <rijo.joseph@myntra.com> Co-authored-by: Đặng Minh Dũng <dungdm93@live.com>
* trino impersonation feature * Extra options label update * Update superset/db_engine_specs/trino.py Co-authored-by: Đặng Minh Dũng <dungdm93@live.com> Co-authored-by: rijojoseph01 <rijo.joseph@myntra.com> Co-authored-by: Đặng Minh Dũng <dungdm93@live.com>
SUMMARY
When using authentication with trino, the current behavior is just to modify the URL to replace the username which will result in an unauthorized exception. This PR will fix this by updating the connection argument with the effective user.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Step 1: Setup database connection to trino with credentials in URL.
Step 2: Enable impersonation from
Other
optionStep 3 : Log in with a different user and run the query via SQL lab and you will see the principal user as admin and user as the logged-in user in trino UI.
ADDITIONAL INFORMATION