diff --git a/superset/app.py b/superset/app.py index d297c4e4e..feaf0d82c 100644 --- a/superset/app.py +++ b/superset/app.py @@ -149,6 +149,7 @@ class SupersetAppInitializer: from superset.databases.api import DatabaseRestApi from superset.datasets.api import DatasetRestApi from superset.queries.api import QueryRestApi + from superset.security.api import SecurityRestApi from superset.queries.saved_queries.api import SavedQueryRestApi from superset.reports.api import ReportScheduleRestApi from superset.reports.logs.api import ReportExecutionLogRestApi @@ -406,7 +407,7 @@ class SupersetAppInitializer: category_label=__("Security"), icon="fa-list-ol", ) - + appbuilder.add_api(SecurityRestApi) # # Conditionally setup email views # diff --git a/superset/security/api.py b/superset/security/api.py new file mode 100644 index 000000000..5aa51f85f --- /dev/null +++ b/superset/security/api.py @@ -0,0 +1,62 @@ +# 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. +import logging + +from flask import Response +from flask_appbuilder import expose +from flask_appbuilder.api import BaseApi, safe +from flask_appbuilder.security.decorators import permission_name, protect +from flask_wtf.csrf import generate_csrf + +from superset.extensions import event_logger + +logger = logging.getLogger(__name__) + + +class SecurityRestApi(BaseApi): + resource_name = "security" + allow_browser_login = True + openapi_spec_tag = "Security" + + @expose("/csrf_token/", methods=["GET"]) + @event_logger.log_this + @protect() + @safe + @permission_name("read") + def csrf_token(self) -> Response: + """ + Return the csrf token + --- + get: + description: >- + Fetch the CSRF token + responses: + 200: + description: Result contains the CSRF token + content: + application/json: + schema: + type: object + properties: + result: + type: string + 401: + $ref: '#/components/responses/401' + 500: + $ref: '#/components/responses/500' + """ + return self.response(200, result=generate_csrf()) diff --git a/superset/views/core.py b/superset/views/core.py index 62b1b498e..155555e4a 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1408,6 +1408,9 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods @event_logger.log_this @expose("/csrf_token/", methods=["GET"]) def csrf_token(self) -> FlaskResponse: + logger.warning( + "This API endpoint is deprecated and will be removed in version 2.0.0" + ) return Response( self.render_template("superset/csrf_token.json"), mimetype="text/json" ) diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index fef99e0b8..a6030b675 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -1369,7 +1369,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): test_client.set_cookie( "localhost", app.config["GLOBAL_ASYNC_QUERIES_JWT_COOKIE_NAME"], "foo" ) - rv = post_assert_metric(test_client, CHART_DATA_URI, request_payload, "data") + rv = test_client.post(CHART_DATA_URI, json=request_payload) self.assertEqual(rv.status_code, 401) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @@ -1444,9 +1444,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): return orig_run(self, force_cached=False) with mock.patch.object(ChartDataCommand, "run", new=mock_run): - rv = self.get_assert_metric( - f"{CHART_DATA_URI}/test-cache-key", "data_from_cache" - ) + rv = self.client.get(f"{CHART_DATA_URI}/test-cache-key",) self.assertEqual(rv.status_code, 401) diff --git a/tests/core_tests.py b/tests/core_tests.py index 912b23bde..3bc230a19 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -612,6 +612,7 @@ class TestCore(SupersetTestCase): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_cache_logging(self): + self.login("admin") store_cache_keys = app.config["STORE_CACHE_KEYS_IN_METADATA_DB"] app.config["STORE_CACHE_KEYS_IN_METADATA_DB"] = True girls_slice = self.get_slice("Girls", db.session) @@ -785,6 +786,7 @@ class TestCore(SupersetTestCase): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_slice_id_is_always_logged_correctly_on_web_request(self): # superset/explore case + self.login("admin") slc = db.session.query(Slice).filter_by(slice_name="Girls").one() qry = db.session.query(models.Log).filter_by(slice_id=slc.id) self.get_resp(slc.slice_url, {"form_data": json.dumps(slc.form_data)}) diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index 9780b1608..f24340095 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -20,20 +20,21 @@ from datetime import datetime import json import unittest from random import random -from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices import pytest from flask import escape, url_for from sqlalchemy import func -from tests.fixtures.unicode_dashboard import load_unicode_dashboard_with_position from tests.test_app import app from superset import db, security_manager from superset.connectors.sqla.models import SqlaTable from superset.models import core as models from superset.models.dashboard import Dashboard from superset.models.slice import Slice +from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices from tests.fixtures.energy_dashboard import load_energy_table_with_slice +from tests.fixtures.public_role import public_role_like_gamma +from tests.fixtures.unicode_dashboard import load_unicode_dashboard_with_position from tests.fixtures.world_bank_dashboard import load_world_bank_dashboard_with_slices from .base_tests import SupersetTestCase @@ -378,6 +379,7 @@ class TestDashboard(SupersetTestCase): self.assertEqual(len(data["slices"]), origin_slices_length - 1) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @pytest.mark.usefixtures("public_role_like_gamma") def test_public_user_dashboard_access(self): table = db.session.query(SqlaTable).filter_by(table_name="birth_names").one() @@ -419,6 +421,7 @@ class TestDashboard(SupersetTestCase): self.revoke_public_access_to_table(table) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + @pytest.mark.usefixtures("public_role_like_gamma") def test_dashboard_with_created_by_can_be_accessed_by_public_users(self): self.logout() table = db.session.query(SqlaTable).filter_by(table_name="birth_names").one() @@ -455,6 +458,7 @@ class TestDashboard(SupersetTestCase): @pytest.mark.usefixtures("load_energy_table_with_slice", "load_dashboard") def test_users_can_view_published_dashboard(self): + self.login("alpha") resp = self.get_resp("/api/v1/dashboard/") self.assertNotIn(f"/superset/dashboard/{pytest.hidden_dash_slug}/", resp) self.assertIn(f"/superset/dashboard/{pytest.published_dash_slug}/", resp) diff --git a/tests/dashboards/security/security_rbac_tests.py b/tests/dashboards/security/security_rbac_tests.py index 41a01fade..19885d958 100644 --- a/tests/dashboards/security/security_rbac_tests.py +++ b/tests/dashboards/security/security_rbac_tests.py @@ -17,6 +17,8 @@ """Unit tests for Superset""" from unittest import mock +import pytest + from tests.dashboards.dashboard_test_utils import * from tests.dashboards.security.base_case import BaseTestDashboardSecurity from tests.dashboards.superset_factory_util import ( @@ -25,6 +27,7 @@ from tests.dashboards.superset_factory_util import ( create_datasource_table_to_db, create_slice_to_db, ) +from tests.fixtures.public_role import public_role_like_gamma @mock.patch.dict( @@ -117,6 +120,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): # post revoke_access_to_dashboard(dashboard_to_access, new_role) + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboard_view__public_user_can_not_access_without_permission(self): dashboard_to_access = create_dashboard_to_db(published=True) self.logout() @@ -127,6 +131,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): # assert self.assert403(response) + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboard_view__public_user_with_dashboard_permission_can_not_access_draft( self, ): @@ -143,6 +148,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): # post revoke_access_to_dashboard(dashboard_to_access, "Public") + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboard_view__public_user_access_with_dashboard_permission(self): # arrange dashboard_to_access = create_dashboard_to_db( @@ -267,6 +273,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): self.login(username) return new_role, draft_dashboards, published_dashboards + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboards_list__public_user_without_any_permissions_get_empty_list( self, ): @@ -278,6 +285,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): # assert self.assert_dashboards_list_view_response(response, 0) + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboards_list__public_user_get_only_published_permitted_dashboards( self, ): @@ -370,6 +378,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): for dash in published_dashboards + draft_dashboards: revoke_access_to_dashboard(dash, new_role) + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboards_api__public_user_without_any_permissions_get_empty_list( self, ): @@ -382,6 +391,7 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity): # assert self.assert_dashboards_api_response(response, 0) + @pytest.mark.usefixtures("public_role_like_gamma") def test_get_dashboards_api__public_user_get_only_published_permitted_dashboards( self, ): diff --git a/tests/fixtures/public_role.py b/tests/fixtures/public_role.py new file mode 100644 index 000000000..0e3dc8cd1 --- /dev/null +++ b/tests/fixtures/public_role.py @@ -0,0 +1,44 @@ +# 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. +import pytest + +from superset.extensions import db, security_manager +from tests.test_app import app + + +@pytest.fixture() +def public_role_like_gamma(): + with app.app_context(): + app.config["PUBLIC_ROLE_LIKE"] = "Gamma" + security_manager.sync_role_definitions() + + yield + + security_manager.get_public_role().permissions = [] + db.session.commit() + + +@pytest.fixture() +def public_role_like_test_role(): + with app.app_context(): + app.config["PUBLIC_ROLE_LIKE"] = "TestRole" + security_manager.sync_role_definitions() + + yield + + security_manager.get_public_role().permissions = [] + db.session.commit() diff --git a/tests/security/api_tests.py b/tests/security/api_tests.py new file mode 100644 index 000000000..d10387768 --- /dev/null +++ b/tests/security/api_tests.py @@ -0,0 +1,57 @@ +# 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. +# isort:skip_file +# pylint: disable=too-many-public-methods, no-self-use, invalid-name, too-many-arguments +"""Unit tests for Superset""" +import json + +from tests.base_tests import SupersetTestCase +from flask_wtf.csrf import generate_csrf + + +class TestSecurityApi(SupersetTestCase): + resource_name = "security" + + def _assert_get_csrf_token(self): + uri = f"api/v1/{self.resource_name}/csrf_token/" + response = self.client.get(uri) + assert response.status_code == 200 + data = json.loads(response.data.decode("utf-8")) + assert data["result"] == generate_csrf() + + def test_get_csrf_token(self): + """ + Security API: Test get CSRF token + """ + self.login(username="admin") + self._assert_get_csrf_token() + + def test_get_csrf_token_gamma(self): + """ + Security API: Test get CSRF token by gamma + """ + self.login(username="gamma") + self._assert_get_csrf_token() + + def test_get_csrf_unauthorized(self): + """ + Security API: Test get CSRF no login + """ + self.logout() + uri = f"api/v1/{self.resource_name}/csrf_token/" + response = self.client.get(uri) + self.assertEqual(response.status_code, 401) diff --git a/tests/security_tests.py b/tests/security_tests.py index 1b7e0b3a5..335fe635a 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -30,7 +30,6 @@ from flask import current_app, g from sqlalchemy import Float, Date, String from superset.models.dashboard import Dashboard -from tests.fixtures.world_bank_dashboard import load_world_bank_dashboard_with_slices from superset import app, appbuilder, db, security_manager, viz, ConnectorRegistry from superset.connectors.druid.models import DruidCluster, DruidDatasource @@ -48,9 +47,14 @@ from .dashboard_utils import ( create_slice, create_dashboard, ) -from .fixtures.energy_dashboard import load_energy_table_with_slice -from .fixtures.unicode_dashboard import load_unicode_dashboard_with_slice from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices +from tests.fixtures.energy_dashboard import load_energy_table_with_slice +from tests.fixtures.public_role import ( + public_role_like_gamma, + public_role_like_test_role, +) +from tests.fixtures.unicode_dashboard import load_unicode_dashboard_with_slice +from tests.fixtures.world_bank_dashboard import load_world_bank_dashboard_with_slices NEW_SECURITY_CONVERGE_VIEWS = ( "Annotation", @@ -567,6 +571,7 @@ class TestRolePermission(SupersetTestCase): ) # wb_health_population slice, has access self.assertNotIn("Girl Name Cloud", data) # birth_names slice, no access + @pytest.mark.usefixtures("public_role_like_gamma") def test_public_sync_role_data_perms(self): """ Security: Tests if the sync role method preserves data access permissions @@ -594,13 +599,11 @@ class TestRolePermission(SupersetTestCase): # Cleanup self.revoke_public_access_to_table(table) + @pytest.mark.usefixtures("public_role_like_test_role") def test_public_sync_role_builtin_perms(self): """ Security: Tests public role creation based on a builtin role """ - current_app.config["PUBLIC_ROLE_LIKE"] = "TestRole" - - security_manager.sync_role_definitions() public_role = security_manager.get_public_role() public_role_resource_names = [ [permission.view_menu.name, permission.permission.name] @@ -609,10 +612,6 @@ class TestRolePermission(SupersetTestCase): for pvm in current_app.config["FAB_ROLES"]["TestRole"]: assert pvm in public_role_resource_names - # Cleanup - current_app.config["PUBLIC_ROLE_LIKE"] = "Gamma" - security_manager.sync_role_definitions() - def test_sqllab_gamma_user_schema_access_to_sqllab(self): session = db.session @@ -815,6 +814,7 @@ class TestRolePermission(SupersetTestCase): self.assert_can_gamma(get_perm_tuples("Gamma")) self.assert_cannot_alpha(get_perm_tuples("Gamma")) + @pytest.mark.usefixtures("public_role_like_gamma") def test_public_permissions_basic(self): self.assert_can_gamma(get_perm_tuples("Public")) diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py index 395948556..ee0fac541 100644 --- a/tests/sqllab_tests.py +++ b/tests/sqllab_tests.py @@ -253,7 +253,7 @@ class TestSqlLab(SupersetTestCase): # Not logged in, should error out resp = self.client.get("/superset/queries/0") # Redirects to the login page - self.assertEqual(403, resp.status_code) + self.assertEqual(401, resp.status_code) # Admin sees queries self.login("admin") @@ -286,7 +286,7 @@ class TestSqlLab(SupersetTestCase): self.logout() resp = self.client.get("/superset/queries/0") # Redirects to the login page - self.assertEqual(403, resp.status_code) + self.assertEqual(401, resp.status_code) def test_search_query_on_db_id(self): self.run_some_queries() diff --git a/tests/superset_test_config.py b/tests/superset_test_config.py index 9398dab19..0925ec11a 100644 --- a/tests/superset_test_config.py +++ b/tests/superset_test_config.py @@ -73,7 +73,6 @@ WTF_CSRF_ENABLED = False FAB_ROLES = {"TestRole": [["Security", "menu_access"], ["List Users", "menu_access"]]} -PUBLIC_ROLE_LIKE = "Gamma" AUTH_ROLE_PUBLIC = "Public" EMAIL_NOTIFICATIONS = False REDIS_HOST = os.environ.get("REDIS_HOST", "localhost")