Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reorganize role permissions #23096

Merged
merged 4 commits into from
Feb 21, 2023
Merged

fix: reorganize role permissions #23096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3ba7c1
fix: reorganize role permissions
dpgaspar Feb 16, 2023
3798618
fix tests
dpgaspar Feb 16, 2023
86f735e
fix tests
dpgaspar Feb 16, 2023
6d1335e
fix tests
dpgaspar Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset/queries/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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"}

Expand Down
47 changes: 39 additions & 8 deletions superset/security/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,40 +154,53 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
READ_ONLY_MODEL_VIEWS = {"Database", "DruidClusterModelView", "DynamicPlugin"}

USER_MODEL_VIEWS = {
"RegisterUserModelView",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @dpgaspar is their any use case to add it. Can you elaborate if their is ?

"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 = {
Expand All @@ -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 = {
Expand All @@ -222,24 +236,41 @@ 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"),
("menu_access", "Query Search"),
}

SQLLAB_EXTRA_PERMISSION_VIEWS = {
("can_csv", "Superset"),
("can_csv", "Superset"), # Deprecated permission remove on 3.0.0
("can_read", "Superset"),
("can_read", "Database"),
}
Expand Down
2 changes: 1 addition & 1 deletion superset/sqllab/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
6 changes: 5 additions & 1 deletion tests/integration_tests/queries/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ def test_get_query_no_data_access(self):
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()
Expand Down Expand Up @@ -383,7 +387,7 @@ def test_get_list_query_no_data_access(self):
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)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/queries/saved_queries/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ def test_export_not_allowed(self):
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)
Expand Down
8 changes: 7 additions & 1 deletion tests/integration_tests/security_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1332,8 +1332,11 @@ def assert_cannot_menu(self, view_menu, permissions_set):
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)
Expand Down Expand Up @@ -1374,7 +1377,6 @@ def assert_can_alpha(self, perm_set):
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)
Expand Down Expand Up @@ -1504,6 +1506,8 @@ def test_alpha_permissions(self):
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):
Expand Down Expand Up @@ -1548,6 +1552,8 @@ def test_gamma_permissions(self):
# 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)
Expand Down