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/07/23 09:23:20 UTC

[GitHub] [incubator-superset] amitNielsen opened a new issue #10408: Proposal for Dashboard Access Permissions

amitNielsen opened a new issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408


   
   
   ## [SIP] Proposal for Dashboard Access Permissions
   
   ### Motivation
   
   As a dashboard provider in an organisation with many subgroups inside I need the ability manage user access to dashboards and different levels of permissions(Viewer,Editor)
   
   ### Proposed Change
   
   Create permissions for each dashboard with the format "dashboard (viewer/editor) access on dashboard_title [dashboard_id]"
   The name of the permission is updated if the dashboard title is changed.
   Add a dashboard permission to a role via the Edit Role form, or, for convenience, add a role to a dashboard via the Edit Dashboard form.
   The permissions "all_dashboard_viewer_access" and "all_dashboard_editor_access" is added to Admin, Alpha, and Gamma roles.
   To allow private dashboards, remove "all_dashboard_access" from a role. This role will then only have access to the dashboards specified. Add a dashboard's permission to a role to "publish" it.
   Note that these permissions do not apply to slices. To make the data in a dashboard truly private, use dashboard permissions with a read only role that has no slice/explore access.
   
   ####**FAB permission view mechanism**
   Use **FAB permission view** mechanism to have new permissions named **dashboard read/edit access**
   and for every dashboard there would be a matching **view_menu** instance that can be used a permission_view and be assigned into Roles:
   
   [![](https://mermaid.ink/img/eyJjb2RlIjoiZ3JhcGggVERcblx0QVtkYXNoYm9hcmQgaW5zdGFuY2VdIC0tPnxoYXMgYSBtYXRjaGluZ3wgQihhYl92aWV3X21lbnUpXG5cdEIgLS0-IHxmb3JtIHRvZ3RoZXJ8IEN7cGVybWlzc2lvbl92aWV3fVxuICAgIERBW2Rhc2hib2FyZCByZWFkIGFjY2Vzc10tLT58aW5zdGFuY2Ugb2Z8IGFiX3Blcm1pc3Npb25cblx0REVBW2Rhc2hib2FyZCBlZGl0IGFjY2Vzc10tLT58aW5zdGFuY2Ugb2Z8IGFiX3Blcm1pc3Npb25cbiAgICBhYl9wZXJtaXNzaW9uIC0tPiB8Zm9ybSB0b2d0aGVyfCBDe2FiX3Blcm1pc3Npb25fdmlld31cblx0QyAtLT58dXNlZCBpbnwgRFthYl9wZXJtaXNzaW9uX3ZpZXdfcm9sZV0iLCJtZXJtYWlkIjp7InRoZW1lIjoiZGVmYXVsdCJ9fQ)](https://mermaid-js.github.io/mermaid-live-editor/#/edit/eyJjb2RlIjoiZ3JhcGggVERcblx0QVtkYXNoYm9hcmQgaW5zdGFuY2VdIC0tPnxoYXMgYSBtYXRjaGluZ3wgQihhYl92aWV3X21lbnUpXG5cdEIgLS0-IHxmb3JtIHRvZ3RoZXJ8IEN7cGVybWlzc2lvbl92aWV3fVxuICAgIERBW2Rhc2hib2FyZCByZWFkIGFjY2Vzc10tLT58aW5zdGFuY2Ugb2Z8IGFiX3Blcm1pc3Npb25cblx0REVBW2Rhc2hib2FyZCBlZGl0IGFjY2Vzc10tLT58aW5zdGFuY2Ugb2Z8IGFiX3Blcm1pc3Npb25cbiAgICBhYl9wZXJtaXNzaW9uIC0tPiB8Zm9ybSB0b2d0aGVyfCBDe2FiX3Blcm1pc3Npb25fdmlld
 31cblx0QyAtLT58dXNlZCBpbnwgRFthYl9wZXJtaXNzaW9uX3ZpZXdfcm9sZV0iLCJtZXJtYWlkIjp7InRoZW1lIjoiZGVmYXVsdCJ9fQ)
   
   Users will be validated with the necessary permissions when fetching dashboards(specific or list)  and will be rejected if they don't have them
   
   #### dashboard edit access (UI & BE)
   ![image info](https://lh3.googleusercontent.com/TkiC5vgRSO5HLymvvet0KOhq6zc59nUXc6tGUNqQcgpbVaYyRhGtScUJ5ihqQBeYWQ3MKlvhqAySRyf7HggqBBbLN7u44rRwqsXhawSPhO4hBW93EFoZjoVUhvE-kSi4fMLp5DgYicKX8u7YR8mLGguk5cxUcK58W4SxMYJm3YdpB69TNscOkuzfvScYk2p9bnCwbTlVX-zoSzj1b5aAH3afXXAUTB_SCXRMCRw-qGrAgb6zDfJD4io4_7S1Ch278aHOF5yKJQPYGrewwbAn19cUDytWlr6_HiG_lxhGY41jLqUcpeg5lZVhAipyi_4HG5oU_edOJpJL8UrDiUHMZ4_kc2Nz58wuP-mX_ZVgWT52jHftzmXacvcI7YFtWM6v-IJwyde0s2zg01WaFuXp_ftUICm6vzuPMbt3hti-gEw0-UtEeudVzjxuVLEOQJJvHLRI8j6jMMFTJCNGuunI42azaJGMm7dxXJDT7Sy6L6UAq0NRrLNgJPdQIPLCi5AUfQYSrDsMrdkM4eAtF_nRMV-Wg_jWXK16pXH8ZQNAOBMBUzB-AhUUseoRC2K3PrayyEPyts3SgWlZUMXGlENlsfCFTj9ySkU2ZboM7t1kv8OOm8hqXCdGYNQ17cz2-iru-X5sfdPJ3yqbTO_NgGA0G-4sobwlfdrvLiGF9PybCIevL8knfVM_w9jlz7zHeQ=w210-h66-no?authuser=0)
   
   *edit dashboard button* will be hidden if user doesn't have edit access for the requested dashboard
   
   save dashboard endpoints will also validate for permissions
   
   
   ### New or Changed Public Interfaces
    #### the following endpoints logic will need to validate dashboard viewer/editor access
   
       BaseSupersetView <-- Dashboard endpoints:
           GET dashboard/new/  -  create new empty dashboard
       SupersetModelView <-- DashboardModelView endpoints:
           GET /dashboard/list,  get all dashboards
       BaseSupersetView <-- Superset endpoints:
           POST dashboad/save_dash/{} ,
           POST GET /dashboard/{}/published
           GET /dashboard/{} - get specific dashboard
       BaseSupersetModelRestApi <-- DashboardResetApi endpoints
           POST,PUT,DELETE,GET /api/v1/dashboard/
   
   ### New dependencies
   
   none
   
   ### Migration Plan and Compatibility
   
   create relevant permissions for all existing dashboards
   assign "all_dashboard_viewer_access" and "all_dashboard_editor_access" to Admin, Alpha, and Gamma roles
   
   ### Rejected Alternatives
   
   none
   


----------------------------------------------------------------
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] issue-label-bot[bot] commented on issue #10408: Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
issue-label-bot[bot] commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-662906502


   Issue-Label Bot is automatically applying the label `#enhancement` to this issue, with a confidence of 0.93. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback! 
   
    Links: [app homepage](https://github.com/marketplace/issue-label-bot), [dashboard](https://mlbot.net/data/apache/incubator-superset) and [code](https://github.com/hamelsmu/MLapp) for this bot.


----------------------------------------------------------------
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] mistercrunch commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-688630012


   Something to clarify here: what does happen when the user has access to the dashboard, but doesn't have access to the underlying datasets?
   
   If we assume that there's a need for people getting access to a dashboard, but not being able to explore the dataset(s), because say, there's some level of detail in the underlying dataset that the granter does not want to expose for some reason. We need to clarify the following topics:
   - when giving access to a dashboard, we need to make it clear to the granter that there are two options here: dashboard only and dashboard+explore on the datasets
   - for users who have access to dashboard but not the dataset, is the "explore this chart" button grayed out? invisible?


----------------------------------------------------------------
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] ktmud commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749337302


   @amitmiran137 If users can switch this flag at the dashboard level, it means at least part of the system will contain dataset-based access control. Will dashboards with `LEVEL_ACCESS_MODE= 'Dashboard'` enabled still subject to any dataset level access control configured? If yes, what's the benefit of having a flag? Can we just: 1). always apply the dataset level control per chart; 2) apply the dashboard access control when a dashboard-level access group is configured?


