refactor(api): csrf token on the new REST API (#13212)

* refactor(api): csrf token on the new REST API

* improve OpenAPI spec description

* fix test

* remove public role like has default for all tests

* fix test

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
         #

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())

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"
         )

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)
 

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)})

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)

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,
     ):

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()

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)

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"))
 

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()

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")