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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
