You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2019/05/21 00:30:10 UTC

[incubator-superset] branch master updated: [security] New, deprecate merge_perm, FAB method is fixed (#7355)

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 74704f6  [security] New, deprecate merge_perm, FAB method is fixed (#7355)
74704f6 is described below

commit 74704f68c79f68de642732c7ece1e2682a5695bb
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Tue May 21 01:30:00 2019 +0100

    [security] New, deprecate merge_perm, FAB method is fixed (#7355)
    
    * [security] New, deprecate merge_perm, FAB method is fixed
    
    * [style] Fix, flakes
    
    * [tests] Fix, change merge_perm to add_permission_view_menu
    
    * [security] Fix, maintain merge_perm for compatibility
    
    * [security] New, deprecation warning on merge_perm method
    
    * [style] Fix, flake8 C812
---
 superset/cli.py                    |  2 +-
 superset/connectors/druid/views.py | 16 +++++++++++-----
 superset/connectors/sqla/views.py  |  8 ++++----
 superset/security.py               | 26 +++++++++++---------------
 superset/views/core.py             |  4 ++--
 tests/access_tests.py              |  4 ++--
 tests/druid_tests.py               |  4 ++--
 7 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/superset/cli.py b/superset/cli.py
index 5242910..7b441b4 100755
--- a/superset/cli.py
+++ b/superset/cli.py
@@ -371,7 +371,7 @@ def load_test_users_run():
             security_manager.add_permission_role(gamma_sqllab_role, perm)
         utils.get_or_create_main_db()
         db_perm = utils.get_main_database(security_manager.get_session).perm
-        security_manager.merge_perm('database_access', db_perm)
+        security_manager.add_permission_view_menu('database_access', db_perm)
         db_pvm = security_manager.find_permission_view_menu(
             view_menu_name=db_perm, permission_name='database_access')
         gamma_sqllab_role.permissions.append(db_pvm)
diff --git a/superset/connectors/druid/views.py b/superset/connectors/druid/views.py
index 4b05d70..99923e0 100644
--- a/superset/connectors/druid/views.py
+++ b/superset/connectors/druid/views.py
@@ -147,11 +147,11 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
 
     def post_add(self, metric):
         if metric.is_restricted:
-            security_manager.merge_perm('metric_access', metric.get_perm())
+            security_manager.add_permission_view_menu('metric_access', metric.get_perm())
 
     def post_update(self, metric):
         if metric.is_restricted:
-            security_manager.merge_perm('metric_access', metric.get_perm())
+            security_manager.add_permission_view_menu('metric_access', metric.get_perm())
 
 
 appbuilder.add_view_no_menu(DruidMetricInlineView)
@@ -202,7 +202,7 @@ class DruidClusterModelView(SupersetModelView, DeleteMixin, YamlExportMixin):  #
     }
 
     def pre_add(self, cluster):
-        security_manager.merge_perm('database_access', cluster.perm)
+        security_manager.add_permission_view_menu('database_access', cluster.perm)
 
     def pre_update(self, cluster):
         self.pre_add(cluster)
@@ -311,9 +311,15 @@ class DruidDatasourceModelView(DatasourceModelView, DeleteMixin, YamlExportMixin
 
     def post_add(self, datasource):
         datasource.refresh_metrics()
-        security_manager.merge_perm('datasource_access', datasource.get_perm())
+        security_manager.add_permission_view_menu(
+            'datasource_access',
+            datasource.get_perm(),
+        )
         if datasource.schema:
-            security_manager.merge_perm('schema_access', datasource.schema_perm)
+            security_manager.add_permission_view_menu(
+                'schema_access',
+                datasource.schema_perm,
+            )
 
     def post_update(self, datasource):
         self.post_add(datasource)
diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py
index a7c77c2..2edf949 100644
--- a/superset/connectors/sqla/views.py
+++ b/superset/connectors/sqla/views.py
@@ -155,11 +155,11 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
 
     def post_add(self, metric):
         if metric.is_restricted:
-            security_manager.merge_perm('metric_access', metric.get_perm())
+            security_manager.add_permission_view_menu('metric_access', metric.get_perm())
 
     def post_update(self, metric):
         if metric.is_restricted:
-            security_manager.merge_perm('metric_access', metric.get_perm())
+            security_manager.add_permission_view_menu('metric_access', metric.get_perm())
 
 
 appbuilder.add_view_no_menu(SqlMetricInlineView)
@@ -283,9 +283,9 @@ class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin):  # noqa
 
     def post_add(self, table, flash_message=True):
         table.fetch_metadata()
-        security_manager.merge_perm('datasource_access', table.get_perm())
+        security_manager.add_permission_view_menu('datasource_access', table.get_perm())
         if table.schema:
-            security_manager.merge_perm('schema_access', table.schema_perm)
+            security_manager.add_permission_view_menu('schema_access', table.schema_perm)
 
         if flash_message:
             flash(_(
diff --git a/superset/security.py b/superset/security.py
index b30b2e5..f8ae057 100644
--- a/superset/security.py
+++ b/superset/security.py
@@ -263,26 +263,22 @@ class SupersetSecurityManager(SecurityManager):
             return [d for d in datasource_names if d in full_names]
 
     def merge_perm(self, permission_name, view_menu_name):
-        # Implementation copied from sm.find_permission_view_menu.
-        # TODO: use sm.find_permission_view_menu once issue
-        #       https://github.com/airbnb/superset/issues/1944 is resolved.
-        permission = self.find_permission(permission_name)
-        view_menu = self.find_view_menu(view_menu_name)
-        pv = None
-        if permission and view_menu:
-            pv = self.get_session.query(self.permissionview_model).filter_by(
-                permission=permission, view_menu=view_menu).first()
-        if not pv and permission_name and view_menu_name:
-            self.add_permission_view_menu(permission_name, view_menu_name)
+        logging.warning(
+            "This method 'merge_perm' is deprecated use add_permission_view_menu",
+        )
+        self.add_permission_view_menu(permission_name, view_menu_name)
 
     def is_user_defined_permission(self, perm):
         return perm.permission.name in self.OBJECT_SPEC_PERMISSIONS
 
     def create_custom_permissions(self):
         # Global perms
-        self.merge_perm('all_datasource_access', 'all_datasource_access')
-        self.merge_perm('all_database_access', 'all_database_access')
-        self.merge_perm('can_only_access_owned_queries', 'can_only_access_owned_queries')
+        self.add_permission_view_menu('all_datasource_access', 'all_datasource_access')
+        self.add_permission_view_menu('all_database_access', 'all_database_access')
+        self.add_permission_view_menu(
+            'can_only_access_owned_queries',
+            'can_only_access_owned_queries',
+        )
 
     def create_missing_perms(self):
         """Creates missing perms for datasources, schemas and metrics"""
@@ -299,7 +295,7 @@ class SupersetSecurityManager(SecurityManager):
         def merge_pv(view_menu, perm):
             """Create permission view menu only if it doesn't exist"""
             if view_menu and perm and (view_menu, perm) not in all_pvs:
-                self.merge_perm(view_menu, perm)
+                self.add_permission_view_menu(view_menu, perm)
 
         logging.info('Creating missing datasource permissions.')
         datasources = ConnectorRegistry.get_all_datasources(db.session)
diff --git a/superset/views/core.py b/superset/views/core.py
index 1f69f40..8f17ea4 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -310,10 +310,10 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):  # noqa
     def pre_add(self, db):
         self.check_extra(db)
         db.set_sqlalchemy_uri(db.sqlalchemy_uri)
-        security_manager.merge_perm('database_access', db.perm)
+        security_manager.add_permission_view_menu('database_access', db.perm)
         # adding a new database we always want to force refresh schema list
         for schema in db.all_schema_names():
-            security_manager.merge_perm(
+            security_manager.add_permission_view_menu(
                 'schema_access', security_manager.get_schema_perm(db, schema))
 
     def pre_update(self, db):
diff --git a/tests/access_tests.py b/tests/access_tests.py
index f608f00..0bc1743 100644
--- a/tests/access_tests.py
+++ b/tests/access_tests.py
@@ -273,7 +273,7 @@ class RequestAccessTests(SupersetTestCase):
         # gamma gets granted database access
         database = session.query(models.Database).first()
 
-        security_manager.merge_perm('database_access', database.perm)
+        security_manager.add_permission_view_menu('database_access', database.perm)
         ds_perm_view = security_manager.find_permission_view_menu(
             'database_access', database.perm)
         security_manager.add_permission_role(
@@ -309,7 +309,7 @@ class RequestAccessTests(SupersetTestCase):
             table_name='wb_health_population').first()
 
         ds.schema = 'temp_schema'
-        security_manager.merge_perm('schema_access', ds.schema_perm)
+        security_manager.add_permission_view_menu('schema_access', ds.schema_perm)
         schema_perm_view = security_manager.find_permission_view_menu(
             'schema_access', ds.schema_perm)
         security_manager.add_permission_role(
diff --git a/tests/druid_tests.py b/tests/druid_tests.py
index 78d1bb8..164b16a 100644
--- a/tests/druid_tests.py
+++ b/tests/druid_tests.py
@@ -289,8 +289,8 @@ class DruidTests(SupersetTestCase):
         db.session.merge(no_gamma_ds)
         db.session.commit()
 
-        security_manager.merge_perm('datasource_access', gamma_ds.perm)
-        security_manager.merge_perm('datasource_access', no_gamma_ds.perm)
+        security_manager.add_permission_view_menu('datasource_access', gamma_ds.perm)
+        security_manager.add_permission_view_menu('datasource_access', no_gamma_ds.perm)
 
         perm = security_manager.find_permission_view_menu(
             'datasource_access', gamma_ds.get_perm())