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

refactor: Removes the deprecated ENABLE_EXPLORE_JSON_CSRF_PROTECTION feature flag #26344

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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: 0 additions & 1 deletion RESOURCES/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ These features flags currently default to True and **will be removed in a future
- DASHBOARD_NATIVE_FILTERS_SET
- DISABLE_DATASET_SOURCE_EDIT
- ENABLE_EXPLORE_DRAG_AND_DROP
- ENABLE_EXPLORE_JSON_CSRF_PROTECTION
- ENABLE_TEMPLATE_REMOVE_FILTERS
- GENERIC_CHART_AXES
- REMOVE_SLICE_LEVEL_LABEL_COLORS
Expand Down
2 changes: 2 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ assists people when migrating to a new version.

### Breaking Changes

- [26344](https://github.com/apache/superset/issues/26344): Removes the deprecated `ENABLE_EXPLORE_JSON_CSRF_PROTECTION` feature flag. The previous value of the feature flag was `False` and now the feature is permanently removed.

### Potential Downtime

### Other
Expand Down
35 changes: 18 additions & 17 deletions docs/docs/installation/configuring-superset.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,27 @@ version: 1

### Configuration

To configure your application, you need to create a file `superset_config.py`. Add this file to your
To configure your application, you need to create a file `superset_config.py`. Add this file to your

`PYTHONPATH` or create an environment variable `SUPERSET_CONFIG_PATH` specifying the full path of the `superset_config.py`.

For example, if deploying on Superset directly on a Linux-based system where your `superset_config.py` is under `/app` directory, you can run:

```bash
export SUPERSET_CONFIG_PATH=/app/superset_config.py
```

If you are using your own custom Dockerfile with official Superset image as base image, then you can add your overrides as shown below:

```bash
COPY --chown=superset superset_config.py /app/
ENV SUPERSET_CONFIG_PATH /app/superset_config.py
```

Docker compose deployments handle application configuration differently. See [https://github.com/apache/superset/tree/master/docker#readme](https://github.com/apache/superset/tree/master/docker#readme) for details.
Docker compose deployments handle application configuration differently. See [https://github.com/apache/superset/tree/master/docker#readme](https://github.com/apache/superset/tree/master/docker#readme) for details.

The following is an example of just a few of the parameters you can set in your `superset_config.py` file:

```
# Superset specific config
ROW_LIMIT = 5000
Expand Down Expand Up @@ -88,7 +91,7 @@ WTF_CSRF_EXEMPT_LIST = [‘’]

#### Adding an initial SECRET_KEY

Superset requires a user-specified SECRET_KEY to start up. This requirement was [added in version 2.1.0 to force secure configurations](https://preset.io/blog/superset-security-update-default-secret_key-vulnerability/). Add a strong SECRET_KEY to your `superset_config.py` file like:
Superset requires a user-specified SECRET_KEY to start up. This requirement was [added in version 2.1.0 to force secure configurations](https://preset.io/blog/superset-security-update-default-secret_key-vulnerability/). Add a strong SECRET_KEY to your `superset_config.py` file like:

```python
SECRET_KEY = 'YOUR_OWN_RANDOM_GENERATED_SECRET_KEY'
Expand All @@ -99,7 +102,7 @@ You can generate a strong secure key with `openssl rand -base64 42`.
#### Rotating to a newer SECRET_KEY

If you wish to change your existing SECRET_KEY, add the existing SECRET_KEY to your `superset_config.py` file as
`PREVIOUS_SECRET_KEY = `and provide your new key as `SECRET_KEY =`. You can find your current SECRET_KEY with these
`PREVIOUS_SECRET_KEY = `and provide your new key as `SECRET_KEY =`. You can find your current SECRET_KEY with these
commands - if running Superset with Docker, execute from within the Superset application container:

```python
Expand All @@ -119,23 +122,21 @@ database engine on a separate host or container.

Superset supports the following database engines/versions:

| Database Engine | Supported Versions |
| --------------------------------------------------------- | ---------------------------------- |
| [PostgreSQL](https://www.postgresql.org/) | 10.X, 11.X, 12.X, 13.X, 14.X, 15.X |
| [MySQL](https://www.mysql.com/) | 5.7, 8.X |

| Database Engine | Supported Versions |
| ----------------------------------------- | ---------------------------------- |
| [PostgreSQL](https://www.postgresql.org/) | 10.X, 11.X, 12.X, 13.X, 14.X, 15.X |
| [MySQL](https://www.mysql.com/) | 5.7, 8.X |

Use the following database drivers and connection strings:

| Database | PyPI package | Connection String |
| ----------------------------------------- | --------------------------------- | ------------------------------------------------------------------------ |
| [PostgreSQL](https://www.postgresql.org/) | `pip install psycopg2` | `postgresql://<UserName>:<DBPassword>@<Database Host>/<Database Name>` |
| [MySQL](https://www.mysql.com/) | `pip install mysqlclient` | `mysql://<UserName>:<DBPassword>@<Database Host>/<Database Name>` |
| Database | PyPI package | Connection String |
| ----------------------------------------- | ------------------------- | ---------------------------------------------------------------------- |
| [PostgreSQL](https://www.postgresql.org/) | `pip install psycopg2` | `postgresql://<UserName>:<DBPassword>@<Database Host>/<Database Name>` |
| [MySQL](https://www.mysql.com/) | `pip install mysqlclient` | `mysql://<UserName>:<DBPassword>@<Database Host>/<Database Name>` |

To configure Superset metastore set `SQLALCHEMY_DATABASE_URI` config key on `superset_config`
to the appropriate connection string.


### Running on a WSGI HTTP Server

While you can run Superset on NGINX or Apache, we recommend using Gunicorn in async mode. This
Expand Down Expand Up @@ -167,7 +168,7 @@ So, when you use `BigQuery` datasource on Superset, you have to use `gunicorn` w

### HTTPS Configuration

You can configure HTTPS upstream via a load balancer or a reverse proxy (such as nginx) and do SSL/TLS Offloading before traffic reaches the Superset application. In this setup, local traffic from a Celery worker taking a snapshot of a chart for Alerts & Reports can access Superset at a `http://` URL, from behind the ingress point.
You can configure HTTPS upstream via a load balancer or a reverse proxy (such as nginx) and do SSL/TLS Offloading before traffic reaches the Superset application. In this setup, local traffic from a Celery worker taking a snapshot of a chart for Alerts & Reports can access Superset at a `http://` URL, from behind the ingress point.
You can also configure [SSL in Gunicorn](https://docs.gunicorn.org/en/stable/settings.html#ssl) (the Python webserver) if you are using an official Superset Docker image.

### Configuration Behind a Load Balancer
Expand All @@ -191,7 +192,7 @@ RequestHeader set X-Forwarded-Proto "https"
### Custom OAuth2 Configuration

Superset is built on Flask-AppBuilder (FAB), which supports many providers out of the box
(GitHub, Twitter, LinkedIn, Google, Azure, etc). Beyond those, Superset can be configured to connect
(GitHub, Twitter, LinkedIn, Google, Azure, etc). Beyond those, Superset can be configured to connect
with other OAuth2 Authorization Server implementations that support “code” authorization.

Make sure the pip package [`Authlib`](https://authlib.org/) is installed on the webserver.
Expand Down Expand Up @@ -358,7 +359,7 @@ You can enable or disable features with flag from `superset_config.py`:
```python
FEATURE_FLAGS = {
'CLIENT_CACHE': False,
'ENABLE_EXPLORE_JSON_CSRF_PROTECTION': False,
'DASHBOARD_RBAC': False,
'PRESTO_EXPAND_DATA': False,
}
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ describe('Native filters', () => {
cy.createSampleDashboards([0]);
});

it.only('Verify that default value is respected after revisit', () => {
it('Verify that default value is respected after revisit', () => {
prepareDashboardFilters([
{ name: 'country_name', column: 'country_name', datasetId: 2 },
]);
Expand Down
8 changes: 0 additions & 8 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,6 @@ class D3Format(TypedDict, total=False):
# editor no longer shows. Currently this is set to false so that the editor
# option does show, but we will be depreciating it.
"DISABLE_LEGACY_DATASOURCE_EDITOR": True,
# For some security concerns, you may need to enforce CSRF protection on
# all query request to explore_json endpoint. In Superset, we use
# `flask-csrf <https://sjl.bitbucket.io/flask-csrf/>`_ add csrf protection
# for all POST requests, but this protection doesn't apply to GET method.
# When ENABLE_EXPLORE_JSON_CSRF_PROTECTION is set to true, your users cannot
# make GET request to explore_json. explore_json accepts both GET and POST request.
# See `PR 7935 <https://github.com/apache/superset/pull/7935>`_ for more details.
"ENABLE_EXPLORE_JSON_CSRF_PROTECTION": False, # deprecated
"ENABLE_TEMPLATE_PROCESSING": False,
"ENABLE_TEMPLATE_REMOVE_FILTERS": True, # deprecated
# Allow for javascript controls components
Expand Down
2 changes: 0 additions & 2 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,6 @@ def explore_json_data(self, cache_key: str) -> FlaskResponse:
return json_error_response(utils.error_msg_from_exception(ex), 400)

EXPLORE_JSON_METHODS = ["POST"]
if not is_feature_enabled("ENABLE_EXPLORE_JSON_CSRF_PROTECTION"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this was False by default, shouldn't we keep the EXPLORE_JSON_METHODS.append("GET") ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! @dpgaspar can you confirm the expected behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geido Confirmed and added the GET method by default as you suggested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my understanding this feature flag would allow explore_json to accept HTTP GETs but still imposing CSRF protection. So when False this endpoint would only accept HTTP POSTs and so would be automatically protected to CSRF. Shouldn't we keep the same behaviour and just allow for POST and be always protected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my understanding this feature flag would allow explore_json to accept HTTP GETs but still imposing CSRF protection.

This feature flag was False by default, allowing GET. There's no other control than excluding GET when this feature flag is True.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed the not on the feature flag check. Makes sense

EXPLORE_JSON_METHODS.append("GET")

@api
@has_access_api
Expand Down
9 changes: 8 additions & 1 deletion tests/integration_tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,15 @@ def test_comments_in_sqlatable_query(self):
self.assertEqual(clean_query, rendered_query)

def test_slice_payload_no_datasource(self):
form_data = {
"viz_type": "dist_bar",
}
self.login(username="admin")
data = self.get_json_resp("/superset/explore_json/", raise_on_error=False)
rv = self.client.post(
"/superset/explore_json/",
data={"form_data": json.dumps(form_data)},
)
data = json.loads(rv.data.decode("utf-8"))

self.assertEqual(
data["errors"][0]["message"],
Expand Down
Loading