fix: SQLLab role permissions (#14372)

* fix: SQLLab role permissions * add missing perm * fix tests * fix security test * fix security test * fix tests
apache · Apr 29, 2021 · 6541a03 · 6541a03
1 parent 1c16261
commit 6541a03
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 23 deletions.
diff --git a/superset/constants.py b/superset/constants.py
@@ -116,6 +116,8 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "data_from_cache": "read",
     "get_charts": "read",
     "get_datasets": "read",
+    "function_names": "read",
+    "available": "read",
 }
 
 EXTRA_FORM_DATA_APPEND_KEYS = {

diff --git a/superset/security/manager.py b/superset/security/manager.py
@@ -184,6 +184,20 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
 
     ACCESSIBLE_PERMS = {"can_userinfo", "resetmypassword"}
 
+    SQLLAB_PERMISSION_VIEWS = {
+        ("can_csv", "Superset"),
+        ("can_read", "SavedQuery"),
+        ("can_read", "Database"),
+        ("can_sql_json", "Superset"),
+        ("can_sqllab_viz", "Superset"),
+        ("can_sqllab_table_viz", "Superset"),
+        ("can_sqllab", "Superset"),
+        ("menu_access", "SQL Lab"),
+        ("menu_access", "SQL Editor"),
+        ("menu_access", "Saved Queries"),
+        ("menu_access", "Query Search"),
+    }
+
     data_access_permissions = (
         "database_access",
         "schema_access",
@@ -820,24 +834,7 @@ def _is_sql_lab_pvm(self, pvm: PermissionView) -> bool:
         :param pvm: The FAB permission/view
         :returns: Whether the FAB object is SQL Lab related
         """
-
-        return (
-            pvm.view_menu.name
-            in {"SQL Lab", "SQL Editor", "Query Search", "Saved Queries"}
-            or pvm.permission.name
-            in {
-                "can_sql_json",
-                "can_csv",
-                "can_search_queries",
-                "can_sqllab_viz",
-                "can_sqllab_table_viz",
-                "can_sqllab",
-            }
-            or (
-                pvm.view_menu.name in self.USER_MODEL_VIEWS
-                and pvm.permission.name == "can_list"
-            )
-        )
+        return (pvm.permission.name, pvm.view_menu.name) in self.SQLLAB_PERMISSION_VIEWS
 
     def _is_granter_pvm(  # pylint: disable=no-self-use
         self, pvm: PermissionView

diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py
@@ -614,9 +614,7 @@ def test_info_security_database(self):
         assert rv.status_code == 200
         assert "can_read" in data["permissions"]
         assert "can_write" in data["permissions"]
-        assert "can_function_names" in data["permissions"]
-        assert "can_available" in data["permissions"]
-        assert len(data["permissions"]) == 4
+        assert len(data["permissions"]) == 2
 
     def test_get_invalid_database_table_metadata(self):
         """

diff --git a/tests/security_tests.py b/tests/security_tests.py
@@ -832,9 +832,18 @@ def test_admin_permissions(self):
 
     def test_sql_lab_permissions(self):
         sql_lab_set = get_perm_tuples("sql_lab")
-        self.assertIn(("can_sql_json", "Superset"), sql_lab_set)
         self.assertIn(("can_csv", "Superset"), sql_lab_set)
-        self.assertIn(("can_search_queries", "Superset"), sql_lab_set)
+        self.assertIn(("can_read", "Database"), sql_lab_set)
+        self.assertIn(("can_read", "SavedQuery"), sql_lab_set)
+        self.assertIn(("can_sql_json", "Superset"), sql_lab_set)
+        self.assertIn(("can_sqllab_viz", "Superset"), sql_lab_set)
+        self.assertIn(("can_sqllab_table_viz", "Superset"), sql_lab_set)
+        self.assertIn(("can_sqllab", "Superset"), sql_lab_set)
+
+        self.assertIn(("menu_access", "SQL Lab"), sql_lab_set)
+        self.assertIn(("menu_access", "SQL Editor"), sql_lab_set)
+        self.assertIn(("menu_access", "Saved Queries"), sql_lab_set)
+        self.assertIn(("menu_access", "Query Search"), sql_lab_set)
 
         self.assert_cannot_alpha(sql_lab_set)