Reimplement permissions fetching to do it in a single transaction (#2…

…1156)

Co-authored-by: Bogdan Kyryliuk <bogdankyryliuk@dropbox.com>

superset/security/manager.py

@@ -24,11 +24,13 @@
     Any,
     Callable,
     cast,
+    DefaultDict,
     Dict,
     List,
     NamedTuple,
     Optional,
     Set,
+    Tuple,
     TYPE_CHECKING,
     Union,
 )
@@ -1802,3 +1804,38 @@ def is_admin(self) -> bool:
         return current_app.config["AUTH_ROLE_ADMIN"] in [
             role.name for role in self.get_user_roles()
         ]
+
+    def get_permissions(
+        self,
+        user: User,
+    ) -> Tuple[Dict[str, List[List[str]]], DefaultDict[str, List[str]]]:
+        if not user.roles:
+            raise AttributeError("User object does not have roles")
+
+        roles = defaultdict(list)
+        permissions = defaultdict(set)
+
+        query = (
+            self.get_session.query(Role.name, Permission.name, ViewMenu.name)
+            .join(assoc_user_role, assoc_user_role.c.role_id == Role.id)
+            .join(Role.permissions)
+            .join(PermissionView.view_menu)
+            .join(PermissionView.permission)
+        )
+
+        if user.is_anonymous:
+            public_role = current_app.config.get("AUTH_ROLE_PUBLIC")
+            query = query.filter(Role.name == public_role)
+        else:
+            query = query.filter(assoc_user_role.c.user_id == user.id)
+
+        rows = query.all()
+        for role, permission, view_menu in rows:
+            if permission in ("datasource_access", "database_access"):
+                permissions[permission].add(view_menu)
+            roles[role].append([permission, view_menu])
+
+        transformed_permissions = defaultdict(list)
+        for perm in permissions:
+            transformed_permissions[perm] = list(permissions[perm])
+        return roles, transformed_permissions

superset/views/utils.py

@@ -15,9 +15,8 @@
 # specific language governing permissions and limitations
 # under the License.
 import logging
-from collections import defaultdict
 from functools import wraps
-from typing import Any, Callable, DefaultDict, Dict, List, Optional, Tuple, Union
+from typing import Any, Callable, Dict, List, Optional, Tuple, Union
 from urllib import parse
 
 import msgpack
@@ -94,34 +93,13 @@ def bootstrap_user_data(user: User, include_perms: bool = False) -> Dict[str, An
         }
 
     if include_perms:
-        roles, permissions = get_permissions(user)
+        roles, permissions = security_manager.get_permissions(user)
         payload["roles"] = roles
         payload["permissions"] = permissions
 
     return payload
 
 
-def get_permissions(
-    user: User,
-) -> Tuple[Dict[str, List[List[str]]], DefaultDict[str, List[str]]]:
-    if not user.roles:
-        raise AttributeError("User object does not have roles")
-
-    roles = defaultdict(list)
-    permissions = defaultdict(set)
-
-    for role in user.roles:
-        permissions_ = security_manager.get_role_permissions(role)
-        for permission in permissions_:
-            if permission[0] in ("datasource_access", "database_access"):
-                permissions[permission[0]].add(permission[1])
-            roles[role.name].append([permission[0], permission[1]])
-    transformed_permissions = defaultdict(list)
-    for perm in permissions:
-        transformed_permissions[perm] = list(permissions[perm])
-    return roles, transformed_permissions
-
-
 def get_viz(
     form_data: FormData,
     datasource_type: str,

tests/integration_tests/security_tests.py

@@ -61,6 +61,7 @@
     load_world_bank_dashboard_with_slices,
     load_world_bank_data,
 )
+from .dashboard_utils import get_table
 
 NEW_SECURITY_CONVERGE_VIEWS = (
     "Annotation",
@@ -73,6 +74,158 @@
     "SavedQuery",
 )
 
