From a21a4175c3fc1ff5c4189bd782be747e0de5a108 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 7 Dec 2021 14:13:44 -0800 Subject: [PATCH 01/31] helper methods and dashboard access --- superset/common/request_contexed_based.py | 16 +-------------- superset/config.py | 2 +- superset/dashboards/filters.py | 24 ++++++++++++++++++----- superset/security/api.py | 2 +- superset/security/guest_token.py | 14 ++++++++++--- superset/security/manager.py | 23 ++++++++++++++++------ superset/views/base.py | 9 +-------- superset/views/core.py | 5 +++-- 8 files changed, 54 insertions(+), 41 deletions(-) diff --git a/superset/common/request_contexed_based.py b/superset/common/request_contexed_based.py index 0b06a0ccbe1d..5d8405e36cf0 100644 --- a/superset/common/request_contexed_based.py +++ b/superset/common/request_contexed_based.py @@ -16,24 +16,10 @@ # under the License. from __future__ import annotations -from typing import List, TYPE_CHECKING - -from flask import g - from superset import conf, security_manager -if TYPE_CHECKING: - from flask_appbuilder.security.sqla.models import Role - - -def get_user_roles() -> List[Role]: - if g.user.is_anonymous: - public_role = conf.get("AUTH_ROLE_PUBLIC") - return [security_manager.get_public_role()] if public_role else [] - return g.user.roles - def is_user_admin() -> bool: - user_roles = [role.name.lower() for role in get_user_roles()] + user_roles = [role.name.lower() for role in security_manager.get_user_roles()] admin_role = conf.get("AUTH_ROLE_ADMIN").lower() return admin_role in user_roles diff --git a/superset/config.py b/superset/config.py index f9b734340dbc..2ff2f1a2389d 100644 --- a/superset/config.py +++ b/superset/config.py @@ -389,7 +389,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # a custom security config could potentially give access to setting filters on # tables that users do not have access to. "ROW_LEVEL_SECURITY": True, - "EMBEDDED_SUPERSET": False, + "EMBEDDED_SUPERSET": False, # This requires that the public role be available # Enables Alerts and reports new implementation "ALERT_REPORTS": False, # Enable experimental feature to search for other dashboards diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index 96584784f449..3d49978702fa 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -16,6 +16,7 @@ # under the License. from typing import Any, Optional +from flask import g from flask_appbuilder.security.sqla.models import Role from flask_babel import lazy_gettext as _ from sqlalchemy import and_, or_ @@ -25,7 +26,8 @@ from superset.models.core import FavStar from superset.models.dashboard import Dashboard from superset.models.slice import Slice -from superset.views.base import BaseFilter, get_user_roles, is_user_admin +from superset.security.guest_token import GuestTokenResourceType, GuestUser +from superset.views.base import BaseFilter, is_user_admin from superset.views.base_api import BaseFavoriteFilter @@ -112,7 +114,7 @@ def apply(self, query: Query, value: Any) -> Query: ) ) - dashboard_rbac_or_filters = [] + feature_flagged_filters = [] if is_feature_enabled("DASHBOARD_RBAC"): roles_based_query = ( db.session.query(Dashboard.id) @@ -121,19 +123,31 @@ def apply(self, query: Query, value: Any) -> Query: and_( Dashboard.published.is_(True), dashboard_has_roles, - Role.id.in_([x.id for x in get_user_roles()]), + Role.id.in_([x.id for x in security_manager.get_user_roles()]), ), ) ) - dashboard_rbac_or_filters.append(Dashboard.id.in_(roles_based_query)) + feature_flagged_filters.append(Dashboard.id.in_(roles_based_query)) + + if is_feature_enabled("EMBEDDED_SUPERSET") and security_manager.is_guest_user( + g.user + ): + guest_user: GuestUser = g.user + embedded_dashboard_ids = [ + r["id"] + for r in guest_user.resources + if r["type"] == GuestTokenResourceType.DASHBOARD + ] + if len(embedded_dashboard_ids) != 0: + feature_flagged_filters.append(Dashboard.id.in_(embedded_dashboard_ids)) query = query.filter( or_( Dashboard.id.in_(owner_ids_query), Dashboard.id.in_(datasource_perm_query), Dashboard.id.in_(users_favorite_dash_query), - *dashboard_rbac_or_filters, + *feature_flagged_filters, ) ) diff --git a/superset/security/api.py b/superset/security/api.py index c0a4a77b2422..54efcd07e0db 100644 --- a/superset/security/api.py +++ b/superset/security/api.py @@ -35,7 +35,7 @@ class UserSchema(Schema): class ResourceSchema(Schema): - type = fields.String(required=True) + type = fields.String(required=True) # todo figure out how to make this an enum id = fields.String(required=True) rls = fields.String() diff --git a/superset/security/guest_token.py b/superset/security/guest_token.py index cbef52f008e3..60add8175400 100644 --- a/superset/security/guest_token.py +++ b/superset/security/guest_token.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from enum import Enum from typing import List, Optional, TypedDict, Union from flask_appbuilder.security.sqla.models import Role @@ -26,17 +27,24 @@ class GuestTokenUser(TypedDict, total=False): last_name: str +class GuestTokenResourceType(Enum): + DASHBOARD = "dashboard" + + class GuestTokenResource(TypedDict): - type: str + type: GuestTokenResourceType id: Union[str, int] rls: Optional[str] +GuestTokenResources = List[GuestTokenResource] + + class GuestToken(TypedDict): iat: float exp: float user: GuestTokenUser - resources: List[GuestTokenResource] + resources: GuestTokenResources class GuestUser(AnonymousUserMixin): @@ -50,7 +58,7 @@ class GuestUser(AnonymousUserMixin): def is_authenticated(self) -> bool: """ This is set to true because guest users should be considered authenticated, - at least in most places. The treatment of this flag is pretty inconsistent. + at least in most places. The treatment of this flag is kind of inconsistent. """ return True diff --git a/superset/security/manager.py b/superset/security/manager.py index 1a0a4532f5cd..6bd31f2e9833 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -68,6 +68,7 @@ from superset.security.guest_token import ( GuestToken, GuestTokenResource, + GuestTokenResources, GuestTokenUser, GuestUser, ) @@ -278,7 +279,7 @@ def can_access(self, permission_name: str, view_name: str) -> bool: """ user = g.user - if user.is_anonymous: + if user.is_anonymous and not self.is_guest_user(user): return self.is_item_public(permission_name, view_name) return self._has_view_access(user, permission_name, view_name) @@ -1097,6 +1098,12 @@ def get_user_by_username( def get_anonymous_user(self) -> User: # pylint: disable=no-self-use return AnonymousUserMixin() + def get_user_roles(self) -> List[Role]: + if g.user.is_anonymous: + public_role = current_app.config.get("AUTH_ROLE_PUBLIC") + return [self.get_public_role()] if public_role else [] + return g.user.roles + def get_rls_filters(self, table: "BaseDatasource") -> List[SqlaQuery]: """ Retrieves the appropriate row level security filters for the current user and @@ -1195,8 +1202,7 @@ def raise_for_user_activity_access(user_id: int) -> None: ) ) - @staticmethod - def raise_for_dashboard_access(dashboard: "Dashboard") -> None: + def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: """ Raise an exception if the user cannot access the dashboard. @@ -1206,14 +1212,15 @@ def raise_for_dashboard_access(dashboard: "Dashboard") -> None: # pylint: disable=import-outside-toplevel from superset import is_feature_enabled from superset.dashboards.commands.exceptions import DashboardAccessDeniedError - from superset.views.base import get_user_roles, is_user_admin + from superset.views.base import is_user_admin from superset.views.utils import is_owner has_rbac_access = True if is_feature_enabled("DASHBOARD_RBAC"): has_rbac_access = any( - dashboard_role.id in [user_role.id for user_role in get_user_roles()] + dashboard_role.id + in [user_role.id for user_role in self.get_user_roles()] for dashboard_role in dashboard.roles ) @@ -1255,7 +1262,7 @@ def _get_current_epoch_time() -> float: return time.time() def create_guest_access_token( - self, user: GuestTokenUser, resources: List[GuestTokenResource] + self, user: GuestTokenUser, resources: GuestTokenResources ) -> bytes: secret = current_app.config["GUEST_TOKEN_JWT_SECRET"] algo = current_app.config["GUEST_TOKEN_JWT_ALGO"] @@ -1319,3 +1326,7 @@ def parse_jwt_guest_token(raw_token: str) -> GuestToken: if token.get("resources") is None: raise ValueError("Guest token does not contain a resources claim") return cast(GuestToken, token) + + @staticmethod + def is_guest_user(user: Any) -> bool: + return hasattr(user, "is_guest_user") and user.is_guest_user diff --git a/superset/views/base.py b/superset/views/base.py index 72dc8053d461..e5fc08200da0 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -263,15 +263,8 @@ def create_table_permissions(table: models.SqlaTable) -> None: security_manager.add_permission_view_menu("schema_access", table.schema_perm) -def get_user_roles() -> List[Role]: - if g.user.is_anonymous: - public_role = conf.get("AUTH_ROLE_PUBLIC") - return [security_manager.find_role(public_role)] if public_role else [] - return g.user.roles - - def is_user_admin() -> bool: - user_roles = [role.name.lower() for role in list(get_user_roles())] + user_roles = [role.name.lower() for role in list(security_manager.get_user_roles())] return "admin" in user_roles diff --git a/superset/views/core.py b/superset/views/core.py index 9557c30e83d9..68f6e78ed64b 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -135,7 +135,6 @@ data_payload_response, generate_download_headers, get_error_msg, - get_user_roles, handle_api_exception, json_error_response, json_errors_response, @@ -1886,7 +1885,9 @@ def publish( # pylint: disable=no-self-use f"ERROR: cannot find dashboard {dashboard_id}", status=404 ) - edit_perm = is_owner(dash, g.user) or admin_role in get_user_roles() + edit_perm = ( + is_owner(dash, g.user) or admin_role in security_manager.get_user_roles() + ) if not edit_perm: username = g.user.username if hasattr(g.user, "username") else "user" return json_error_response( From ad3572e54304323dac86023fc8907e54acb15686 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Sat, 11 Dec 2021 02:20:07 -0800 Subject: [PATCH 02/31] guest token dashboard authz --- superset/security/manager.py | 51 +++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 6bd31f2e9833..9a7752830108 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -69,6 +69,7 @@ GuestToken, GuestTokenResource, GuestTokenResources, + GuestTokenResourceType, GuestTokenUser, GuestUser, ) @@ -1068,11 +1069,17 @@ def raise_for_access( assert datasource + is_dashboard_access_check_applicable = feature_flag_manager.is_feature_enabled( + "DASHBOARD_RBAC" + ) or feature_flag_manager.is_feature_enabled( + "EMBEDDED_SUPERSET" + ) + if not ( self.can_access_schema(datasource) or self.can_access("datasource_access", datasource.perm or "") or ( - feature_flag_manager.is_feature_enabled("DASHBOARD_RBAC") + is_dashboard_access_check_applicable and self.can_access_based_on_dashboard(datasource) ) ): @@ -1215,19 +1222,25 @@ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: from superset.views.base import is_user_admin from superset.views.utils import is_owner - has_rbac_access = True - - if is_feature_enabled("DASHBOARD_RBAC"): - has_rbac_access = any( + def has_rbac_access() -> bool: + return is_feature_enabled("DASHBOARD_RBAC") and any( dashboard_role.id in [user_role.id for user_role in self.get_user_roles()] for dashboard_role in dashboard.roles ) + has_published_access = ( + not is_feature_enabled("DASHBOARD_RBAC") + and not self.is_guest_user(g.user) + and dashboard.published + ) + can_access = ( is_user_admin() or is_owner(dashboard, g.user) - or (dashboard.published and has_rbac_access) + or has_published_access + or has_rbac_access() + or self.has_guest_access(GuestTokenResourceType.DASHBOARD, dashboard.id) or (not dashboard.published and not dashboard.roles) ) @@ -1329,4 +1342,30 @@ def parse_jwt_guest_token(raw_token: str) -> GuestToken: @staticmethod def is_guest_user(user: Any) -> bool: + # pylint: disable=import-outside-toplevel + from superset import is_feature_enabled + + if not is_feature_enabled("EMBEDDED_SUPERSET"): + return False return hasattr(user, "is_guest_user") and user.is_guest_user + + def get_current_guest_user_if_guest(self) -> Optional[GuestUser]: + # pylint: disable=import-outside-toplevel + from superset.extensions import feature_flag_manager + + if self.is_guest_user(g.user): + return g.user + return None + + def has_guest_access( + self, resource_type: GuestTokenResourceType, id: Union[str, int] + ) -> bool: + user = self.get_current_guest_user_if_guest() + if not user: + return False + + strid = str(id) + for resource in user.resources: + if resource["type"] == resource_type.value and str(resource["id"]) == strid: + return True + return False From 4fd8715b42185477706bdbd1493635636abe3a22 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Sat, 11 Dec 2021 02:35:24 -0800 Subject: [PATCH 03/31] adjust csrf exempt list --- superset/config.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/superset/config.py b/superset/config.py index 2ff2f1a2389d..f7a5bc521f5a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -191,7 +191,11 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: WTF_CSRF_ENABLED = True # Add endpoints that need to be exempt from CSRF protection -WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.data.api.data"] +WTF_CSRF_EXEMPT_LIST = [ + "superset.views.core.log", + "superset.views.core.explore_json", + "superset.charts.data.api.data", +] # Whether to run the web server in debug mode or not DEBUG = os.environ.get("FLASK_ENV") == "development" From 7353826f4fc73c752df0d5193b1e888ebcebcc6b Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Sat, 11 Dec 2021 02:35:35 -0800 Subject: [PATCH 04/31] eums don't work that way --- superset/dashboards/filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index 3d49978702fa..e398af97b744 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -137,7 +137,7 @@ def apply(self, query: Query, value: Any) -> Query: embedded_dashboard_ids = [ r["id"] for r in guest_user.resources - if r["type"] == GuestTokenResourceType.DASHBOARD + if r["type"] == GuestTokenResourceType.DASHBOARD.value ] if len(embedded_dashboard_ids) != 0: feature_flagged_filters.append(Dashboard.id.in_(embedded_dashboard_ids)) From 5c0a7f5bbf029fdeaeab075f48e5abb0cabae67f Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Mon, 20 Dec 2021 13:31:39 -0800 Subject: [PATCH 05/31] Remove unnecessary import --- superset/security/manager.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 9a7752830108..e9e2c614a5e2 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1350,8 +1350,6 @@ def is_guest_user(user: Any) -> bool: return hasattr(user, "is_guest_user") and user.is_guest_user def get_current_guest_user_if_guest(self) -> Optional[GuestUser]: - # pylint: disable=import-outside-toplevel - from superset.extensions import feature_flag_manager if self.is_guest_user(g.user): return g.user From 2141f2dee0fe4dd53a9f2b978dcd86676b4cded1 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 21 Dec 2021 18:45:06 -0800 Subject: [PATCH 06/31] move row level security tests to their own file --- .../security/row_level_security_tests.py | 195 ++++++++++++++++++ tests/integration_tests/security_tests.py | 169 --------------- 2 files changed, 195 insertions(+), 169 deletions(-) create mode 100644 tests/integration_tests/security/row_level_security_tests.py diff --git a/tests/integration_tests/security/row_level_security_tests.py b/tests/integration_tests/security/row_level_security_tests.py new file mode 100644 index 000000000000..d159c27fa8b1 --- /dev/null +++ b/tests/integration_tests/security/row_level_security_tests.py @@ -0,0 +1,195 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# isort:skip_file +import re +from typing import Any, Dict + +import pytest +from flask import g + +from superset import db, security_manager +from superset.connectors.sqla.models import RowLevelSecurityFilter, SqlaTable +from ..base_tests import SupersetTestCase + + +class TestRowLevelSecurity(SupersetTestCase): + """ + Testing Row Level Security + """ + + rls_entry = None + query_obj: Dict[str, Any] = dict( + groupby=[], + metrics=None, + filter=[], + is_timeseries=False, + columns=["value"], + granularity=None, + from_dttm=None, + to_dttm=None, + extras={}, + ) + NAME_AB_ROLE = "NameAB" + NAME_Q_ROLE = "NameQ" + NAMES_A_REGEX = re.compile(r"name like 'A%'") + NAMES_B_REGEX = re.compile(r"name like 'B%'") + NAMES_Q_REGEX = re.compile(r"name like 'Q%'") + BASE_FILTER_REGEX = re.compile(r"gender = 'boy'") + + def setUp(self): + session = db.session + + # Create roles + security_manager.add_role(self.NAME_AB_ROLE) + security_manager.add_role(self.NAME_Q_ROLE) + gamma_user = security_manager.find_user(username="gamma") + gamma_user.roles.append(security_manager.find_role(self.NAME_AB_ROLE)) + gamma_user.roles.append(security_manager.find_role(self.NAME_Q_ROLE)) + self.create_user_with_roles("NoRlsRoleUser", ["Gamma"]) + session.commit() + + # Create regular RowLevelSecurityFilter (energy_usage, unicode_test) + self.rls_entry1 = RowLevelSecurityFilter() + self.rls_entry1.tables.extend( + session.query(SqlaTable) + .filter(SqlaTable.table_name.in_(["energy_usage", "unicode_test"])) + .all() + ) + self.rls_entry1.filter_type = "Regular" + self.rls_entry1.clause = "value > {{ cache_key_wrapper(1) }}" + self.rls_entry1.group_key = None + self.rls_entry1.roles.append(security_manager.find_role("Gamma")) + self.rls_entry1.roles.append(security_manager.find_role("Alpha")) + db.session.add(self.rls_entry1) + + # Create regular RowLevelSecurityFilter (birth_names name starts with A or B) + self.rls_entry2 = RowLevelSecurityFilter() + self.rls_entry2.tables.extend( + session.query(SqlaTable) + .filter(SqlaTable.table_name.in_(["birth_names"])) + .all() + ) + self.rls_entry2.filter_type = "Regular" + self.rls_entry2.clause = "name like 'A%' or name like 'B%'" + self.rls_entry2.group_key = "name" + self.rls_entry2.roles.append(security_manager.find_role("NameAB")) + db.session.add(self.rls_entry2) + + # Create Regular RowLevelSecurityFilter (birth_names name starts with Q) + self.rls_entry3 = RowLevelSecurityFilter() + self.rls_entry3.tables.extend( + session.query(SqlaTable) + .filter(SqlaTable.table_name.in_(["birth_names"])) + .all() + ) + self.rls_entry3.filter_type = "Regular" + self.rls_entry3.clause = "name like 'Q%'" + self.rls_entry3.group_key = "name" + self.rls_entry3.roles.append(security_manager.find_role("NameQ")) + db.session.add(self.rls_entry3) + + # Create Base RowLevelSecurityFilter (birth_names boys) + self.rls_entry4 = RowLevelSecurityFilter() + self.rls_entry4.tables.extend( + session.query(SqlaTable) + .filter(SqlaTable.table_name.in_(["birth_names"])) + .all() + ) + self.rls_entry4.filter_type = "Base" + self.rls_entry4.clause = "gender = 'boy'" + self.rls_entry4.group_key = "gender" + self.rls_entry4.roles.append(security_manager.find_role("Admin")) + db.session.add(self.rls_entry4) + + db.session.commit() + + def tearDown(self): + session = db.session + session.delete(self.rls_entry1) + session.delete(self.rls_entry2) + session.delete(self.rls_entry3) + session.delete(self.rls_entry4) + session.delete(security_manager.find_role("NameAB")) + session.delete(security_manager.find_role("NameQ")) + session.delete(self.get_user("NoRlsRoleUser")) + session.commit() + + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_rls_filter_alters_energy_query(self): + g.user = self.get_user(username="alpha") + tbl = self.get_table(name="energy_usage") + sql = tbl.get_query_str(self.query_obj) + assert tbl.get_extra_cache_keys(self.query_obj) == [1] + assert "value > 1" in sql + + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_rls_filter_doesnt_alter_energy_query(self): + g.user = self.get_user( + username="admin" + ) # self.login() doesn't actually set the user + tbl = self.get_table(name="energy_usage") + sql = tbl.get_query_str(self.query_obj) + assert tbl.get_extra_cache_keys(self.query_obj) == [] + assert "value > 1" not in sql + + @pytest.mark.usefixtures("load_unicode_dashboard_with_slice") + def test_multiple_table_filter_alters_another_tables_query(self): + g.user = self.get_user( + username="alpha" + ) # self.login() doesn't actually set the user + tbl = self.get_table(name="unicode_test") + sql = tbl.get_query_str(self.query_obj) + assert tbl.get_extra_cache_keys(self.query_obj) == [1] + assert "value > 1" in sql + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_rls_filter_alters_gamma_birth_names_query(self): + g.user = self.get_user(username="gamma") + tbl = self.get_table(name="birth_names") + sql = tbl.get_query_str(self.query_obj) + + # establish that the filters are grouped together correctly with + # ANDs, ORs and parens in the correct place + assert ( + "WHERE ((name like 'A%'\n or name like 'B%')\n OR (name like 'Q%'))\n AND (gender = 'boy');" + in sql + ) + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_rls_filter_alters_no_role_user_birth_names_query(self): + g.user = self.get_user(username="NoRlsRoleUser") + tbl = self.get_table(name="birth_names") + sql = tbl.get_query_str(self.query_obj) + + # gamma's filters should not be present query + assert not self.NAMES_A_REGEX.search(sql) + assert not self.NAMES_B_REGEX.search(sql) + assert not self.NAMES_Q_REGEX.search(sql) + # base query should be present + assert self.BASE_FILTER_REGEX.search(sql) + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_rls_filter_doesnt_alter_admin_birth_names_query(self): + g.user = self.get_user(username="admin") + tbl = self.get_table(name="birth_names") + sql = tbl.get_query_str(self.query_obj) + + # no filters are applied for admin user + assert not self.NAMES_A_REGEX.search(sql) + assert not self.NAMES_B_REGEX.search(sql) + assert not self.NAMES_Q_REGEX.search(sql) + assert not self.BASE_FILTER_REGEX.search(sql) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 2168e8c1f41f..15f06577b496 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1048,175 +1048,6 @@ def test_raise_for_access_viz(self, mock_can_access_schema, mock_can_access): security_manager.raise_for_access(viz=test_viz) -class TestRowLevelSecurity(SupersetTestCase): - """ - Testing Row Level Security - """ - - rls_entry = None - query_obj: Dict[str, Any] = dict( - groupby=[], - metrics=None, - filter=[], - is_timeseries=False, - columns=["value"], - granularity=None, - from_dttm=None, - to_dttm=None, - extras={}, - ) - NAME_AB_ROLE = "NameAB" - NAME_Q_ROLE = "NameQ" - NAMES_A_REGEX = re.compile(r"name like 'A%'") - NAMES_B_REGEX = re.compile(r"name like 'B%'") - NAMES_Q_REGEX = re.compile(r"name like 'Q%'") - BASE_FILTER_REGEX = re.compile(r"gender = 'boy'") - - def setUp(self): - session = db.session - - # Create roles - security_manager.add_role(self.NAME_AB_ROLE) - security_manager.add_role(self.NAME_Q_ROLE) - gamma_user = security_manager.find_user(username="gamma") - gamma_user.roles.append(security_manager.find_role(self.NAME_AB_ROLE)) - gamma_user.roles.append(security_manager.find_role(self.NAME_Q_ROLE)) - self.create_user_with_roles("NoRlsRoleUser", ["Gamma"]) - session.commit() - - # Create regular RowLevelSecurityFilter (energy_usage, unicode_test) - self.rls_entry1 = RowLevelSecurityFilter() - self.rls_entry1.tables.extend( - session.query(SqlaTable) - .filter(SqlaTable.table_name.in_(["energy_usage", "unicode_test"])) - .all() - ) - self.rls_entry1.filter_type = "Regular" - self.rls_entry1.clause = "value > {{ cache_key_wrapper(1) }}" - self.rls_entry1.group_key = None - self.rls_entry1.roles.append(security_manager.find_role("Gamma")) - self.rls_entry1.roles.append(security_manager.find_role("Alpha")) - db.session.add(self.rls_entry1) - - # Create regular RowLevelSecurityFilter (birth_names name starts with A or B) - self.rls_entry2 = RowLevelSecurityFilter() - self.rls_entry2.tables.extend( - session.query(SqlaTable) - .filter(SqlaTable.table_name.in_(["birth_names"])) - .all() - ) - self.rls_entry2.filter_type = "Regular" - self.rls_entry2.clause = "name like 'A%' or name like 'B%'" - self.rls_entry2.group_key = "name" - self.rls_entry2.roles.append(security_manager.find_role("NameAB")) - db.session.add(self.rls_entry2) - - # Create Regular RowLevelSecurityFilter (birth_names name starts with Q) - self.rls_entry3 = RowLevelSecurityFilter() - self.rls_entry3.tables.extend( - session.query(SqlaTable) - .filter(SqlaTable.table_name.in_(["birth_names"])) - .all() - ) - self.rls_entry3.filter_type = "Regular" - self.rls_entry3.clause = "name like 'Q%'" - self.rls_entry3.group_key = "name" - self.rls_entry3.roles.append(security_manager.find_role("NameQ")) - db.session.add(self.rls_entry3) - - # Create Base RowLevelSecurityFilter (birth_names boys) - self.rls_entry4 = RowLevelSecurityFilter() - self.rls_entry4.tables.extend( - session.query(SqlaTable) - .filter(SqlaTable.table_name.in_(["birth_names"])) - .all() - ) - self.rls_entry4.filter_type = "Base" - self.rls_entry4.clause = "gender = 'boy'" - self.rls_entry4.group_key = "gender" - self.rls_entry4.roles.append(security_manager.find_role("Admin")) - db.session.add(self.rls_entry4) - - db.session.commit() - - def tearDown(self): - session = db.session - session.delete(self.rls_entry1) - session.delete(self.rls_entry2) - session.delete(self.rls_entry3) - session.delete(self.rls_entry4) - session.delete(security_manager.find_role("NameAB")) - session.delete(security_manager.find_role("NameQ")) - session.delete(self.get_user("NoRlsRoleUser")) - session.commit() - - @pytest.mark.usefixtures("load_energy_table_with_slice") - def test_rls_filter_alters_energy_query(self): - g.user = self.get_user(username="alpha") - tbl = self.get_table(name="energy_usage") - sql = tbl.get_query_str(self.query_obj) - assert tbl.get_extra_cache_keys(self.query_obj) == [1] - assert "value > 1" in sql - - @pytest.mark.usefixtures("load_energy_table_with_slice") - def test_rls_filter_doesnt_alter_energy_query(self): - g.user = self.get_user( - username="admin" - ) # self.login() doesn't actually set the user - tbl = self.get_table(name="energy_usage") - sql = tbl.get_query_str(self.query_obj) - assert tbl.get_extra_cache_keys(self.query_obj) == [] - assert "value > 1" not in sql - - @pytest.mark.usefixtures("load_unicode_dashboard_with_slice") - def test_multiple_table_filter_alters_another_tables_query(self): - g.user = self.get_user( - username="alpha" - ) # self.login() doesn't actually set the user - tbl = self.get_table(name="unicode_test") - sql = tbl.get_query_str(self.query_obj) - assert tbl.get_extra_cache_keys(self.query_obj) == [1] - assert "value > 1" in sql - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_rls_filter_alters_gamma_birth_names_query(self): - g.user = self.get_user(username="gamma") - tbl = self.get_table(name="birth_names") - sql = tbl.get_query_str(self.query_obj) - - # establish that the filters are grouped together correctly with - # ANDs, ORs and parens in the correct place - assert ( - "WHERE ((name like 'A%'\n or name like 'B%')\n OR (name like 'Q%'))\n AND (gender = 'boy');" - in sql - ) - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_rls_filter_alters_no_role_user_birth_names_query(self): - g.user = self.get_user(username="NoRlsRoleUser") - tbl = self.get_table(name="birth_names") - sql = tbl.get_query_str(self.query_obj) - - # gamma's filters should not be present query - assert not self.NAMES_A_REGEX.search(sql) - assert not self.NAMES_B_REGEX.search(sql) - assert not self.NAMES_Q_REGEX.search(sql) - # base query should be present - assert self.BASE_FILTER_REGEX.search(sql) - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_rls_filter_doesnt_alter_admin_birth_names_query(self): - g.user = self.get_user(username="admin") - tbl = self.get_table(name="birth_names") - sql = tbl.get_query_str(self.query_obj) - - # no filters are applied for admin user - assert not self.NAMES_A_REGEX.search(sql) - assert not self.NAMES_B_REGEX.search(sql) - assert not self.NAMES_Q_REGEX.search(sql) - assert not self.BASE_FILTER_REGEX.search(sql) - - class TestAccessRequestEndpoints(SupersetTestCase): def test_access_request_disabled(self): with patch.object(AccessRequestsModelView, "is_enabled", return_value=False): From bfdbad5955404ea6a40c91c8c16691a9c95f8ffb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 22 Dec 2021 12:09:43 -0800 Subject: [PATCH 07/31] a bit of refactoring --- superset/security/manager.py | 14 +- .../security/guest_token_security_tests.py | 47 +++++ tox.ini | 180 ------------------ 3 files changed, 55 insertions(+), 186 deletions(-) create mode 100644 tests/integration_tests/security/guest_token_security_tests.py delete mode 100644 tox.ini diff --git a/superset/security/manager.py b/superset/security/manager.py index e9e2c614a5e2..73e818491d71 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1071,8 +1071,8 @@ def raise_for_access( is_dashboard_access_check_applicable = feature_flag_manager.is_feature_enabled( "DASHBOARD_RBAC" - ) or feature_flag_manager.is_feature_enabled( - "EMBEDDED_SUPERSET" + ) or self.is_guest_user( + g.user ) if not ( @@ -1315,10 +1315,12 @@ def get_guest_user_from_request(self, req: Request) -> Optional[GuestUser]: logger.warning("Invalid guest token", exc_info=True) return None else: - return self.guest_user_cls( - token=token, - roles=[self.find_role(current_app.config["GUEST_ROLE_NAME"])], - ) + return self.get_guest_user_from_token(token) + + def get_guest_user_from_token(self, token: GuestToken) -> GuestUser: + return self.guest_user_cls( + token=token, roles=[self.find_role(current_app.config["GUEST_ROLE_NAME"])], + ) @staticmethod def parse_jwt_guest_token(raw_token: str) -> GuestToken: diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py new file mode 100644 index 000000000000..5f4a60a54ea8 --- /dev/null +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -0,0 +1,47 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Unit tests for Superset""" +from unittest import mock + +import pytest +from flask import g + +from integration_tests.base_tests import SupersetTestCase +from superset import db, security_manager +from superset.models.dashboard import Dashboard +from superset.security.guest_token import GuestUser +from tests.integration_tests.fixtures.birth_names_dashboard import ( + load_birth_names_dashboard_with_slices, +) +from tests.integration_tests.fixtures.public_role import public_role_like_gamma +from tests.integration_tests.fixtures.query_context import get_query_context + + +@mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, +) +class TestDashboardGuestTokenSecurity(SupersetTestCase): + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_dashboard_access_filter_as_guest(self): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + g.user = security_manager.get_guest_user_from_token({ + "user": {}, + "resources": [{"type": "dashboard", "id": dash.id}] + }) + + security_manager.raise_for_access(datasource=dash.datasources[0]) diff --git a/tox.ini b/tox.ini deleted file mode 100644 index 839bce55867e..000000000000 --- a/tox.ini +++ /dev/null @@ -1,180 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with -# this work for additional information regarding copyright ownership. -# The ASF licenses this file to You under the Apache License, Version 2.0 -# (the "License"); you may not use this file except in compliance with -# the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -# Remember to start celery workers to run celery tests, e.g. -# celery --app=superset.tasks.celery_app:app worker -Ofair -c 2 -[testenv] -basepython = python3.8 -ignore_basepython_conflict = true -commands = - superset db upgrade - superset init - # use -s to be able to use break pointers. - # no args or tests/* can be passed as an argument to run all tests - pytest -s {posargs} -deps = - -rrequirements/testing.txt -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - mysql: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 - postgres: SUPERSET__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://superset:superset@localhost/test - sqlite: SUPERSET__SQLALCHEMY_DATABASE_URI = sqlite:////{envtmpdir}/superset.db - mysql-presto: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 - # docker run -p 8080:8080 --name presto prestosql/presto - mysql-presto: SUPERSET__SQLALCHEMY_EXAMPLES_URI = presto://localhost:8080/memory/default - # based on https://github.com/big-data-europe/docker-hadoop - # clone the repo & run docker-compose up -d to test locally - mysql-hive: SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset?charset=utf8 - mysql-hive: SUPERSET__SQLALCHEMY_EXAMPLES_URI = hive://localhost:10000/default - # make sure that directory is accessible by docker - hive: UPLOAD_FOLDER = /tmp/.superset/app/static/uploads/ -usedevelop = true -allowlist_externals = - npm - pkill - -[testenv:cypress] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-dashboard] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh dashboard -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-explore] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh explore -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-sqllab] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh sqllab -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:cypress-sqllab-backend-persist] -setenv = - PYTHONPATH = {toxinidir} - SUPERSET_TESTENV = true - SUPERSET_CONFIG = tests.integration_tests.superset_test_config - SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true -commands = - npm install -g npm@'>=6.5.0' - pip install -e {toxinidir}/ - {toxinidir}/superset-frontend/cypress_build.sh sqllab -commands_post = - pkill -if "python {envbindir}/flask" - -[testenv:eslint] -changedir = {toxinidir}/superset-frontend -commands = - npm run lint -deps = - -[testenv:fossa] -commands = - {toxinidir}/scripts/fossa.sh -deps = -passenv = * - -[testenv:javascript] -commands = - npm install -g npm@'>=6.5.0' - {toxinidir}/superset-frontend/js_build.sh -deps = - -[testenv:license-check] -commands = - {toxinidir}/scripts/check_license.sh -passenv = * -whitelist_externals = - {toxinidir}/scripts/check_license.sh -deps = - -[testenv:pre-commit] -commands = - pre-commit run --all-files -deps = - -rrequirements/integration.txt -skip_install = true - -[testenv:pylint] -commands = - pylint superset -deps = - -rrequirements/testing.txt - -[testenv:thumbnails] -setenv = - SUPERSET_CONFIG = tests.integration_tests.superset_test_config_thumbnails -deps = - -rrequirements/testing.txt - -[tox] -envlist = - cypress-dashboard - cypress-explore - cypress-sqllab - cypress-sqllab-backend-persist - eslint - fossa - javascript - license-check - pre-commit - pylint -skipsdist = true From 36a1224496eb494806bbe7f1cc85167f87f23a28 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 22 Dec 2021 12:11:24 -0800 Subject: [PATCH 08/31] add guest token security tests --- .../security/guest_token_security_tests.py | 47 +++++++++++++++---- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index 5f4a60a54ea8..a962b68e8436 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -19,9 +19,10 @@ import pytest from flask import g - from integration_tests.base_tests import SupersetTestCase + from superset import db, security_manager +from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard from superset.security.guest_token import GuestUser from tests.integration_tests.fixtures.birth_names_dashboard import ( @@ -34,14 +35,42 @@ @mock.patch.dict( "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) -class TestDashboardGuestTokenSecurity(SupersetTestCase): - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") +@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") +class TestGuestTokenSecurity(SupersetTestCase): def test_dashboard_access_filter_as_guest(self): dash = db.session.query(Dashboard).filter_by(slug="births").first() - g.user = security_manager.get_guest_user_from_token({ - "user": {}, - "resources": [{"type": "dashboard", "id": dash.id}] - }) + dataset = list(dash.datasources)[0] + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": dash.id}]} + ) + + security_manager.raise_for_access(datasource=dataset) + + def test_dashboard_access_filter_as_unauthorized_guest(self): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + dataset = list(dash.datasources)[0] + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": dash.id + 1}]} + ) + + with self.assertRaises(SupersetSecurityException): + security_manager.raise_for_access(datasource=dataset) + + def test_chart_access_filter_as_guest(self): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + chart = dash.slices[0] + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": dash.id}]} + ) + + security_manager.raise_for_access(viz=chart) + + def test_chart_access_filter_as_unauthorized_guest(self): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + chart = dash.slices[0] + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": dash.id + 1}]} + ) - security_manager.raise_for_access(datasource=dash.datasources[0]) + with self.assertRaises(SupersetSecurityException): + security_manager.raise_for_access(viz=chart) From 7181333a89c69d3b1d9724a1edac5706d12000c0 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 5 Jan 2022 17:53:01 -0800 Subject: [PATCH 09/31] refactor tests --- .../security/guest_token_security_tests.py | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index a962b68e8436..42cc7407f49c 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -37,40 +37,37 @@ ) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") class TestGuestTokenSecurity(SupersetTestCase): - def test_dashboard_access_filter_as_guest(self): - dash = db.session.query(Dashboard).filter_by(slug="births").first() - dataset = list(dash.datasources)[0] - g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": dash.id}]} + def setUp(self) -> None: + self.dash = db.session.query(Dashboard).filter_by(slug="births").first() + self.authorized_guest = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id}]} + ) + self.unauthorized_guest = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id + 1}]} ) + def test_dashboard_access_filter_as_guest(self): + dataset = list(self.dash.datasources)[0] + g.user = self.authorized_guest + security_manager.raise_for_access(datasource=dataset) def test_dashboard_access_filter_as_unauthorized_guest(self): - dash = db.session.query(Dashboard).filter_by(slug="births").first() - dataset = list(dash.datasources)[0] - g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": dash.id + 1}]} - ) + dataset = list(self.dash.datasources)[0] + g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(datasource=dataset) def test_chart_access_filter_as_guest(self): - dash = db.session.query(Dashboard).filter_by(slug="births").first() - chart = dash.slices[0] - g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": dash.id}]} - ) + chart = self.dash.slices[0] + g.user = self.authorized_guest security_manager.raise_for_access(viz=chart) def test_chart_access_filter_as_unauthorized_guest(self): - dash = db.session.query(Dashboard).filter_by(slug="births").first() - chart = dash.slices[0] - g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": dash.id + 1}]} - ) + chart = self.dash.slices[0] + g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(viz=chart) From 27a2ec3dbfc26ceea68f5cc216eda43c6d09fe00 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 5 Jan 2022 17:54:44 -0800 Subject: [PATCH 10/31] clean imports --- tests/integration_tests/security_tests.py | 24 +++-------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 15f06577b496..7b3edfd5d8bf 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -15,27 +15,25 @@ # specific language governing permissions and limitations # under the License. # isort:skip_file -import dataclasses import inspect -import re import time import unittest from collections import namedtuple from unittest import mock from unittest.mock import Mock, patch -from typing import Any, Dict +from typing import Any import jwt import prison import pytest -from flask import current_app, g +from flask import current_app from superset.models.dashboard import Dashboard from superset import app, appbuilder, db, security_manager, viz, ConnectorRegistry from superset.connectors.druid.models import DruidCluster, DruidDatasource -from superset.connectors.sqla.models import RowLevelSecurityFilter, SqlaTable +from superset.connectors.sqla.models import SqlaTable from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import SupersetSecurityException from superset.models.core import Database @@ -45,22 +43,6 @@ from superset.views.access_requests import AccessRequestsModelView from .base_tests import SupersetTestCase -from tests.integration_tests.fixtures.birth_names_dashboard import ( - load_birth_names_dashboard_with_slices, -) -from tests.integration_tests.fixtures.energy_dashboard import ( - load_energy_table_with_slice, -) -from tests.integration_tests.fixtures.public_role import ( - public_role_like_gamma, - public_role_like_test_role, -) -from tests.integration_tests.fixtures.unicode_dashboard import ( - load_unicode_dashboard_with_slice, -) -from tests.integration_tests.fixtures.world_bank_dashboard import ( - load_world_bank_dashboard_with_slices, -) NEW_SECURITY_CONVERGE_VIEWS = ( "Annotation", From 09aed96e678e5fdb02b0c1b2b34446ccfa399390 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 14:19:51 -0800 Subject: [PATCH 11/31] variable names can be too long apparently --- superset/security/manager.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 73e818491d71..4d9f77c0fa44 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1069,17 +1069,15 @@ def raise_for_access( assert datasource - is_dashboard_access_check_applicable = feature_flag_manager.is_feature_enabled( + should_check_dashboard_access = feature_flag_manager.is_feature_enabled( "DASHBOARD_RBAC" - ) or self.is_guest_user( - g.user - ) + ) or self.is_guest_user(g.user) if not ( self.can_access_schema(datasource) or self.can_access("datasource_access", datasource.perm or "") or ( - is_dashboard_access_check_applicable + should_check_dashboard_access and self.can_access_based_on_dashboard(datasource) ) ): From c7bfa96dd17d417695d92ca3c95846efb3cc56d1 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 14:25:13 -0800 Subject: [PATCH 12/31] missing argument to get_user_roles --- superset/security/manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 4d9f77c0fa44..933ff1d6f3d9 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1103,11 +1103,11 @@ def get_user_by_username( def get_anonymous_user(self) -> User: # pylint: disable=no-self-use return AnonymousUserMixin() - def get_user_roles(self) -> List[Role]: - if g.user.is_anonymous: + def get_user_roles(self, user: User) -> List[Role]: + if user.is_anonymous: public_role = current_app.config.get("AUTH_ROLE_PUBLIC") return [self.get_public_role()] if public_role else [] - return g.user.roles + return user.roles def get_rls_filters(self, table: "BaseDatasource") -> List[SqlaQuery]: """ @@ -1223,7 +1223,7 @@ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: def has_rbac_access() -> bool: return is_feature_enabled("DASHBOARD_RBAC") and any( dashboard_role.id - in [user_role.id for user_role in self.get_user_roles()] + in [user_role.id for user_role in self.get_user_roles(g.user)] for dashboard_role in dashboard.roles ) From aa672b9b7ac270c59914362c59878ff1c212b8c3 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 14:26:25 -0800 Subject: [PATCH 13/31] don't redefine builtins --- superset/security/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 933ff1d6f3d9..fa8ab71d67e7 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1356,13 +1356,13 @@ def get_current_guest_user_if_guest(self) -> Optional[GuestUser]: return None def has_guest_access( - self, resource_type: GuestTokenResourceType, id: Union[str, int] + self, resource_type: GuestTokenResourceType, resource_id: Union[str, int] ) -> bool: user = self.get_current_guest_user_if_guest() if not user: return False - strid = str(id) + strid = str(resource_id) for resource in user.resources: if resource["type"] == resource_type.value and str(resource["id"]) == strid: return True From 091786d1e60af9931d6b392604fe2c5a89bb0d19 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 14:28:07 -0800 Subject: [PATCH 14/31] remove unused imports --- superset/security/manager.py | 1 - superset/views/base.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index fa8ab71d67e7..48d3c9eec6a1 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -67,7 +67,6 @@ from superset.exceptions import SupersetSecurityException from superset.security.guest_token import ( GuestToken, - GuestTokenResource, GuestTokenResources, GuestTokenResourceType, GuestTokenUser, diff --git a/superset/views/base.py b/superset/views/base.py index e5fc08200da0..5b84ec36751d 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -38,7 +38,7 @@ from flask_appbuilder.actions import action from flask_appbuilder.forms import DynamicForm from flask_appbuilder.models.sqla.filters import BaseFilter -from flask_appbuilder.security.sqla.models import Role, User +from flask_appbuilder.security.sqla.models import User from flask_appbuilder.widgets import ListWidget from flask_babel import get_locale, gettext as __, lazy_gettext as _ from flask_jwt_extended.exceptions import NoAuthorizationError From 5e23ce9dcef758ad00ade9311006efd0fc0d7cf6 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 14:33:41 -0800 Subject: [PATCH 15/31] fix test import --- tests/integration_tests/security/guest_token_security_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index 42cc7407f49c..2fdb1bd1db1d 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -19,12 +19,12 @@ import pytest from flask import g -from integration_tests.base_tests import SupersetTestCase from superset import db, security_manager from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard from superset.security.guest_token import GuestUser +from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, ) From a62b2f516ef0ae4c9177bce46db18cc30a00b6f3 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 14:57:43 -0800 Subject: [PATCH 16/31] default to global user when getting roles --- superset/security/manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 48d3c9eec6a1..0dfaafa533ac 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1102,7 +1102,9 @@ def get_user_by_username( def get_anonymous_user(self) -> User: # pylint: disable=no-self-use return AnonymousUserMixin() - def get_user_roles(self, user: User) -> List[Role]: + def get_user_roles(self, user: Optional[User] = None) -> List[Role]: + if not user: + user = g.user if user.is_anonymous: public_role = current_app.config.get("AUTH_ROLE_PUBLIC") return [self.get_public_role()] if public_role else [] From 4d5a69158be7fc83d5001a4ac8ad7f223441b595 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 15:35:39 -0800 Subject: [PATCH 17/31] missing import --- tests/integration_tests/security_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 087b3fee6793..ebbe18aafcb9 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -53,6 +53,7 @@ ) from tests.integration_tests.fixtures.world_bank_dashboard import ( load_world_bank_dashboard_with_slices, + load_world_bank_data, ) NEW_SECURITY_CONVERGE_VIEWS = ( From 13a2038ad1fd8b4e706f0c37922dd287fbd4cc7a Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 15:53:15 -0800 Subject: [PATCH 18/31] mock it --- .../security/guest_token_security_tests.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index 2fdb1bd1db1d..840ef6944724 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -16,6 +16,7 @@ # under the License. """Unit tests for Superset""" from unittest import mock +from unittest.mock import patch import pytest from flask import g @@ -46,28 +47,32 @@ def setUp(self) -> None: {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id + 1}]} ) - def test_dashboard_access_filter_as_guest(self): + @patch("superset.security.manager.g") + def test_dashboard_access_filter_as_guest(self, mock_g): dataset = list(self.dash.datasources)[0] - g.user = self.authorized_guest + mock_g.user = self.authorized_guest security_manager.raise_for_access(datasource=dataset) - def test_dashboard_access_filter_as_unauthorized_guest(self): + @patch("superset.security.manager.g") + def test_dashboard_access_filter_as_unauthorized_guest(self, mock_g): dataset = list(self.dash.datasources)[0] - g.user = self.unauthorized_guest + mock_g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(datasource=dataset) - def test_chart_access_filter_as_guest(self): + @patch("superset.security.manager.g") + def test_chart_access_filter_as_guest(self, mock_g): chart = self.dash.slices[0] - g.user = self.authorized_guest + mock_g.user = self.authorized_guest security_manager.raise_for_access(viz=chart) - def test_chart_access_filter_as_unauthorized_guest(self): + @patch("superset.security.manager.g") + def test_chart_access_filter_as_unauthorized_guest(self, mock_g): chart = self.dash.slices[0] - g.user = self.unauthorized_guest + mock_g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(viz=chart) From 0be28dab75023d565b6566a7945627a825545ecb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 15:57:40 -0800 Subject: [PATCH 19/31] test get_user_roles --- .../security/guest_token_security_tests.py | 11 +++++++++++ tests/integration_tests/security_tests.py | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index 840ef6944724..b74fae1223d5 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -76,3 +76,14 @@ def test_chart_access_filter_as_unauthorized_guest(self, mock_g): with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(viz=chart) + + def test_get_guest_user_roles_explicit(self): + roles = security_manager.get_user_roles(self.authorized_guest) + self.assertEqual(self.authorized_guest.roles, roles) + + @patch("superset.security.manager.g") + def test_get_guest_user_roles_implicit(self, mock_g): + mock_g.user = self.authorized_guest + + roles = security_manager.get_user_roles(self.authorized_guest) + self.assertEqual(self.authorized_guest.roles, roles) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index ebbe18aafcb9..2f6480aecd90 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1041,6 +1041,19 @@ def test_raise_for_access_viz(self, mock_can_access_schema, mock_can_access): with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(viz=test_viz) + @patch("superset.security.manager.g") + def test_get_user_roles(self, mock_g): + admin = security_manager.find_user("admin") + mock_g.user = admin + roles = security_manager.get_user_roles() + self.assertEqual(admin.roles, roles) + + @patch("superset.security.manager.g") + def test_get_anonymous_roles(self, mock_g): + mock_g.user = security_manager.get_anonymous_user() + roles = security_manager.get_user_roles() + self.assertEqual([security_manager.get_public_role()], roles) + class TestAccessRequestEndpoints(SupersetTestCase): def test_access_request_disabled(self): From c5bded9bb24b0037d1e945ce03769bd68df9e801 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 17:11:34 -0800 Subject: [PATCH 20/31] infer g.user for ease of tests --- superset/security/manager.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 0dfaafa533ac..40620ebbc3c7 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1068,9 +1068,10 @@ def raise_for_access( assert datasource - should_check_dashboard_access = feature_flag_manager.is_feature_enabled( - "DASHBOARD_RBAC" - ) or self.is_guest_user(g.user) + should_check_dashboard_access = ( + feature_flag_manager.is_feature_enabled("DASHBOARD_RBAC") + or self.is_guest_user() + ) if not ( self.can_access_schema(datasource) @@ -1230,7 +1231,7 @@ def has_rbac_access() -> bool: has_published_access = ( not is_feature_enabled("DASHBOARD_RBAC") - and not self.is_guest_user(g.user) + and not self.is_guest_user() and dashboard.published ) @@ -1342,17 +1343,19 @@ def parse_jwt_guest_token(raw_token: str) -> GuestToken: return cast(GuestToken, token) @staticmethod - def is_guest_user(user: Any) -> bool: + def is_guest_user(user: Optional[Any] = None) -> bool: # pylint: disable=import-outside-toplevel from superset import is_feature_enabled if not is_feature_enabled("EMBEDDED_SUPERSET"): return False + if not user: + user = g.user return hasattr(user, "is_guest_user") and user.is_guest_user def get_current_guest_user_if_guest(self) -> Optional[GuestUser]: - if self.is_guest_user(g.user): + if self.is_guest_user(): return g.user return None From ebf9400fff1787142ddb0c7b0882f672a8c22a96 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 22:13:37 -0800 Subject: [PATCH 21/31] remove redundant check --- superset/security/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 40620ebbc3c7..e6b152747391 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -279,7 +279,7 @@ def can_access(self, permission_name: str, view_name: str) -> bool: """ user = g.user - if user.is_anonymous and not self.is_guest_user(user): + if user.is_anonymous: return self.is_item_public(permission_name, view_name) return self._has_view_access(user, permission_name, view_name) From deceb33b6ecc4e2c507447eb100635cdadd35114 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 22:14:35 -0800 Subject: [PATCH 22/31] tests for guest user security manager fns --- .../security/guest_token_security_tests.py | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index b74fae1223d5..e877f8f36ac7 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -19,12 +19,10 @@ from unittest.mock import patch import pytest -from flask import g from superset import db, security_manager from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard -from superset.security.guest_token import GuestUser from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, @@ -87,3 +85,23 @@ def test_get_guest_user_roles_implicit(self, mock_g): roles = security_manager.get_user_roles(self.authorized_guest) self.assertEqual(self.authorized_guest.roles, roles) + + def test_user_is_not_guest(self): + is_guest = security_manager.is_guest_user(security_manager.find_user("admin")) + self.assertTrue(is_guest) + + def test_anon_is_not_guest(self): + is_guest = security_manager.is_guest_user(security_manager.get_anonymous_user()) + self.assertTrue(is_guest) + + def test_guest_is_guest(self): + is_guest = security_manager.is_guest_user(self.authorized_guest) + self.assertTrue(is_guest) + + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=False, + ) + def test_is_guest_user_flag_off(self): + is_guest = security_manager.is_guest_user(self.authorized_guest) + self.assertFalse(is_guest) From 128f3a09f9b2db1c1d156fefb1947da6ba984eae Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 13 Jan 2022 22:15:22 -0800 Subject: [PATCH 23/31] use algo to get rid of warning messages --- tests/integration_tests/security_tests.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 2f6480aecd90..48ded730aad1 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1170,7 +1170,11 @@ def test_create_guest_access_token(self, get_time_mock): token = security_manager.create_guest_access_token(user, resources) # unfortunately we cannot mock time in the jwt lib - decoded_token = jwt.decode(token, self.app.config["GUEST_TOKEN_JWT_SECRET"]) + decoded_token = jwt.decode( + token, + self.app.config["GUEST_TOKEN_JWT_SECRET"], + algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]], + ) self.assertEqual(user, decoded_token["user"]) self.assertEqual(resources, decoded_token["resources"]) From ede136716c8d35d3d2d88baa7abf31f242f8e23d Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 18 Jan 2022 01:57:08 -0800 Subject: [PATCH 24/31] tweaking access checks --- superset/security/manager.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index e6b152747391..22ed41569761 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1222,25 +1222,20 @@ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: from superset.views.base import is_user_admin from superset.views.utils import is_owner - def has_rbac_access() -> bool: - return is_feature_enabled("DASHBOARD_RBAC") and any( + has_rbac_access = True + + if is_feature_enabled("DASHBOARD_RBAC"): + has_rbac_access = any( dashboard_role.id - in [user_role.id for user_role in self.get_user_roles(g.user)] + in [user_role.id for user_role in self.get_user_roles()] for dashboard_role in dashboard.roles ) - has_published_access = ( - not is_feature_enabled("DASHBOARD_RBAC") - and not self.is_guest_user() - and dashboard.published - ) - can_access = ( is_user_admin() or is_owner(dashboard, g.user) - or has_published_access - or has_rbac_access() or self.has_guest_access(GuestTokenResourceType.DASHBOARD, dashboard.id) + or (dashboard.published and has_rbac_access) or (not dashboard.published and not dashboard.roles) ) From 38a89ae3eeab6236b7fbdde6d087dd4c940ec0ed Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 18 Jan 2022 12:13:36 -0800 Subject: [PATCH 25/31] fix guest token security tests --- .../security/guest_token_security_tests.py | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index e877f8f36ac7..df76417d6fd3 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -16,9 +16,9 @@ # under the License. """Unit tests for Superset""" from unittest import mock -from unittest.mock import patch import pytest +from flask import g from superset import db, security_manager from superset.exceptions import SupersetSecurityException @@ -26,16 +26,35 @@ from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, + load_birth_names_data, ) -from tests.integration_tests.fixtures.public_role import public_role_like_gamma -from tests.integration_tests.fixtures.query_context import get_query_context + + +@mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, +) +class TestGuestUserSecurity(SupersetTestCase): + def test_user_is_not_guest(self): + is_guest = security_manager.is_guest_user(security_manager.find_user("admin")) + self.assertFalse(is_guest) + + def test_anon_is_not_guest(self): + is_guest = security_manager.is_guest_user(security_manager.get_anonymous_user()) + self.assertFalse(is_guest) + + def test_guest_is_guest(self): + authorized_guest = security_manager.get_guest_user_from_token( + {"user": {}, "resources": []} + ) + is_guest = security_manager.is_guest_user(authorized_guest) + self.assertTrue(is_guest) @mock.patch.dict( "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") -class TestGuestTokenSecurity(SupersetTestCase): +class TestGuestTokenDashboardAccess(SupersetTestCase): def setUp(self) -> None: self.dash = db.session.query(Dashboard).filter_by(slug="births").first() self.authorized_guest = security_manager.get_guest_user_from_token( @@ -45,32 +64,28 @@ def setUp(self) -> None: {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id + 1}]} ) - @patch("superset.security.manager.g") - def test_dashboard_access_filter_as_guest(self, mock_g): + def test_dashboard_access_filter_as_guest(self): dataset = list(self.dash.datasources)[0] - mock_g.user = self.authorized_guest + g.user = self.authorized_guest security_manager.raise_for_access(datasource=dataset) - @patch("superset.security.manager.g") - def test_dashboard_access_filter_as_unauthorized_guest(self, mock_g): + def test_dashboard_access_filter_as_unauthorized_guest(self): dataset = list(self.dash.datasources)[0] - mock_g.user = self.unauthorized_guest + g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(datasource=dataset) - @patch("superset.security.manager.g") - def test_chart_access_filter_as_guest(self, mock_g): + def test_chart_access_filter_as_guest(self): chart = self.dash.slices[0] - mock_g.user = self.authorized_guest + g.user = self.authorized_guest security_manager.raise_for_access(viz=chart) - @patch("superset.security.manager.g") - def test_chart_access_filter_as_unauthorized_guest(self, mock_g): + def test_chart_access_filter_as_unauthorized_guest(self): chart = self.dash.slices[0] - mock_g.user = self.unauthorized_guest + g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): security_manager.raise_for_access(viz=chart) @@ -79,25 +94,12 @@ def test_get_guest_user_roles_explicit(self): roles = security_manager.get_user_roles(self.authorized_guest) self.assertEqual(self.authorized_guest.roles, roles) - @patch("superset.security.manager.g") - def test_get_guest_user_roles_implicit(self, mock_g): - mock_g.user = self.authorized_guest + def test_get_guest_user_roles_implicit(self): + g.user = self.authorized_guest roles = security_manager.get_user_roles(self.authorized_guest) self.assertEqual(self.authorized_guest.roles, roles) - def test_user_is_not_guest(self): - is_guest = security_manager.is_guest_user(security_manager.find_user("admin")) - self.assertTrue(is_guest) - - def test_anon_is_not_guest(self): - is_guest = security_manager.is_guest_user(security_manager.get_anonymous_user()) - self.assertTrue(is_guest) - - def test_guest_is_guest(self): - is_guest = security_manager.is_guest_user(self.authorized_guest) - self.assertTrue(is_guest) - @mock.patch.dict( "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=False, From 1927a1ceb22cc6f539b45c7b7552ed8e9dd99391 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 18 Jan 2022 14:32:17 -0800 Subject: [PATCH 26/31] missing imports --- .../security/row_level_security_tests.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration_tests/security/row_level_security_tests.py b/tests/integration_tests/security/row_level_security_tests.py index d159c27fa8b1..665666cb61f5 100644 --- a/tests/integration_tests/security/row_level_security_tests.py +++ b/tests/integration_tests/security/row_level_security_tests.py @@ -24,6 +24,18 @@ from superset import db, security_manager from superset.connectors.sqla.models import RowLevelSecurityFilter, SqlaTable from ..base_tests import SupersetTestCase +from tests.integration_tests.fixtures.birth_names_dashboard import ( + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) +from tests.integration_tests.fixtures.energy_dashboard import ( + load_energy_table_with_slice, + load_energy_table_data, +) +from tests.integration_tests.fixtures.unicode_dashboard import ( + load_unicode_dashboard_with_slice, + load_unicode_data, +) class TestRowLevelSecurity(SupersetTestCase): From 128b90cfa43f226c782af61b9929d50a3e1025a7 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 18 Jan 2022 15:31:09 -0800 Subject: [PATCH 27/31] more tests --- .../security/guest_token_security_tests.py | 82 ++++++++++++++++++- 1 file changed, 78 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index df76417d6fd3..6d1efe1f17c1 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -23,6 +23,7 @@ from superset import db, security_manager from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard +from superset.security.guest_token import GuestTokenResourceType from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, @@ -34,27 +35,100 @@ "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) class TestGuestUserSecurity(SupersetTestCase): - def test_user_is_not_guest(self): + def test_is_guest_user__regular_user(self): is_guest = security_manager.is_guest_user(security_manager.find_user("admin")) self.assertFalse(is_guest) - def test_anon_is_not_guest(self): + def test_is_guest_user__anonymous(self): is_guest = security_manager.is_guest_user(security_manager.get_anonymous_user()) self.assertFalse(is_guest) - def test_guest_is_guest(self): + def test_is_guest_user__guest_user(self): authorized_guest = security_manager.get_guest_user_from_token( {"user": {}, "resources": []} ) is_guest = security_manager.is_guest_user(authorized_guest) self.assertTrue(is_guest) + def test_get_guest_user__regular_user(self): + g.user = security_manager.find_user("admin") + guest_user = security_manager.get_current_guest_user_if_guest() + self.assertIsNone(guest_user) + + def test_get_guest_user__anonymous_user(self): + g.user = security_manager.get_anonymous_user() + guest_user = security_manager.get_current_guest_user_if_guest() + self.assertIsNone(guest_user) + + def test_get_guest_user__guest_user(self): + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": []} + ) + guest_user = security_manager.get_current_guest_user_if_guest() + self.assertEqual(guest_user, g.user) + + def test_has_guest_access__regular_user(self): + g.user = security_manager.find_user("admin") + has_guest_access = security_manager.has_guest_access( + GuestTokenResourceType.DASHBOARD, 42 + ) + self.assertFalse(has_guest_access) + + def test_has_guest_access__anonymous_user(self): + g.user = security_manager.get_anonymous_user() + has_guest_access = security_manager.has_guest_access( + GuestTokenResourceType.DASHBOARD, 42 + ) + self.assertFalse(has_guest_access) + + def test_has_guest_access__authorized_guest_user(self): + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": 42}]} + ) + has_guest_access = security_manager.has_guest_access( + GuestTokenResourceType.DASHBOARD, 42 + ) + self.assertTrue(has_guest_access) + + def test_has_guest_access__authorized_guest_user__non_zero_resource_index(self): + g.user = security_manager.get_guest_user_from_token( + { + "user": {}, + "resources": [ + {"type": "dashboard", "id": 24}, + {"type": "dashboard", "id": 42}, + ], + } + ) + has_guest_access = security_manager.has_guest_access( + GuestTokenResourceType.DASHBOARD, 42 + ) + self.assertTrue(has_guest_access) + + def test_has_guest_access__unauthorized_guest_user__different_resource_id(self): + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": 24}]} + ) + has_guest_access = security_manager.has_guest_access( + GuestTokenResourceType.DASHBOARD, 42 + ) + self.assertFalse(has_guest_access) + + def test_has_guest_access__unauthorized_guest_user__different_resource_type(self): + g.user = security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dirt", "id": 42}]} + ) + has_guest_access = security_manager.has_guest_access( + GuestTokenResourceType.DASHBOARD, 42 + ) + self.assertFalse(has_guest_access) + @mock.patch.dict( "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") -class TestGuestTokenDashboardAccess(SupersetTestCase): +class TestGuestUserDashboardAccess(SupersetTestCase): def setUp(self) -> None: self.dash = db.session.query(Dashboard).filter_by(slug="births").first() self.authorized_guest = security_manager.get_guest_user_from_token( From b73a81fdd97168a2bbb12b71ab529e29539e2aa5 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 19 Jan 2022 21:15:37 -0800 Subject: [PATCH 28/31] more testing and also some small refactoring --- superset/security/manager.py | 34 +++-- .../security/guest_token_security_tests.py | 131 +++++++++++------- 2 files changed, 99 insertions(+), 66 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 22ed41569761..566e36e76130 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1222,22 +1222,28 @@ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: from superset.views.base import is_user_admin from superset.views.utils import is_owner - has_rbac_access = True - - if is_feature_enabled("DASHBOARD_RBAC"): - has_rbac_access = any( - dashboard_role.id - in [user_role.id for user_role in self.get_user_roles()] - for dashboard_role in dashboard.roles + def has_rbac_access() -> bool: + return ( + is_feature_enabled("DASHBOARD_RBAC") + and dashboard.published + and any( + dashboard_role.id + in [user_role.id for user_role in self.get_user_roles()] + for dashboard_role in dashboard.roles + ) ) - can_access = ( - is_user_admin() - or is_owner(dashboard, g.user) - or self.has_guest_access(GuestTokenResourceType.DASHBOARD, dashboard.id) - or (dashboard.published and has_rbac_access) - or (not dashboard.published and not dashboard.roles) - ) + if self.is_guest_user(): + can_access = self.has_guest_access( + GuestTokenResourceType.DASHBOARD, dashboard.id + ) + else: + can_access = ( + is_user_admin() + or is_owner(dashboard, g.user) + or has_rbac_access() + or (not dashboard.published and not dashboard.roles) + ) if not can_access: raise DashboardAccessDeniedError() diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index 6d1efe1f17c1..9ca34198dbdf 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -21,9 +21,11 @@ from flask import g from superset import db, security_manager +from superset.dashboards.commands.exceptions import DashboardAccessDeniedError from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard from superset.security.guest_token import GuestTokenResourceType +from superset.sql_parse import Table from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, @@ -35,6 +37,16 @@ "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) class TestGuestUserSecurity(SupersetTestCase): + # This test doesn't use a dashboard fixture, the next test does. + # That way tests are faster. + + resource_id = 42 + + def authorized_guest(self): + return security_manager.get_guest_user_from_token( + {"user": {}, "resources": [{"type": "dashboard", "id": self.resource_id}]} + ) + def test_is_guest_user__regular_user(self): is_guest = security_manager.is_guest_user(security_manager.find_user("admin")) self.assertFalse(is_guest) @@ -44,12 +56,17 @@ def test_is_guest_user__anonymous(self): self.assertFalse(is_guest) def test_is_guest_user__guest_user(self): - authorized_guest = security_manager.get_guest_user_from_token( - {"user": {}, "resources": []} - ) - is_guest = security_manager.is_guest_user(authorized_guest) + is_guest = security_manager.is_guest_user(self.authorized_guest()) self.assertTrue(is_guest) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=False, + ) + def test_is_guest_user__flag_off(self): + is_guest = security_manager.is_guest_user(self.authorized_guest()) + self.assertFalse(is_guest) + def test_get_guest_user__regular_user(self): g.user = security_manager.find_user("admin") guest_user = security_manager.get_current_guest_user_if_guest() @@ -61,68 +78,76 @@ def test_get_guest_user__anonymous_user(self): self.assertIsNone(guest_user) def test_get_guest_user__guest_user(self): - g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": []} - ) + g.user = self.authorized_guest() guest_user = security_manager.get_current_guest_user_if_guest() self.assertEqual(guest_user, g.user) def test_has_guest_access__regular_user(self): g.user = security_manager.find_user("admin") has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, 42 + GuestTokenResourceType.DASHBOARD, self.resource_id ) self.assertFalse(has_guest_access) def test_has_guest_access__anonymous_user(self): g.user = security_manager.get_anonymous_user() has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, 42 + GuestTokenResourceType.DASHBOARD, self.resource_id ) self.assertFalse(has_guest_access) def test_has_guest_access__authorized_guest_user(self): - g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": 42}]} - ) + g.user = self.authorized_guest() has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, 42 + GuestTokenResourceType.DASHBOARD, self.resource_id ) self.assertTrue(has_guest_access) def test_has_guest_access__authorized_guest_user__non_zero_resource_index(self): - g.user = security_manager.get_guest_user_from_token( - { - "user": {}, - "resources": [ - {"type": "dashboard", "id": 24}, - {"type": "dashboard", "id": 42}, - ], - } - ) + guest = self.authorized_guest() + guest.resources = [ + {"type": "dashboard", "id": self.resource_id - 1} + ] + guest.resources + g.user = guest + has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, 42 + GuestTokenResourceType.DASHBOARD, self.resource_id ) self.assertTrue(has_guest_access) def test_has_guest_access__unauthorized_guest_user__different_resource_id(self): g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dashboard", "id": 24}]} + { + "user": {}, + "resources": [{"type": "dashboard", "id": self.resource_id - 1}], + } ) has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, 42 + GuestTokenResourceType.DASHBOARD, self.resource_id ) self.assertFalse(has_guest_access) def test_has_guest_access__unauthorized_guest_user__different_resource_type(self): g.user = security_manager.get_guest_user_from_token( - {"user": {}, "resources": [{"type": "dirt", "id": 42}]} + {"user": {}, "resources": [{"type": "dirt", "id": self.resource_id}]} ) has_guest_access = security_manager.has_guest_access( - GuestTokenResourceType.DASHBOARD, 42 + GuestTokenResourceType.DASHBOARD, self.resource_id ) self.assertFalse(has_guest_access) + def test_get_guest_user_roles_explicit(self): + guest = self.authorized_guest() + roles = security_manager.get_user_roles(guest) + self.assertEqual(guest.roles, roles) + + def test_get_guest_user_roles_implicit(self): + guest = self.authorized_guest() + g.user = guest + + roles = security_manager.get_user_roles() + self.assertEqual(guest.roles, roles) + @mock.patch.dict( "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, @@ -138,46 +163,48 @@ def setUp(self) -> None: {"user": {}, "resources": [{"type": "dashboard", "id": self.dash.id + 1}]} ) - def test_dashboard_access_filter_as_guest(self): - dataset = list(self.dash.datasources)[0] + def test_chart_raise_for_access_as_guest(self): + chart = self.dash.slices[0] g.user = self.authorized_guest - security_manager.raise_for_access(datasource=dataset) + security_manager.raise_for_access(viz=chart) - def test_dashboard_access_filter_as_unauthorized_guest(self): - dataset = list(self.dash.datasources)[0] + def test_chart_raise_for_access_as_unauthorized_guest(self): + chart = self.dash.slices[0] g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): - security_manager.raise_for_access(datasource=dataset) + security_manager.raise_for_access(viz=chart) - def test_chart_access_filter_as_guest(self): - chart = self.dash.slices[0] + def test_dataset_raise_for_access_as_guest(self): + dataset = self.dash.slices[0].datasource g.user = self.authorized_guest - security_manager.raise_for_access(viz=chart) + security_manager.raise_for_access(datasource=dataset) - def test_chart_access_filter_as_unauthorized_guest(self): - chart = self.dash.slices[0] + def test_dataset_raise_for_access_as_unauthorized_guest(self): + dataset = self.dash.slices[0].datasource g.user = self.unauthorized_guest with self.assertRaises(SupersetSecurityException): - security_manager.raise_for_access(viz=chart) + security_manager.raise_for_access(datasource=dataset) - def test_get_guest_user_roles_explicit(self): - roles = security_manager.get_user_roles(self.authorized_guest) - self.assertEqual(self.authorized_guest.roles, roles) + def test_guest_token_does_not_grant_access_to_underlying_table(self): + sqla_table = self.dash.slices[0].table + table = Table(table=sqla_table.table_name) - def test_get_guest_user_roles_implicit(self): g.user = self.authorized_guest - roles = security_manager.get_user_roles(self.authorized_guest) - self.assertEqual(self.authorized_guest.roles, roles) + with self.assertRaises(Exception): + security_manager.raise_for_access(table=table, database=sqla_table.database) - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - EMBEDDED_SUPERSET=False, - ) - def test_is_guest_user_flag_off(self): - is_guest = security_manager.is_guest_user(self.authorized_guest) - self.assertFalse(is_guest) + def test_raise_for_dashboard_access_as_guest(self): + g.user = self.authorized_guest + + security_manager.raise_for_dashboard_access(self.dash) + + def test_raise_for_dashboard_access_as_unauthorized_guest(self): + g.user = self.unauthorized_guest + + with self.assertRaises(DashboardAccessDeniedError): + security_manager.raise_for_dashboard_access(self.dash) From eccec4d4ba41ab2285068d439a6f8d73f6c29c08 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 20 Jan 2022 13:01:35 -0800 Subject: [PATCH 29/31] move validation out of parsing --- superset/security/manager.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 566e36e76130..14b6d8960116 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1310,13 +1310,17 @@ def get_guest_user_from_request(self, req: Request) -> Optional[GuestUser]: try: token = self.parse_jwt_guest_token(raw_token) + if token.get("user") is None: + raise ValueError("Guest token does not contain a user claim") + if token.get("resources") is None: + raise ValueError("Guest token does not contain a resources claim") except Exception: # pylint: disable=broad-except # The login manager will handle sending 401s. # We don't need to send a special error message. logger.warning("Invalid guest token", exc_info=True) return None else: - return self.get_guest_user_from_token(token) + return self.get_guest_user_from_token(cast(GuestToken, token)) def get_guest_user_from_token(self, token: GuestToken) -> GuestUser: return self.guest_user_cls( @@ -1324,24 +1328,15 @@ def get_guest_user_from_token(self, token: GuestToken) -> GuestUser: ) @staticmethod - def parse_jwt_guest_token(raw_token: str) -> GuestToken: + def parse_jwt_guest_token(raw_token: str) -> Dict[str, Any]: """ - Parses and validates a guest token. - Raises an error if the jwt is invalid: - if it is not signed with our secret, - or if required claims are not present. + Parses a guest token. Raises an error if the jwt fails standard claims checks. :param raw_token: the token gotten from the request :return: the same token that was passed in, tested but unchanged """ secret = current_app.config["GUEST_TOKEN_JWT_SECRET"] algo = current_app.config["GUEST_TOKEN_JWT_ALGO"] - - token = jwt.decode(raw_token, secret, algorithms=[algo]) - if token.get("user") is None: - raise ValueError("Guest token does not contain a user claim") - if token.get("resources") is None: - raise ValueError("Guest token does not contain a resources claim") - return cast(GuestToken, token) + return jwt.decode(raw_token, secret, algorithms=[algo]) @staticmethod def is_guest_user(user: Optional[Any] = None) -> bool: From e833ef57bc77a051c013c89831585c90237106fb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 20 Jan 2022 15:13:18 -0800 Subject: [PATCH 30/31] fix dashboard access check again --- superset/security/manager.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 14b6d8960116..5993f952f347 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1212,6 +1212,8 @@ def raise_for_user_activity_access(user_id: int) -> None: def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: """ Raise an exception if the user cannot access the dashboard. + This does not check for the required role/permission pairs, + it only concerns itself with entity relationships. :param dashboard: Dashboard the user wants access to :raises DashboardAccessDeniedError: If the user cannot access the resource @@ -1223,14 +1225,10 @@ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: from superset.views.utils import is_owner def has_rbac_access() -> bool: - return ( - is_feature_enabled("DASHBOARD_RBAC") - and dashboard.published - and any( - dashboard_role.id - in [user_role.id for user_role in self.get_user_roles()] - for dashboard_role in dashboard.roles - ) + return (not is_feature_enabled("DASHBOARD_RBAC")) or any( + dashboard_role.id + in [user_role.id for user_role in self.get_user_roles()] + for dashboard_role in dashboard.roles ) if self.is_guest_user(): @@ -1241,7 +1239,7 @@ def has_rbac_access() -> bool: can_access = ( is_user_admin() or is_owner(dashboard, g.user) - or has_rbac_access() + or (dashboard.published and has_rbac_access()) or (not dashboard.published and not dashboard.roles) ) From 5c3a5fb4fe6b4f7d5617d6023147b3a21f84a3cf Mon Sep 17 00:00:00 2001 From: Lily Kuang Date: Fri, 21 Jan 2022 12:15:01 -0800 Subject: [PATCH 31/31] add more test --- tests/integration_tests/security_tests.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 48ded730aad1..2c6b365b8b0b 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1211,3 +1211,26 @@ def test_get_guest_user_expired_token(self, get_time_mock): guest_user = security_manager.get_guest_user_from_request(fake_request) self.assertIsNone(guest_user) + + def test_get_guest_user_no_user(self): + user = None + resources = [{"type": "dashboard", "id": 1}] + token = security_manager.create_guest_access_token(user, resources) + fake_request = FakeRequest() + fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = token + guest_user = security_manager.get_guest_user_from_request(fake_request) + + self.assertIsNone(guest_user) + self.assertRaisesRegex(ValueError, "Guest token does not contain a user claim") + + def test_get_guest_user_no_resource(self): + user = {"username": "test_guest"} + resources = [] + token = security_manager.create_guest_access_token(user, resources) + fake_request = FakeRequest() + fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = token + guest_user = security_manager.get_guest_user_from_request(fake_request) + + self.assertRaisesRegex( + ValueError, "Guest token does not contain a resources claim" + )