----------------------------------------------------------------
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] always-busy commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
always-busy commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-744395108


   good SIP


----------------------------------------------------------------
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] amitmiran137 edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749339421


   dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   The problem with option 2 :  you want to give viewing access and not ownership for managing the dashboard 


----------------------------------------------------------------
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] altef commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
altef commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-740772147


   This looks awesome!  (Though I am admittedly biased; dashboard level permissions are pretty high on my Superset wishlist.)


----------------------------------------------------------------
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] amitmiran137 commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-740433428


   Calling for responses for the final summary of this sip
   anyone?
   


----------------------------------------------------------------
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] amitmiran137 commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-744637262


   > Personally, I feel that all data access should be based on the role-based access control of the underlying Data Warehouse.
   > In many companies, Superset will not be the sole tool to access the data. So, if data access were to be controlled on a BI tool level, there would be a severe overhead of implementation the same data governance rules for each of the tools and access methods.
   > I'd greatly prefer to focus on getting der User Impersonation (a.k.a. User Principal Push Down) feature robustly working for the supported databases.
   > This way we would not require Dashboard Level Access Control at all: the users can access the charts depending on whether they are allowed to access the underlying tables/views.
   > All other solutions may rip holes into the company's data governance model in my eyes.
   > However, I am aware, that features like Caching of the chart's query results get much more complex, especially when the Data Warehouse supports column- or row-based access control.
   
   I totally agree with the use case described above btw.
   but there are use-cases like internal organization data access that does require any governance but just access on the end delivery to the client which are the dashboards themselves 
   
   this is we it should be up to the organization to decide with what mechanisem they want to use:
   either `dataset_level` or `dashboard_level`


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697830709


   Hey @john-bodley just noticed this PR merged #11024 
   it actually fully covers the dashboard_edit_access permission concept so I think I'm going to change the proposal to focus only on access in the general concept of it
   
   


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749393014


   > dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   This will be a major turn off for any organization who needs dataset level access control. 
   
   My 1) and 2) were not two options but two steps of one solution. Basically we keep current dataset level access control unchanged but add a layer of dashboard access control which blocks users from even viewing the dashboard.
   
   In terms of actual implementation, you could still leverage the existing RBAC by adding an `roles` attribute to dashboards and a custom `can_access_dashboard`/`can_edit_dashboard` method to [`SecurityManager`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L108):
   
   1. Add a new column `roles` to the `Dashboard` model, which stores FAB roles that corresponds to a business unit/user group's dashboard view or edit role. We don't allow specifying view access by users as it unnecessarily complicates things.
   2. When publishing a dashboard, users choose which roles/user groups have access to this dashboard
   3. Add `dashboard_access` to [`OBJECT_SPEC_PERMISSIONS`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L175)
   4. Add `can_access_dashboard` and `can_edit_dashboard` to `SecurityManager` which passes the right permission names and view names to `can_access` based on roles associated with the dashboard. E.g.
       ```python
       if not dashboard.roles:
           return True
       for role in dashboard.roles:
           if self.can_access("dashboard_access", dashboard.perm_for_role(role, edit=False)):
               return True
       return False
       ```
   4. Place these checks manually in the API, just like what we do [for datasources](https://github.com/airbnb/incubator-superset/blob/ce1abc98df307b770d68603c8652d0dfe4a6deae/superset%2Fviews%2Fcore.py#L263).
   
   In short, I don't think an "access mode" switch is necessary, as current security model seems to already suffice in supporting the additional layer of role-baed dashboard-level access control.
   
   There could be a toggle to allow pulling query results from controlled datasource even if users don't have direct access to datasource---but that has its a whole other level of complexity and doesn't seem to be blocking us from implementing the basic dashboard-level access control.
   
   This is obviously a popular user demand and I'm all for addressing it, but let's make sure the final solution is as prudent as possible.


----------------------------------------------------------------
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] ktmud commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-741012382


   > > I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   > > I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii). Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok, the access patterns are likely more secure (people could exploit dashboard level access controls), and doesn't require additional logic or development of request/approval/management flow.
   > 
   > there is a IV) option which is co-existing (i) or (ii) , depending whether the feature flag is off or on
   
   I'm a strong believer that feature flags should only be used for experimental features, not something we will support in the long term. If we find value in one way or the other, we should just make one of them the default and clean up the other.
   
   In this case, I don't see why we can't add the dashboard-level access on top of the dataset access control. 
   
   
   
   
   


