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!: remove ROW_LEVEL_SECURITY feature flag (permanently enable) #19230

Merged
merged 10 commits into from
Mar 31, 2022
1 change: 0 additions & 1 deletion RELEASING/release-notes-1-0/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion RESOURCES/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
]
Expand Down
13 changes: 1 addition & 12 deletions superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions superset/initialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
),
)

#
Expand Down
5 changes: 1 addition & 4 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
40 changes: 0 additions & 40 deletions tests/integration_tests/sqla_views_tests.py

This file was deleted.