Skip to content

Commit

Permalink
chore(rls): move to feature flag and disable related view
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro committed Nov 5, 2020
1 parent 52145f8 commit 44a8649
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 17 deletions.
2 changes: 2 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `ENABLE_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 :)

Expand Down
2 changes: 1 addition & 1 deletion superset/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def init_views(self) -> None:
category_label=__("Manage"),
category_icon="",
)
if self.config["ENABLE_ROW_LEVEL_SECURITY"]:
if feature_flag_manager.is_feature_enabled("ENABLE_ROW_LEVEL_SECURITY"):
appbuilder.add_view(
RowLevelSecurityFiltersModelView,
"Row Level Security",
Expand Down
4 changes: 2 additions & 2 deletions superset/common/query_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
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
Expand Down Expand Up @@ -207,7 +207,7 @@ def cache_key(self, query_obj: QueryObject, **kwargs: Any) -> Optional[str]:
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("ENABLE_ROW_LEVEL_SECURITY")
and self.datasource.is_rls_supported
else [],
changed_on=self.datasource.changed_on,
Expand Down
16 changes: 8 additions & 8 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,14 @@ def _try_json_readsha( # pylint: disable=unused-argument
"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.
"ENABLE_ROW_LEVEL_SECURITY": False,
}

# Set the default view to card/grid view if thumbnail support is enabled.
Expand Down Expand Up @@ -876,14 +884,6 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
"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:
Expand Down
4 changes: 2 additions & 2 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,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 config["ENABLE_ROW_LEVEL_SECURITY"]:
if is_feature_enabled("ENABLE_ROW_LEVEL_SECURITY"):
where_clause_and += self._get_sqla_row_level_filters(template_processor)
if extras:
where = extras.get("where")
Expand Down Expand Up @@ -1508,7 +1508,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 config["ENABLE_ROW_LEVEL_SECURITY"] and self.is_rls_supported:
if is_feature_enabled("ENABLE_ROW_LEVEL_SECURITY") and self.is_rls_supported:
templatable_statements += [
f.clause for f in security_manager.get_rls_filters(self)
]
Expand Down
1 change: 0 additions & 1 deletion superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 3 additions & 2 deletions superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
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 (
Expand Down Expand Up @@ -462,7 +462,8 @@ def cache_key(self, query_obj: QueryObjectDict, **extra: Any) -> str:
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("ENABLE_ROW_LEVEL_SECURITY")
and self.datasource.is_rls_supported
else []
)
cache_dict["changed_on"] = self.datasource.changed_on
Expand Down
2 changes: 1 addition & 1 deletion tests/superset_test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"SHARE_QUERIES_VIA_KV_STORE": True,
"ENABLE_TEMPLATE_PROCESSING": True,
"ENABLE_REACT_CRUD_VIEWS": os.environ.get("ENABLE_REACT_CRUD_VIEWS", False),
"ENABLE_ROW_LEVEL_SECURITY": True,
}


Expand All @@ -73,7 +74,6 @@ def GET_FEATURE_FLAGS_FUNC(ff):
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")
Expand Down

0 comments on commit 44a8649

Please sign in to comment.