[security] Adding docstrings and type hints (#7952)

superset/security.py

@@ -17,9 +17,10 @@
 # pylint: disable=C,R,W
 """A set of constants and methods to manage permissions and security"""
 import logging
-from typing import List
+from typing import Callable, List, Optional, Set, Tuple, TYPE_CHECKING, Union
 
 from flask import current_app, g
+from flask_appbuilder import Model
 from flask_appbuilder.security.sqla import models as ab_models
 from flask_appbuilder.security.sqla.manager import SecurityManager
 from flask_appbuilder.security.views import (
@@ -30,11 +31,17 @@ from flask_appbuilder.security.views import (
 )
 from flask_appbuilder.widgets import ListWidget
 from sqlalchemy import or_
+from sqlalchemy.engine.base import Connection
+from sqlalchemy.orm.mapper import Mapper
 
 from superset import sql_parse
 from superset.connectors.connector_registry import ConnectorRegistry
 from superset.exceptions import SupersetSecurityException
-from superset.utils.core import DatasourceName
+
+if TYPE_CHECKING:
+    from superset.models.core import Database, BaseDatasource
+
+from superset.utils.core import DatasourceName  # noqa: I202
 
 
 class SupersetSecurityListWidget(ListWidget):
@@ -124,12 +131,35 @@ class SupersetSecurityManager(SecurityManager):
 
     ACCESSIBLE_PERMS = {"can_userinfo"}
 
-    def get_schema_perm(self, database, schema):
-        if schema:
-            return "[{}].[{}]".format(database, schema)
+    def get_schema_perm(
+        self, database: Union["Database", str], schema: Optional[str] = None
+    ) -> Optional[str]:
+        """
+        Return the database specific schema permission.
+
+        :param database: The Superset database or database name
+        :param schema: The Superset schema name
+        :return: The database specific schema permission
+        """
+
+        if schema:
+            return f"[{database}].[{schema}]"
+
+        return None
+
+    def can_access(self, permission_name: str, view_name: str) -> bool:
+        """
+        Return True if the user can access the FAB permission/view, False
+        otherwise.
+
+        Note this method adds protection from has_access failing from missing
+        permission/view entries.
+
+        :param permission_name: The FAB permission name
+        :param view_name: The FAB view-menu name
+        :returns: Whether the use can access the FAB permission/view
+        """
 
-    def can_access(self, permission_name, view_name):
-        """Protecting from has_access failing from missing perms/view"""
         user = g.user
         if user.is_anonymous:
             return self.is_item_public(permission_name, view_name)
@@ -137,100 +167,223 @@ class SupersetSecurityManager(SecurityManager):
 
     def can_only_access_owned_queries(self) -> bool:
         """
-        can_access check for custom can_only_access_owned_queries permissions.
+        Return True if the user can only access owned queries, False otherwise.
 
-        :returns: True if current user can access custom permissions
+        :returns: Whether the use can only access owned queries
         """
         return self.can_access(
             "can_only_access_owned_queries", "can_only_access_owned_queries"
         )
 
-    def all_datasource_access(self):
+    def all_datasource_access(self) -> bool:
+        """
+        Return True if the user can access all Superset datasources, False otherwise.
+
+        :returns: Whether the user can access all Superset datasources
+        """
+
         return self.can_access("all_datasource_access", "all_datasource_access")
 
-    def all_database_access(self):
+    def all_database_access(self) -> bool:
+        """
+        Return True if the user can access all Superset databases, False otherwise.
+
+        :returns: Whether the user can access all Superset databases
+        """
+
         return self.can_access("all_database_access", "all_database_access")
 
-    def database_access(self, database):
+    def database_access(self, database: "Database") -> bool:
+        """
+        Return True if the user can access the Superset database, False otherwise.
+
+        :param database: The Superset database
+        :returns: Whether the user can access the Superset database
+        """
+
         return (
-            self.all_database_access()
+            self.all_datasource_access()
+            or self.all_database_access()
             or self.can_access("database_access", database.perm)
-            or self.all_datasource_access()
         )
 
-    def schema_access(self, datasource):
+    def schema_access(self, datasource: "BaseDatasource") -> bool:
+        """
+        Return True if the user can access the schema associated with the Superset
+        datasource, False otherwise.
+
+        Note for Druid datasources the database and schema are akin to the Druid cluster
+        and datasource name prefix, i.e., [schema.]datasource, respectively.
+
+        :param datasource: The Superset datasource
+        :returns: Whether the user can access the datasource's schema
+        """
+
         return (
-            self.database_access(datasource.database)
-            or self.all_datasource_access()
+            self.all_datasource_access()
+            or self.database_access(datasource.database)
             or self.can_access("schema_access", datasource.schema_perm)
         )
 
-    def datasource_access(self, datasource):
+    def datasource_access(self, datasource: "BaseDatasource") -> bool:
+        """
+        Return True if the user can access the Superset datasource, False otherwise.
+
+        :param datasource: The Superset datasource
+        :returns: Whether the use can access the Superset datasource
+        """
+
         return self.schema_access(datasource) or self.can_access(
             "datasource_access", datasource.perm
         )
 
-    def get_datasource_access_error_msg(self, datasource):
-        return """This endpoint requires the datasource {}, database or
-            `all_datasource_access` permission""".format(
-            datasource.name
-        )
+    def get_datasource_access_error_msg(self, datasource: "BaseDatasource") -> str:
+        """
+        Return the error message for the denied Superset datasource.
+
+        :param datasource: The denied Superset datasource
+        :returns: The error message
+        """
+
+        return f"""This endpoint requires the datasource {datasource.name}, database or
+            `all_datasource_access` permission"""
+
+    def get_datasource_access_link(self, datasource: "BaseDatasource") -> Optional[str]:
+        """
+        Return the link for the denied Superset datasource.
+
+        :param datasource: The denied Superset datasource
+        :returns: The access URL
+        """
 
-    def get_datasource_access_link(self, datasource):
         from superset import conf
 
         return conf.get("PERMISSION_INSTRUCTIONS_LINK")
 
-    def get_table_access_error_msg(self, table_name):
-        return """You need access to the following tables: {}, all database access or
-              `all_datasource_access` permission""".format(
-            table_name
-        )
+    def get_table_access_error_msg(self, tables: List[str]) -> str:
+        """
+        Return the error message for the denied SQL tables.
+
+        Note the table names conform to the [[cluster.]schema.]table construct.
+
+        :param tables: The list of denied SQL table names
+        :returns: The error message
+        """
+
+        return f"""You need access to the following tables: {", ".join(tables)}, all
+            database access or `all_datasource_access` permission"""
+
+    def get_table_access_link(self, tables: List[str]) -> Optional[str]:
+        """
+        Return the access link for the denied SQL tables.
+
+        Note the table names conform to the [[cluster.]schema.]table construct.
+
+        :param tables: The list of denied SQL table names
+        :returns: The access URL
+        """
 
-    def get_table_access_link(self, tables):
         from superset import conf
 
         return conf.get("PERMISSION_INSTRUCTIONS_LINK")
 
-    def datasource_access_by_name(self, database, datasource_name, schema=None):
+    def _datasource_access_by_name(
+        self, database: "Database", table_name: str, schema: str = None
+    ) -> bool:
+        """
+        Return True if the user can access the SQL table, False otherwise.
+
+        :param database: The SQL database
+        :param table_name: The SQL table name
+        :param schema: The Superset schema
+        :returns: Whether the use can access the SQL table
+        """
+
         from superset import db
 
         if self.database_access(database) or self.all_datasource_access():
             return True
 
         schema_perm = self.get_schema_perm(database, schema)
-        if schema and self.can_access("schema_access", schema_perm):
+        if schema_perm and self.can_access("schema_access", schema_perm):
             return True
 
         datasources = ConnectorRegistry.query_datasources_by_name(
-            db.session, database, datasource_name, schema=schema
+            db.session, database, table_name, schema=schema
         )
         for datasource in datasources:
             if self.can_access("datasource_access", datasource.perm):
                 return True
         return False
 
-    def get_schema_and_table(self, table_in_query, schema):
+    def _get_schema_and_table(
+        self, table_in_query: str, schema: str
+    ) -> Tuple[str, str]:
+        """
+        Return the SQL schema/table tuple associated with the table extracted from the
+        SQL query.
+
+        Note the table name conforms to the [[cluster.]schema.]table construct.
+
+        :param table_in_query: The SQL table name
+        :param schema: The fallback SQL schema if not present in the table name
+        :returns: The SQL schema/table tuple
+        """
+
         table_name_pieces = table_in_query.split(".")
         if len(table_name_pieces) == 3:
-            return tuple(table_name_pieces[1:])
+            return tuple(table_name_pieces[1:])  # noqa: T484
         elif len(table_name_pieces) == 2:
-            return tuple(table_name_pieces)
+            return tuple(table_name_pieces)  # noqa: T484
         return (schema, table_name_pieces[0])
 
-    def datasource_access_by_fullname(self, database, table_in_query, schema):
-        table_schema, table_name = self.get_schema_and_table(table_in_query, schema)
-        return self.datasource_access_by_name(database, table_name, schema=table_schema)
+    def _datasource_access_by_fullname(
+        self, database: "Database", table_in_query: str, schema: str
+    ) -> bool:
+        """
+        Return True if the user can access the table extracted from the SQL query, False
+        otherwise.
+
+        Note the table name conforms to the [[cluster.]schema.]table construct.
+
+        :param database: The Superset database
+        :param table_in_query: The SQL table name
+        :param schema: The fallback SQL schema, i.e., if not present in the table name
+        :returns: Whether the user can access the SQL table
+        """
+
+        table_schema, table_name = self._get_schema_and_table(table_in_query, schema)
+        return self._datasource_access_by_name(
+            database, table_name, schema=table_schema
+        )
+
+    def rejected_tables(self, sql: str, database: "Database", schema: str) -> List[str]:
+        """
+        Return the list of rejected SQL table names.
+
+        Note the rejected table names conform to the [[cluster.]schema.]table construct.
+
+        :param sql: The SQL statement
+        :param database: The SQL database
+        :param schema: The SQL database schema
+        :returns: The rejected table names
+        """
 
-    def rejected_datasources(self, sql, database, schema):
         superset_query = sql_parse.ParsedQuery(sql)
+
         return [
             t
             for t in superset_query.tables
-            if not self.datasource_access_by_fullname(database, t, schema)
+            if not self._datasource_access_by_fullname(database, t, schema)
         ]
 
-    def user_datasource_perms(self):
+    def _user_datasource_perms(self) -> Set[str]:
+        """
+        Return the set of FAB permission view-menu names the user can access.
+
+        :returns: The set of FAB permission view-menu names
+        """
+
         datasource_perms = set()
         for r in g.user.roles:
             for perm in r.permissions:
@@ -238,7 +391,18 @@ class SupersetSecurityManager(SecurityManager):
                     datasource_perms.add(perm.view_menu.name)
         return datasource_perms
 
-    def schemas_accessible_by_user(self, database, schemas, hierarchical=True):
+    def schemas_accessible_by_user(
+        self, database: "Database", schemas: List[str], hierarchical: bool = True
+    ) -> List[str]:
+        """
+        Return the sorted list of SQL schemas accessible by the user.
+
+        :param database: The SQL database
+        :param schemas: The list of eligible SQL schemas
+        :param hierarchical: Whether to check using the hierarchical permission logic
+        :returns: The list of accessible SQL schemas
+        """
+
         from superset import db
         from superset.connectors.sqla.models import SqlaTable
 
@@ -250,10 +414,10 @@ class SupersetSecurityManager(SecurityManager):
         subset = set()
         for schema in schemas:
             schema_perm = self.get_schema_perm(database, schema)
-            if self.can_access("schema_access", schema_perm):
+            if schema_perm and self.can_access("schema_access", schema_perm):
                 subset.add(schema)
 
-        perms = self.user_datasource_perms()
+        perms = self._user_datasource_perms()
         if perms:
             tables = (
                 db.session.query(SqlaTable)
@@ -266,8 +430,20 @@ class SupersetSecurityManager(SecurityManager):
         return sorted(list(subset))
 
     def get_datasources_accessible_by_user(
-        self, database, datasource_names: List[DatasourceName], schema: str = None
+        self,
+        database: "Database",
+        datasource_names: List[DatasourceName],
+        schema: Optional[str] = None,
     ) -> List[DatasourceName]:
+        """
+        Return the list of SQL tables accessible by the user.
+
+        :param database: The SQL database
+        :param datasource_names: The list of eligible SQL tables w/ schema
+        :param schema: The fallback SQL schema if not present in the table name
+        :returns: The list of accessible SQL tables w/ schema
+        """
+
         from superset import db
 
         if self.database_access(database) or self.all_datasource_access():
@@ -275,10 +451,10 @@ class SupersetSecurityManager(SecurityManager):
 
         if schema:
             schema_perm = self.get_schema_perm(database, schema)
-            if self.can_access("schema_access", schema_perm):
+            if schema_perm and self.can_access("schema_access", schema_perm):
                 return datasource_names
 
-        user_perms = self.user_datasource_perms()
+        user_perms = self._user_datasource_perms()
         user_datasources = ConnectorRegistry.query_datasources_by_permissions(
             db.session, database, user_perms
         )
@@ -289,26 +465,48 @@ class SupersetSecurityManager(SecurityManager):
             full_names = {d.full_name for d in user_datasources}
             return [d for d in datasource_names if d in full_names]
 
-    def merge_perm(self, permission_name, view_menu_name):
+    def merge_perm(self, permission_name: str, view_menu_name: str) -> None:
+        """
+        Add the FAB permission/view-menu.
+
+        :param permission_name: The FAB permission name
+        :param view_menu_names: The FAB view-menu name
+        :see: SecurityManager.add_permission_view_menu
+        """
+
         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):
+    def _is_user_defined_permission(self, perm: Model) -> bool:
+        """
+        Return True if the FAB permission is user defined, False otherwise.
+
+        :param perm: The FAB permission
+        :returns: Whether the FAB permission is user defined
+        """
+
         return perm.permission.name in self.OBJECT_SPEC_PERMISSIONS
 
-    def create_custom_permissions(self):
-        # Global perms
+    def create_custom_permissions(self) -> None:
+        """
+        Create custom FAB permissions.
+        """
+
         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"""
+    def create_missing_perms(self) -> None:
+        """
+        Creates missing FAB permissions for datasources, schemas and metrics.
+        """
+
         from superset import db
+        from superset.connectors.base.models import BaseMetric
         from superset.models import core as models
 
         logging.info("Fetching a set of all perms to lookup which ones are missing")
@@ -334,7 +532,7 @@ class SupersetSecurityManager(SecurityManager):
             merge_pv("database_access", database.perm)
 
         logging.info("Creating missing metrics permissions")
-        metrics = []
+        metrics: List[BaseMetric] = []
         for datasource_class in ConnectorRegistry.sources.values():
             metrics += list(db.session.query(datasource_class.metric_class).all())
 
@@ -342,14 +540,17 @@ class SupersetSecurityManager(SecurityManager):
             if metric.is_restricted:
                 merge_pv("metric_access", metric.perm)
 
-    def clean_perms(self):
-        """FAB leaves faulty permissions that need to be cleaned up"""
+    def clean_perms(self) -> None:
+        """
+        Clean up the FAB faulty permissions.
+        """
+
         logging.info("Cleaning faulty perms")
         sesh = self.get_session
         pvms = sesh.query(ab_models.PermissionView).filter(
             or_(
-                ab_models.PermissionView.permission == None,  # NOQA
-                ab_models.PermissionView.view_menu == None,  # NOQA
+                ab_models.PermissionView.permission == None,  # noqa
+                ab_models.PermissionView.view_menu == None,  # noqa
             )
         )
         deleted_count = pvms.delete()
@@ -357,8 +558,11 @@ class SupersetSecurityManager(SecurityManager):
         if deleted_count:
             logging.info("Deleted {} faulty permissions".format(deleted_count))
 
-    def sync_role_definitions(self):
-        """Inits the Superset application with security roles and such"""
+    def sync_role_definitions(self) -> None:
+        """
+        Initialize the Superset application with security roles and such.
+        """
+
         from superset import conf
 
         logging.info("Syncing role definition")
@@ -366,14 +570,14 @@ class SupersetSecurityManager(SecurityManager):
         self.create_custom_permissions()
 
         # Creating default roles
-        self.set_role("Admin", self.is_admin_pvm)
-        self.set_role("Alpha", self.is_alpha_pvm)
-        self.set_role("Gamma", self.is_gamma_pvm)
-        self.set_role("granter", self.is_granter_pvm)
-        self.set_role("sql_lab", self.is_sql_lab_pvm)
+        self.set_role("Admin", self._is_admin_pvm)
+        self.set_role("Alpha", self._is_alpha_pvm)
+        self.set_role("Gamma", self._is_gamma_pvm)
+        self.set_role("granter", self._is_granter_pvm)
+        self.set_role("sql_lab", self._is_sql_lab_pvm)
 
         if conf.get("PUBLIC_ROLE_LIKE_GAMMA", False):
-            self.set_role("Public", self.is_gamma_pvm)
+            self.set_role("Public", self._is_gamma_pvm)
 
         self.create_missing_perms()
 
@@ -381,7 +585,14 @@ class SupersetSecurityManager(SecurityManager):
         self.get_session.commit()
         self.clean_perms()
 
-    def set_role(self, role_name, pvm_check):
+    def set_role(self, role_name: str, pvm_check: Callable) -> None:
+        """
+        Set the FAB permission/views for the role.
+
+        :param role_name: The FAB role name
+        :param pvm_check: The FAB permission/view check
+        """
+
         logging.info("Syncing {} perms".format(role_name))
         sesh = self.get_session
         pvms = sesh.query(ab_models.PermissionView).all()
@@ -392,8 +603,17 @@ class SupersetSecurityManager(SecurityManager):
         sesh.merge(role)
         sesh.commit()
 
-    def is_admin_only(self, pvm):
-        # not readonly operations on read only model views allowed only for admins
+    def _is_admin_only(self, pvm: Model) -> bool:
+        """
+        Return True if the FAB permission/view is accessible to only Admin users,
+        False otherwise.
+
+        Note readonly operations on read only model views are allowed only for admins.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is accessible to only Admin users
+        """
+
         if (
             pvm.view_menu.name in self.READ_ONLY_MODEL_VIEWS
             and pvm.permission.name not in self.READ_ONLY_PERMISSION
@@ -404,7 +624,15 @@ class SupersetSecurityManager(SecurityManager):
             or pvm.permission.name in self.ADMIN_ONLY_PERMISSIONS
         )
 
-    def is_alpha_only(self, pvm):
+    def _is_alpha_only(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the FAB permission/view is accessible to only Alpha users,
+        False otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is accessible to only Alpha users
+        """
+
         if (
             pvm.view_menu.name in self.GAMMA_READ_ONLY_MODEL_VIEWS
             and pvm.permission.name not in self.READ_ONLY_PERMISSION
@@ -415,25 +643,65 @@ class SupersetSecurityManager(SecurityManager):
             or pvm.permission.name in self.ALPHA_ONLY_PERMISSIONS
         )
 
-    def is_accessible_to_all(self, pvm):
+    def _is_accessible_to_all(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the FAB permission/view is accessible to all, False
+        otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is accessible to all users
+        """
+
         return pvm.permission.name in self.ACCESSIBLE_PERMS
 
-    def is_admin_pvm(self, pvm):
-        return not self.is_user_defined_permission(pvm)
+    def _is_admin_pvm(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the FAB permission/view is Admin user related, False
+        otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is Admin related
+        """
+
+        return not self._is_user_defined_permission(pvm)
+
+    def _is_alpha_pvm(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the FAB permission/view is Alpha user related, False
+        otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is Alpha related
+        """
 
-    def is_alpha_pvm(self, pvm):
         return not (
-            self.is_user_defined_permission(pvm) or self.is_admin_only(pvm)
-        ) or self.is_accessible_to_all(pvm)
+            self._is_user_defined_permission(pvm) or self._is_admin_only(pvm)
+        ) or self._is_accessible_to_all(pvm)
+
+    def _is_gamma_pvm(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the FAB permission/view is Gamma user related, False
+        otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is Gamma related
+        """
 
-    def is_gamma_pvm(self, pvm):
         return not (
-            self.is_user_defined_permission(pvm)
-            or self.is_admin_only(pvm)
-            or self.is_alpha_only(pvm)
-        ) or self.is_accessible_to_all(pvm)
+            self._is_user_defined_permission(pvm)
+            or self._is_admin_only(pvm)
+            or self._is_alpha_only(pvm)
+        ) or self._is_accessible_to_all(pvm)
+
+    def _is_sql_lab_pvm(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the FAB permission/view is SQL Lab related, False
+        otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the FAB object is SQL Lab related
+        """
 
-    def is_sql_lab_pvm(self, pvm):
         return (
             pvm.view_menu.name
             in {"SQL Lab", "SQL Editor", "Query Search", "Saved Queries"}
@@ -451,10 +719,28 @@ class SupersetSecurityManager(SecurityManager):
             )
         )
 
-    def is_granter_pvm(self, pvm):
+    def _is_granter_pvm(self, pvm: PermissionModelView) -> bool:
+        """
+        Return True if the user can grant the FAB permission/view, False
+        otherwise.
+
+        :param pvm: The FAB permission/view
+        :returns: Whether the user can grant the FAB permission/view
+        """
+
         return pvm.permission.name in {"can_override_role_permissions", "can_approve"}
 
-    def set_perm(self, mapper, connection, target):  # noqa
+    def set_perm(
+        self, mapper: Mapper, connection: Connection, target: "BaseDatasource"
+    ) -> None:
+        """
+        Set the datasource permissions.
+
+        :param mapper: The table mappper
+        :param connection: The DB-API connection
+        :param target: The mapped instance being persisted
+        """
+
         if target.perm != target.get_perm():
             link_table = target.__table__
             connection.execute(
@@ -497,7 +783,14 @@ class SupersetSecurityManager(SecurityManager):
                 )
             )
 
-    def assert_datasource_permission(self, datasource):
+    def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
+        """
+        Assert the the user has permission to access the Superset datasource.
+
+        :param datasource: The Superset datasource
+        :rasies SupersetSecurityException: If the user does not have permission
+        """
+
         if not self.datasource_access(datasource):
             raise SupersetSecurityException(
                 self.get_datasource_access_error_msg(datasource),

superset/views/core.py

@@ -2399,7 +2399,7 @@ class Superset(BaseSupersetView):
             )
 
         query = db.session.query(Query).filter_by(results_key=key).one()
-        rejected_tables = security_manager.rejected_datasources(
+        rejected_tables = security_manager.rejected_tables(
             query.sql, query.database, query.schema
         )
         if rejected_tables:
@@ -2521,7 +2521,7 @@ class Superset(BaseSupersetView):
         if not mydb:
             json_error_response("Database with id {} is missing.".format(database_id))
 
-        rejected_tables = security_manager.rejected_datasources(sql, mydb, schema)
+        rejected_tables = security_manager.rejected_tables(sql, mydb, schema)
         if rejected_tables:
             return json_error_response(
                 security_manager.get_table_access_error_msg(rejected_tables),
@@ -2645,7 +2645,7 @@ class Superset(BaseSupersetView):
         logging.info("Exporting CSV file [{}]".format(client_id))
         query = db.session.query(Query).filter_by(client_id=client_id).one()
 
-        rejected_tables = security_manager.rejected_datasources(
+        rejected_tables = security_manager.rejected_tables(
             query.sql, query.database, query.schema
         )
         if rejected_tables:

tests/security_tests.py

@@ -113,12 +113,12 @@ class RolePermissionTests(SupersetTestCase):
 
     def test_is_admin_only(self):
         self.assertFalse(
-            security_manager.is_admin_only(
+            security_manager._is_admin_only(
                 security_manager.find_permission_view_menu("can_show", "TableModelView")
             )
         )
         self.assertFalse(
-            security_manager.is_admin_only(
+            security_manager._is_admin_only(
                 security_manager.find_permission_view_menu(
                     "all_datasource_access", "all_datasource_access"
                 )
@@ -126,27 +126,27 @@ class RolePermissionTests(SupersetTestCase):
         )
 
         self.assertTrue(
-            security_manager.is_admin_only(
+            security_manager._is_admin_only(
                 security_manager.find_permission_view_menu("can_delete", "DatabaseView")
             )
         )
         if app.config.get("ENABLE_ACCESS_REQUEST"):
             self.assertTrue(
-                security_manager.is_admin_only(
+                security_manager._is_admin_only(
                     security_manager.find_permission_view_menu(
                         "can_show", "AccessRequestsModelView"
                     )
                 )
             )
         self.assertTrue(
-            security_manager.is_admin_only(
+            security_manager._is_admin_only(
                 security_manager.find_permission_view_menu(
                     "can_edit", "UserDBModelView"
                 )
             )
         )
         self.assertTrue(
-            security_manager.is_admin_only(
+            security_manager._is_admin_only(
                 security_manager.find_permission_view_menu("can_approve", "Superset")
             )
         )
@@ -156,41 +156,41 @@ class RolePermissionTests(SupersetTestCase):
     )
     def test_is_alpha_only(self):
         self.assertFalse(
-            security_manager.is_alpha_only(
+            security_manager._is_alpha_only(
                 security_manager.find_permission_view_menu("can_show", "TableModelView")
             )
         )
 
         self.assertTrue(
-            security_manager.is_alpha_only(
+            security_manager._is_alpha_only(
                 security_manager.find_permission_view_menu(
                     "muldelete", "TableModelView"
                 )
             )
         )
         self.assertTrue(
-            security_manager.is_alpha_only(
+            security_manager._is_alpha_only(
                 security_manager.find_permission_view_menu(
                     "all_datasource_access", "all_datasource_access"
                 )
             )
         )
         self.assertTrue(
-            security_manager.is_alpha_only(
+            security_manager._is_alpha_only(
                 security_manager.find_permission_view_menu(
                     "can_edit", "SqlMetricInlineView"
                 )
             )
         )
         self.assertTrue(
-            security_manager.is_alpha_only(
+            security_manager._is_alpha_only(
                 security_manager.find_permission_view_menu(
                     "can_delete", "DruidMetricInlineView"
                 )
             )
         )
         self.assertTrue(
-            security_manager.is_alpha_only(
+            security_manager._is_alpha_only(
                 security_manager.find_permission_view_menu(
                     "all_database_access", "all_database_access"
                 )
@@ -199,7 +199,7 @@ class RolePermissionTests(SupersetTestCase):
 
     def test_is_gamma_pvm(self):
         self.assertTrue(
-            security_manager.is_gamma_pvm(
+            security_manager._is_gamma_pvm(
                 security_manager.find_permission_view_menu("can_show", "TableModelView")
             )
         )