diff --git a/superset/config.py b/superset/config.py index 8d00e519a8ed3..43567c01c5ea2 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1101,7 +1101,9 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # Define a list of usernames to be excluded from all dropdown lists of users # Owners, filters for created_by, etc. -EXCLUDE_USER_USERNAMES: List[str] = [] +# The users can also be excluded by overriding the get_exclude_users_from_lists method +# in security manager +EXCLUDE_USERS_FROM_LISTS: Optional[List[str]] = None # This auth provider is used by background (offline) tasks that need to access diff --git a/superset/security/manager.py b/superset/security/manager.py index 260dbb49ee7d4..eaf827ef90463 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1702,6 +1702,22 @@ def on_permission_view_after_delete( :param target: The mapped instance being persisted """ + @staticmethod + def get_exclude_users_from_lists() -> List[str]: + """ + Override to dynamically identify a list of usernames to exclude from + all UI dropdown lists, owners, created_by filters etc... + + It will exclude all users from the all endpoints of the form + ``/api/v1//related/`` + + Optionally you can also exclude them using the `EXCLUDE_USERS_FROM_LISTS` + config setting. + + :return: A list of usernames + """ + return [] + def raise_for_access( # pylint: disable=too-many-arguments,too-many-locals self, diff --git a/superset/views/filters.py b/superset/views/filters.py index 090234421ae63..7f3dfd29306e7 100644 --- a/superset/views/filters.py +++ b/superset/views/filters.py @@ -57,7 +57,7 @@ def apply(self, query: Query, value: Optional[Any]) -> Query: class BaseFilterRelatedUsers(BaseFilter): # pylint: disable=too-few-public-methods """ - Filter to apply on related users. Will exclude users in EXCLUDE_USER_USERNAMES + Filter to apply on related users. Will exclude users in EXCLUDE_USERS_FROM_LISTS Use in the api by adding something like: ``` @@ -73,9 +73,10 @@ class BaseFilterRelatedUsers(BaseFilter): # pylint: disable=too-few-public-meth def apply(self, query: Query, value: Optional[Any]) -> Query: user_model = security_manager.user_model - query_ = query.filter( - and_( - user_model.username.not_in(current_app.config["EXCLUDE_USER_USERNAMES"]) - ) + exclude_users = ( + security_manager.get_exclude_users_from_lists() + if current_app.config["EXCLUDE_USERS_FROM_LISTS"] is None + else current_app.config["EXCLUDE_USERS_FROM_LISTS"] ) + query_ = query.filter(and_(user_model.username.not_in(exclude_users))) return query_ diff --git a/tests/integration_tests/base_api_tests.py b/tests/integration_tests/base_api_tests.py index ce99a778549de..66544f1447e05 100644 --- a/tests/integration_tests/base_api_tests.py +++ b/tests/integration_tests/base_api_tests.py @@ -16,6 +16,8 @@ # under the License. # isort:skip_file import json +from unittest.mock import patch + from tests.integration_tests.fixtures.world_bank_dashboard import ( load_world_bank_dashboard_with_slices, load_world_bank_data, @@ -32,6 +34,7 @@ from superset.views.base_api import BaseSupersetModelRestApi, requires_json from .base_tests import SupersetTestCase +from .conftest import with_config class Model1Api(BaseSupersetModelRestApi): @@ -288,13 +291,13 @@ def test_get_filter_related_owners(self): # TODO Check me assert expected_results == sorted_results + @with_config({"EXCLUDE_USERS_FROM_LISTS": ["gamma"]}) def test_get_base_filter_related_owners(self): """ API: Test get base filter related owners """ self.login(username="admin") uri = f"api/v1/{self.resource_name}/related/owners" - self.app.config["EXCLUDE_USER_USERNAMES"] = ["gamma"] gamma_user = ( db.session.query(security_manager.user_model) .filter(security_manager.user_model.username == "gamma") @@ -309,8 +312,33 @@ def test_get_base_filter_related_owners(self): assert response["count"] == len(users) - 1 response_users = [result["text"] for result in response["result"]] assert "gamma user" not in response_users - # revert the config change - self.app.config["EXCLUDE_USER_USERNAMES"] = [] + + @patch( + "superset.security.SupersetSecurityManager.get_exclude_users_from_lists", + return_value=["gamma"], + ) + def test_get_base_filter_related_owners_on_sm( + self, mock_get_exclude_users_from_list + ): + """ + API: Test get base filter related owners using security manager + """ + self.login(username="admin") + uri = f"api/v1/{self.resource_name}/related/owners" + gamma_user = ( + db.session.query(security_manager.user_model) + .filter(security_manager.user_model.username == "gamma") + .one_or_none() + ) + assert gamma_user is not None + users = db.session.query(security_manager.user_model).all() + + rv = self.client.get(uri) + assert rv.status_code == 200 + response = json.loads(rv.data.decode("utf-8")) + assert response["count"] == len(users) - 1 + response_users = [result["text"] for result in response["result"]] + assert "gamma user" not in response_users def test_get_ids_related_owners(self): """ diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py index c117422adc1bc..463f93b833681 100644 --- a/tests/integration_tests/conftest.py +++ b/tests/integration_tests/conftest.py @@ -19,7 +19,7 @@ import contextlib import functools import os -from typing import Any, Callable, Optional, TYPE_CHECKING +from typing import Any, Callable, Dict, Optional, TYPE_CHECKING from unittest.mock import patch import pytest @@ -255,6 +255,38 @@ def wrapper(*args, **kwargs): return decorate +def with_config(override_config: Dict[str, Any]): + """ + Use this decorator to mock specific config keys. + + Usage: + + class TestYourFeature(SupersetTestCase): + + @with_config({"SOME_CONFIG": True}) + def test_your_config(self): + self.assertEqual(curren_app.config["SOME_CONFIG"), True) + + """ + + def decorate(test_fn): + config_backup = {} + + def wrapper(*args, **kwargs): + from flask import current_app + + for key, value in override_config.items(): + config_backup[key] = current_app.config[key] + current_app.config[key] = value + test_fn(*args, **kwargs) + for key, value in config_backup.items(): + current_app.config[key] = value + + return functools.update_wrapper(wrapper, test_fn) + + return decorate + + @pytest.fixture def virtual_dataset(): from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn