You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by dp...@apache.org on 2022/09/29 11:30:30 UTC
[superset] branch master updated: feat: new config to filter specific users from dropdown lists (#21515)
This is an automated email from the ASF dual-hosted git repository.
dpgaspar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new ab7cfec975 feat: new config to filter specific users from dropdown lists (#21515)
ab7cfec975 is described below
commit ab7cfec975b3f06eb386e14532d42bc6a02a0687
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Thu Sep 29 12:30:07 2022 +0100
feat: new config to filter specific users from dropdown lists (#21515)
---
superset/charts/api.py | 7 +++--
superset/config.py | 7 +++++
superset/dashboards/api.py | 6 +++-
superset/datasets/api.py | 8 +++--
superset/queries/api.py | 7 +++--
superset/reports/api.py | 5 ++-
superset/security/manager.py | 16 ++++++++++
superset/views/filters.py | 34 +++++++++++++++++++-
tests/integration_tests/base_api_tests.py | 52 +++++++++++++++++++++++++++++++
tests/integration_tests/conftest.py | 34 +++++++++++++++++++-
10 files changed, 166 insertions(+), 10 deletions(-)
diff --git a/superset/charts/api.py b/superset/charts/api.py
index 9fb9872048..30bf782730 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -80,7 +80,7 @@ from superset.views.base_api import (
requires_json,
statsd_metrics,
)
-from superset.views.filters import FilterRelatedOwners
+from superset.views.filters import BaseFilterRelatedUsers, FilterRelatedOwners
logger = logging.getLogger(__name__)
config = app.config
@@ -239,7 +239,10 @@ class ChartRestApi(BaseSupersetModelRestApi):
"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 e659d7ba83..43567c01c5 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 9f440566cf..5d503b09d9 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -94,7 +94,7 @@ from superset.views.base_api import (
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 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"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 86faa8bb9d..65b660dba6 100644
--- a/superset/datasets/api.py
+++ b/superset/datasets/api.py
@@ -71,7 +71,7 @@ from superset.views.base_api import (
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 460e2dd466..83cb504937 100644
--- a/superset/queries/api.py
+++ b/superset/queries/api.py
@@ -24,7 +24,7 @@ from superset.models.sql_lab import Query
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 46480da999..565d05c703 100644
--- a/superset/reports/api.py
+++ b/superset/reports/api.py
@@ -57,7 +57,7 @@ from superset.views.base_api import (
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 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
]
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 260dbb49ee..eaf827ef90 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -1702,6 +1702,22 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
: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/<modelview>/related/<column>``
+
+ 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 3a503e66b6..7f3dfd2930 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 @@ class FilterRelatedOwners(BaseFilter): # pylint: disable=too-few-public-methods
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 16cc8cc18d..66544f1447 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.models.dashboard import Dashboard
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 @@ class ApiOwnersTestCaseMixin:
# 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 c117422adc..463f93b833 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -19,7 +19,7 @@ from __future__ import annotations
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 with_feature_flags(**mock_feature_flags):
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