From ab3770667c0b11043b177838f8c2eddd717fcfcc Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Thu, 31 Mar 2022 14:05:17 -0700 Subject: [PATCH] chore!: remove `ROW_LEVEL_SECURITY` feature flag (permanently enable) (#19230) * permanently turn on rls feature flag * unused imports * docs * docs * unused import --- RELEASING/release-notes-1-0/README.md | 1 - RESOURCES/FEATURE_FLAGS.md | 1 - UPDATING.md | 1 + superset/config.py | 8 ----- superset/connectors/sqla/models.py | 5 ++- superset/connectors/sqla/views.py | 13 +------ superset/initialization/__init__.py | 3 -- superset/security/manager.py | 5 +-- tests/integration_tests/sqla_views_tests.py | 40 --------------------- 9 files changed, 5 insertions(+), 72 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 77e381c7275cc..26ac6bfde9435 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -52,7 +52,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/UPDATING.md b/UPDATING.md index a911b4b541291..865b3907d5ff5 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -29,6 +29,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 (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. - [19168](https://github.com/apache/superset/pull/19168): Celery upgrade to 5.X has breaking changes on it's command line invocation. Please follow: https://docs.celeryq.dev/en/stable/whatsnew-5.2.html#step-1-adjust-your-command-line-invocation Consider migrating you celery config if you haven't already: https://docs.celeryq.dev/en/stable/userguide/configuration.html#conf-old-settings-map diff --git a/superset/config.py b/superset/config.py index 4c68aa25515dd..62198c484c64a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -397,14 +397,6 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: "DASHBOARD_FILTERS_EXPERIMENTAL": False, "GLOBAL_ASYNC_QUERIES": False, "VERSIONED_EXPORT": True, - # 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 291ddb3764859..0600f3b10a582 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -1393,8 +1393,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: @@ -1810,7 +1809,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 116b18869d009..aa176ba2b8642 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -22,15 +22,13 @@ from flask import current_app, flash, Markup, redirect from flask_appbuilder import CompactCRUDMixin, expose 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 lazy_gettext as _ -from werkzeug.exceptions import NotFound 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 @@ -328,15 +326,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 5b11f0930a782..e600b0f2f5e7b 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -277,9 +277,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 ab21dbd54eed4..f57f1166ce394 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1222,11 +1222,8 @@ 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 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)