----------------------------------------------------------------
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] yishaifri commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
yishaifri commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-666120185


   yes!! cool idea


----------------------------------------------------------------
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 issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697151432


   Currently only owners + admins are able to edit dashboards. I'm interested in knowing how the current view access and ownership controls are suffice in order to provide the `viewer` and `editor` type roles.


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-662910820


   @villebro the SIP we discussed


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749393014


   > dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   This will be a major turn off for any organization who needs dataset level access control. 
   
   My 1) and 2) were not two options but two steps of one solution. Basically we keep current dataset level access control unchanged but add a new layer of dashboard access control.
   
   In terms of actual implementation, you could still leverage the existing RBAC by adding an `roles` attribute to dashboards and a custom `can_access_dashboard`/`can_edit_dashboard` method to [`SecurityManager`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L108):
   
   1. Add a new column `roles` to the `Dashboard` model, which stores FAB roles that corresponds to a business unit/user group's dashboard view or edit role. We don't allow specifying view access by users as it unnecessarily complicates things.
   2. When publishing a dashboard, users choose which roles/user groups have access to this dashboard
   3. Add `dashboard_access` to [`OBJECT_SPEC_PERMISSIONS`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L175)
   4. Add `can_access_dashboard` and `can_edit_dashboard` to `SecurityManager` which passes the right permission names and view names to `can_access` based on roles associated with the dashboard. E.g.
       ```python
       if not dashboard.roles:
           return True
       for role in dashboard.roles:
           if self.can_access("dashboard_access", dashboard.perm_for_role(role, edit=False)):
               return True
       return False
       ```
   4. Place these checks manually in the API, just like what we do [for datasources](https://github.com/airbnb/incubator-superset/blob/ce1abc98df307b770d68603c8652d0dfe4a6deae/superset%2Fviews%2Fcore.py#L263).
   
   In short, I don't think an "access mode" switch is necessary, as current security model seems to already suffice in supporting the additional layer of role-based dashboard-level access control and the only extra work is adding a new `roles` column (like we already have in [`datasource.perm`](https://github.com/airbnb/incubator-superset/blob/7ae8cd07cc2942ae99fcf8ae95aba9ffd65682b9/superset%2Fconnectors%2Fbase%2Fmodels.py#L107) and `datasource.schema_perm`, except we add `roles` instead of `perm` to have the ability to enforce foreign key check.)
   
   There could be a toggle to allow pulling query results from controlled datasource even if users don't have direct access to datasource---but that has its own level of complexity and doesn't seem to be blocking us from implementing the basic dashboard-level access control.
   
   This is obviously a popular user demand and I'm all for addressing it, but let's make sure the final solution is as prudent as possible.


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-688648112


   @mistercrunch 
   
   * when giving access to a dashboard, so dashboard itself is accessible but then each one of the chart are doing additional calls to get the underlying data from the **datasource**, so those calls will also need to be validated that although there is no direct access to the **datasource** the access to that dashboard is allowing to pull data for the need of viewing it inside that specific dashboard
   
   Giving access to a specific dashboard can be sufficient enough for the user to view the dashboard without the complexicity of adding all underlying datasources 👍 🚀 
   
   * for users who have access to dashboard but not the dataset, the explore chart should be invisible to simplify **dashboard viewers** to my opinion same as edit dashboard should be invisible


----------------------------------------------------------------
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] amitNielsen edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-673435376


   #10594
   work started


