Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: deprecate rls base related filters #24128

Merged
merged 4 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ assists people when migrating to a new version.

### Breaking Changes

- [24128](https://github.com/apache/superset/pull/24128) The `RLS_BASE_RELATED_FIELD_FILTERS` config parameter has been removed. Now the Tables dropdown will feature the same tables that the user is able to see elsewhere in the application using the standard `DatasourceFilter`, and the Roles dropdown will be filtered using the filter defined in `EXTRA_RELATED_QUERY_FILTERS["role"]`.
- [23785](https://github.com/apache/superset/pull/23785) Deprecated the following feature flags: `CLIENT_CACHE`, `DASHBOARD_CACHE`, `DASHBOARD_FILTERS_EXPERIMENTAL`, `DASHBOARD_NATIVE_FILTERS`, `DASHBOARD_NATIVE_FILTERS_SET`, `DISABLE_DATASET_SOURCE_EDIT`, `ENABLE_EXPLORE_JSON_CSRF_PROTECTION`, `REMOVE_SLICE_LEVEL_LABEL_COLORS`. It also removed `DASHBOARD_EDIT_CHART_IN_NEW_TAB` as the feature is supported without the need for a feature flag.
- [22801](https://github.com/apache/superset/pull/22801): The Thumbnails feature has been changed to execute as the currently logged in user by default, falling back to the selenium user for anonymous users. To continue always using the selenium user, please add the following to your `superset_config.py`: `THUMBNAILS_EXECUTE_AS = ["selenium"]`
- [22799](https://github.com/apache/superset/pull/22799): Alerts & Reports has been changed to execute as the owner of the alert/report by default, giving priority to the last modifier and then the creator if either is contained within the list of owners, otherwise the first owner will be used. To continue using the selenium user, please add the following to your `superset_config.py`: `ALERT_REPORTS_EXECUTE_AS = ["selenium"]`
Expand Down
4 changes: 3 additions & 1 deletion superset-frontend/src/features/rls/RowLevelSecurityModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,9 @@ function RowLevelSecurityModal(props: RowLevelSecurityModalProps) {

<StyledInputContainer>
<div className="control-label">
{t('Roles')}{' '}
{currentRule.filter_type === FilterType.BASE
? t('Excluded roles')
: t('Roles')}{' '}
<InfoTooltip
tooltip={t(
'For regular filters, these are the roles this filter will be applied to. For base filters, these are the roles that the filter DOES NOT apply to, e.g. Admin if admin should see all data.',
Expand Down
13 changes: 0 additions & 13 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
from celery.schedules import crontab
from dateutil import tz
from flask import Blueprint
from flask_appbuilder.models.filters import BaseFilter
from flask_appbuilder.security.manager import AUTH_DB
from pandas._libs.parsers import STR_NA_VALUES # pylint: disable=no-name-in-module
from sqlalchemy.orm.query import Query
Expand Down Expand Up @@ -1377,18 +1376,6 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
"force_https_permanent": False,
}

# It is possible to customize which tables and roles are featured in the RLS
# dropdown. When set, this dict is assigned to `filter_rel_fields`
# on `RLSRestApi`. Example:
#
# from flask_appbuilder.models.sqla import filters

# RLS_BASE_RELATED_FIELD_FILTERS = {
# "tables": [["table_name", filters.FilterStartsWith, "birth"]],
# "roles": [["name", filters.FilterContains, "Admin"]]
# }
RLS_BASE_RELATED_FIELD_FILTERS: Dict[str, BaseFilter] = {}

#
# Flask session cookie options
#
Expand Down
8 changes: 6 additions & 2 deletions superset/row_level_security/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from flask_babel import ngettext
from marshmallow import ValidationError

from superset import app
from superset.commands.exceptions import (
DatasourceNotFoundValidationError,
RolesNotFoundValidationError,
Expand All @@ -44,11 +43,13 @@
RLSPutSchema,
RLSShowSchema,
)
from superset.views.base import DatasourceFilter
from superset.views.base_api import (
BaseSupersetModelRestApi,
requires_json,
statsd_metrics,
)
from superset.views.filters import BaseFilterRelatedRoles

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -122,7 +123,10 @@ class RLSRestApi(BaseSupersetModelRestApi):
edit_model_schema = RLSPutSchema()

allowed_rel_fields = {"tables", "roles"}
base_related_field_filters = app.config["RLS_BASE_RELATED_FIELD_FILTERS"]
base_related_field_filters = {
"tables": [["id", DatasourceFilter, lambda: []]],
"roles": [["id", BaseFilterRelatedRoles, lambda: []]],
}

@expose("/", methods=("POST",))
@protect()
Expand Down
68 changes: 17 additions & 51 deletions tests/integration_tests/security/row_level_security_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,58 +586,24 @@ def test_table_related_filter(self):
assert len(result) == 1
assert {"birth_names"} == received_tables

@mock.patch(
"superset.row_level_security.api.RLSRestApi.base_related_field_filters",
{"roles": [["name", filters.FilterEqual, "Admin"]]},
)
def test_role_related_filter(self):
self.login("Admin")

params = prison.dumps({"page": 0, "page_size": 10})

rv = self.client.get(f"/api/v1/rowlevelsecurity/related/roles?q={params}")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
result = data["result"]
received_roles = set([role["text"] for role in result])

assert data["count"] == 1
assert len(result) == 1
assert {"Admin"} == received_roles

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
@pytest.mark.usefixtures("load_energy_table_with_slice")
@mock.patch(
"superset.row_level_security.api.RLSRestApi.base_related_field_filters",
{
"tables": [["table_name", filters.FilterStartsWith, "birth"]],
"roles": [["name", filters.FilterEqual, "Admin"]],
},
)
def test_table_and_role_related_filter(self):
self.login("Admin")

params = prison.dumps({"page": 0, "page_size": 10})

rv = self.client.get(f"/api/v1/rowlevelsecurity/related/tables?q={params}")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
result = data["result"]
received_tables = set([table["text"].split(".")[-1] for table in result])

assert data["count"] == 1
assert len(result) == 1
assert {"birth_names"} == received_tables

rv = self.client.get(f"/api/v1/rowlevelsecurity/related/roles?q={params}")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
result = data["result"]
received_roles = set([role["text"] for role in result])
def test_get_all_related_roles_with_with_extra_filters(self):
"""
API: Test get filter related roles with extra related query filters
"""
self.login(username="admin")

assert data["count"] == 1
assert len(result) == 1
assert {"Admin"} == received_roles
def _base_filter(query):
return query.filter_by(name="Alpha")

with mock.patch.dict(
"superset.views.filters.current_app.config",
{"EXTRA_RELATED_QUERY_FILTERS": {"role": _base_filter}},
):
rv = self.client.get(f"/api/v1/rowlevelsecurity/related/roles")
assert rv.status_code == 200
response = json.loads(rv.data.decode("utf-8"))
response_roles = [result["text"] for result in response["result"]]
assert response_roles == ["Alpha"]


RLS_ALICE_REGEX = re.compile(r"name = 'Alice'")
Expand Down