Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dpgaspar committed Sep 22, 2022
1 parent 9edb010 commit 0d62500
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 10 deletions.
4 changes: 3 additions & 1 deletion superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
11 changes: 6 additions & 5 deletions superset/views/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
```
Expand All @@ -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_
34 changes: 31 additions & 3 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,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")
Expand All @@ -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):
"""
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 0d62500

Please sign in to comment.