+GAMMA_ROLE_PERMISSIONS = {
+    "Gamma": [
+        ["menu_access", "List Users"],
+        ["can_read", "SavedQuery"],
+        ["can_write", "SavedQuery"],
+        ["can_read", "CssTemplate"],
+        ["can_write", "CssTemplate"],
+        ["can_read", "ReportSchedule"],
+        ["can_write", "ReportSchedule"],
+        ["can_read", "Chart"],
+        ["can_write", "Chart"],
+        ["can_read", "Annotation"],
+        ["can_write", "Annotation"],
+        ["can_read", "Dataset"],
+        ["can_read", "Dashboard"],
+        ["can_write", "Dashboard"],
+        ["can_read", "Database"],
+        ["can_read", "Query"],
+        ["can_this_form_post", "ResetMyPasswordView"],
+        ["can_this_form_get", "ResetMyPasswordView"],
+        ["can_this_form_post", "UserInfoEditView"],
+        ["can_this_form_get", "UserInfoEditView"],
+        ["can_userinfo", "UserDBModelView"],
+        ["resetmypassword", "UserDBModelView"],
+        ["can_get", "OpenApi"],
+        ["can_show", "SwaggerView"],
+        ["can_get", "MenuApi"],
+        ["can_list", "AsyncEventsRestApi"],
+        ["can_read", "AdvancedDataType"],
+        ["can_invalidate", "CacheRestApi"],
+        ["can_export", "Chart"],
+        ["can_read", "DashboardFilterStateRestApi"],
+        ["can_write", "DashboardFilterStateRestApi"],
+        ["can_read", "DashboardPermalinkRestApi"],
+        ["can_write", "DashboardPermalinkRestApi"],
+        ["can_delete_embedded", "Dashboard"],
+        ["can_get_embedded", "Dashboard"],
+        ["can_export", "Dashboard"],
+        ["can_read", "EmbeddedDashboard"],
+        ["can_read", "Explore"],
+        ["can_read", "ExploreFormDataRestApi"],
+        ["can_write", "ExploreFormDataRestApi"],
+        ["can_read", "ExplorePermalinkRestApi"],
+        ["can_write", "ExplorePermalinkRestApi"],
+        ["can_delete", "FilterSets"],
+        ["can_list", "FilterSets"],
+        ["can_edit", "FilterSets"],
+        ["can_add", "FilterSets"],
+        ["can_import_", "ImportExportRestApi"],
+        ["can_export", "ImportExportRestApi"],
+        ["can_export", "SavedQuery"],
+        ["can_show", "DynamicPlugin"],
+        ["can_list", "DynamicPlugin"],
+        ["can_time_range", "Api"],
+        ["can_query_form_data", "Api"],
+        ["can_query", "Api"],
+        ["can_this_form_post", "CsvToDatabaseView"],
+        ["can_this_form_get", "CsvToDatabaseView"],
+        ["can_this_form_post", "ExcelToDatabaseView"],
+        ["can_this_form_get", "ExcelToDatabaseView"],
+        ["can_this_form_post", "ColumnarToDatabaseView"],
+        ["can_this_form_get", "ColumnarToDatabaseView"],
+        ["can_get", "Datasource"],
+        ["can_external_metadata", "Datasource"],
+        ["can_external_metadata_by_name", "Datasource"],
+        ["can_get_value", "KV"],
+        ["can_store", "KV"],
+        ["can_my_queries", "SqlLab"],
+        ["can_created_dashboards", "Superset"],
+        ["can_testconn", "Superset"],
+        ["can_estimate_query_cost", "Superset"],
+        ["can_explore", "Superset"],
+        ["can_fetch_datasource_metadata", "Superset"],
+        ["can_search_queries", "Superset"],
+        ["can_save_dash", "Superset"],
+        ["can_dashboard_permalink", "Superset"],
+        ["can_warm_up_cache", "Superset"],
+        ["can_request_access", "Superset"],
+        ["can_datasources", "Superset"],
+        ["can_available_domains", "Superset"],
+        ["can_dashboard", "Superset"],
+        ["can_annotation_json", "Superset"],
+        ["can_created_slices", "Superset"],
+        ["can_slice_json", "Superset"],
+        ["can_profile", "Superset"],
+        ["can_filter", "Superset"],
+        ["can_validate_sql_json", "Superset"],
+        ["can_slice", "Superset"],
+        ["can_sqllab", "Superset"],
+        ["can_log", "Superset"],
+        ["can_recent_activity", "Superset"],
+        ["can_tables", "Superset"],
+        ["can_fave_slices", "Superset"],
+        ["can_sqllab_viz", "Superset"],
+        ["can_fave_dashboards", "Superset"],
+        ["can_results", "Superset"],
+        ["can_extra_table_metadata", "Superset"],
+        ["can_schemas_access_for_file_upload", "Superset"],
+        ["can_fave_dashboards_by_username", "Superset"],
+        ["can_csv", "Superset"],
+        ["can_add_slices", "Superset"],
+        ["can_explore_json", "Superset"],
+        ["can_sqllab_history", "Superset"],
+        ["can_import_dashboards", "Superset"],
+        ["can_sqllab_table_viz", "Superset"],
+        ["can_stop_query", "Superset"],
+        ["can_favstar", "Superset"],
+        ["can_copy_dash", "Superset"],
+        ["can_queries", "Superset"],
+        ["can_user_slices", "Superset"],
+        ["can_delete", "TableSchemaView"],
+        ["can_post", "TableSchemaView"],
+        ["can_expanded", "TableSchemaView"],
+        ["can_get", "TabStateView"],
+        ["can_post", "TabStateView"],
+        ["can_migrate_query", "TabStateView"],
+        ["can_put", "TabStateView"],
+        ["can_activate", "TabStateView"],
+        ["can_delete", "TabStateView"],
+        ["can_delete_query", "TabStateView"],
+        ["can_get", "TagView"],
+        ["can_tagged_objects", "TagView"],
+        ["can_post", "TagView"],
+        ["can_delete", "TagView"],
+        ["can_suggestions", "TagView"],
+        ["can_read", "SecurityRestApi"],
+        ["menu_access", "List Roles"],
+        ["menu_access", "Action Log"],
+        ["menu_access", "Access requests"],
+        ["menu_access", "Home"],
+        ["menu_access", "Annotation Layers"],
+        ["menu_access", "Plugins"],
+        ["menu_access", "Import Dashboards"],
+        ["menu_access", "Alerts & Report"],
+        ["menu_access", "Dashboards"],
+        ["menu_access", "Charts"],
+        ["menu_access", "SQL Editor"],
+        ["menu_access", "Saved Queries"],
+        ["menu_access", "Query Search"],
+        ["menu_access", "Data"],
+        ["menu_access", "Databases"],
+        ["menu_access", "Datasets"],
+        ["can_share_dashboard", "Superset"],
+        ["can_share_chart", "Superset"],
+    ],
+    "schema_access_role": [["schema_access", "[examples].[temp_schema]"]],
+    "dummy_role": [
+        ["datasource_access", "[examples].[wb_health_population](id:1)"],
+        ["database_access", "[examples].(id:1)"],
+    ],
+}
+
 
 def get_perm_tuples(role_name):
     perm_set = set()
