You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2020/11/05 10:20:23 UTC

[incubator-superset] branch master updated: chore(rls): move to feature flag and disable related view (#11575)

This is an automated email from the ASF dual-hosted git repository.

villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 600a6fa  chore(rls): move to feature flag and disable related view (#11575)
600a6fa is described below

commit 600a6fa92a0bbe5bfd93371db5ced6af556c3697
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu Nov 5 12:19:48 2020 +0200

    chore(rls): move to feature flag and disable related view (#11575)
    
    * chore(rls): move to feature flag and disable related view
    
    * rename feature flag
---
 UPDATING.md                        |  2 ++
 superset/app.py                    |  2 +-
 superset/common/query_context.py   |  4 ++--
 superset/config.py                 | 16 ++++++++--------
 superset/connectors/sqla/models.py |  4 ++--
 superset/connectors/sqla/views.py  |  1 -
 superset/security/manager.py       |  2 ++
 superset/viz.py                    |  5 +++--
 superset/viz_sip38.py              |  2 +-
 tests/superset_test_config.py      |  2 +-
 10 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/UPDATING.md b/UPDATING.md
index 556ab8a..e56deff 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -23,6 +23,8 @@ assists people when migrating to a new version.
 
 ## Next
 
+* [11575](https://github.com/apache/incubator-superset/pull/11575) The Row Level Security (RLS) config flag has been moved to a feature flag. To migrate, add `ROW_LEVEL_SECURITY: True` to the `FEATURE_FLAGS` dict in `superset_config.py`.
+
 * NOTICE: config flag ENABLE_REACT_CRUD_VIEWS has been set to `True` by default, set to `False` if
   you prefer to vintage look and feel :)
 
diff --git a/superset/app.py b/superset/app.py
index dc8d612..427ac11 100644
--- a/superset/app.py
+++ b/superset/app.py
@@ -262,7 +262,7 @@ class SupersetAppInitializer:
             category_label=__("Manage"),
             category_icon="",
         )
-        if self.config["ENABLE_ROW_LEVEL_SECURITY"]:
+        if feature_flag_manager.is_feature_enabled("ROW_LEVEL_SECURITY"):
             appbuilder.add_view(
                 RowLevelSecurityFiltersModelView,
                 "Row Level Security",
diff --git a/superset/common/query_context.py b/superset/common/query_context.py
index 61f038f..350243c 100644
--- a/superset/common/query_context.py
+++ b/superset/common/query_context.py
@@ -24,7 +24,7 @@ import numpy as np
 import pandas as pd
 from flask_babel import gettext as _
 
-from superset import app, cache, db, security_manager
+from superset import app, cache, db, is_feature_enabled, security_manager
 from superset.common.query_object import QueryObject
 from superset.connectors.base.models import BaseDatasource
 from superset.connectors.connector_registry import ConnectorRegistry
@@ -207,7 +207,7 @@ class QueryContext:
                 datasource=self.datasource.uid,
                 extra_cache_keys=extra_cache_keys,
                 rls=security_manager.get_rls_ids(self.datasource)
-                if config["ENABLE_ROW_LEVEL_SECURITY"]
+                if is_feature_enabled("ROW_LEVEL_SECURITY")
                 and self.datasource.is_rls_supported
                 else [],
                 changed_on=self.datasource.changed_on,
diff --git a/superset/config.py b/superset/config.py
index 936230d..5af1bc2 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -324,6 +324,14 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = {
     "ESCAPE_MARKDOWN_HTML": False,
     "SIP_34_ANNOTATIONS_UI": 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": False,
 }
 
 # Set the default view to card/grid view if thumbnail support is enabled.
@@ -876,14 +884,6 @@ TALISMAN_CONFIG = {
     "force_https_permanent": 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.
-ENABLE_ROW_LEVEL_SECURITY = False
 # It is possible to customize which tables and roles are featured in the RLS
 # dropdown. When set, this dict is assigned to `add_form_query_rel_fields` and
 # `edit_form_query_rel_fields` on `RowLevelSecurityFiltersModelView`. Example:
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index c4bf979..a7c078d 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -1112,7 +1112,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
                         raise QueryObjectValidationError(
                             _("Invalid filter operation type: %(op)s", op=op)
                         )
-        if config["ENABLE_ROW_LEVEL_SECURITY"]:
+        if is_feature_enabled("ROW_LEVEL_SECURITY"):
             where_clause_and += self._get_sqla_row_level_filters(template_processor)
         if extras:
             where = extras.get("where")
@@ -1508,7 +1508,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
             templatable_statements.append(extras["where"])
         if "having" in extras:
             templatable_statements.append(extras["having"])
-        if config["ENABLE_ROW_LEVEL_SECURITY"] and self.is_rls_supported:
+        if is_feature_enabled("ROW_LEVEL_SECURITY") and 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 1b54669..e2ef794 100644
--- a/superset/connectors/sqla/views.py
+++ b/superset/connectors/sqla/views.py
@@ -359,7 +359,6 @@ class TableModelView(  # pylint: disable=too-many-ancestors
     related_views = [
         TableColumnInlineView,
         SqlMetricInlineView,
-        RowLevelSecurityFiltersModelView,
     ]
     base_order = ("changed_on", "desc")
     search_columns = ("database", "schema", "table_name", "owners", "is_sqllab_view")
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 17db91e..081bf14 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -135,6 +135,8 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
         "RoleModelView",
         "LogModelView",
         "Security",
+        "Row Level Security",
+        "Row Level Security Filters",
         "RowLevelSecurityFiltersModelView",
     } | USER_MODEL_VIEWS
 
diff --git a/superset/viz.py b/superset/viz.py
index 921daf4..e41b4ed 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -53,7 +53,7 @@ from flask_babel import lazy_gettext as _
 from geopy.point import Point
 from pandas.tseries.frequencies import to_offset
 
-from superset import app, cache, db, security_manager
+from superset import app, cache, db, is_feature_enabled, security_manager
 from superset.constants import NULL_STRING
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import (
@@ -462,7 +462,8 @@ class BaseViz:
         cache_dict["extra_cache_keys"] = self.datasource.get_extra_cache_keys(query_obj)
         cache_dict["rls"] = (
             security_manager.get_rls_ids(self.datasource)
-            if config["ENABLE_ROW_LEVEL_SECURITY"] and self.datasource.is_rls_supported
+            if is_feature_enabled("ROW_LEVEL_SECURITY")
+            and self.datasource.is_rls_supported
             else []
         )
         cache_dict["changed_on"] = self.datasource.changed_on
diff --git a/superset/viz_sip38.py b/superset/viz_sip38.py
index d9f6f72..a1ab19b 100644
--- a/superset/viz_sip38.py
+++ b/superset/viz_sip38.py
@@ -444,7 +444,7 @@ class BaseViz:
         cache_dict["extra_cache_keys"] = self.datasource.get_extra_cache_keys(query_obj)
         cache_dict["rls"] = (
             security_manager.get_rls_ids(self.datasource)
-            if config["ENABLE_ROW_LEVEL_SECURITY"]
+            if config["ROW_LEVEL_SECURITY"]
             else []
         )
         cache_dict["changed_on"] = self.datasource.changed_on
diff --git a/tests/superset_test_config.py b/tests/superset_test_config.py
index 6697207..f74272e 100644
--- a/tests/superset_test_config.py
+++ b/tests/superset_test_config.py
@@ -56,6 +56,7 @@ FEATURE_FLAGS = {
     "SHARE_QUERIES_VIA_KV_STORE": True,
     "ENABLE_TEMPLATE_PROCESSING": True,
     "ENABLE_REACT_CRUD_VIEWS": os.environ.get("ENABLE_REACT_CRUD_VIEWS", False),
+    "ROW_LEVEL_SECURITY": True,
 }
 
 
@@ -73,7 +74,6 @@ FAB_ROLES = {"TestRole": [["Security", "menu_access"], ["List Users", "menu_acce
 PUBLIC_ROLE_LIKE = "Gamma"
 AUTH_ROLE_PUBLIC = "Public"
 EMAIL_NOTIFICATIONS = False
-ENABLE_ROW_LEVEL_SECURITY = True
 CACHE_CONFIG = {"CACHE_TYPE": "simple"}
 REDIS_HOST = os.environ.get("REDIS_HOST", "localhost")
 REDIS_PORT = os.environ.get("REDIS_PORT", "6379")