Skip to content

Commit

Permalink
chore(config): Migrating ENABLE_JAVASCRIPT_CONTROLS from app config…
Browse files Browse the repository at this point in the history
… to a feature flag (#19113)

* first pass migrating config to ff

* nixing a console log from testing

* adding an entry to `UPDATING.md`

* linting ✨

* Adding ENABLE_JAVASCRIPT_CONTROLS to FEATURE_FLAGS.md

* no longer in need of state!

* Turning the flag back off

* linting... le sigh

* and more linting...
  • Loading branch information
rusackas authored Mar 14, 2022
1 parent 8234395 commit 76b4a14
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 19 deletions.
1 change: 1 addition & 0 deletions RESOURCES/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
// 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';

Expand Down Expand Up @@ -66,15 +71,12 @@ function jsFunctionControl(
{extraDescr}
</div>
),
mapStateToProps: state => ({
// eslint-disable-next-line no-negated-condition
warning: !state.common.conf.ENABLE_JAVASCRIPT_CONTROLS
? t(
'This functionality is disabled in your environment for security reasons.',
)
: null,
readOnly: !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: !isFeatureEnabled(FeatureFlag.ENABLE_JAVASCRIPT_CONTROLS),
};
}

Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/spec/fixtures/mockDashboardInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
},
};
11 changes: 5 additions & 6 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": 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,
Expand Down Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions superset/views/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
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
Expand All @@ -55,7 +55,7 @@


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"]


Expand Down

0 comments on commit 76b4a14

Please sign in to comment.