----------------------------------------------------------------
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] bkyryliuk commented on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697546551


   > Currently only owners + admins are able to edit dashboards. I'm interested in knowing whether the current view access and ownership controls are suffice in order to provide the "viewer" and "editor" functions?
   > 
   > This seems like a far simpler approach as is consistent with the slice model and requires no code change.
   
   agree with @john-bodley  proposal, it looks like current permission model supports this use case.


----------------------------------------------------------------
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] itsik-avidan commented on issue #10408: [SIP] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
itsik-avidan commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-662910684


   👍 
   


----------------------------------------------------------------
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] altef commented on issue #10408: [SIP] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
altef commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-663725882


   This would be wonderful for my use-case as well


----------------------------------------------------------------
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] villebro commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
villebro commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-742440029


   > > > I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   > > > I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii). Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok, the access patterns are likely more secure (people could exploit dashboard level access controls), and doesn't require additional logic or development of request/approval/management flow.
   > > 
   > > 
   > > there is a IV) option which is co-existing (i) or (ii) , depending whether the feature flag is off or on
   > 
   > I'm a strong believer that feature flags should only be used for experimental features, not something we will support in the long term. If we find value in one way or the other, we should just make one of them the default and clean up the other.
   > 
   > In this case, I don't see why we can't add the dashboard-level access on top of the dataset access control.
   
   The way I see it this will be implemented as a config flag, so one can choose to use datasource and/or dashboard access control, defaulting to the current datasource option.


----------------------------------------------------------------
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] hyramduke commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
hyramduke commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-672064868


   +1


----------------------------------------------------------------
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] amitmiran137 commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749224632


   This Flag can be introduced at the global level for a default behaviour and at the  dashboard level for overriding the default
   
   @ktmud 


----------------------------------------------------------------
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 issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697673946


   I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   
   I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii).  Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok and doesn't require additional logic or development of request/approval/management flow.


----------------------------------------------------------------
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] amitmiran137 commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-748535237


   If anybody still haven't voted 
   https://lists.apache.org/x/thread.html/r91c0acde742a9a719a30938d708e945890ade944ea476dc1def61975%40%3Cdev.superset.apache.org%3E


----------------------------------------------------------------
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 edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697673946


   I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   
   I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii).  Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok, the access patterns are likely more secure (people could exploit dashboard level access controls), and doesn't require additional logic or development of request/approval/management flow.


----------------------------------------------------------------
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 edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697151432


   Currently only owners + admins are able to edit dashboards. I'm interested in knowing how the current view access and ownership controls are suffice in order to provide the "viewer" and "editor" type roles.


----------------------------------------------------------------
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] amitmiran137 commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749339421


   dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   


----------------------------------------------------------------
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] amitmiran137 edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749339421


   @ktmud 
   dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   The problem with option 2 :  you want to give viewing access and not ownership for managing the dashboard 
   
   How would you suggest to  manage list of users have view access to a dashboard ?
   Isn't a list of users equals to a role ?
   Why create a new access model instead of leveraging the RBAC existing one 


