diff --git a/superset/charts/api.py b/superset/charts/api.py index 8a111b005d2c2..46279e7a295a8 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -80,7 +80,7 @@ requires_json, statsd_metrics, ) -from superset.views.filters import FilterRelatedOwners +from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners logger = logging.getLogger(__name__) config = app.config @@ -234,7 +234,10 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "slices": ("slice_name", "asc"), "owners": ("first_name", "asc"), } - + filter_rel_fields = { + "owners": [["id", BaseFilterRelatedUsers, lambda: []]], + "created_by": [["id", BaseFilterRelatedUsers, lambda: []]], + } related_field_filters = { "owners": RelatedFieldFilter("first_name", FilterRelatedOwners), "created_by": RelatedFieldFilter("first_name", FilterRelatedOwners), diff --git a/superset/config.py b/superset/config.py index e659d7ba83023..43567c01c5ea2 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1099,6 +1099,13 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument return msg +# Define a list of usernames to be excluded from all dropdown lists of users +# Owners, filters for created_by, etc. +# 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 # protected resources. Can be overridden by end users in order to support # custom auth mechanisms diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 9f440566cf5cb..5d503b09d908b 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -94,7 +94,7 @@ requires_json, statsd_metrics, ) -from superset.views.filters import FilterRelatedOwners +from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners logger = logging.getLogger(__name__) @@ -240,6 +240,10 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "owners": ("first_name", "asc"), "roles": ("name", "asc"), } + filter_rel_fields = { + "owners": [["id", BaseFilterRelatedUsers, lambda: []]], + "created_by": [["id", BaseFilterRelatedUsers, lambda: []]], + } related_field_filters = { "owners": RelatedFieldFilter("first_name", FilterRelatedOwners), "roles": RelatedFieldFilter("name", FilterRelatedRoles), diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 86faa8bb9df1c..65b660dba640b 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -71,7 +71,7 @@ requires_json, statsd_metrics, ) -from superset.views.filters import FilterRelatedOwners +from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners logger = logging.getLogger(__name__) @@ -214,6 +214,11 @@ class DatasetRestApi(BaseSupersetModelRestApi): "extra", ] openapi_spec_tag = "Datasets" + + filter_rel_fields = { + "owners": [["id", BaseFilterRelatedUsers, lambda: []]], + "database": [["id", DatabaseFilter, lambda: []]], + } related_field_filters = { "owners": RelatedFieldFilter("first_name", FilterRelatedOwners), "database": "database_name", @@ -223,7 +228,6 @@ class DatasetRestApi(BaseSupersetModelRestApi): "id": [DatasetCertifiedFilter], } search_columns = ["id", "database", "owners", "schema", "sql", "table_name"] - filter_rel_fields = {"database": [["id", DatabaseFilter, lambda: []]]} allowed_rel_fields = {"database", "owners"} allowed_distinct_fields = {"schema"} diff --git a/superset/queries/api.py b/superset/queries/api.py index 460e2dd4667e0..83cb504937a4c 100644 --- a/superset/queries/api.py +++ b/superset/queries/api.py @@ -24,7 +24,7 @@ from superset.queries.filters import QueryFilter from superset.queries.schemas import openapi_spec_methods_override, QuerySchema from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter -from superset.views.filters import FilterRelatedOwners +from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners logger = logging.getLogger(__name__) @@ -109,7 +109,10 @@ class QueryRestApi(BaseSupersetModelRestApi): "tab_name", "user.first_name", ] - + filter_rel_fields = { + "created_by": [["id", BaseFilterRelatedUsers, lambda: []]], + "user": [["id", BaseFilterRelatedUsers, lambda: []]], + } related_field_filters = { "created_by": RelatedFieldFilter("first_name", FilterRelatedOwners), "user": RelatedFieldFilter("first_name", FilterRelatedOwners), diff --git a/superset/reports/api.py b/superset/reports/api.py index 46480da9995ca..565d05c7033e5 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -57,7 +57,7 @@ requires_json, statsd_metrics, ) -from superset.views.filters import FilterRelatedOwners +from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners logger = logging.getLogger(__name__) @@ -204,10 +204,13 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]: ] search_filters = {"name": [ReportScheduleAllTextFilter]} allowed_rel_fields = {"owners", "chart", "dashboard", "database", "created_by"} + filter_rel_fields = { "chart": [["id", ChartFilter, lambda: []]], "dashboard": [["id", DashboardAccessFilter, lambda: []]], "database": [["id", DatabaseFilter, lambda: []]], + "owners": [["id", BaseFilterRelatedUsers, lambda: []]], + "created_by": [["id", BaseFilterRelatedUsers, lambda: []]], } text_field_rel_fields = { "dashboard": "dashboard_title", 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 3a503e66b614a..7f3dfd29306e7 100644 --- a/superset/views/filters.py +++ b/superset/views/filters.py @@ -14,15 +14,19 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import logging from typing import Any, cast, Optional +from flask import current_app from flask_appbuilder.models.filters import BaseFilter from flask_babel import lazy_gettext -from sqlalchemy import or_ +from sqlalchemy import and_, or_ from sqlalchemy.orm import Query from superset import security_manager +logger = logging.getLogger(__name__) + class FilterRelatedOwners(BaseFilter): # pylint: disable=too-few-public-methods @@ -48,3 +52,31 @@ def apply(self, query: Query, value: Optional[Any]) -> Query: user_model.username.ilike(like_value), ) ) + + +class BaseFilterRelatedUsers(BaseFilter): # pylint: disable=too-few-public-methods + + """ + Filter to apply on related users. Will exclude users in EXCLUDE_USERS_FROM_LISTS + + Use in the api by adding something like: + ``` + filter_rel_fields = { + "owners": [["id", BaseFilterRelatedUsers, lambda: []]], + "created_by": [["id", BaseFilterRelatedUsers, lambda: []]], + } + ``` + """ + + name = lazy_gettext("username") + arg_name = "username" + + def apply(self, query: Query, value: Optional[Any]) -> Query: + user_model = security_manager.user_model + 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 16cc8cc18d045..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,6 +291,55 @@ 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" + 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 + + @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): """ API: Test get filter related owners 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