feat(databases): security perm simplification (#12036)

* feat(databases): security perm simplification

* fix tests

* fix JS tests

superset-frontend/spec/javascripts/views/CRUD/data/database/DatabaseList_spec.jsx

@@ -62,7 +62,7 @@ const mockUser = {
 };
 
 fetchMock.get(databasesInfoEndpoint, {
-  permissions: ['can_delete'],
+  permissions: ['can_write'],
 });
 fetchMock.get(databasesEndpoint, {
   result: mockdatabases,

superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx

@@ -145,11 +145,11 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
     setDatabaseModalOpen(true);
   }
 
-  const canCreate = hasPerm('can_add');
-  const canEdit = hasPerm('can_edit');
-  const canDelete = hasPerm('can_delete');
+  const canCreate = hasPerm('can_write');
+  const canEdit = hasPerm('can_write');
+  const canDelete = hasPerm('can_write');
   const canExport =
-    hasPerm('can_mulexport') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
+    hasPerm('can_read') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
 
   const menuData: SubMenuProps = {
     activeChild: 'Databases',

superset/constants.py

@@ -82,6 +82,7 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "edit": "write",
     "list": "read",
     "muldelete": "write",
+    "mulexport": "read",
     "show": "read",
     "new": "write",
     "yaml_export": "read",
@@ -98,11 +99,15 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "post": "write",
     "put": "write",
     "related": "read",
+    "related_objects": "read",
+    "schemas": "read",
+    "select_star": "read",
+    "table_metadata": "read",
+    "test_connection": "read",
     "favorite_status": "read",
     "thumbnail": "read",
     "import_": "write",
     "refresh": "read",
-    "related_objects": "read",
     "cache_screenshot": "read",
     "screenshot": "read",
     "data": "read",

superset/databases/api.py

@@ -38,7 +38,7 @@
 from superset import event_logger
 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.databases.commands.create import CreateDatabaseCommand
 from superset.databases.commands.delete import DeleteDatabaseCommand
 from superset.databases.commands.exceptions import (
@@ -92,8 +92,9 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "test_connection",
         "related_objects",
     }
-    class_permission_name = "DatabaseView"
     resource_name = "database"
+    class_permission_name = "Database"
+    method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
     allow_browser_login = True
     base_filters = [["id", DatabaseFilter, lambda: []]]
     show_columns = [

superset/migrations/versions/42b4c9e01447_security_converge_databases.py

@@ -0,0 +1,82 @@
+# 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 databases
+
+Revision ID: 42b4c9e01447
+Revises: 5daced1f0e76
+Create Date: 2020-12-14 10:49:36.110805
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "42b4c9e01447"
+down_revision = "1f6dca87d1a2"
+
+import sqlalchemy as sa
+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 = {"Database": ("can_read", "can_write",)}
+PVM_MAP = {
+    Pvm("DatabaseView", "can_add"): (Pvm("Database", "can_write"),),
+    Pvm("DatabaseView", "can_delete"): (Pvm("Database", "can_write"),),
+    Pvm("DatabaseView", "can_edit",): (Pvm("Database", "can_write"),),
+    Pvm("DatabaseView", "can_list",): (Pvm("Database", "can_read"),),
+    Pvm("DatabaseView", "can_mulexport",): (Pvm("Database", "can_read"),),
+    Pvm("DatabaseView", "can_post",): (Pvm("Database", "can_write"),),
+    Pvm("DatabaseView", "can_show",): (Pvm("Database", "can_read"),),
+    Pvm("DatabaseView", "muldelete",): (Pvm("Database", "can_write"),),
+    Pvm("DatabaseView", "yaml_export",): (Pvm("Database", "can_read"),),
+}
+
+
+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/database/views.py

@@ -30,7 +30,7 @@
 import superset.models.core as models
 from superset import app, db, is_feature_enabled
 from superset.connectors.sqla.models import SqlaTable
-from superset.constants import RouteMethod
+from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.exceptions import CertificateException
 from superset.sql_parse import Table
 from superset.typing import FlaskResponse
@@ -81,6 +81,10 @@ class DatabaseView(
     DatabaseMixin, SupersetModelView, DeleteMixin, YamlExportMixin
 ):  # pylint: disable=too-many-ancestors
     datamodel = SQLAInterface(models.Database)
+
+    class_permission_name = "Database"
+    method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
     include_route_methods = RouteMethod.CRUD_SET
 
     add_template = "superset/models/database/add.html"

tests/databases/api_tests.py

@@ -562,6 +562,20 @@ def test_get_table_metadata(self):
         self.assertTrue(len(response["columns"]) > 5)
         self.assertTrue(response.get("selectStar").startswith("SELECT"))
 
+    def test_info_security_database(self):
+        """
+        Database API: Test info security
+        """
+        self.login(username="admin")
+        params = {"keys": ["permissions"]}
+        uri = f"api/v1/database/_info?q={prison.dumps(params)}"
+        rv = self.get_assert_metric(uri, "info")
+        data = json.loads(rv.data.decode("utf-8"))
+        assert rv.status_code == 200
+        assert "can_read" in data["permissions"]
+        assert "can_write" in data["permissions"]
+        assert len(data["permissions"]) == 2
+
     def test_get_invalid_database_table_metadata(self):
         """
         Database API: Test get invalid database from table metadata
@@ -874,7 +888,9 @@ def test_export_database_not_allowed(self):
         argument = [database.id]
         uri = f"api/v1/database/export/?q={prison.dumps(argument)}"
         rv = self.client.get(uri)
-        assert rv.status_code == 401
+        # export only requires can_read now, but gamma need to have explicit access to
+        # view the database
+        assert rv.status_code == 404
 
     def test_export_database_non_existing(self):
         """

tests/security_tests.py

@@ -50,6 +50,7 @@
 
 NEW_SECURITY_CONVERGE_VIEWS = (
     "Annotation",
+    "Database",
     "Dataset",
     "Dashboard",
     "CssTemplate",
@@ -701,7 +702,7 @@ def assert_cannot_alpha(self, perm_set):
         self.assert_cannot_write("UserDBModelView", perm_set)
 
     def assert_can_admin(self, perm_set):
-        self.assert_can_all("DatabaseView", perm_set)
+        self.assert_can_all("Database", perm_set)
         self.assert_can_all("RoleModelView", perm_set)
         self.assert_can_all("UserDBModelView", perm_set)