----------------------------------------------------------------
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] rumbin commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
rumbin commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-741536660


   Personally, I feel that all data access should be based on the role-based access control of the underlying Data Warehouse.
   In many companies, Superset will not be the sole tool to access the data. So, if data access were to be controlled on a BI tool level, there would be a severe overhead of implementation the same data governance rules for each of the tools and access methods.
   I'd greatly prefer to focus on getting der User Impersonation (a.k.a. User Principal Push Down) feature robustly working for the supported databases.
   This way we would not require Dashboard Level Access Control at all: the users can access the charts depending on whether they are allowed to access the underlying tables/views.
   All other solutions may rip holes into the company's data governance model in my eyes.
   However, I am aware, that features like Caching of the chart's query results get much more complex, especially when the Data Warehouse supports column- or row-based access control.


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749393014


   > dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   This will be a major turn off for any organization who needs dataset level access control. 
   
   My 1) and 2) were not two options but two steps of one solution. Basically we keep current dataset level access control unchanged but add a layer of dashboard access control which blocks users from even viewing the dashboard.
   
   In terms of actual implementation, you could still leverage the existing RBAC by adding an `roles` attribute to dashboards and a custom `can_access_dashboard`/`can_edit_dashboard` method to [`SecurityManager`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L108):
   
   1. Add a new column `roles` to the `Dashboard` model, which stores FAB roles that corresponds to a business unit/user group's dashboard view or edit role. We don't allow specifying view access by users as it unnecessarily complicates things.
   2. When publishing a dashboard, users choose which roles/user groups have access to this dashboard
   3. Add `dashboard_access` to [`OBJECT_SPEC_PERMISSIONS`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L175)
   4. Add `can_access_dashboard` and `can_edit_dashboard` to `SecurityManager` which passes the right permission names and view names to `can_access` based on roles associated with the dashboard. E.g.
       ```python
       if not dashboard.roles:
           return True
       for role in dashboard.roles:
           if self.can_access("dashboard_access", dashboard.perm_for_role(role, edit=False)):
               return True
       return False
       ```
   4. Place these checks manually in the API, just like what we do [for datasources](https://github.com/airbnb/incubator-superset/blob/ce1abc98df307b770d68603c8652d0dfe4a6deae/superset%2Fviews%2Fcore.py#L263).
   
   In short, I don't think an "access mode" switch is necessary, as current security model seems to already suffice in supporting the additional layer of role-baed dashboard-level access control.
   
   This is a real user demand and I'm all for addressing it, but just want to make sure we don't introduce unnecessary complexity here.


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-673435376


   #10594 https://github.com/apache/incubator-superset/pull/10594
   work started


----------------------------------------------------------------
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] shashankkoppar commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
shashankkoppar commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-735742781


   I would also like this! It would be handy to share dashboard level access! Since most of engineers working on this are non tech, so they wont understand if which dataset or database or stuff .
   
   Current dashboard permissions is heavily dependent on dataset permissions.
   In our scenario, we have dashboards which we want to share to different clients which use same dataset, then it is difficult, since they both can see all dashboards created by this dataset. 
   Would like to know when it is possible to have dashboard level access!  
   


----------------------------------------------------------------
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] amitNielsen edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-675565953


   #5483


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749337302






----------------------------------------------------------------
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] amitmiran137 edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-744637262


   > Personally, I feel that all data access should be based on the role-based access control of the underlying Data Warehouse.
   > In many companies, Superset will not be the sole tool to access the data. So, if data access were to be controlled on a BI tool level, there would be a severe overhead of implementation the same data governance rules for each of the tools and access methods.
   > I'd greatly prefer to focus on getting der User Impersonation (a.k.a. User Principal Push Down) feature robustly working for the supported databases.
   > This way we would not require Dashboard Level Access Control at all: the users can access the charts depending on whether they are allowed to access the underlying tables/views.
   > All other solutions may rip holes into the company's data governance model in my eyes.
   > However, I am aware, that features like Caching of the chart's query results get much more complex, especially when the Data Warehouse supports column- or row-based access control.
   
   I totally agree with the use case described above btw.
   but there are use-cases like internal organization data access that does require any governance but just access on the end delivery to the client which are the dashboards themselves 
   
   this is we it should be up to the organization to decide with what mechanism they want to use:
   either `dataset_level` or `dashboard_level`


