From 7c29571b9a5b13dde081ce982610edf20d2eb6e8 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 24 Feb 2023 10:33:31 +0000 Subject: [PATCH 1/6] fix: refuse to start with default secret on non debug envs --- docker/.env-non-dev | 1 + superset/config.py | 3 ++- superset/initialization/__init__.py | 14 +++++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docker/.env-non-dev b/docker/.env-non-dev index 0ae4c1c7932bb..726b0bb167bf7 100644 --- a/docker/.env-non-dev +++ b/docker/.env-non-dev @@ -42,6 +42,7 @@ REDIS_PORT=6379 FLASK_ENV=production SUPERSET_ENV=production SUPERSET_LOAD_EXAMPLES=yes +SUPERSET_SECRET_KEY=TEST_NON_DEV_SECRET CYPRESS_CONFIG=false SUPERSET_PORT=8088 MAPBOX_API_KEY='' diff --git a/superset/config.py b/superset/config.py index 5489d5f67dbfe..35a1b4ce2279a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -188,7 +188,8 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: SQLALCHEMY_TRACK_MODIFICATIONS = False # --------------------------------------------------------- -# Your App secret key. Make sure you override it on superset_config.py. +# Your App secret key. Make sure you override it on superset_config.py +# or use `SUPERSET_SECRET_KEY` environment variable. # Use a strong complex alphanumeric string and use a tool to help you generate # a sufficiently random sequence, ex: openssl rand -base64 42" SECRET_KEY = CHANGE_ME_SECRET_KEY diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index cda0651456b9f..655a88bd14e5d 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -458,7 +458,7 @@ def init_app_in_ctx(self) -> None: self.init_views() def check_secret_key(self) -> None: - if self.config["SECRET_KEY"] == CHANGE_ME_SECRET_KEY: + def display_default_secret_key_warning() -> None: top_banner = 80 * "-" + "\n" + 36 * " " + "WARNING\n" + 80 * "-" bottom_banner = 80 * "-" + "\n" + 80 * "-" logger.warning(top_banner) @@ -471,6 +471,18 @@ def check_secret_key(self) -> None: ) logger.warning(bottom_banner) + if self.config["SECRET_KEY"] == CHANGE_ME_SECRET_KEY: + if "SUPERSET_SECRET_KEY" in os.environ: + self.config["SECRET_KEY"] = os.environ["SUPERSET_SECRET_KEY"] + return + if self.superset_app.debug: + logger.warning("Debug mode identified with default secret key") + display_default_secret_key_warning() + return + display_default_secret_key_warning() + logger.error("Refusing to start due to insecure SECRET_KEY") + exit(1) + def init_app(self) -> None: """ Main entry point which will delegate to other methods in From 72c5ecfbed5398ea11ab56109ccc95b8ae45b9b8 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 24 Feb 2023 10:40:49 +0000 Subject: [PATCH 2/6] don't crash on tests --- superset/initialization/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 655a88bd14e5d..c7387f3d6896b 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -475,7 +475,7 @@ def display_default_secret_key_warning() -> None: if "SUPERSET_SECRET_KEY" in os.environ: self.config["SECRET_KEY"] = os.environ["SUPERSET_SECRET_KEY"] return - if self.superset_app.debug: + if self.superset_app.debug or self.superset_app.config["TESTING"]: logger.warning("Debug mode identified with default secret key") display_default_secret_key_warning() return From ec5cd169295f2d19cdb0785aaf190806a4247a56 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 24 Feb 2023 10:55:33 +0000 Subject: [PATCH 3/6] lint --- superset/initialization/__init__.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index c7387f3d6896b..65420cd80fd7d 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -18,6 +18,7 @@ import logging import os +import sys from typing import Any, Callable, Dict, TYPE_CHECKING import wtforms_json @@ -458,7 +459,7 @@ def init_app_in_ctx(self) -> None: self.init_views() def check_secret_key(self) -> None: - def display_default_secret_key_warning() -> None: + def log_default_secret_key_warning() -> None: top_banner = 80 * "-" + "\n" + 36 * " " + "WARNING\n" + 80 * "-" bottom_banner = 80 * "-" + "\n" + 80 * "-" logger.warning(top_banner) @@ -475,13 +476,17 @@ def display_default_secret_key_warning() -> None: if "SUPERSET_SECRET_KEY" in os.environ: self.config["SECRET_KEY"] = os.environ["SUPERSET_SECRET_KEY"] return - if self.superset_app.debug or self.superset_app.config["TESTING"]: + if ( + self.superset_app.debug + or self.superset_app.config["TESTING"] + or "PYTEST_CURRENT_TEST" in os.environ + ): logger.warning("Debug mode identified with default secret key") - display_default_secret_key_warning() + log_default_secret_key_warning() return - display_default_secret_key_warning() + log_default_secret_key_warning() logger.error("Refusing to start due to insecure SECRET_KEY") - exit(1) + sys.exit(1) def init_app(self) -> None: """ From b3a212db7e893c6733fa77d058fb90150a40a783 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 24 Feb 2023 11:23:01 +0000 Subject: [PATCH 4/6] identify pytest is running --- superset/initialization/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 65420cd80fd7d..d9ed6bdbb419b 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -479,7 +479,8 @@ def log_default_secret_key_warning() -> None: if ( self.superset_app.debug or self.superset_app.config["TESTING"] - or "PYTEST_CURRENT_TEST" in os.environ + # There must be a better way + or "pytest" in sys.modules ): logger.warning("Debug mode identified with default secret key") log_default_secret_key_warning() From 306f48593f82e20e3bbf26249484ceac7055fa45 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 24 Feb 2023 11:56:26 +0000 Subject: [PATCH 5/6] add docs and improve env var use --- UPDATING.md | 1 + docs/docs/installation/configuring-superset.mdx | 4 ++-- superset/config.py | 2 +- superset/initialization/__init__.py | 3 --- tests/unit_tests/config_test.py | 4 ++++ 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 4c6f62e89d74b..9c7b1cf49e4fe 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,7 @@ assists people when migrating to a new version. ## Next +- [23186](https://github.com/apache/superset/pull/23186): Superset will refuse to start if a default `SECRET_KEY` is detected on a non Flask debug setting. - [22022](https://github.com/apache/superset/pull/22022): HTTP API endpoints `/superset/approve` and `/superset/request_access` have been deprecated and their HTTP methods were changed from GET to POST - [20606](https://github.com/apache/superset/pull/20606): When user clicks on chart title or "Edit chart" button in Dashboard page, Explore opens in the same tab. Clicking while holding cmd/ctrl opens Explore in a new tab. To bring back the old behaviour (always opening Explore in a new tab), flip feature flag `DASHBOARD_EDIT_CHART_IN_NEW_TAB` to `True`. - [20799](https://github.com/apache/superset/pull/20799): Presto and Trino engine will now display tracking URL for running queries in SQL Lab. If for some reason you don't want to show the tracking URL (for example, when your data warehouse hasn't enabled access for to Presto or Trino UI), update `TRACKING_URL_TRANSFORMER` in `config.py` to return `None`. diff --git a/docs/docs/installation/configuring-superset.mdx b/docs/docs/installation/configuring-superset.mdx index aefc12d603061..916c28b49522d 100644 --- a/docs/docs/installation/configuring-superset.mdx +++ b/docs/docs/installation/configuring-superset.mdx @@ -23,8 +23,8 @@ SUPERSET_WEBSERVER_PORT = 8088 # Your App secret key will be used for securely signing the session cookie # and encrypting sensitive information on the database # Make sure you are changing this key for your deployment with a strong key. -# You can generate a strong key using `openssl rand -base64 42` - +# You can generate a strong key using `openssl rand -base64 42`. +# Alternatively you can set it with `SUPERSET_SECRET_KEY` environment variable. SECRET_KEY = 'YOUR_OWN_RANDOM_GENERATED_SECRET_KEY' # The SQLAlchemy connection string to your database backend diff --git a/superset/config.py b/superset/config.py index 35a1b4ce2279a..2a39864922d93 100644 --- a/superset/config.py +++ b/superset/config.py @@ -192,7 +192,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # or use `SUPERSET_SECRET_KEY` environment variable. # Use a strong complex alphanumeric string and use a tool to help you generate # a sufficiently random sequence, ex: openssl rand -base64 42" -SECRET_KEY = CHANGE_ME_SECRET_KEY +SECRET_KEY = os.environ.get("SUPERSET_SECRET_KEY") or CHANGE_ME_SECRET_KEY # The SQLAlchemy connection string. SQLALCHEMY_DATABASE_URI = "sqlite:///" + os.path.join(DATA_DIR, "superset.db") diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index d9ed6bdbb419b..8d5508fb8dd75 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -473,9 +473,6 @@ def log_default_secret_key_warning() -> None: logger.warning(bottom_banner) if self.config["SECRET_KEY"] == CHANGE_ME_SECRET_KEY: - if "SUPERSET_SECRET_KEY" in os.environ: - self.config["SECRET_KEY"] = os.environ["SUPERSET_SECRET_KEY"] - return if ( self.superset_app.debug or self.superset_app.config["TESTING"] diff --git a/tests/unit_tests/config_test.py b/tests/unit_tests/config_test.py index 021193a6cd36e..5f21b0555104d 100644 --- a/tests/unit_tests/config_test.py +++ b/tests/unit_tests/config_test.py @@ -73,6 +73,10 @@ def apply_dttm_defaults(table: "SqlaTable", dttm_defaults: Dict[str, Any]) -> No dbcol.expression = dttm_column_defaults["expression"] +def test_default_secret_key() -> None: + check_secret_key + + @pytest.fixture def test_table(session: Session) -> "SqlaTable": """ From 9edf0c49f78dc533019bb11800e4c89cfabe0719 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 24 Feb 2023 11:58:59 +0000 Subject: [PATCH 6/6] revert --- tests/unit_tests/config_test.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/unit_tests/config_test.py b/tests/unit_tests/config_test.py index 5f21b0555104d..021193a6cd36e 100644 --- a/tests/unit_tests/config_test.py +++ b/tests/unit_tests/config_test.py @@ -73,10 +73,6 @@ def apply_dttm_defaults(table: "SqlaTable", dttm_defaults: Dict[str, Any]) -> No dbcol.expression = dttm_column_defaults["expression"] -def test_default_secret_key() -> None: - check_secret_key - - @pytest.fixture def test_table(session: Session) -> "SqlaTable": """