chore: deprecate rls base related filters (#24128)

UPDATING.md

@@ -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"]`

superset-frontend/src/features/rls/RowLevelSecurityModal.tsx

@@ -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.',

superset/config.py

@@ -53,7 +53,6 @@ from cachelib.base import BaseCache
 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
@@ -1377,18 +1376,6 @@ TALISMAN_CONFIG = {
     "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
 #

superset/row_level_security/api.py

@@ -24,7 +24,6 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface
 from flask_babel import ngettext
 from marshmallow import ValidationError
 
-from superset import app
 from superset.commands.exceptions import (
     DatasourceNotFoundValidationError,
     RolesNotFoundValidationError,
@@ -44,11 +43,13 @@ from superset.row_level_security.schemas import (
     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__)
 
@@ -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()

tests/integration_tests/security/row_level_security_tests.py

@@ -586,58 +586,24 @@ class TestRowLevelSecurityWithRelatedAPI(SupersetTestCase):
         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")
+    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")
 
-        params = prison.dumps({"page": 0, "page_size": 10})
+        def _base_filter(query):
+            return query.filter_by(name="Alpha")
 
-        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])
-
-        assert data["count"] == 1
-        assert len(result) == 1
-        assert {"Admin"} == received_roles
+        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'")