----------------------------------------------------------------
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] ktmud commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749393014


   > dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   This will be a major turn off for any organization who needs dataset level access control. 
   
   My 1) and 2) were not two options but two steps of one solution. Basically we keep current dataset level access control unchanged but add a layer of dashboard access control which blocks users from even viewing the dashboard.
   
   In terms of actual implementation, you could still leverage the existing RBAC by adding an `roles` attribute to dashboards and a custom `can_access_dashboard`/`can_edit_dashboard` method to [`SecurityManager`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L108):
   
   1. Add a new column `roles` to the `Dashboard` model, which stores FAB roles that corresponds to a business unit/user group's dashboard view or edit role. We don't allow specifying view access by users as it unnecessarily complicates things.
   2. When publishing a dashboard, users choose which roles/user groups have access to this dashboard
   3. Add `can_access_dashboard` and `can_edit_dashboard` to `SecurityManager` which passes the right permission names and view names to `can_access` based on roles associated with the dashboard. E.g.
       ```python
       if not dashboard.roles:
           return True
       for role in dashboard.roles:
           if self.can_access("can_view", f."{role.name}.dashboard"):
               return True
       return False
       ```
   4. Place these checks manually in the API, just like what we do [for datasources](https://github.com/airbnb/incubator-superset/blob/ce1abc98df307b770d68603c8652d0dfe4a6deae/superset%2Fviews%2Fcore.py#L263).
   
   In short, I don't think an "access mode" switch is necessary, as current security model seems to suffice in adding the additional layer of dashboard-level of access control, all we need to do is adding two extra columns.
   
   There is also no need to manually create a new role whenever a dashboard is published as all new dashboards will be accessible to all by default.
   
   This is a real user demand and I'm all for addressing it, but let's find the most sensible solution with long-term maintainability in mind!


----------------------------------------------------------------
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] willbarrett commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
willbarrett commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-668210106


   To comply with the current usage of Admin/Alpha/Gamma, I'd recommend only adding `all_dashboard_editor_access` to Admin and Alpha - Gamma is currently a much more limited user that requires explicit access to data.


----------------------------------------------------------------
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] amitNielsen edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
amitNielsen edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697836762


   > I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   > 
   > I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii). Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok, the access patterns are likely more secure (people could exploit dashboard level access controls), and doesn't require additional logic or development of request/approval/management flow.
   
   there is a IV) option which is co-existing (i) or (ii) , depending whether the feature flag is off or on


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-675565953


   #5438


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749337302


   @amitmiran137 If users can switch this flag at the dashboard level, it means at least part of the system will contain dataset-based access control. Will dashboards with `LEVEL_ACCESS_MODE= 'Dashboard'` enabled still subject to any dataset level access control configured? If yes, what's the benefit of having a flag? Can we just: 1). always apply the dataset level control to the charts; 2) apply the dashboard access control when a dashboard-level access group is configured?


----------------------------------------------------------------
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] amitNielsen edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
amitNielsen edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697836762


   > I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   > 
   > I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii). Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok, the access patterns are likely more secure (people could exploit dashboard level access controls), and doesn't require additional logic or development of request/approval/management flow.
   
   there is a IV) option which is co-existing (i) or (ii) , depending whether the feature flag is off or on


----------------------------------------------------------------
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] villebro commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
villebro commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-740534018


   To echo @amitmiran137 's request, it would be important for the community to be vocal about the updated proposal in the SIP description. Without comments either for or against the proposal, it's very difficult to assess whether or not the current proposal is in line with how the community expects dashboard access control to work. So if this feature is something you are interested in seeing implemented, please read through the description of the SIP and post your thoughts here.


----------------------------------------------------------------
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] bkyryliuk commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-668228105


   Would it make sense to consider automating creation of the datasource permissions while granting access to the dashboard. E.g. user will request the access to the tables that back the dashboard & owner will be able to grant / approve those with 1 click ? 
   
   Creating dashboard permissions to the user opens up some edge-cases as there will be 2 ways to validate the access, access to dashboard that can change underlying sql & datasource access [ table, schema, database level]:
   1. owner changes the dashboard and grants access to the undesired 
   2. what will be the explore from dashboard behavior for the read only users and how it will interact with the dashboard permissions ?
   


----------------------------------------------------------------
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 edited a comment on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697151432


   Currently only owners + admins are able to edit dashboards. I'm interested in knowing whether the current view access and ownership controls are suffice in order to provide the "viewer" and "editor" functions?
   
   This seems like a far simpler approach as is consistent with the slice model and requires no code change.


----------------------------------------------------------------
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] amitNielsen removed a comment on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen removed a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-673435376


   #10594
   work started


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP-51] Proposal for Dashboard Level Access

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-697836762


   > I partially misspoke earlier, currently there are no access controls explicitly at the dashboard level, it's merely a series of [rules](https://github.com/apache/incubator-superset/blob/e4ffaecc72afb706a31d66d626e3c15c94b3a995/superset/views/dashboard/filters.py#L33-L36).
   > 
   > I do think the community needs to collectively decide whether security should be at i) the datasource level (either Superset datasource or the underlying database, schema, etc.), the ii) chart/dashboard level, or iii) a combination of both (i) and (ii). Currently it's (i) (for right or wrong) and aspects of dashboard level access could be achieved by row level access and/or dashboard specific Superset datasources. There is additional overhead with this approach, however it's simpler to grok, the access patterns are likely more secure (people could exploit dashboard level access controls), and doesn't require additional logic or development of request/approval/management flow.
   
   there is a V) option which is co-existing (i) or (ii) , depending whether the feature flag is off or on


