You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/03/30 23:45:13 UTC

[GitHub] [incubator-superset] tooptoop4 opened a new pull request #9422: Fix user impersonation with presto

tooptoop4 opened a new pull request #9422: Fix user impersonation with presto
URL: https://github.com/apache/incubator-superset/pull/9422
 
 
   ### CATEGORY
   
   Choose one
   
   - [x] Bug Fix
   - [ ] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   Fixes https://github.com/apache/incubator-superset/issues/9406
   PyHive 0.6.2 release includes https://github.com/dropbox/PyHive/pull/309 making this change possible.
   In presto, credentials are authenticated against a 'principal', that principal can then impersonate a 'user' so queries can be executed with authorisation rules applied to the user (not the principal).
   Benefit: a single shared presto data source can be created in superset with a generic account (superuser aka the principal) in the presto connection. Then each user that logs into superset ui does not need to create new datasource, they can go straight to SQLLab and run presto queries on the shared datasource which are being authorised under their username.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth commented on pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
DRavikanth commented on pull request #9422:
URL: https://github.com/apache/superset/pull/9422#issuecomment-800705920


   @tooptoop4 curious to know why this PR was ever merged to superset branch?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] john-bodley commented on a change in pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9422: Fix user impersonation with presto
URL: https://github.com/apache/incubator-superset/pull/9422#discussion_r400652220
 
 

 ##########
 File path: superset/models/core.py
 ##########
 @@ -313,6 +314,9 @@ def get_sqla_engine(
         if connect_args:
             params["connect_args"] = connect_args
 
+        if user_name and self.impersonate_user and str(sqlalchemy_url).startswith('presto'):
 
 Review comment:
   See ^^. You should update `get_configuration_for_impersonation` to handle this. Note given that this isn't part of the `connection`  key the  `get_configuration_for_impersonation` needs be refactored to be `get_connection_args_for_impersonation`.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] tooptoop4 commented on issue #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
tooptoop4 commented on issue #9422: Fix user impersonation with presto
URL: https://github.com/apache/incubator-superset/pull/9422#issuecomment-606449620
 
 
   @john-bodley current behavior before this pr seems to modify username in url so it expects all users to have same password as the generic acct !?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] john-bodley commented on issue #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #9422: Fix user impersonation with presto
URL: https://github.com/apache/incubator-superset/pull/9422#issuecomment-606858588
 
 
   @tooptoop4 you could do something similar to https://github.com/apache/incubator-superset/pull/9405 to achieve what you require which also updates `connect_args`. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] john-bodley commented on a change in pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9422: Fix user impersonation with presto
URL: https://github.com/apache/incubator-superset/pull/9422#discussion_r400650359
 
 

 ##########
 File path: superset/models/core.py
 ##########
 @@ -288,9 +288,10 @@ def get_sqla_engine(
         # If using MySQL or Presto for example, will set url.username
         # If using Hive, will not do anything yet since that relies on a
         # configuration parameter instead.
-        self.db_engine_spec.modify_url_for_impersonation(
-            sqlalchemy_url, self.impersonate_user, effective_username
-        )
+        if not user_name or not str(sqlalchemy_url).startswith('presto'):
 
 Review comment:
   There shouldn't be any database specific logic in `core.py` hence the reason for the `self.db_engine_spec.modify_url_for_impersonation(...)` method. You should do something like [this](https://github.com/apache/incubator-superset/blob/9f5f8e5d92f4d346acbedfbd2a1105f7194fea78/superset/db_engine_specs/hive.py#L396).

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] etr2460 commented on issue #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #9422: Fix user impersonation with presto
URL: https://github.com/apache/incubator-superset/pull/9422#issuecomment-606326687
 
 
   @john-bodley this might be interesting to you

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] tooptoop4 commented on pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
tooptoop4 commented on pull request #9422:
URL: https://github.com/apache/superset/pull/9422#issuecomment-801030000


   @DRavikanth I'm not a committer


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] tooptoop4 commented on pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
tooptoop4 commented on pull request #9422:
URL: https://github.com/apache/incubator-superset/pull/9422#issuecomment-636396492


   crispy


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] stale[bot] closed pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #9422:
URL: https://github.com/apache/incubator-superset/pull/9422


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] stale[bot] commented on pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #9422:
URL: https://github.com/apache/incubator-superset/pull/9422#issuecomment-636385428


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [incubator-superset] stale[bot] commented on pull request #9422: Fix user impersonation with presto

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #9422:
URL: https://github.com/apache/incubator-superset/pull/9422#issuecomment-665990852


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org