From 40c5d7cd633b53de01379a514ca4ff82d41e9b6f Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 15 Jul 2020 10:09:23 +0300 Subject: [PATCH 1/3] chore: prefer allow/deny terminology --- UPDATING.md | 2 ++ superset/config.py | 12 ++++++------ superset/connectors/druid/models.py | 6 +++--- superset/db_engine_specs/base.py | 6 +++--- superset/models/datasource_access_request.py | 6 +++--- superset/utils/pandas_postprocessing.py | 12 ++++++------ superset/viz.py | 2 +- superset/viz_sip38.py | 2 +- 8 files changed, 25 insertions(+), 23 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 3b2e3e31949e6..c46a238a463da 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,6 +23,8 @@ assists people when migrating to a new version. ## Next +* XXX: References to blacklst/whitelist language have been replaced with more appropriate alternatives. All configs refencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`. + * [9964](https://github.com/apache/incubator-superset/pull/9964): Breaking change on Flask-AppBuilder 3. If you're using OAuth, find out what needs to be changed [here](https://github.com/dpgaspar/Flask-AppBuilder/blob/master/README.rst#change-log). * [10233](https://github.com/apache/incubator-superset/pull/10233): a change which deprecates the `ENABLE_FLASK_COMPRESS` config option in favor of the Flask-Compress `COMPRESS_REGISTER` config option which serves the same purpose. diff --git a/superset/config.py b/superset/config.py index 74e7fec7109c3..9ded3d17f0e9c 100644 --- a/superset/config.py +++ b/superset/config.py @@ -381,8 +381,8 @@ def _try_json_readsha( # pylint: disable=unused-argument # List of time grains to disable in the application (see list of builtin # time grains in superset/db_engine_specs.builtin_time_grains). # For example: to disable 1 second time grain: -# TIME_GRAIN_BLACKLIST = ['PT1S'] -TIME_GRAIN_BLACKLIST: List[str] = [] +# TIME_GRAIN_DENYLIST = ['PT1S'] +TIME_GRAIN_DENYLIST: List[str] = [] # Additional time grains to be supported using similar definitions as in # superset/db_engine_specs.builtin_time_grains. @@ -402,17 +402,17 @@ def _try_json_readsha( # pylint: disable=unused-argument # --------------------------------------------------- # List of viz_types not allowed in your environment -# For example: Blacklist pivot table and treemap: -# VIZ_TYPE_BLACKLIST = ['pivot_table', 'treemap'] +# For example: Disable pivot table and treemap: +# VIZ_TYPE_DENYLIST = ['pivot_table', 'treemap'] # --------------------------------------------------- -VIZ_TYPE_BLACKLIST: List[str] = [] +VIZ_TYPE_DENYLIST: List[str] = [] # --------------------------------------------------- # List of data sources not to be refreshed in druid cluster # --------------------------------------------------- -DRUID_DATA_SOURCE_BLACKLIST: List[str] = [] +DRUID_DATA_SOURCE_DENYLIST: List[str] = [] # -------------------------------------------------- # Modules, datasources and middleware to be registered diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index e9c6029e7720c..ffd178d7c0a2f 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -206,11 +206,11 @@ def refresh_datasources( If ``datasource_name`` is specified, only that datasource is updated """ ds_list = self.get_datasources() - blacklist = conf.get("DRUID_DATA_SOURCE_BLACKLIST", []) + denylist = conf.get("DRUID_DATA_SOURCE_DENYLIST", []) ds_refresh: List[str] = [] if not datasource_name: - ds_refresh = list(filter(lambda ds: ds not in blacklist, ds_list)) - elif datasource_name not in blacklist and datasource_name in ds_list: + ds_refresh = list(filter(lambda ds: ds not in denylist, ds_list)) + elif datasource_name not in denylist and datasource_name in ds_list: ds_refresh.append(datasource_name) else: return diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index a32c21af8015e..881599015afac 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -268,7 +268,7 @@ def get_time_grains(cls) -> Tuple[TimeGrain, ...]: def get_time_grain_expressions(cls) -> Dict[Optional[str], str]: """ Return a dict of all supported time grains including any potential added grains - but excluding any potentially blacklisted grains in the config file. + but excluding any potentially disabled grains in the config file. :return: All time grain expressions supported by the engine """ @@ -276,8 +276,8 @@ def get_time_grain_expressions(cls) -> Dict[Optional[str], str]: time_grain_expressions = cls._time_grain_expressions.copy() grain_addon_expressions = config["TIME_GRAIN_ADDON_EXPRESSIONS"] time_grain_expressions.update(grain_addon_expressions.get(cls.engine, {})) - blacklist: List[str] = config["TIME_GRAIN_BLACKLIST"] - for key in blacklist: + denylist: List[str] = config["TIME_GRAIN_DENYLIST"] + for key in denylist: time_grain_expressions.pop(key) return time_grain_expressions diff --git a/superset/models/datasource_access_request.py b/superset/models/datasource_access_request.py index 8940611b5eceb..974633294b3fc 100644 --- a/superset/models/datasource_access_request.py +++ b/superset/models/datasource_access_request.py @@ -42,7 +42,7 @@ class DatasourceAccessRequest(Model, AuditMixinNullable): datasource_id = Column(Integer) datasource_type = Column(String(200)) - ROLES_BLACKLIST = set(config["ROBOT_PERMISSION_ROLES"]) + ROLES_DENYLIST = set(config["ROBOT_PERMISSION_ROLES"]) @property def cls_model(self) -> Type["BaseDatasource"]: @@ -72,7 +72,7 @@ def roles_with_datasource(self) -> str: perm = self.datasource.perm # pylint: disable=no-member pv = security_manager.find_permission_view_menu("datasource_access", perm) for role in pv.role: - if role.name in self.ROLES_BLACKLIST: + if role.name in self.ROLES_DENYLIST: continue # pylint: disable=no-member href = ( @@ -95,7 +95,7 @@ def user_roles(self) -> str: f"created_by={self.created_by.username}&role_to_extend={role.name}" ) link = 'Extend {} Role'.format(href, role.name) - if role.name in self.ROLES_BLACKLIST: + if role.name in self.ROLES_DENYLIST: link = "{} Role".format(role.name) action_list = action_list + "
  • " + link + "
    • " return "" diff --git a/superset/utils/pandas_postprocessing.py b/superset/utils/pandas_postprocessing.py index 12b49bc9e4d9d..e71df74717df8 100644 --- a/superset/utils/pandas_postprocessing.py +++ b/superset/utils/pandas_postprocessing.py @@ -26,7 +26,7 @@ from superset.exceptions import QueryObjectValidationError from superset.utils.core import DTTM_ALIAS, PostProcessingContributionOrientation -WHITELIST_NUMPY_FUNCTIONS = ( +ALLOWLIST_NUMPY_FUNCTIONS = ( "average", "argmin", "argmax", @@ -49,7 +49,7 @@ "var", ) -WHITELIST_ROLLING_FUNCTIONS = ( +DENYLIST_ROLLING_FUNCTIONS = ( "count", "corr", "cov", @@ -65,7 +65,7 @@ "quantile", ) -WHITELIST_CUMULATIVE_FUNCTIONS = ( +ALLOWLIST_CUMULATIVE_FUNCTIONS = ( "cummax", "cummin", "cumprod", @@ -142,7 +142,7 @@ def _get_aggregate_funcs( _("Operator undefined for aggregator: %(name)s", name=name,) ) operator = agg_obj["operator"] - if operator not in WHITELIST_NUMPY_FUNCTIONS or not hasattr(np, operator): + if operator not in ALLOWLIST_NUMPY_FUNCTIONS or not hasattr(np, operator): raise QueryObjectValidationError( _("Invalid numpy function: %(operator)s", operator=operator,) ) @@ -331,7 +331,7 @@ def rolling( # pylint: disable=too-many-arguments kwargs["win_type"] = win_type df_rolling = df_rolling.rolling(**kwargs) - if rolling_type not in WHITELIST_ROLLING_FUNCTIONS or not hasattr( + if rolling_type not in DENYLIST_ROLLING_FUNCTIONS or not hasattr( df_rolling, rolling_type ): raise QueryObjectValidationError( @@ -422,7 +422,7 @@ def cum(df: DataFrame, columns: Dict[str, str], operator: str) -> DataFrame: """ df_cum = df[columns.keys()] operation = "cum" + operator - if operation not in WHITELIST_CUMULATIVE_FUNCTIONS or not hasattr( + if operation not in ALLOWLIST_CUMULATIVE_FUNCTIONS or not hasattr( df_cum, operation ): raise QueryObjectValidationError( diff --git a/superset/viz.py b/superset/viz.py index 2067f7c22fda7..6e6f86d77687c 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -2940,6 +2940,6 @@ def get_data(self, df: pd.DataFrame) -> VizData: if ( inspect.isclass(o) and issubclass(o, BaseViz) - and o.viz_type not in config["VIZ_TYPE_BLACKLIST"] + and o.viz_type not in config["VIZ_TYPE_DENYLIST"] ) } diff --git a/superset/viz_sip38.py b/superset/viz_sip38.py index 7d615d14adc73..aa7a4e1f7798a 100644 --- a/superset/viz_sip38.py +++ b/superset/viz_sip38.py @@ -2823,6 +2823,6 @@ def get_data(self, df: pd.DataFrame) -> VizData: if ( inspect.isclass(o) and issubclass(o, BaseViz) - and o.viz_type not in config["VIZ_TYPE_BLACKLIST"] + and o.viz_type not in config["VIZ_TYPE_DENYLIST"] ) } From 3cd624d09bf1aae647962e825d12c4d7a3edcb8d Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 15 Jul 2020 10:11:13 +0300 Subject: [PATCH 2/3] fix tests --- tests/db_engine_specs/base_engine_spec_tests.py | 4 ++-- tests/security_tests.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/db_engine_specs/base_engine_spec_tests.py b/tests/db_engine_specs/base_engine_spec_tests.py index f4fc64ba58531..a79d0bc6378e1 100644 --- a/tests/db_engine_specs/base_engine_spec_tests.py +++ b/tests/db_engine_specs/base_engine_spec_tests.py @@ -151,9 +151,9 @@ def test_limit_with_non_token_limit(self): """SELECT 'LIMIT 777'""", """SELECT 'LIMIT 777'\nLIMIT 1000""" ) - def test_time_grain_blacklist(self): + def test_time_grain_denylist(self): with app.app_context(): - app.config["TIME_GRAIN_BLACKLIST"] = ["PT1M"] + app.config["TIME_GRAIN_DENYLIST"] = ["PT1M"] time_grain_functions = SqliteEngineSpec.get_time_grain_expressions() self.assertNotIn("PT1M", time_grain_functions) diff --git a/tests/security_tests.py b/tests/security_tests.py index 278400f260ff2..fb8e81ee53733 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -784,9 +784,9 @@ def assert_can_all(view_menu): def test_views_are_secured(self): """Preventing the addition of unsecured views without has_access decorator""" # These FAB views are secured in their body as opposed to by decorators - method_whitelist = ("action", "action_post") + method_allowlist = ("action", "action_post") # List of redirect & other benign views - views_whitelist = [ + views_allowlist = [ ["MyIndexView", "index"], ["UtilView", "back"], ["LocaleView", "index"], @@ -807,8 +807,8 @@ def test_views_are_secured(self): view_class, predicate=inspect.ismethod ): if ( - name not in method_whitelist - and [class_name, name] not in views_whitelist + name not in method_allowlist + and [class_name, name] not in views_allowlist and hasattr(value, "_urls") and not hasattr(value, "_permission_name") ): From e6760fd1b6df37a83100f1ffc7f3103177bc1114 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 15 Jul 2020 10:13:31 +0300 Subject: [PATCH 3/3] add PR reference --- UPDATING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPDATING.md b/UPDATING.md index c46a238a463da..5b4369114e28e 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,7 +23,7 @@ assists people when migrating to a new version. ## Next -* XXX: References to blacklst/whitelist language have been replaced with more appropriate alternatives. All configs refencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`. +* [10320](https://github.com/apache/incubator-superset/pull/10320): References to blacklst/whitelist language have been replaced with more appropriate alternatives. All configs refencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`. * [9964](https://github.com/apache/incubator-superset/pull/9964): Breaking change on Flask-AppBuilder 3. If you're using OAuth, find out what needs to be changed [here](https://github.com/dpgaspar/Flask-AppBuilder/blob/master/README.rst#change-log).