----------------------------------------------------------------
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] amitNielsen commented on issue #10408: [SIP-51] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
amitNielsen commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-671860828


   Hey @bkyryliuk 
   We plan to automate creation of the datasource permissions while granting access to the dashboard externally to superset by create a matching role for each created dashboard and assigning all the relevant permissions to it(Dashboard, datasources)
   
   I think that explore from dashboard should be available for only **dashboard edit access** permission and owners


----------------------------------------------------------------
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] ktmud commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-748831643


   > The way I see it this will be implemented as a config flag, so one can choose to use datasource and/or dashboard access control, defaulting to the current datasource option.
   
   Where is this config flag set? Globally by Superset admins or at dashboard level? Can companies decide to use both modes at the same time? I.e., certain group of users can access some dashboards, but not all data in that dashboard since the data query may still go through a custom SecurityManager for access control.
   


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749393014


   > dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   This will be a major turn off for any organization who needs dataset level access control. 
   
   My 1) and 2) were not two options but two steps of one solution. Basically we keep current dataset level access control unchanged but add a new layer of dashboard access control.
   
   In terms of actual implementation, you could still leverage the existing RBAC by adding an `roles` attribute to dashboards and a custom `can_access_dashboard`/`can_edit_dashboard` method to [`SecurityManager`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L108):
   
   1. Add a new column `roles` to the `Dashboard` model, which stores FAB roles that corresponds to a business unit/user group's dashboard view or edit role. We don't allow specifying view access by users as it unnecessarily complicates things.
   2. When publishing a dashboard, users choose which roles/user groups have access to this dashboard
   3. Add `dashboard_access` to [`OBJECT_SPEC_PERMISSIONS`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L175)
   4. Add `can_access_dashboard` and `can_edit_dashboard` to `SecurityManager` which passes the right permission names and view names to `can_access` based on roles associated with the dashboard. E.g.
       ```python
       if not dashboard.roles:
           return True
       for role in dashboard.roles:
           if self.can_access("dashboard_access", dashboard.perm_for_role(role, edit=False)):
               return True
       return False
       ```
   4. Place these checks manually in the API, just like what we do [for datasources](https://github.com/airbnb/incubator-superset/blob/ce1abc98df307b770d68603c8652d0dfe4a6deae/superset%2Fviews%2Fcore.py#L263).
   
   In short, I don't think an "access mode" switch is necessary, as current security model seems to already suffice in supporting the additional layer of role-based dashboard-level access control and the only extra work is adding a new `roles` column (like we already have in [`datasource.perm`](https://github.com/airbnb/incubator-superset/blob/7ae8cd07cc2942ae99fcf8ae95aba9ffd65682b9/superset%2Fconnectors%2Fbase%2Fmodels.py#L107) and `datasource.schema_perm`, except we add `roles` instead of `perm` to have the ability to enforce foreign key check.)
   
   There could be a toggle to allow pulling query results from controlled datasource even if users don't have direct access to datasource---but that has its a whole other level of complexity and doesn't seem to be blocking us from implementing the basic dashboard-level access control.
   
   This is obviously a popular user demand and I'm all for addressing it, but let's make sure the final solution is as prudent as possible.


----------------------------------------------------------------
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] ktmud edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749393014


   > dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   This will be a major turn off for any organization who needs dataset level access control. 
   
   My 1) and 2) were not two options but two steps of one solution. Basically we keep current dataset level access control unchanged but add a new layer of dashboard access control.
   
   In terms of actual implementation, you could still leverage the existing RBAC by adding an `roles` attribute to dashboards and a custom `can_access_dashboard`/`can_edit_dashboard` method to [`SecurityManager`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L108):
   
   1. Add a new column `roles` to the `Dashboard` model, which stores FAB roles that corresponds to a business unit/user group's dashboard view or edit role. We don't allow specifying view access by users as it unnecessarily complicates things.
   2. When publishing a dashboard, users choose which roles/user groups have access to this dashboard
   3. Add `dashboard_access` to [`OBJECT_SPEC_PERMISSIONS`](https://github.com/airbnb/incubator-superset/blob/2f0add3aec8f168fb8c37c648f0e29e507f39294/superset%2Fsecurity%2Fmanager.py#L175)
   4. Add `can_access_dashboard` and `can_edit_dashboard` to `SecurityManager` which passes the right permission names and view names to `can_access` based on roles associated with the dashboard. E.g.
       ```python
       if not dashboard.roles:
           return True
       for role in dashboard.roles:
           if self.can_access("dashboard_access", dashboard.perm_for_role(role, edit=False)):
               return True
       return False
       ```
   4. Place these checks manually in the API, just like what we do [for datasources](https://github.com/airbnb/incubator-superset/blob/ce1abc98df307b770d68603c8652d0dfe4a6deae/superset%2Fviews%2Fcore.py#L263).
   
   In short, I don't think an "access mode" switch is necessary, as current security model seems to already suffice in supporting the additional layer of role-baed dashboard-level access control.
   
   There could be a toggle to allow pulling query results from controlled datasource even if users don't have direct access to datasource---but that has its a whole other level of complexity and doesn't seem to be blocking us from implementing the basic dashboard-level access control.
   
   This is obviously a popular user demand and I'm all for addressing it, but let's make sure the final solution is as prudent as possible.


----------------------------------------------------------------
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] SaharExelate commented on issue #10408: [SIP] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
SaharExelate commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-664279433


   Cool!


----------------------------------------------------------------
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] amitmiran137 edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-744637262


   > Personally, I feel that all data access should be based on the role-based access control of the underlying Data Warehouse.
   > In many companies, Superset will not be the sole tool to access the data. So, if data access were to be controlled on a BI tool level, there would be a severe overhead of implementation the same data governance rules for each of the tools and access methods.
   > I'd greatly prefer to focus on getting der User Impersonation (a.k.a. User Principal Push Down) feature robustly working for the supported databases.
   > This way we would not require Dashboard Level Access Control at all: the users can access the charts depending on whether they are allowed to access the underlying tables/views.
   > All other solutions may rip holes into the company's data governance model in my eyes.
   > However, I am aware, that features like Caching of the chart's query results get much more complex, especially when the Data Warehouse supports column- or row-based access control.
   
   I totally agree with the use case described above btw.
   but there are use-cases like internal organization data access that doesn't require any governance but just access on the end delivery to the client which are the dashboards themselves 
   
   this is we it should be up to the organization to decide with what mechanism they want to use:
   either `dataset_level` or `dashboard_level`


