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(config): Migrating ENABLE_JAVASCRIPT_CONTROLS from app config to a feature flag #19113

Merged
merged 9 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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