Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(rls): move to feature flag and disable related view #11575

Merged
merged 2 commits into from
Nov 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 `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("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("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.
"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("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("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("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 superset/viz_sip38.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ def cache_key(self, query_obj, **extra):
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
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),
"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