----------------------------------------------------------------
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] amitmiran137 edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-749339421


   @ktmud 
   dashboards with LEVEL_ACCESS_MODE= 'Dashboard' enabled will not be subject to any dataset level access.
   
   The problem with option 2 :  you want to give viewing access and not ownership for managing the dashboard 


----------------------------------------------------------------
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] JKHeadley commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
JKHeadley commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-746849472


   This is exactly what my company needs. A simple way to share dashboards with clients without giving them access to the underlying datasource.


----------------------------------------------------------------
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] amitmiran137 edited a comment on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
amitmiran137 edited a comment on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-744637262


   > Personally, I feel that all data access should be based on the role-based access control of the underlying Data Warehouse.
   > In many companies, Superset will not be the sole tool to access the data. So, if data access were to be controlled on a BI tool level, there would be a severe overhead of implementation the same data governance rules for each of the tools and access methods.
   > I'd greatly prefer to focus on getting der User Impersonation (a.k.a. User Principal Push Down) feature robustly working for the supported databases.
   > This way we would not require Dashboard Level Access Control at all: the users can access the charts depending on whether they are allowed to access the underlying tables/views.
   > All other solutions may rip holes into the company's data governance model in my eyes.
   > However, I am aware, that features like Caching of the chart's query results get much more complex, especially when the Data Warehouse supports column- or row-based access control.
   
   I totally agree with the use case described above btw.
   but there are use-cases like internal organization data access that doesn't require any governance but just access on the end delivery to the client which are the dashboards themselves 
   
   this is why we it should be up to the organization to decide with what mechanism they want to use:
   either `dataset_level` or `dashboard_level`


----------------------------------------------------------------
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] villebro commented on issue #10408: [SIP-51] Dashboard/Dataset Level Access

Posted by GitBox <gi...@apache.org>.
villebro commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-725919143


   Thanks for updating the SIP @amitNielsen ! I think the current state of the proposal captures the motivations and required changes well.
   
   If we can keep this behind a feature flag and not cause backwards incompatible changes, then I'm all for the change. By introducing the option to allow access to dashboards/charts with a signed token provided by the backend, we should be able to do this securely, without compromising leakage. I believe it should be possible to keep the necessary code changes mostly within the security manager, avoiding unnecessary additional code complexity.
   
   I envision a flow that would look something like this:
   1) User requests dashboard. If user is not permitted to access the dashboard, a 403 is returned. If user does have access, a signed token based on the user id and dashboard are returned with the bootstrap data.
   2) the dashboard requests data for all charts in the dashboard, appending the token to the requests.
   3) the backend verifies that the token is valid, and ensures that the chart is indeed in the dashboard. If something doesn't add up, a 403 is returned, otherwise the chart data is returned.
   


----------------------------------------------------------------
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] LiranBri commented on issue #10408: [SIP] Proposal for Dashboard Access Permissions

Posted by GitBox <gi...@apache.org>.
LiranBri commented on issue #10408:
URL: https://github.com/apache/incubator-superset/issues/10408#issuecomment-664242672


   +1


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