From bdf7bd484deb2e9dbebfbc875d84a4815c53bdee Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 16 Mar 2022 17:37:08 -0700 Subject: [PATCH 1/5] permanently turn on rls feature flag --- RELEASING/release-notes-1-0/README.md | 1 - RESOURCES/FEATURE_FLAGS.md | 1 - superset/config.py | 8 ----- superset/connectors/sqla/models.py | 5 ++- superset/connectors/sqla/views.py | 9 ----- superset/initialization/__init__.py | 3 -- superset/security/manager.py | 2 +- tests/integration_tests/sqla_views_tests.py | 40 --------------------- 8 files changed, 3 insertions(+), 66 deletions(-) delete mode 100644 tests/integration_tests/sqla_views_tests.py diff --git a/RELEASING/release-notes-1-0/README.md b/RELEASING/release-notes-1-0/README.md index d476cb6e1b8d9..ed1eeea0dab07 100644 --- a/RELEASING/release-notes-1-0/README.md +++ b/RELEASING/release-notes-1-0/README.md @@ -96,7 +96,6 @@ Some of the new features in this release are disabled by default. Each has a fea | Dashboard Native Filters | `DASHBOARD_NATIVE_FILTERS: True` | | | Alerts & Reporting | `ALERT_REPORTS: True` | [Celery workers configured & celery beat process](https://superset.apache.org/docs/installation/async-queries-celery) | | Homescreen Thumbnails | `THUMBNAILS: TRUE, THUMBNAIL_CACHE_CONFIG: CacheConfig = { "CACHE_TYPE": "null", "CACHE_NO_NULL_WARNING": True}`| selenium, pillow 7, celery | -| Row Level Security | `ROW_LEVEL_SECURITY` | | [Extra Documentation](https://superset.apache.org/docs/security#row-level-security) | Dynamic Viz Plugin Import | `DYNAMIC_PLUGINS: True` | | # Stability and Bugfixes diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index 69f8f8a5a691d..732eb07dd9a2b 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -53,7 +53,6 @@ These features flags are **safe for production** and have been tested. - ESCAPE_MARKDOWN_HTML - ENABLE_TEMPLATE_PROCESSING - LISTVIEWS_DEFAULT_CARD_VIEW -- ROW_LEVEL_SECURITY - SCHEDULED_QUERIES [(docs)](https://superset.apache.org/docs/installation/alerts-reports) - SQL_VALIDATORS_BY_ENGINE [(docs)](https://superset.apache.org/docs/installation/sql-templating) - SQLLAB_BACKEND_PERSISTENCE diff --git a/superset/config.py b/superset/config.py index 6579719ea81cc..084cc14d3add3 100644 --- a/superset/config.py +++ b/superset/config.py @@ -408,14 +408,6 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: "DASHBOARD_FILTERS_EXPERIMENTAL": False, "GLOBAL_ASYNC_QUERIES": False, "VERSIONED_EXPORT": False, - # Note that: RowLevelSecurityFilter is only given by default to the Admin role - # and the Admin Role does have the all_datasources security permission. - # But, if users create a specific role with access to RowLevelSecurityFilter MVC - # and a custom datasource access, the table dropdown will not be correctly filtered - # by that custom datasource access. So we are assuming a default security config, - # a custom security config could potentially give access to setting filters on - # tables that users do not have access to. - "ROW_LEVEL_SECURITY": True, "EMBEDDED_SUPERSET": False, # Enables Alerts and reports new implementation "ALERT_REPORTS": False, diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 9cc2f8a78136b..11ae91a51cdf4 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -1335,8 +1335,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma raise QueryObjectValidationError( _("Invalid filter operation type: %(op)s", op=op) ) - if is_feature_enabled("ROW_LEVEL_SECURITY"): - where_clause_and += self._get_sqla_row_level_filters(template_processor) + where_clause_and += self._get_sqla_row_level_filters(template_processor) if extras: where = extras.get("where") if where: @@ -1744,7 +1743,7 @@ def has_extra_cache_key_calls(self, query_obj: QueryObjectDict) -> bool: templatable_statements.append(extras["where"]) if "having" in extras: templatable_statements.append(extras["having"]) - if is_feature_enabled("ROW_LEVEL_SECURITY") and self.is_rls_supported: + if self.is_rls_supported: templatable_statements += [ f.clause for f in security_manager.get_rls_filters(self) ] diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index fef8a2d8a4356..a735ee736481a 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -374,15 +374,6 @@ class RowLevelSecurityFiltersModelView(SupersetModelView, DeleteMixin): add_form_query_rel_fields = app.config["RLS_FORM_QUERY_REL_FIELDS"] edit_form_query_rel_fields = add_form_query_rel_fields - @staticmethod - def is_enabled() -> bool: - return is_feature_enabled("ROW_LEVEL_SECURITY") - - @before_request - def ensure_enabled(self) -> None: - if not self.is_enabled(): - raise NotFound() - class TableModelView( # pylint: disable=too-many-ancestors DatasourceModelView, DeleteMixin, YamlExportMixin diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 59e204d6d65e8..d860935bf6ec1 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -280,9 +280,6 @@ def init_views(self) -> None: category="Security", category_label=__("Security"), icon="fa-lock", - menu_cond=lambda: feature_flag_manager.is_feature_enabled( - "ROW_LEVEL_SECURITY" - ), ) # diff --git a/superset/security/manager.py b/superset/security/manager.py index eb068c81fbb11..68d0d0e17971f 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1230,7 +1230,7 @@ def get_rls_cache_key(self, datasource: "BaseDatasource") -> List[str]: from superset import is_feature_enabled rls_ids = [] - if is_feature_enabled("ROW_LEVEL_SECURITY") and datasource.is_rls_supported: + if datasource.is_rls_supported: rls_ids = self.get_rls_ids(datasource) rls_str = [str(rls_id) for rls_id in rls_ids] guest_rls = self.get_guest_rls_filters_str(datasource) diff --git a/tests/integration_tests/sqla_views_tests.py b/tests/integration_tests/sqla_views_tests.py deleted file mode 100644 index 92c350c8785c9..0000000000000 --- a/tests/integration_tests/sqla_views_tests.py +++ /dev/null @@ -1,40 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -from tests.integration_tests.base_tests import SupersetTestCase -from tests.integration_tests.conftest import with_feature_flags - - -class TestRowLevelSecurityFiltersModelView(SupersetTestCase): - @with_feature_flags(ROW_LEVEL_SECURITY=False) - def test_rls_disabled(self): - """ - RLS Filters Model View: Responds not found when disabled - """ - self.login(username="admin") - uri = "/rowlevelsecurityfiltersmodelview/api" - rv = self.client.get(uri) - self.assertEqual(rv.status_code, 404) - - @with_feature_flags(ROW_LEVEL_SECURITY=True) - def test_rls_enabled(self): - """ - RLS Filters Model View: Responds successfully when enabled - """ - self.login(username="admin") - uri = "/rowlevelsecurityfiltersmodelview/api" - rv = self.client.get(uri) - self.assertEqual(rv.status_code, 200) From be93a61c396d023a42da2afd9dbf4d5e727312ca Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 16 Mar 2022 17:52:40 -0700 Subject: [PATCH 2/5] unused imports --- superset/connectors/sqla/views.py | 2 -- superset/security/manager.py | 3 --- 2 files changed, 5 deletions(-) diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index a735ee736481a..a687daf79dcbb 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -24,11 +24,9 @@ from flask_appbuilder import CompactCRUDMixin, expose from flask_appbuilder.actions import action from flask_appbuilder.fieldwidgets import Select2Widget -from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.decorators import has_access from flask_babel import gettext as __, lazy_gettext as _ -from werkzeug.exceptions import NotFound from wtforms.ext.sqlalchemy.fields import QuerySelectField from wtforms.validators import Regexp diff --git a/superset/security/manager.py b/superset/security/manager.py index 68d0d0e17971f..42afa9616ce53 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1226,9 +1226,6 @@ def get_guest_rls_filters_str(self, table: "BaseDatasource") -> List[str]: return [f.get("clause", "") for f in self.get_guest_rls_filters(table)] def get_rls_cache_key(self, datasource: "BaseDatasource") -> List[str]: - # pylint: disable=import-outside-toplevel - from superset import is_feature_enabled - rls_ids = [] if datasource.is_rls_supported: rls_ids = self.get_rls_ids(datasource) From a527069bf39c32ce7073e277f9bbef3e531142ec Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 16 Mar 2022 17:55:38 -0700 Subject: [PATCH 3/5] docs --- UPDATING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UPDATING.md b/UPDATING.md index ea9f02094a52c..7ce8799ef8fb7 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -28,6 +28,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [19230](https://github.com/apache/superset/pull/19230): The `ROW_LEVEL_SECURITY` feature flag has been removed. Row Level Security is now always on. - [19113](https://github.com/apache/superset/pull/19113): The `ENABLE_JAVASCRIPT_CONTROLS` setting has moved from app config to a feature flag. Any deployments who overrode this setting will now need to override the feature flag from here onward. - [18976](https://github.com/apache/superset/pull/18976): When running the app in debug mode, the app will default to use `SimpleCache` for `FILTER_STATE_CACHE_CONFIG` and `EXPLORE_FORM_DATA_CACHE_CONFIG`. When running in non-debug mode, a cache backend will need to be defined, otherwise the application will fail to start. For installations using Redis or other caching backends, it is recommended to use the same backend for both cache configs. - [17881](https://github.com/apache/superset/pull/17881): Previously simple adhoc filter values on string columns were stripped of enclosing single and double quotes. To fully support literal quotes in filters, both single and double quotes will no longer be removed from filter values. From 4e093a4988815e41ae256800a0ad8638ef825aa8 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Wed, 16 Mar 2022 18:46:24 -0700 Subject: [PATCH 4/5] docs --- UPDATING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPDATING.md b/UPDATING.md index 7ce8799ef8fb7..0d9e1fc19b2b0 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -28,7 +28,7 @@ assists people when migrating to a new version. ### Breaking Changes -- [19230](https://github.com/apache/superset/pull/19230): The `ROW_LEVEL_SECURITY` feature flag has been removed. Row Level Security is now always on. +- [19230](https://github.com/apache/superset/pull/19230): The `ROW_LEVEL_SECURITY` feature flag has been removed (permanently enabled). Any deployments which had set this flag to false will need to verify that the presence of the Row Level Security feature does not interfere with their use case. - [19113](https://github.com/apache/superset/pull/19113): The `ENABLE_JAVASCRIPT_CONTROLS` setting has moved from app config to a feature flag. Any deployments who overrode this setting will now need to override the feature flag from here onward. - [18976](https://github.com/apache/superset/pull/18976): When running the app in debug mode, the app will default to use `SimpleCache` for `FILTER_STATE_CACHE_CONFIG` and `EXPLORE_FORM_DATA_CACHE_CONFIG`. When running in non-debug mode, a cache backend will need to be defined, otherwise the application will fail to start. For installations using Redis or other caching backends, it is recommended to use the same backend for both cache configs. - [17881](https://github.com/apache/superset/pull/17881): Previously simple adhoc filter values on string columns were stripped of enclosing single and double quotes. To fully support literal quotes in filters, both single and double quotes will no longer be removed from filter values. From 23426b2ba1ac050ea3f99ba18532ee1087d95f28 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 31 Mar 2022 10:35:38 -0700 Subject: [PATCH 5/5] unused import --- superset/connectors/sqla/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 65c348bb28c8c..700a220994a1e 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -30,7 +30,7 @@ from wtforms.ext.sqlalchemy.fields import QuerySelectField from wtforms.validators import Regexp -from superset import app, db, is_feature_enabled +from superset import app, db from superset.connectors.base.views import DatasourceModelView from superset.connectors.sqla import models from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod