feat(dashboards): security perm simplification

superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx

@@ -63,7 +63,7 @@ const mockUser = {
 };
 
 fetchMock.get(dashboardsInfoEndpoint, {
-  permissions: ['can_list', 'can_edit', 'can_delete'],
+  permissions: ['can_read', 'can_write'],
 });
 fetchMock.get(dashboardOwnersEndpoint, {
   result: [],

superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx

@@ -61,9 +61,9 @@ function DashboardCard({
   favoriteStatus,
   saveFavoriteStatus,
 }: DashboardCardProps) {
-  const canEdit = hasPerm('can_edit');
-  const canDelete = hasPerm('can_delete');
-  const canExport = hasPerm('can_mulexport');
+  const canEdit = hasPerm('can_write');
+  const canDelete = hasPerm('can_write');
+  const canExport = hasPerm('can_read');
 
   const menu = (
     <Menu>

superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx

@@ -121,10 +121,10 @@ function DashboardList(props: DashboardListProps) {
     refreshData();
   };
 
-  const canCreate = hasPerm('can_add');
-  const canEdit = hasPerm('can_edit');
-  const canDelete = hasPerm('can_delete');
-  const canExport = hasPerm('can_mulexport');
+  const canCreate = hasPerm('can_write');
+  const canEdit = hasPerm('can_write');
+  const canDelete = hasPerm('can_write');
+  const canExport = hasPerm('can_read');
 
   const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 

superset/constants.py

@@ -78,10 +78,12 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "api_update": "write",
     "delete": "write",
     "download": "read",
+    "download_dashboards": "read",
     "edit": "write",
     "list": "read",
     "muldelete": "write",
     "show": "read",
+    "new": "write",
 }
 
 MODEL_API_RW_METHOD_PERMISSION_MAP = {
@@ -95,4 +97,7 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "post": "write",
     "put": "write",
     "related": "read",
+    "favorite_status": "read",
+    "thumbnail": "read",
+    "import_": "write",
 }

superset/dashboards/api.py

@@ -32,7 +32,7 @@
 from superset import is_feature_enabled, thumbnail_cache
 from superset.commands.exceptions import CommandInvalidError
 from superset.commands.importers.v1.utils import remove_root
-from superset.constants import RouteMethod
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.dashboards.commands.bulk_delete import BulkDeleteDashboardCommand
 from superset.dashboards.commands.create import CreateDashboardCommand
 from superset.dashboards.commands.delete import DeleteDashboardCommand
@@ -93,7 +93,9 @@ class DashboardRestApi(BaseSupersetModelRestApi):
     resource_name = "dashboard"
     allow_browser_login = True
 
-    class_permission_name = "DashboardModelView"
+    class_permission_name = "Dashboard"
+    method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
     show_columns = [
         "id",
         "charts",

superset/migrations/versions/1f6dca87d1a2_security_converge_dashboards.py

@@ -0,0 +1,89 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""security converge dashboards
+
+Revision ID: 1f6dca87d1a2
+Revises: 5daced1f0e76
+Create Date: 2020-12-11 11:45:25.051084
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "1f6dca87d1a2"
+down_revision = "5daced1f0e76"
+
+
+from alembic import op
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm import Session
+
+from superset.migrations.shared.security_converge import (
+    add_pvms,
+    get_reversed_new_pvms,
+    get_reversed_pvm_map,
+    migrate_roles,
+    Pvm,
+)
+
+NEW_PVMS = {"Dashboard": ("can_read", "can_write",)}
+PVM_MAP = {
+    Pvm("DashboardModelView", "can_add"): (Pvm("Dashboard", "can_write"),),
+    Pvm("DashboardModelView", "can_delete"): (Pvm("Dashboard", "can_write"),),
+    Pvm("DashboardModelView", "can_download_dashboards",): (
+        Pvm("Dashboard", "can_read"),
+    ),
+    Pvm("DashboardModelView", "can_edit",): (Pvm("Dashboard", "can_write"),),
+    Pvm("DashboardModelView", "can_favorite_status",): (Pvm("Dashboard", "can_read"),),
+    Pvm("DashboardModelView", "can_list",): (Pvm("Dashboard", "can_read"),),
+    Pvm("DashboardModelView", "can_mulexport",): (Pvm("Dashboard", "can_read"),),
+    Pvm("DashboardModelView", "can_show",): (Pvm("Dashboard", "can_read"),),
+    Pvm("DashboardModelView", "muldelete",): (Pvm("Dashboard", "can_write"),),
+    Pvm("DashboardModelView", "mulexport",): (Pvm("Dashboard", "can_read"),),
+    Pvm("DashboardModelViewAsync", "can_list",): (Pvm("Dashboard", "can_read"),),
+    Pvm("DashboardModelViewAsync", "muldelete",): (Pvm("Dashboard", "can_write"),),
+    Pvm("DashboardModelViewAsync", "mulexport",): (Pvm("Dashboard", "can_read"),),
+    Pvm("Dashboard", "can_new",): (Pvm("Dashboard", "can_write"),),
+}
+
+
+def upgrade():
+    bind = op.get_bind()
+    session = Session(bind=bind)
+
+    # Add the new permissions on the migration itself
+    add_pvms(session, NEW_PVMS)
+    migrate_roles(session, PVM_MAP)
+    try:
+        session.commit()
+    except SQLAlchemyError as ex:
+        print(f"An error occurred while upgrading permissions: {ex}")
+        session.rollback()
+
+
+def downgrade():
+    bind = op.get_bind()
+    session = Session(bind=bind)
+
+    # Add the old permissions on the migration itself
+    add_pvms(session, get_reversed_new_pvms(PVM_MAP))
+    migrate_roles(session, get_reversed_pvm_map(PVM_MAP))
+    try:
+        session.commit()
+    except SQLAlchemyError as ex:
+        print(f"An error occurred while downgrading permissions: {ex}")
+        session.rollback()
+    pass

superset/views/core.py

@@ -911,7 +911,7 @@ def save_or_overwrite_slice(  # pylint: disable=too-many-arguments,too-many-loca
         elif new_dashboard_name:
             # Creating and adding to a new dashboard
             # check create dashboard permissions
-            dash_add_perm = security_manager.can_access("can_add", "DashboardModelView")
+            dash_add_perm = security_manager.can_access("can_write", "Dashboard")
             if not dash_add_perm:
                 return json_error_response(
                     _("You don't have the rights to ")

superset/views/dashboard/views.py

@@ -25,7 +25,7 @@
 from flask_babel import gettext as __, lazy_gettext as _
 
 from superset import db, event_logger, is_feature_enabled
-from superset.constants import RouteMethod
+from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.models.dashboard import Dashboard as DashboardModel
 from superset.typing import FlaskResponse
 from superset.utils import core as utils
@@ -46,6 +46,9 @@ class DashboardModelView(
     datamodel = SQLAInterface(DashboardModel)
     # TODO disable api_read and api_delete (used by cypress)
     # once we move to ChartRestModelApi
+    class_permission_name = "Dashboard"
+    method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
     include_route_methods = RouteMethod.CRUD_SET | {
         RouteMethod.API_READ,
         RouteMethod.API_DELETE,
@@ -106,6 +109,9 @@ def pre_update(self, item: "DashboardModelView") -> None:
 class Dashboard(BaseSupersetView):
     """The base views for Superset!"""
 
+    class_permission_name = "Dashboard"
+    method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
     @has_access
     @expose("/new/")
     def new(self) -> FlaskResponse:  # pylint: disable=no-self-use
@@ -120,6 +126,9 @@ def new(self) -> FlaskResponse:  # pylint: disable=no-self-use
 
 class DashboardModelViewAsync(DashboardModelView):  # pylint: disable=too-many-ancestors
     route_base = "/dashboardasync"
+    class_permission_name = "Dashboard"
+    method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
     include_route_methods = {RouteMethod.API_READ}
 
     list_columns = [

tests/base_api_tests.py

@@ -32,17 +32,17 @@
 class Model1Api(BaseSupersetModelRestApi):
     datamodel = SQLAInterface(Dashboard)
     allow_browser_login = True
-    class_permission_name = "DashboardModelView"
+    class_permission_name = "Dashboard"
     method_permission_name = {
-        "get_list": "list",
-        "get": "show",
-        "export": "mulexport",
-        "post": "add",
-        "put": "edit",
-        "delete": "delete",
-        "bulk_delete": "delete",
-        "info": "list",
-        "related": "list",
+        "get_list": "read",
+        "get": "read",
+        "export": "read",
+        "post": "write",
+        "put": "write",
+        "delete": "write",
+        "bulk_delete": "write",
+        "info": "read",
+        "related": "read",
     }
 
 

tests/security_tests.py

@@ -48,7 +48,7 @@
 from .fixtures.energy_dashboard import load_energy_table_with_slice
 from .fixtures.unicode_dashboard import load_unicode_dashboard_with_slice
 
-NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery")
+NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery", "Dashboard")
 
 
 def get_perm_tuples(role_name):
@@ -648,7 +648,7 @@ def assert_can_gamma(self, perm_set):
 
         # make sure that user can create slices and dashboards
         self.assert_can_all("SliceModelView", perm_set)
-        self.assert_can_all("DashboardModelView", perm_set)
+        self.assert_can_all("Dashboard", perm_set)
 
         self.assertIn(("can_add_slices", "Superset"), perm_set)
         self.assertIn(("can_copy_dash", "Superset"), perm_set)
@@ -832,37 +832,19 @@ def test_granter_permissions(self):
         self.assert_cannot_alpha(granter_set)
 
     def test_gamma_permissions(self):
-        def assert_can_read(view_menu):
-            self.assertIn(("can_list", view_menu), gamma_perm_set)
-
-        def assert_can_write(view_menu):
-            self.assertIn(("can_add", view_menu), gamma_perm_set)
-            self.assertIn(("can_delete", view_menu), gamma_perm_set)
-            self.assertIn(("can_edit", view_menu), gamma_perm_set)
-
-        def assert_cannot_write(view_menu):
-            self.assertNotIn(("can_add", view_menu), gamma_perm_set)
-            self.assertNotIn(("can_delete", view_menu), gamma_perm_set)
-            self.assertNotIn(("can_edit", view_menu), gamma_perm_set)
-            self.assertNotIn(("can_save", view_menu), gamma_perm_set)
-
-        def assert_can_all(view_menu):
-            assert_can_read(view_menu)
-            assert_can_write(view_menu)
-
         gamma_perm_set = set()
         for perm in security_manager.find_role("Gamma").permissions:
             gamma_perm_set.add((perm.permission.name, perm.view_menu.name))
 
         # check read only perms
-        assert_can_read("TableModelView")
+        self.assert_can_read("TableModelView", gamma_perm_set)
 
         # make sure that user can create slices and dashboards
-        assert_can_all("SliceModelView")
-        assert_can_all("DashboardModelView")
+        self.assert_can_all("SliceModelView", gamma_perm_set)
+        self.assert_can_all("Dashboard", gamma_perm_set)
 
-        assert_cannot_write("UserDBModelView")
-        assert_cannot_write("RoleModelView")
+        self.assert_cannot_write("UserDBModelView", gamma_perm_set)
+        self.assert_cannot_write("RoleModelView", gamma_perm_set)
 
         self.assertIn(("can_add_slices", "Superset"), gamma_perm_set)
         self.assertIn(("can_copy_dash", "Superset"), gamma_perm_set)