Skip to content

Commit

Permalink
feat: new config to filter specific users from dropdown lists (#21515)
Browse files Browse the repository at this point in the history
  • Loading branch information
dpgaspar authored Sep 29, 2022
1 parent b787c3f commit ab7cfec
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 10 deletions.
7 changes: 5 additions & 2 deletions superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -239,7 +239,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),
Expand Down
7 changes: 7 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion superset/dashboards/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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),
Expand Down
8 changes: 6 additions & 2 deletions superset/datasets/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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",
Expand All @@ -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"}

Expand Down
7 changes: 5 additions & 2 deletions superset/queries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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),
Expand Down
5 changes: 4 additions & 1 deletion superset/reports/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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",
Expand Down
16 changes: 16 additions & 0 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/<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,
Expand Down
34 changes: 33 additions & 1 deletion superset/views/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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_
52 changes: 52 additions & 0 deletions tests/integration_tests/base_api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down
34 changes: 33 additions & 1 deletion tests/integration_tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ab7cfec

Please sign in to comment.