From 5d9ae8f62608869dce26090ab263764e9c4f68c7 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Wed, 2 Mar 2022 08:24:29 -0700 Subject: [PATCH 1/9] first pass migrating config to ff --- .../superset-ui-core/src/utils/featureFlags.ts | 1 + .../src/utilities/Shared_DeckGL.jsx | 9 ++++++--- superset-frontend/spec/fixtures/mockDashboardInfo.js | 2 +- superset/config.py | 11 +++++------ superset/views/utils.py | 6 ++++-- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index cab4f72741167..8ed617cc3e631 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -49,6 +49,7 @@ export enum FeatureFlag { ENABLE_DND_WITH_CLICK_UX = 'ENABLE_DND_WITH_CLICK_UX', FORCE_DATABASE_CONNECTIONS_SSL = 'FORCE_DATABASE_CONNECTIONS_SSL', ENABLE_TEMPLATE_REMOVE_FILTERS = 'ENABLE_TEMPLATE_REMOVE_FILTERS', + ENABLE_JAVASCRIPT_CONTROLS = 'ENABLE_JAVASCRIPT_CONTROLS', DASHBOARD_RBAC = 'DASHBOARD_RBAC', ALERTS_ATTACH_REPORTS = 'ALERTS_ATTACH_REPORTS', ALLOW_FULL_CSV_EXPORT = 'ALLOW_FULL_CSV_EXPORT', diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx index b801338f0c285..9b1b51bb48333 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx @@ -20,7 +20,7 @@ // These are control configurations that are shared ONLY within the DeckGL viz plugin repo. import React from 'react'; -import { t, validateNonEmpty } from '@superset-ui/core'; +import { FeatureFlag, isFeatureEnabled, t, validateNonEmpty } from '@superset-ui/core'; import { D3_FORMAT_OPTIONS, sharedControls } from '@superset-ui/chart-controls'; import { columnChoices, PRIMARY_COLOR } from './controls'; @@ -45,6 +45,9 @@ const jsFunctionInfo = ( ); +const jsControlConfig = isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS); +console.log(jsControlConfig,"!!!!!!!"); + function jsFunctionControl( label, description, @@ -68,12 +71,12 @@ function jsFunctionControl( ), mapStateToProps: state => ({ // eslint-disable-next-line no-negated-condition - warning: !state.common.conf.ENABLE_JAVASCRIPT_CONTROLS + warning: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS) ? t( 'This functionality is disabled in your environment for security reasons.', ) : null, - readOnly: !state.common.conf.ENABLE_JAVASCRIPT_CONTROLS, + readOnly: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS), }), }; } diff --git a/superset-frontend/spec/fixtures/mockDashboardInfo.js b/superset-frontend/spec/fixtures/mockDashboardInfo.js index 4aaba5a52cfa2..c11ec7f88a35d 100644 --- a/superset-frontend/spec/fixtures/mockDashboardInfo.js +++ b/superset-frontend/spec/fixtures/mockDashboardInfo.js @@ -34,6 +34,6 @@ export default { dash_save_perm: true, common: { flash_messages: [], - conf: { ENABLE_JAVASCRIPT_CONTROLS: false, SUPERSET_WEBSERVER_TIMEOUT: 60 }, + conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 }, }, }; diff --git a/superset/config.py b/superset/config.py index c6f2269b94e61..ed3da12af29b3 100644 --- a/superset/config.py +++ b/superset/config.py @@ -374,6 +374,11 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: "ENABLE_EXPLORE_JSON_CSRF_PROTECTION": False, "ENABLE_TEMPLATE_PROCESSING": False, "ENABLE_TEMPLATE_REMOVE_FILTERS": False, + # Allow for javascript controls components + # this enables programmers to customize certain charts (like the + # geospatial ones) by inputing javascript in controls. This exposes + # an XSS security vulnerability + "ENABLE_JAVASCRIPT_CONTROLS": True, "KV_STORE": False, # When this feature is enabled, nested types in Presto will be # expanded into extra columns and/or arrays. This is experimental, @@ -1021,12 +1026,6 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name # } ALLOWED_EXTRA_AUTHENTICATIONS: Dict[str, Dict[str, Callable[..., Any]]] = {} -# Allow for javascript controls components -# this enables programmers to customize certain charts (like the -# geospatial ones) by inputing javascript in controls. This exposes -# an XSS security vulnerability -ENABLE_JAVASCRIPT_CONTROLS = False - # The id of a template dashboard that should be copied to every new user DASHBOARD_TEMPLATE_ID = None diff --git a/superset/views/utils.py b/superset/views/utils.py index cac0c4f7465f0..ae43477f5a4e7 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -40,7 +40,8 @@ SupersetException, SupersetSecurityException, ) -from superset.extensions import cache_manager, security_manager + +from superset.extensions import cache_manager, feature_flag_manager, security_manager from superset.legacy import update_time_range from superset.models.core import Database from superset.models.dashboard import Dashboard @@ -55,7 +56,8 @@ REJECTED_FORM_DATA_KEYS: List[str] = [] -if not app.config["ENABLE_JAVASCRIPT_CONTROLS"]: + +if not feature_flag_manager.is_feature_enabled("ENABLE_JAVASCRIPT_CONTROLS"): REJECTED_FORM_DATA_KEYS = ["js_tooltip", "js_onclick_href", "js_data_mutator"] From 8beb9358d67e5e505c406ba21f95e399b370bb29 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 16:24:12 -0700 Subject: [PATCH 2/9] nixing a console log from testing --- .../legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx index 9b1b51bb48333..9972f425f2b24 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx @@ -46,7 +46,6 @@ const jsFunctionInfo = ( ); const jsControlConfig = isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS); -console.log(jsControlConfig,"!!!!!!!"); function jsFunctionControl( label, From 34ba2bd8f837e4b1b6b5270102f822c4df2ad24e Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 16:38:49 -0700 Subject: [PATCH 3/9] adding an entry to `UPDATING.md` --- UPDATING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UPDATING.md b/UPDATING.md index 2df0b3bdcb329..977a63ac54b16 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -28,6 +28,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [19113](https://github.com/apache/superset/pull/19113): The `ENABLE_JAVASCRIPT_CONTROLS` setting has moved from app config to a feature flag. Any deployments who overrode this setting will now need to override the feature flag from here onward. - [18976](https://github.com/apache/superset/pull/18976): When running the app in debug mode, the app will default to use `SimpleCache` for `FILTER_STATE_CACHE_CONFIG` and `EXPLORE_FORM_DATA_CACHE_CONFIG`. When running in non-debug mode, a cache backend will need to be defined, otherwise the application will fail to start. For installations using Redis or other caching backends, it is recommended to use the same backend for both cache configs. - [17881](https://github.com/apache/superset/pull/17881): Previously simple adhoc filter values on string columns were stripped of enclosing single and double quotes. To fully support literal quotes in filters, both single and double quotes will no longer be removed from filter values. - [17984](https://github.com/apache/superset/pull/17984): Default Flask SECRET_KEY has changed for security reasons. You should always override with your own secret. Set `PREVIOUS_SECRET_KEY` (ex: PREVIOUS_SECRET_KEY = "\2\1thisismyscretkey\1\2\\e\\y\\y\\h") with your previous key and use `superset re-encrypt-secrets` to rotate you current secrets From eae79d2381fc3a52d752d81b494065e13b71c886 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 17:36:37 -0700 Subject: [PATCH 4/9] linting :sparkles: --- .../src/utilities/Shared_DeckGL.jsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx index 9972f425f2b24..c7ac2b0e1f795 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx @@ -20,7 +20,12 @@ // These are control configurations that are shared ONLY within the DeckGL viz plugin repo. import React from 'react'; -import { FeatureFlag, isFeatureEnabled, t, validateNonEmpty } from '@superset-ui/core'; +import { + FeatureFlag, + isFeatureEnabled, + t, + validateNonEmpty, +} from '@superset-ui/core'; import { D3_FORMAT_OPTIONS, sharedControls } from '@superset-ui/chart-controls'; import { columnChoices, PRIMARY_COLOR } from './controls'; @@ -45,7 +50,9 @@ const jsFunctionInfo = ( ); -const jsControlConfig = isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS); +const jsControlConfig = isFeatureEnabled( + FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS, +); function jsFunctionControl( label, From ec60a86b67ee6bedf18ae4675845f84d550e38a2 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 17:57:55 -0700 Subject: [PATCH 5/9] Adding ENABLE_JAVASCRIPT_CONTROLS to FEATURE_FLAGS.md --- RESOURCES/FEATURE_FLAGS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index c19fac7b06947..69f8f8a5a691d 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -43,6 +43,7 @@ These features are **finished** but currently being tested. They are usable, but - GLOBAL_ASYNC_QUERIES [(docs)](https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries) - OMNIBAR - VERSIONED_EXPORT +- ENABLE_JAVASCRIPT_CONTROLS ## Stable These features flags are **safe for production** and have been tested. From fda05d39b70c440759f40d581e7c2560fbc1bd8e Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 21:40:45 -0700 Subject: [PATCH 6/9] no longer in need of state! --- .../src/utilities/Shared_DeckGL.jsx | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx index c7ac2b0e1f795..f665c118eaccb 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/Shared_DeckGL.jsx @@ -50,10 +50,6 @@ const jsFunctionInfo = ( ); -const jsControlConfig = isFeatureEnabled( - FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS, -); - function jsFunctionControl( label, description, @@ -75,15 +71,12 @@ function jsFunctionControl( {extraDescr} ), - mapStateToProps: state => ({ - // eslint-disable-next-line no-negated-condition - warning: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS) - ? t( - 'This functionality is disabled in your environment for security reasons.', - ) - : null, - readOnly: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS), - }), + warning: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS) + ? t( + 'This functionality is disabled in your environment for security reasons.', + ) + : null, + readOnly: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS), }; } From c4ba1b73f8bf2fa887848d09e329a3935466dc5c Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 21:58:37 -0700 Subject: [PATCH 7/9] Turning the flag back off --- superset/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/config.py b/superset/config.py index ed3da12af29b3..6579719ea81cc 100644 --- a/superset/config.py +++ b/superset/config.py @@ -378,7 +378,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # this enables programmers to customize certain charts (like the # geospatial ones) by inputing javascript in controls. This exposes # an XSS security vulnerability - "ENABLE_JAVASCRIPT_CONTROLS": True, + "ENABLE_JAVASCRIPT_CONTROLS": False, "KV_STORE": False, # When this feature is enabled, nested types in Presto will be # expanded into extra columns and/or arrays. This is experimental, From 4177545f118d4b0d7f2e0eecec8ac1771034c783 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 22:00:03 -0700 Subject: [PATCH 8/9] linting... le sigh --- superset/views/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/views/utils.py b/superset/views/utils.py index ae43477f5a4e7..b1e77cb7cb41a 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -40,7 +40,6 @@ SupersetException, SupersetSecurityException, ) - from superset.extensions import cache_manager, feature_flag_manager, security_manager from superset.legacy import update_time_range from superset.models.core import Database From 94e458c40932edc42f23a083a1dd1a448ab6ee0f Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 10 Mar 2022 22:01:24 -0700 Subject: [PATCH 9/9] and more linting... --- superset/views/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/views/utils.py b/superset/views/utils.py index b1e77cb7cb41a..17ec6ea1088c9 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -55,7 +55,6 @@ REJECTED_FORM_DATA_KEYS: List[str] = [] - if not feature_flag_manager.is_feature_enabled("ENABLE_JAVASCRIPT_CONTROLS"): REJECTED_FORM_DATA_KEYS = ["js_tooltip", "js_onclick_href", "js_data_mutator"]