[GitHub] [incubator-superset] mistercrunch opened a new issue #7510: [WiP] [SIP-19] Simplify Superset's set of permissions

[GitBox <gi...@apache.org>]

mistercrunch opened a new issue #7510: [WiP] [SIP-19] Simplify Superset's set of permissions 
URL: https://github.com/apache/incubator-superset/issues/7510
 
 
   ## [SIP-19] Simplify Superset's permissions 
   
   ### Motivation
   
   First let's clarify that this is about feature-related permissions, as opposed to data-access permissions. Also note that what I'll refer to as an "atomic permission" is a combination of what FAB calls a "view_menu" and a "permission".
   
   Also note that this change will be powered by recently released FAB features enabling more control over permission definition, as well as migration tools making it straightforward to map and migrate existing role from old to new permission. More information about related FAB features can be found here:
   https://flask-appbuilder.readthedocs.io/en/latest/security.html#permission-customization 
   
   The bulk of feature-related permissions are dynamically generated by Flask App Builder (FAB). For context, FAB generates:
   * a set of permission (`can_show`, `can_list`, `can_add`, `can_download`, `can_delete`, `can_update`, `can_muldelete `) for each `ModelView`. Typically each model has one or many ModelViews
   * a perm for each view method (commonly referred to as "endpoint")
   * a perm for each menu entry
   
   Now Superset overtime has grown to ship with ~280+ permissions. Most of these permissions are unintelligible to users, the UI that exposes them suffers from too much options. In the current state, it almost only makes sense to generate roles programmatically since the cardinality of permissions is so high. 
   
   While we want the atomicity of permissions to cover most use cases, we want for permissions to be easy to document and reason about. In cases where it would be unreasonable to have one permission and not another closely related one, we'd like to merge them as a single permission.
   
   ### Proposed Change
   
   First, group ModelView-related permissions into 2 simple permissions: `read` and `write`, based on this mapping rule:
   ```
       method_permission_name = {
           'add': 'write',
           'delete': 'write',
           'download': 'write',
           'edit': 'write',
           'list': 'read',
           'muldelete': 'write',
           'show': 'read',
       }
   ```
   This assumes that if you can edit, you can also delete or add (`write`). Simarly if you can `read` you can `show` or `list`.
   
   Second, rename and group ModelView names. For clarity drop the "ModelView" suffix and match the Model's name. For examples, `DashboardModelView` view_menu becomes `Dashboard` for permission purposes. This in turns takes care of the secondary ModelView derivatives like `DashboardAsyncModelView` and groups it with other `Dashboard`-related ModelView for permission-related purposes.
   
   Third, models that are related and tightly coupled, for example every Models living around the connectors should refer to the same set of permissions. DruidDatasource, SqlaTables, and their respective Metric and Column models can all go under `Datasource` permissions. Either you can read or write on `Datasource` or you don't. No one needs to be able to edit metrics but unable to edit columns or datasource property. 
   
   This results in the following mappings (Google Spreadsheet):
   https://docs.google.com/spreadsheets/d/1VzBRUsrf_aMS_QkMXoIaLwpETfJmH3qms3kePywBESI/edit?usp=sharing
   
   ### New or Changed Public Interfaces
   
   While `Alpha` and `Gamma` will be migrated and effectively the same, and existing roles converged, there may be existing scripts that would be incompatible with this new world. 
   
   ### New dependencies
   
   N/A
   
   ### Migration Plan and Compatibility
   
   Users will have to run `flask fab security-converge` as part of releasing the new version including this feature.
   
   ### Related future work
   
   * mapping views (endpoint) permission to existing ModelView-related permissions. For instance if we had an existing view permissions called "Superset - delete_dashboards", we could map it to the ModelView atomic permission `Dashboard - can_write`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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