From 790eb9e0db7092da0fb299290a2490f8c1ace3c3 Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Tue, 20 Dec 2022 12:49:29 +0800 Subject: [PATCH 1/4] Fixes server error on embed due to breaking change on flask-login Due to a breaking change in Flask-Login (https://github.com/maxcountryman/flask-login/pull/378) the code for logging in our the AnonymousUser breaks. Unfortunately, Flask-Login not only renames the method we need, but also makes it quasi-private. We can switch to a different public util function Flask-Login offers since at least version 0.3.0. In all versions I checked it essentially executes the same steps as `reload_user(...)` did (it additionally signals the login event internally, which shouldn't cause issues). Fixes #21987 --- superset/embedded/view.py | 7 +++---- superset/views/dashboard/views.py | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/superset/embedded/view.py b/superset/embedded/view.py index c3c6c39ba25ad..8dd383aadafec 100644 --- a/superset/embedded/view.py +++ b/superset/embedded/view.py @@ -19,10 +19,10 @@ from flask import abort, g, request from flask_appbuilder import expose -from flask_login import AnonymousUserMixin, LoginManager +from flask_login import AnonymousUserMixin, login_user from flask_wtf.csrf import same_origin -from superset import event_logger, is_feature_enabled, security_manager +from superset import event_logger, is_feature_enabled from superset.embedded.dao import EmbeddedDAO from superset.superset_typing import FlaskResponse from superset.utils import core as utils @@ -68,8 +68,7 @@ def embedded( # Log in as an anonymous user, just for this view. # This view needs to be visible to all users, # and building the page fails if g.user and/or ctx.user aren't present. - login_manager: LoginManager = security_manager.lm - login_manager.reload_user(AnonymousUserMixin()) + login_user(AnonymousUserMixin(), force=True) add_extra_log_payload( embedded_dashboard_id=uuid, diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 32f0189d70462..52cb2da82e911 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -24,7 +24,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.decorators import has_access from flask_babel import gettext as __, lazy_gettext as _ -from flask_login import AnonymousUserMixin, LoginManager +from flask_login import AnonymousUserMixin, login_user from superset import db, event_logger, is_feature_enabled, security_manager from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod @@ -149,8 +149,7 @@ def embedded( # Log in as an anonymous user, just for this view. # This view needs to be visible to all users, # and building the page fails if g.user and/or ctx.user aren't present. - login_manager: LoginManager = security_manager.lm - login_manager.reload_user(AnonymousUserMixin()) + login_user(AnonymousUserMixin(), force=True) add_extra_log_payload( dashboard_id=dashboard_id_or_slug, From be46675a46f15d054a3eb97d48b518c29cf19db5 Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Wed, 11 Jan 2023 19:55:08 +0800 Subject: [PATCH 2/4] Adds integration test to catch issues for embed view --- .../integration_tests/embedded/view_tests.py | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 tests/integration_tests/embedded/view_tests.py diff --git a/tests/integration_tests/embedded/view_tests.py b/tests/integration_tests/embedded/view_tests.py new file mode 100644 index 0000000000000..db5390a43b504 --- /dev/null +++ b/tests/integration_tests/embedded/view_tests.py @@ -0,0 +1,63 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from unittest import mock + +import pytest + +from superset import db +from superset.embedded.dao import EmbeddedDAO +from superset.models.dashboard import Dashboard +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.fixtures.birth_names_dashboard import ( + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) + + +class TestEmbeddedDashboardView(SupersetTestCase): + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=True, + ) + def test_get_embedded_dashboard(self): + self.dash = db.session.query(Dashboard).filter_by(slug="births").first() + self.embedded = EmbeddedDAO.upsert(self.dash, []) + uri = f"embedded/{self.embedded.uuid}" + response = self.client.get(uri) + self.assert200(response) + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=True, + ) + def test_get_embedded_dashboard_referrer_not_allowed(self): + self.dash = db.session.query(Dashboard).filter_by(slug="births").first() + self.embedded = EmbeddedDAO.upsert(self.dash, ["test.example.com"]) + uri = f"embedded/{self.embedded.uuid}" + response = self.client.get(uri) + self.assert403(response) + + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=True, + ) + def test_get_embedded_dashboard_non_found(self): + uri = f"embedded/bad-uuid" + response = self.client.get(uri) + self.assert404(response) From c0b3679dc63eafa96375640abe7640bd6f08a85e Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Thu, 12 Jan 2023 10:41:55 +0800 Subject: [PATCH 3/4] Refactor embed dashboard view test to functional pytest --- tests/integration_tests/embedded/test_view.py | 70 +++++++++++++++++++ .../integration_tests/embedded/view_tests.py | 63 ----------------- 2 files changed, 70 insertions(+), 63 deletions(-) create mode 100644 tests/integration_tests/embedded/test_view.py delete mode 100644 tests/integration_tests/embedded/view_tests.py diff --git a/tests/integration_tests/embedded/test_view.py b/tests/integration_tests/embedded/test_view.py new file mode 100644 index 0000000000000..6bf86fea9851a --- /dev/null +++ b/tests/integration_tests/embedded/test_view.py @@ -0,0 +1,70 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from typing import TYPE_CHECKING +from unittest import mock + +import pytest + +from superset import db +from superset.embedded.dao import EmbeddedDAO +from superset.models.dashboard import Dashboard +from tests.integration_tests.fixtures.birth_names_dashboard import ( + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) +from tests.integration_tests.fixtures.client import client + +if TYPE_CHECKING: + from typing import Any + + from flask.testing import FlaskClient + + +@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") +@mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=True, +) +def test_get_embedded_dashboard(client: "FlaskClient[Any]"): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + embedded = EmbeddedDAO.upsert(dash, []) + uri = f"embedded/{embedded.uuid}" + response = client.get(uri) + assert response.status_code == 200 + + +@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") +@mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=True, +) +def test_get_embedded_dashboard_referrer_not_allowed(client: "FlaskClient[Any]"): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + embedded = EmbeddedDAO.upsert(dash, ["test.example.com"]) + uri = f"embedded/{embedded.uuid}" + response = client.get(uri) + assert response.status_code == 403 + + +@mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + EMBEDDED_SUPERSET=True, +) +def test_get_embedded_dashboard_non_found(client: "FlaskClient[Any]"): + uri = f"embedded/bad-uuid" + response = client.get(uri) + assert response.status_code == 404 diff --git a/tests/integration_tests/embedded/view_tests.py b/tests/integration_tests/embedded/view_tests.py deleted file mode 100644 index db5390a43b504..0000000000000 --- a/tests/integration_tests/embedded/view_tests.py +++ /dev/null @@ -1,63 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -from unittest import mock - -import pytest - -from superset import db -from superset.embedded.dao import EmbeddedDAO -from superset.models.dashboard import Dashboard -from tests.integration_tests.base_tests import SupersetTestCase -from tests.integration_tests.fixtures.birth_names_dashboard import ( - load_birth_names_dashboard_with_slices, - load_birth_names_data, -) - - -class TestEmbeddedDashboardView(SupersetTestCase): - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - EMBEDDED_SUPERSET=True, - ) - def test_get_embedded_dashboard(self): - self.dash = db.session.query(Dashboard).filter_by(slug="births").first() - self.embedded = EmbeddedDAO.upsert(self.dash, []) - uri = f"embedded/{self.embedded.uuid}" - response = self.client.get(uri) - self.assert200(response) - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - EMBEDDED_SUPERSET=True, - ) - def test_get_embedded_dashboard_referrer_not_allowed(self): - self.dash = db.session.query(Dashboard).filter_by(slug="births").first() - self.embedded = EmbeddedDAO.upsert(self.dash, ["test.example.com"]) - uri = f"embedded/{self.embedded.uuid}" - response = self.client.get(uri) - self.assert403(response) - - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - EMBEDDED_SUPERSET=True, - ) - def test_get_embedded_dashboard_non_found(self): - uri = f"embedded/bad-uuid" - response = self.client.get(uri) - self.assert404(response) From a4b3224cbf113c37585cbc95b017e699ca00d1d6 Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Thu, 12 Jan 2023 16:56:13 +0800 Subject: [PATCH 4/4] Make use of __future__ for prettier types --- tests/integration_tests/embedded/test_view.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/embedded/test_view.py b/tests/integration_tests/embedded/test_view.py index 6bf86fea9851a..9f524e9c09e2b 100644 --- a/tests/integration_tests/embedded/test_view.py +++ b/tests/integration_tests/embedded/test_view.py @@ -14,6 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from __future__ import annotations + from typing import TYPE_CHECKING from unittest import mock @@ -39,7 +41,7 @@ "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) -def test_get_embedded_dashboard(client: "FlaskClient[Any]"): +def test_get_embedded_dashboard(client: FlaskClient[Any]): dash = db.session.query(Dashboard).filter_by(slug="births").first() embedded = EmbeddedDAO.upsert(dash, []) uri = f"embedded/{embedded.uuid}" @@ -52,7 +54,7 @@ def test_get_embedded_dashboard(client: "FlaskClient[Any]"): "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) -def test_get_embedded_dashboard_referrer_not_allowed(client: "FlaskClient[Any]"): +def test_get_embedded_dashboard_referrer_not_allowed(client: FlaskClient[Any]): dash = db.session.query(Dashboard).filter_by(slug="births").first() embedded = EmbeddedDAO.upsert(dash, ["test.example.com"]) uri = f"embedded/{embedded.uuid}" @@ -64,7 +66,7 @@ def test_get_embedded_dashboard_referrer_not_allowed(client: "FlaskClient[Any]") "superset.extensions.feature_flag_manager._feature_flags", EMBEDDED_SUPERSET=True, ) -def test_get_embedded_dashboard_non_found(client: "FlaskClient[Any]"): +def test_get_embedded_dashboard_non_found(client: FlaskClient[Any]): uri = f"embedded/bad-uuid" response = client.get(uri) assert response.status_code == 404