From 6541a03d0b91b2482db7f70b776a67de93f9e280 Mon Sep 17 00:00:00 2001
From: Daniel Vaz Gaspar <danielvazgaspar@gmail.com>
Date: Thu, 29 Apr 2021 15:58:08 +0100
Subject: [PATCH] fix: SQLLab role permissions (#14372)

* fix: SQLLab role permissions

* add missing perm

* fix tests

* fix security test

* fix security test

* fix tests
---
 superset/constants.py        |  2 ++
 superset/security/manager.py | 33 +++++++++++++++------------------
 tests/databases/api_tests.py |  4 +---
 tests/security_tests.py      | 13 +++++++++++--
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/superset/constants.py b/superset/constants.py
index f69b7e92c6875..5fd59bd2896fb 100644
--- 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
index de608da165971..47b9bc2d94d1e 100644
--- 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
index fc12fe0a4acd7..1ce922f09b002 100644
--- 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
index f7e55174db924..ee8b5aab0c29d 100644
--- 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)