From d4362a367657f1a70ba04f6401c75ee2441f0b40 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Tue, 21 Feb 2023 10:19:42 +0000 Subject: [PATCH] fix: reorganize role permissions (#23096) --- superset/queries/api.py | 2 +- superset/security/manager.py | 47 +++++++++++++++---- superset/sqllab/api.py | 2 +- tests/integration_tests/queries/api_tests.py | 6 ++- .../queries/saved_queries/api_tests.py | 2 +- tests/integration_tests/security_tests.py | 8 +++- 6 files changed, 54 insertions(+), 13 deletions(-) diff --git a/superset/queries/api.py b/superset/queries/api.py index 1784edf167..b5737ea812 100644 --- a/superset/queries/api.py +++ b/superset/queries/api.py @@ -137,6 +137,7 @@ class QueryRestApi(BaseSupersetModelRestApi): base_related_field_filters = { "created_by": [["id", BaseFilterRelatedUsers, lambda: []]], "user": [["id", BaseFilterRelatedUsers, lambda: []]], + "database": [["id", DatabaseFilter, lambda: []]], } related_field_filters = { "created_by": RelatedFieldFilter("first_name", FilterRelatedOwners), @@ -145,7 +146,6 @@ class QueryRestApi(BaseSupersetModelRestApi): search_columns = ["changed_on", "database", "sql", "status", "user", "start_time"] - base_related_field_filters = {"database": [["id", DatabaseFilter, lambda: []]]} allowed_rel_fields = {"database", "user"} allowed_distinct_fields = {"status"} diff --git a/superset/security/manager.py b/superset/security/manager.py index b3fa1a6c53..bc6ffe943d 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -154,40 +154,53 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods READ_ONLY_MODEL_VIEWS = {"Database", "DruidClusterModelView", "DynamicPlugin"} USER_MODEL_VIEWS = { + "RegisterUserModelView", "UserDBModelView", "UserLDAPModelView", + "UserInfoEditView", "UserOAuthModelView", "UserOIDModelView", "UserRemoteUserModelView", } GAMMA_READ_ONLY_MODEL_VIEWS = { + "Annotation", + "CssTemplate", "Dataset", "Datasource", - "CssTemplate", } | READ_ONLY_MODEL_VIEWS ADMIN_ONLY_VIEW_MENUS = { + "Access Requests", "AccessRequestsModelView", - "SQL Lab", + "Action Log", + "Log", + "List Users", + "List Roles", "Refresh Druid Metadata", "ResetPasswordView", "RoleModelView", - "Log", - "Security", "Row Level Security", "Row Level Security Filters", "RowLevelSecurityFiltersModelView", + "Security", + "SQL Lab", } | USER_MODEL_VIEWS ALPHA_ONLY_VIEW_MENUS = { "Manage", "CSS Templates", + "Annotation Layers", "Queries", "Import dashboards", "Upload a CSV", "ReportSchedule", "Alerts & Report", + "TableSchemaView", + "CsvToDatabaseView", + "ColumnarToDatabaseView", + "ExcelToDatabaseView", + "ImportExportRestApi", } ADMIN_ONLY_PERMISSIONS = { @@ -199,6 +212,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "all_query_access", "can_grant_guest_token", "can_set_embedded", + "can_warm_up_cache", } READ_ONLY_PERMISSION = { @@ -222,16 +236,33 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "datasource_access", } - ACCESSIBLE_PERMS = {"can_userinfo", "resetmypassword"} + ACCESSIBLE_PERMS = {"can_userinfo", "resetmypassword", "can_recent_activity"} SQLLAB_ONLY_PERMISSIONS = { ("can_my_queries", "SqlLab"), ("can_read", "SavedQuery"), - ("can_sql_json", "Superset"), + ("can_write", "SavedQuery"), + ("can_export", "SavedQuery"), + ("can_read", "Query"), + ("can_export_csv", "Query"), + ("can_get_results", "SQLLab"), + ("can_execute_sql_query", "SQLLab"), + ("can_export_csv", "SQLLab"), + ("can_sql_json", "Superset"), # Deprecated permission remove on 3.0.0 ("can_sqllab_history", "Superset"), ("can_sqllab_viz", "Superset"), - ("can_sqllab_table_viz", "Superset"), + ("can_sqllab_table_viz", "Superset"), # Deprecated permission remove on 3.0.0 ("can_sqllab", "Superset"), + ("can_stop_query", "Superset"), # Deprecated permission remove on 3.0.0 + ("can_test_conn", "Superset"), # Deprecated permission remove on 3.0.0 + ("can_search_queries", "Superset"), # Deprecated permission remove on 3.0.0 + ("can_activate", "TabStateView"), + ("can_get", "TabStateView"), + ("can_delete_query", "TabStateView"), + ("can_post", "TabStateView"), + ("can_delete", "TabStateView"), + ("can_put", "TabStateView"), + ("can_migrate_query", "TabStateView"), ("menu_access", "SQL Lab"), ("menu_access", "SQL Editor"), ("menu_access", "Saved Queries"), @@ -239,7 +270,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods } SQLLAB_EXTRA_PERMISSION_VIEWS = { - ("can_csv", "Superset"), + ("can_csv", "Superset"), # Deprecated permission remove on 3.0.0 ("can_read", "Superset"), ("can_read", "Database"), } diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py index afe68323fb..5915601c0d 100644 --- a/superset/sqllab/api.py +++ b/superset/sqllab/api.py @@ -68,7 +68,7 @@ class SqlLabRestApi(BaseSupersetApi): resource_name = "sqllab" allow_browser_login = True - class_permission_name = "Query" + class_permission_name = "SQLLab" execute_model_schema = ExecutePayloadSchema() diff --git a/tests/integration_tests/queries/api_tests.py b/tests/integration_tests/queries/api_tests.py index b491cf8498..7abcb31df1 100644 --- a/tests/integration_tests/queries/api_tests.py +++ b/tests/integration_tests/queries/api_tests.py @@ -202,6 +202,10 @@ class TestQueryApi(SupersetTestCase): gamma2 = self.create_user( "gamma_2", "password", "Gamma", email="gamma2@superset.org" ) + # Add SQLLab role to these gamma users, so they have access to queries + sqllab_role = self.get_role("sql_lab") + gamma1.roles.append(sqllab_role) + gamma2.roles.append(sqllab_role) gamma1_client_id = self.get_random_string() gamma2_client_id = self.get_random_string() @@ -383,7 +387,7 @@ class TestQueryApi(SupersetTestCase): sql="SELECT col1, col2 from table1", ) - self.login(username="gamma") + self.login(username="gamma_sqllab") arguments = {"filters": [{"col": "sql", "opr": "sw", "value": "SELECT col1"}]} uri = f"api/v1/query/?q={prison.dumps(arguments)}" rv = self.client.get(uri) diff --git a/tests/integration_tests/queries/saved_queries/api_tests.py b/tests/integration_tests/queries/saved_queries/api_tests.py index 91843b0085..09929e4d23 100644 --- a/tests/integration_tests/queries/saved_queries/api_tests.py +++ b/tests/integration_tests/queries/saved_queries/api_tests.py @@ -748,7 +748,7 @@ class TestSavedQueryApi(SupersetTestCase): db.session.query(SavedQuery).filter(SavedQuery.created_by == admin).first() ) - self.login(username="gamma") + self.login(username="gamma_sqllab") argument = [sample_query.id] uri = f"api/v1/saved_query/export/?q={prison.dumps(argument)}" rv = self.client.get(uri) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 6ee6bc6524..c65f5a6dd8 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1332,8 +1332,11 @@ class TestRolePermission(SupersetTestCase): self.assertNotIn(("menu_access", view_menu), permissions_set) def assert_cannot_gamma(self, perm_set): + self.assert_cannot_write("Annotation", perm_set) self.assert_cannot_write("CssTemplate", perm_set) + self.assert_cannot_menu("SQL Lab", perm_set) self.assert_cannot_menu("CSS Templates", perm_set) + self.assert_cannot_menu("Annotation Layers", perm_set) self.assert_cannot_menu("Manage", perm_set) self.assert_cannot_menu("Queries", perm_set) self.assert_cannot_menu("Import dashboards", perm_set) @@ -1374,7 +1377,6 @@ class TestRolePermission(SupersetTestCase): self.assert_can_all("Annotation", perm_set) self.assert_can_all("CssTemplate", perm_set) self.assert_can_all("Dataset", perm_set) - self.assert_can_read("Query", perm_set) self.assert_can_read("Database", perm_set) self.assertIn(("can_import_dashboards", "Superset"), perm_set) self.assertIn(("can_this_form_post", "CsvToDatabaseView"), perm_set) @@ -1504,6 +1506,8 @@ class TestRolePermission(SupersetTestCase): self.assert_can_gamma(alpha_perm_tuples) self.assert_can_alpha(alpha_perm_tuples) self.assert_cannot_alpha(alpha_perm_tuples) + self.assertNotIn(("can_this_form_get", "UserInfoEditView"), alpha_perm_tuples) + self.assertNotIn(("can_this_form_post", "UserInfoEditView"), alpha_perm_tuples) @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") def test_admin_permissions(self): @@ -1548,6 +1552,8 @@ class TestRolePermission(SupersetTestCase): # make sure that user can create slices and dashboards self.assert_can_all("Dashboard", gamma_perm_set) self.assert_can_read("Dataset", gamma_perm_set) + self.assert_can_read("Annotation", gamma_perm_set) + self.assert_can_read("CssTemplate", gamma_perm_set) # make sure that user can create slices and dashboards self.assert_can_all("Chart", gamma_perm_set)