From d5c008dd99af4fb73ad972aed02802973c4a9d95 Mon Sep 17 00:00:00 2001 From: Ben Reinhart Date: Fri, 21 May 2021 14:29:52 -0700 Subject: [PATCH] chore: Perform feature/config condition checks at request time (#14684) * chore: conditional Home link rendering * chore: conditional RowLevelSecurity rendering * chore: Conditional KV rendering * chore: Conditional TagView rendering * chore: Conditional import dashboards link * chore: Conditional upload csv/excel links * chore: Conditional log api and view rendering * chore: Conditionally render email schedules * chore: Conditionally render alert views * chore: Conditionally render alerts/reports * chore: Conditionally render access requests * chore: Conditionally render druid views * Remove unnecessary folder * Consistent naming * Cleanup * Remove object from class * Clean up test file * Clean up test file * Fix lint error * Better naming and follow conventions * Use assertLess over assertNotEqual * Assert less than 400 * Fix failing test --- superset/app.py | 336 +++++++++++++++-------------- superset/connectors/druid/views.py | 29 ++- superset/connectors/sqla/views.py | 11 + superset/views/access_requests.py | 12 ++ superset/views/alerts.py | 24 ++- superset/views/key_value.py | 13 +- superset/views/log/api.py | 12 ++ superset/views/log/views.py | 12 ++ superset/views/schedules.py | 13 +- superset/views/tags.py | 13 +- tests/alerts_tests.py | 51 ++++- tests/core_tests.py | 22 +- tests/druid_tests.py | 79 +++++++ tests/log_api_tests.py | 13 +- tests/log_model_view_tests.py | 37 ++++ tests/schedules_test.py | 29 +++ tests/security_tests.py | 17 ++ tests/sqla_views_tests.py | 40 ++++ tests/tagging_tests.py | 26 +-- 19 files changed, 593 insertions(+), 196 deletions(-) create mode 100644 tests/log_model_view_tests.py create mode 100644 tests/sqla_views_tests.py diff --git a/superset/app.py b/superset/app.py index a01d19afe..7b984df03 100644 --- a/superset/app.py +++ b/superset/app.py @@ -226,10 +226,12 @@ class SupersetAppInitializer: # # Setup regular views # - if appbuilder.app.config["LOGO_TARGET_PATH"]: - appbuilder.add_link( - "Home", label=__("Home"), href="/superset/welcome/", - ) + appbuilder.add_link( + "Home", + label=__("Home"), + href="/superset/welcome/", + cond=lambda: bool(appbuilder.app.config["LOGO_TARGET_PATH"]), + ) appbuilder.add_view( AnnotationLayerModelView, "Annotation Layers", @@ -294,15 +296,17 @@ class SupersetAppInitializer: category_label=__("Manage"), category_icon="", ) - if feature_flag_manager.is_feature_enabled("ROW_LEVEL_SECURITY"): - appbuilder.add_view( - RowLevelSecurityFiltersModelView, - "Row Level Security", - label=__("Row level security"), - category="Security", - category_label=__("Security"), - icon="fa-lock", - ) + appbuilder.add_view( + RowLevelSecurityFiltersModelView, + "Row Level Security", + label=__("Row level security"), + category="Security", + category_label=__("Security"), + icon="fa-lock", + menu_cond=lambda: feature_flag_manager.is_feature_enabled( + "ROW_LEVEL_SECURITY" + ), + ) # # Setup views with no menu @@ -314,10 +318,7 @@ class SupersetAppInitializer: appbuilder.add_view_no_menu(Dashboard) appbuilder.add_view_no_menu(DashboardModelViewAsync) appbuilder.add_view_no_menu(Datasource) - - if feature_flag_manager.is_feature_enabled("KV_STORE"): - appbuilder.add_view_no_menu(KV) - + appbuilder.add_view_no_menu(KV) appbuilder.add_view_no_menu(R) appbuilder.add_view_no_menu(SavedQueryView) appbuilder.add_view_no_menu(SavedQueryViewApi) @@ -330,23 +331,23 @@ class SupersetAppInitializer: appbuilder.add_view_no_menu(TableModelView) appbuilder.add_view_no_menu(TableSchemaView) appbuilder.add_view_no_menu(TabStateView) - - if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): - appbuilder.add_view_no_menu(TagView) + appbuilder.add_view_no_menu(TagView) # # Add links # - if not feature_flag_manager.is_feature_enabled("VERSIONED_EXPORT"): - appbuilder.add_link( - "Import Dashboards", - label=__("Import Dashboards"), - href="/superset/import_dashboards/", - icon="fa-cloud-upload", - category="Manage", - category_label=__("Manage"), - category_icon="fa-wrench", - ) + appbuilder.add_link( + "Import Dashboards", + label=__("Import Dashboards"), + href="/superset/import_dashboards/", + icon="fa-cloud-upload", + category="Manage", + category_label=__("Manage"), + category_icon="fa-wrench", + cond=lambda: not feature_flag_manager.is_feature_enabled( + "VERSIONED_EXPORT" + ), + ) appbuilder.add_link( "SQL Editor", label=_("SQL Editor"), @@ -371,49 +372,54 @@ class SupersetAppInitializer: category="SQL Lab", category_label=__("SQL Lab"), ) - if self.config["CSV_EXTENSIONS"].intersection( - self.config["ALLOWED_EXTENSIONS"] - ): + appbuilder.add_link( + "Upload a CSV", + label=__("Upload a CSV"), + href="/csvtodatabaseview/form", + icon="fa-upload", + category="Data", + category_label=__("Data"), + category_icon="fa-wrench", + cond=lambda: bool( + self.config["CSV_EXTENSIONS"].intersection( + self.config["ALLOWED_EXTENSIONS"] + ) + ), + ) + + try: + import xlrd # pylint: disable=unused-import + appbuilder.add_link( - "Upload a CSV", - label=__("Upload a CSV"), - href="/csvtodatabaseview/form", + "Upload Excel", + label=__("Upload Excel"), + href="/exceltodatabaseview/form", icon="fa-upload", category="Data", category_label=__("Data"), category_icon="fa-wrench", + cond=lambda: bool( + self.config["EXCEL_EXTENSIONS"].intersection( + self.config["ALLOWED_EXTENSIONS"] + ) + ), ) - try: - import xlrd # pylint: disable=unused-import - - if self.config["EXCEL_EXTENSIONS"].intersection( - self.config["ALLOWED_EXTENSIONS"] - ): - appbuilder.add_link( - "Upload Excel", - label=__("Upload Excel"), - href="/exceltodatabaseview/form", - icon="fa-upload", - category="Data", - category_label=__("Data"), - category_icon="fa-wrench", - ) except ImportError: pass - # - # Conditionally setup log views - # - if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]: - appbuilder.add_api(LogRestApi) - appbuilder.add_view( - LogModelView, - "Action Log", - label=__("Action Log"), - category="Security", - category_label=__("Security"), - icon="fa-list-ol", - ) + appbuilder.add_api(LogRestApi) + appbuilder.add_view( + LogModelView, + "Action Log", + label=__("Action Log"), + category="Security", + category_label=__("Security"), + icon="fa-list-ol", + menu_cond=lambda: ( + self.config["FAB_ADD_SECURITY_VIEWS"] + and self.config["SUPERSET_LOG_VIEW"] + ), + ) appbuilder.add_api(SecurityRestApi) # # Conditionally setup email views @@ -423,109 +429,125 @@ class SupersetAppInitializer: "ENABLE_SCHEDULED_EMAIL_REPORTS " "is deprecated and will be removed in version 2.0.0" ) - appbuilder.add_separator("Manage") - appbuilder.add_view( - DashboardEmailScheduleView, - "Dashboard Email Schedules", - label=__("Dashboard Emails"), - category="Manage", - category_label=__("Manage"), - icon="fa-search", - ) - appbuilder.add_view( - SliceEmailScheduleView, - "Chart Emails", - label=__("Chart Email Schedules"), - category="Manage", - category_label=__("Manage"), - icon="fa-search", - ) + + appbuilder.add_separator( + "Manage", cond=lambda: self.config["ENABLE_SCHEDULED_EMAIL_REPORTS"] + ) + appbuilder.add_view( + DashboardEmailScheduleView, + "Dashboard Email Schedules", + label=__("Dashboard Emails"), + category="Manage", + category_label=__("Manage"), + icon="fa-search", + menu_cond=lambda: self.config["ENABLE_SCHEDULED_EMAIL_REPORTS"], + ) + appbuilder.add_view( + SliceEmailScheduleView, + "Chart Emails", + label=__("Chart Email Schedules"), + category="Manage", + category_label=__("Manage"), + icon="fa-search", + menu_cond=lambda: self.config["ENABLE_SCHEDULED_EMAIL_REPORTS"], + ) if self.config["ENABLE_ALERTS"]: logging.warning( "ENABLE_ALERTS is deprecated and will be removed in version 2.0.0" ) - appbuilder.add_view( - AlertModelView, - "Alerts", - label=__("Alerts"), - category="Manage", - category_label=__("Manage"), - icon="fa-exclamation-triangle", - ) - appbuilder.add_view_no_menu(AlertLogModelView) - appbuilder.add_view_no_menu(AlertObservationModelView) - if feature_flag_manager.is_feature_enabled("ALERT_REPORTS"): - appbuilder.add_view( - AlertView, - "Alerts & Report", - label=__("Alerts & Reports"), - category="Manage", - category_label=__("Manage"), - icon="fa-exclamation-triangle", - ) - appbuilder.add_view_no_menu(ReportView) + appbuilder.add_view( + AlertModelView, + "Alerts", + label=__("Alerts"), + category="Manage", + category_label=__("Manage"), + icon="fa-exclamation-triangle", + menu_cond=lambda: bool(self.config["ENABLE_ALERTS"]), + ) + appbuilder.add_view_no_menu(AlertLogModelView) + appbuilder.add_view_no_menu(AlertObservationModelView) + + appbuilder.add_view( + AlertView, + "Alerts & Report", + label=__("Alerts & Reports"), + category="Manage", + category_label=__("Manage"), + icon="fa-exclamation-triangle", + menu_cond=lambda: feature_flag_manager.is_feature_enabled("ALERT_REPORTS"), + ) + appbuilder.add_view_no_menu(ReportView) + + appbuilder.add_view( + AccessRequestsModelView, + "Access requests", + label=__("Access requests"), + category="Security", + category_label=__("Security"), + icon="fa-table", + menu_cond=lambda: bool(self.config["ENABLE_ACCESS_REQUEST"]), + ) # - # Conditionally add Access Request Model View + # Druid Views # - if self.config["ENABLE_ACCESS_REQUEST"]: - appbuilder.add_view( - AccessRequestsModelView, - "Access requests", - label=__("Access requests"), - category="Security", - category_label=__("Security"), - icon="fa-table", - ) + appbuilder.add_separator( + "Data", cond=lambda: bool(self.config["DRUID_IS_ACTIVE"]) + ) + appbuilder.add_view( + DruidDatasourceModelView, + "Druid Datasources", + label=__("Druid Datasources"), + category="Data", + category_label=__("Data"), + icon="fa-cube", + menu_cond=lambda: bool(self.config["DRUID_IS_ACTIVE"]), + ) + appbuilder.add_view( + DruidClusterModelView, + name="Druid Clusters", + label=__("Druid Clusters"), + icon="fa-cubes", + category="Data", + category_label=__("Data"), + category_icon="fa-database", + menu_cond=lambda: bool(self.config["DRUID_IS_ACTIVE"]), + ) + appbuilder.add_view_no_menu(DruidMetricInlineView) + appbuilder.add_view_no_menu(DruidColumnInlineView) + appbuilder.add_view_no_menu(Druid) - # - # Conditionally setup Druid Views - # - if self.config["DRUID_IS_ACTIVE"]: - appbuilder.add_separator("Data") - appbuilder.add_view( - DruidDatasourceModelView, - "Druid Datasources", - label=__("Druid Datasources"), - category="Data", - category_label=__("Data"), - icon="fa-cube", - ) - appbuilder.add_view( - DruidClusterModelView, - name="Druid Clusters", - label=__("Druid Clusters"), - icon="fa-cubes", - category="Data", - category_label=__("Data"), - category_icon="fa-database", - ) - appbuilder.add_view_no_menu(DruidMetricInlineView) - appbuilder.add_view_no_menu(DruidColumnInlineView) - appbuilder.add_view_no_menu(Druid) - - if self.config["DRUID_METADATA_LINKS_ENABLED"]: - appbuilder.add_link( - "Scan New Datasources", - label=__("Scan New Datasources"), - href="/druid/scan_new_datasources/", - category="Data", - category_label=__("Data"), - category_icon="fa-database", - icon="fa-refresh", - ) - appbuilder.add_link( - "Refresh Druid Metadata", - label=__("Refresh Druid Metadata"), - href="/druid/refresh_datasources/", - category="Data", - category_label=__("Data"), - category_icon="fa-database", - icon="fa-cog", - ) - appbuilder.add_separator("Data") + appbuilder.add_link( + "Scan New Datasources", + label=__("Scan New Datasources"), + href="/druid/scan_new_datasources/", + category="Data", + category_label=__("Data"), + category_icon="fa-database", + icon="fa-refresh", + cond=lambda: bool( + self.config["DRUID_IS_ACTIVE"] + and self.config["DRUID_METADATA_LINKS_ENABLED"] + ), + ) + appbuilder.add_link( + "Refresh Druid Metadata", + label=__("Refresh Druid Metadata"), + href="/druid/refresh_datasources/", + category="Data", + category_label=__("Data"), + category_icon="fa-database", + icon="fa-cog", + cond=lambda: bool( + self.config["DRUID_IS_ACTIVE"] + and self.config["DRUID_METADATA_LINKS_ENABLED"] + ), + ) + appbuilder.add_separator( + "Data", cond=lambda: bool(self.config["DRUID_IS_ACTIVE"]) + ) def init_app_in_ctx(self) -> None: """ diff --git a/superset/connectors/druid/views.py b/superset/connectors/druid/views.py index 4b2b45bb6..5454a41d3 100644 --- a/superset/connectors/druid/views.py +++ b/superset/connectors/druid/views.py @@ -19,12 +19,14 @@ import json import logging from datetime import datetime -from flask import flash, Markup, redirect +from flask import current_app as 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 import StringField from wtforms.ext.sqlalchemy.fields import QuerySelectField @@ -49,7 +51,18 @@ from superset.views.base import ( logger = logging.getLogger(__name__) -class DruidColumnInlineView(CompactCRUDMixin, SupersetModelView): +class EnsureEnabledMixin: + @staticmethod + def is_enabled() -> bool: + return bool(app.config["DRUID_IS_ACTIVE"]) + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() + + +class DruidColumnInlineView(CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView): datamodel = SQLAInterface(models.DruidColumn) include_route_methods = RouteMethod.RELATED_VIEW_SET @@ -136,7 +149,7 @@ class DruidColumnInlineView(CompactCRUDMixin, SupersetModelView): self.post_update(item) -class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): +class DruidMetricInlineView(CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView): datamodel = SQLAInterface(models.DruidMetric) include_route_methods = RouteMethod.RELATED_VIEW_SET @@ -189,7 +202,9 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): edit_form_extra_fields = add_form_extra_fields -class DruidClusterModelView(SupersetModelView, DeleteMixin, YamlExportMixin): +class DruidClusterModelView( + EnsureEnabledMixin, SupersetModelView, DeleteMixin, YamlExportMixin, +): datamodel = SQLAInterface(models.DruidCluster) include_route_methods = RouteMethod.CRUD_SET list_title = _("Druid Clusters") @@ -251,7 +266,9 @@ class DruidClusterModelView(SupersetModelView, DeleteMixin, YamlExportMixin): DeleteMixin._delete(self, pk) -class DruidDatasourceModelView(DatasourceModelView, DeleteMixin, YamlExportMixin): +class DruidDatasourceModelView( + EnsureEnabledMixin, DatasourceModelView, DeleteMixin, YamlExportMixin, +): datamodel = SQLAInterface(models.DruidDatasource) include_route_methods = RouteMethod.CRUD_SET list_title = _("Druid Datasources") @@ -367,7 +384,7 @@ class DruidDatasourceModelView(DatasourceModelView, DeleteMixin, YamlExportMixin DeleteMixin._delete(self, pk) -class Druid(BaseSupersetView): +class Druid(EnsureEnabledMixin, BaseSupersetView): """The base views for Superset!""" @has_access diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 8f2681dd2..f7f6742ab 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -24,9 +24,11 @@ from flask import current_app, flash, Markup, redirect from flask_appbuilder import CompactCRUDMixin, expose from flask_appbuilder.actions import action 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 gettext as __, lazy_gettext as _ +from werkzeug.exceptions import NotFound from wtforms.ext.sqlalchemy.fields import QuerySelectField from wtforms.validators import Regexp @@ -369,6 +371,15 @@ class RowLevelSecurityFiltersModelView( # pylint: disable=too-many-ancestors 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/views/access_requests.py b/superset/views/access_requests.py index abcf4be50..bca2e7bfc 100644 --- a/superset/views/access_requests.py +++ b/superset/views/access_requests.py @@ -14,8 +14,11 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from flask import current_app as app +from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import lazy_gettext as _ +from werkzeug.exceptions import NotFound from superset.constants import RouteMethod from superset.views.base import DeleteMixin, SupersetModelView @@ -44,3 +47,12 @@ class AccessRequestsModelView( # pylint: disable=too-many-ancestors "roles_with_datasource": _("Roles to grant"), "created_on": _("Created On"), } + + @staticmethod + def is_enabled() -> bool: + return bool(app.config["ENABLE_ACCESS_REQUEST"]) + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() diff --git a/superset/views/alerts.py b/superset/views/alerts.py index 46538dedd..bf3ea7ae5 100644 --- a/superset/views/alerts.py +++ b/superset/views/alerts.py @@ -17,14 +17,15 @@ """ DEPRECATION NOTICE: this module is deprecated and will be removed on 2.0. """ - from croniter import croniter -from flask import abort, flash, Markup +from flask import abort, current_app as app, flash, Markup from flask_appbuilder import CompactCRUDMixin, permission_name from flask_appbuilder.api import expose +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 superset import is_feature_enabled from superset.constants import RouteMethod @@ -40,8 +41,19 @@ from .base import BaseSupersetView, SupersetModelView # TODO: access control rules for this module +class EnsureEnabledMixin: + @staticmethod + def is_enabled() -> bool: + return bool(app.config["ENABLE_ALERTS"]) + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() + + class AlertLogModelView( - CompactCRUDMixin, SupersetModelView + CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView ): # pylint: disable=too-many-ancestors datamodel = SQLAInterface(AlertLog) include_route_methods = {RouteMethod.LIST} | {"show"} @@ -55,7 +67,7 @@ class AlertLogModelView( class AlertObservationModelView( - CompactCRUDMixin, SupersetModelView + CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView ): # pylint: disable=too-many-ancestors datamodel = SQLAInterface(SQLObservation) include_route_methods = {RouteMethod.LIST} | {"show"} @@ -110,7 +122,9 @@ class ReportView(BaseAlertReportView): class_permission_name = "ReportSchedule" -class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors +class AlertModelView( + EnsureEnabledMixin, SupersetModelView +): # pylint: disable=too-many-ancestors datamodel = SQLAInterface(Alert) route_base = "/alerts" include_route_methods = RouteMethod.CRUD_SET | {"log"} diff --git a/superset/views/key_value.py b/superset/views/key_value.py index 117d66489..8f8aa9978 100644 --- a/superset/views/key_value.py +++ b/superset/views/key_value.py @@ -17,9 +17,11 @@ import simplejson as json from flask import request, Response from flask_appbuilder import expose +from flask_appbuilder.hooks import before_request from flask_appbuilder.security.decorators import has_access_api +from werkzeug.exceptions import NotFound -from superset import db, event_logger +from superset import db, event_logger, is_feature_enabled from superset.models import core as models from superset.typing import FlaskResponse from superset.utils import core as utils @@ -30,6 +32,15 @@ class KV(BaseSupersetView): """Used for storing and retrieving key value pairs""" + @staticmethod + def is_enabled() -> bool: + return is_feature_enabled("KV_STORE") + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() + @event_logger.log_this @has_access_api @expose("/store/", methods=["POST"]) diff --git a/superset/views/log/api.py b/superset/views/log/api.py index f4436635d..a92626df4 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -14,6 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from flask import current_app as app +from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface import superset.models.core as models @@ -42,3 +44,13 @@ class LogRestApi(LogMixin, BaseSupersetModelRestApi): "referrer", ] show_columns = list_columns + + @staticmethod + def is_enabled() -> bool: + return app.config["FAB_ADD_SECURITY_VIEWS"] and app.config["SUPERSET_LOG_VIEW"] + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + return self.response_404() + return None diff --git a/superset/views/log/views.py b/superset/views/log/views.py index 02205622a..6cc8d2ffd 100644 --- a/superset/views/log/views.py +++ b/superset/views/log/views.py @@ -14,7 +14,10 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from flask import current_app as app +from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface +from werkzeug.exceptions import NotFound import superset.models.core as models from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod @@ -28,3 +31,12 @@ class LogModelView(LogMixin, SupersetModelView): # pylint: disable=too-many-anc include_route_methods = {RouteMethod.LIST, RouteMethod.SHOW} class_permission_name = "Log" method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP + + @staticmethod + def is_enabled() -> bool: + return app.config["FAB_ADD_SECURITY_VIEWS"] and app.config["SUPERSET_LOG_VIEW"] + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() diff --git a/superset/views/schedules.py b/superset/views/schedules.py index d46339067..93a3568c3 100644 --- a/superset/views/schedules.py +++ b/superset/views/schedules.py @@ -23,11 +23,13 @@ from typing import Type, Union import simplejson as json from croniter import croniter -from flask import flash, g, Markup +from flask import current_app as app, flash, g, Markup from flask_appbuilder import expose +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 import BooleanField, Form, StringField from superset import db, security_manager @@ -54,6 +56,15 @@ class EmailScheduleView( include_route_methods = RouteMethod.CRUD_SET _extra_data = {"test_email": False, "test_email_recipients": None} + @staticmethod + def is_enabled() -> bool: + return app.config["ENABLE_SCHEDULED_EMAIL_REPORTS"] + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() + @property def schedule_type(self) -> str: raise NotImplementedError() diff --git a/superset/views/tags.py b/superset/views/tags.py index 2bcc0c7c9..c6fac2ff7 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -21,11 +21,13 @@ from typing import Any, Dict, List import simplejson as json from flask import request, Response from flask_appbuilder import expose +from flask_appbuilder.hooks import before_request from flask_appbuilder.security.decorators import has_access_api from jinja2.sandbox import SandboxedEnvironment from sqlalchemy import and_, func +from werkzeug.exceptions import NotFound -from superset import db, utils +from superset import db, is_feature_enabled, utils from superset.jinja_context import ExtraCache from superset.models.dashboard import Dashboard from superset.models.slice import Slice @@ -47,6 +49,15 @@ def process_template(content: str) -> str: class TagView(BaseSupersetView): + @staticmethod + def is_enabled() -> bool: + return is_feature_enabled("TAGGING_SYSTEM") + + @before_request + def ensure_enabled(self) -> None: + if not self.is_enabled(): + raise NotFound() + @has_access_api @expose("/tags/suggestions/", methods=["GET"]) def suggestions(self) -> FlaskResponse: # pylint: disable=no-self-use diff --git a/tests/alerts_tests.py b/tests/alerts_tests.py index bf6f6a62d..fec524476 100644 --- a/tests/alerts_tests.py +++ b/tests/alerts_tests.py @@ -17,7 +17,6 @@ """Unit tests for alerting in Superset""" import json import logging -from typing import Optional from unittest.mock import patch import pytest @@ -41,6 +40,12 @@ from superset.tasks.schedules import ( validate_observations, ) from superset.utils import core as utils +from superset.views.alerts import ( + AlertLogModelView, + AlertModelView, + AlertObservationModelView, +) +from tests.base_tests import SupersetTestCase from tests.test_app import app from tests.utils import read_fixture @@ -359,3 +364,47 @@ def test_deliver_alert_screenshot( "|*Explore in Superset*>", "title": f"[Alert] {alert.label}", } + + +class TestAlertsEndpoints(SupersetTestCase): + def test_log_model_view_disabled(self): + with patch.object(AlertLogModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/alertlogmodelview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_log_model_view_enabled(self): + with patch.object(AlertLogModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/alertlogmodelview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + def test_model_view_disabled(self): + with patch.object(AlertModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/alerts/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_model_view_enabled(self): + with patch.object(AlertModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/alerts/list/" + rv = self.client.get(uri) + self.assertNotEqual(rv.status_code, 404) + + def test_observation_view_disabled(self): + with patch.object(AlertObservationModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/alertobservationmodelview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_observation_view_enabled(self): + with patch.object(AlertObservationModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/alertobservationmodelview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) diff --git a/tests/core_tests.py b/tests/core_tests.py index f176d8fc0..abf0ac3d4 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -32,23 +32,22 @@ import pytz import random import re import unittest -from unittest import mock, skipUnless +from unittest import mock import pandas as pd import sqlalchemy as sqla from sqlalchemy.exc import SQLAlchemyError from superset.models.cache import CacheKey from superset.utils.core import get_example_database +from tests.conftest import with_feature_flags from tests.fixtures.energy_dashboard import load_energy_table_with_slice from tests.test_app import app import superset.views.utils from superset import ( dataframe, db, - jinja_context, security_manager, sql_lab, - is_feature_enabled, ) from superset.connectors.sqla.models import SqlaTable from superset.db_engine_specs.base import BaseEngineSpec @@ -657,10 +656,19 @@ class TestCore(SupersetTestCase): db.session.delete(model_url) db.session.commit() - @skipUnless( - (is_feature_enabled("KV_STORE")), "skipping as /kv/ endpoints are not enabled" - ) - def test_kv(self): + @with_feature_flags(KV_STORE=False) + def test_kv_disabled(self): + self.login(username="admin") + + resp = self.client.get("/kv/10001/") + self.assertEqual(404, resp.status_code) + + value = json.dumps({"data": "this is a test"}) + resp = self.client.post("/kv/store/", data=dict(data=value)) + self.assertEqual(resp.status_code, 404) + + @with_feature_flags(KV_STORE=True) + def test_kv_enabled(self): self.login(username="admin") resp = self.client.get("/kv/10001/") diff --git a/tests/druid_tests.py b/tests/druid_tests.py index fd5806c40..09f2d6101 100644 --- a/tests/druid_tests.py +++ b/tests/druid_tests.py @@ -24,6 +24,13 @@ from unittest.mock import Mock, patch from tests.test_app import app from superset import db, security_manager +from superset.connectors.druid.views import ( + Druid, + DruidClusterModelView, + DruidColumnInlineView, + DruidDatasourceModelView, + DruidMetricInlineView, +) from .base_tests import SupersetTestCase @@ -585,5 +592,77 @@ class TestDruid(SupersetTestCase): self.assertEqual(col_names, {"__time", "dim1", "dim2", "metric1"}) +class TestDruidViewEnabling(SupersetTestCase): + def test_druid_disabled(self): + with patch.object(Druid, "is_enabled", return_value=False): + self.login("admin") + uri = "/druid/refresh_datasources/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_druid_enabled(self): + with patch.object(Druid, "is_enabled", return_value=True): + self.login("admin") + uri = "/druid/refresh_datasources/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + def test_druid_cluster_disabled(self): + with patch.object(DruidClusterModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/druidclustermodelview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_druid_cluster_enabled(self): + with patch.object(DruidClusterModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/druidclustermodelview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + def test_druid_column_disabled(self): + with patch.object(DruidColumnInlineView, "is_enabled", return_value=False): + self.login("admin") + uri = "/druidcolumninlineview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_druid_column_enabled(self): + with patch.object(DruidColumnInlineView, "is_enabled", return_value=True): + self.login("admin") + uri = "/druidcolumninlineview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + def test_druid_datasource_disabled(self): + with patch.object(DruidDatasourceModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/druiddatasourcemodelview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_druid_datasource_enabled(self): + with patch.object(DruidDatasourceModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/druiddatasourcemodelview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + def test_druid_metric_disabled(self): + with patch.object(DruidMetricInlineView, "is_enabled", return_value=False): + self.login("admin") + uri = "/druidmetricinlineview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_druid_metric_enabled(self): + with patch.object(DruidMetricInlineView, "is_enabled", return_value=True): + self.login("admin") + uri = "/druidmetricinlineview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + if __name__ == "__main__": unittest.main() diff --git a/tests/log_api_tests.py b/tests/log_api_tests.py index 98535c422..5885301e9 100644 --- a/tests/log_api_tests.py +++ b/tests/log_api_tests.py @@ -21,10 +21,11 @@ from typing import Optional import prison from flask_appbuilder.security.sqla.models import User +from unittest.mock import patch -import tests.test_app from superset import db from superset.models.core import Log +from superset.views.log.api import LogRestApi from .base_tests import SupersetTestCase @@ -64,6 +65,16 @@ class TestLogApi(SupersetTestCase): db.session.commit() return log + def test_not_enabled(self): + with patch.object(LogRestApi, "is_enabled", return_value=False): + admin_user = self.get_user("admin") + self.insert_log("some_action", admin_user) + self.login(username="admin") + arguments = {"filters": [{"col": "action", "opr": "sw", "value": "some_"}]} + uri = f"api/v1/log/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + def test_get_list(self): """ Log API: Test get list diff --git a/tests/log_model_view_tests.py b/tests/log_model_view_tests.py new file mode 100644 index 000000000..fa80240a1 --- /dev/null +++ b/tests/log_model_view_tests.py @@ -0,0 +1,37 @@ +# 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 unittest.mock import patch + +from superset.views.log.views import LogModelView + +from .base_tests import SupersetTestCase + + +class TestLogModelView(SupersetTestCase): + def test_disabled(self): + with patch.object(LogModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/logmodelview/list/" + rv = self.client.get(uri) + self.assert404(rv) + + def test_enabled(self): + with patch.object(LogModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/logmodelview/list/" + rv = self.client.get(uri) + self.assert200(rv) diff --git a/tests/schedules_test.py b/tests/schedules_test.py index bc7f4a06d..dd7a2a71c 100644 --- a/tests/schedules_test.py +++ b/tests/schedules_test.py @@ -16,6 +16,7 @@ # under the License. # isort:skip_file from datetime import datetime, timedelta +from superset.views.schedules import DashboardEmailScheduleView, SliceEmailScheduleView from unittest.mock import Mock, patch, PropertyMock from flask_babel import gettext as __ @@ -555,6 +556,34 @@ class TestSchedules(SupersetTestCase): }, ) + def test_dashboard_disabled(self): + with patch.object(DashboardEmailScheduleView, "is_enabled", return_value=False): + self.login("admin") + uri = "/dashboardemailscheduleview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_dashboard_enabled(self): + with patch.object(DashboardEmailScheduleView, "is_enabled", return_value=True): + self.login("admin") + uri = "/dashboardemailscheduleview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + + def test_slice_disabled(self): + with patch.object(SliceEmailScheduleView, "is_enabled", return_value=False): + self.login("admin") + uri = "/sliceemailscheduleview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_slice_enabled(self): + with patch.object(SliceEmailScheduleView, "is_enabled", return_value=True): + self.login("admin") + uri = "/sliceemailscheduleview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) + def test_slack_client_compatibility(): c2 = WebClient() diff --git a/tests/security_tests.py b/tests/security_tests.py index ee8b5aab0..ae636a75a 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -38,6 +38,7 @@ from superset.models.core import Database from superset.models.slice import Slice from superset.sql_parse import Table from superset.utils.core import get_example_database +from superset.views.access_requests import AccessRequestsModelView from .base_tests import SupersetTestCase from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices @@ -1203,3 +1204,19 @@ class TestRowLevelSecurity(SupersetTestCase): assert not self.NAMES_B_REGEX.search(sql) assert not self.NAMES_Q_REGEX.search(sql) assert not self.BASE_FILTER_REGEX.search(sql) + + +class TestAccessRequestEndpoints(SupersetTestCase): + def test_access_request_disabled(self): + with patch.object(AccessRequestsModelView, "is_enabled", return_value=False): + self.login("admin") + uri = "/accessrequestsmodelview/list/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_access_request_enabled(self): + with patch.object(AccessRequestsModelView, "is_enabled", return_value=True): + self.login("admin") + uri = "/accessrequestsmodelview/list/" + rv = self.client.get(uri) + self.assertLess(rv.status_code, 400) diff --git a/tests/sqla_views_tests.py b/tests/sqla_views_tests.py new file mode 100644 index 000000000..532d6e8eb --- /dev/null +++ b/tests/sqla_views_tests.py @@ -0,0 +1,40 @@ +# 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.base_tests import SupersetTestCase +from 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) diff --git a/tests/tagging_tests.py b/tests/tagging_tests.py index 77c80d7e3..d19f2abe1 100644 --- a/tests/tagging_tests.py +++ b/tests/tagging_tests.py @@ -15,25 +15,19 @@ # specific language governing permissions and limitations # under the License. -from unittest import skipUnless - -from superset import is_feature_enabled from tests.base_tests import SupersetTestCase +from tests.conftest import with_feature_flags class TestTagging(SupersetTestCase): - @skipUnless( - (is_feature_enabled("TAGGING_SYSTEM") == False), - "skipping as tagging endpoints are not enabled", - ) - def test_tag_view_attachment_disabled(self): - response = self.client.get("/tagview/tagged_objects/") + @with_feature_flags(TAGGING_SYSTEM=False) + def test_tag_view_disabled(self): + self.login("admin") + response = self.client.get("/tagview/tags/suggestions/") self.assertEqual(404, response.status_code) - @skipUnless( - is_feature_enabled("TAGGING_SYSTEM"), - "skipping as tagging endpoints are enabled", - ) - def test_tag_view_attachment_enabled(self): - response = self.client.get("/tagview/tagged_objects/") - self.assertEqual(200, response.status_code) + @with_feature_flags(TAGGING_SYSTEM=True) + def test_tag_view_enabled(self): + self.login("admin") + response = self.client.get("/tagview/tags/suggestions/") + self.assertNotEqual(404, response.status_code)