[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

@@ -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)

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 @@ def pre_add(self, datasource):
 
     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)

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 @@ def pre_add(self, table):
 
     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(_(

superset/security.py

@@ -263,26 +263,22 @@ def accessible_by_user(self, database, datasource_names, schema=None):
             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 @@ def create_missing_perms(self):
         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)

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):

tests/access_tests.py

@@ -273,7 +273,7 @@ def test_clean_requests_after_db_grant(self):
         # 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 @@ def test_clean_requests_after_schema_grant(self):
             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(

tests/druid_tests.py

@@ -289,8 +289,8 @@ def test_filter_druid_datasource(self):
         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())