@@ -1051,6 +1204,54 @@ def test_views_are_secured(self):
             view_str = "\n".join([str(v) for v in unsecured_views])
             raise Exception(f"Some views are not secured:\n{view_str}")
 
+    @patch("superset.utils.core.g")
+    @patch("superset.security.manager.g")
+    def test_get_permissions_gamma_user(self, mock_sm_g, mock_g):
+        session = db.session
+        role_name = "dummy_role"
+        gamma_user = security_manager.find_user(username="gamma")
+        security_manager.add_role(role_name)
+        dummy_role = security_manager.find_role(role_name)
+        gamma_user.roles.append(dummy_role)
+
+        table = (
+            db.session.query(SqlaTable)
+            .filter_by(table_name="wb_health_population")
+            .one()
+        )
+        table_perm = table.perm
+        security_manager.add_permission_role(
+            dummy_role,
+            security_manager.find_permission_view_menu("datasource_access", table_perm),
+        )
+        security_manager.add_permission_role(
+            dummy_role,
+            security_manager.find_permission_view_menu(
+                "database_access", table.database.perm
+            ),
+        )
+
+        session.commit()
+
+        mock_g.user = mock_sm_g.user = security_manager.find_user("gamma")
+        with self.client.application.test_request_context():
+            roles, permissions = security_manager.get_permissions(mock_g.user)
+            assert "dummy_role" in roles
+            assert "Gamma" in roles
+            assert sorted(roles["Gamma"]) == sorted(GAMMA_ROLE_PERMISSIONS["Gamma"])
+            assert sorted(roles["schema_access_role"]) == sorted(
+                GAMMA_ROLE_PERMISSIONS["schema_access_role"]
+            )
+
+            assert len(permissions) == 2
+            assert "[examples].(id:" in permissions["database_access"][0]
+            assert "[examples].[" in permissions["datasource_access"][0]
+
+        # cleanup
+        gamma_user = security_manager.find_user(username="gamma")
+        gamma_user.roles.remove(security_manager.find_role(role_name))
+        session.commit()
+
 
 class TestSecurityManager(SupersetTestCase):
     """