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
diff --git a/superset/app.py b/superset/app.py
index d297c4e..feaf0d8 100644
--- a/superset/app.py
+++ b/superset/app.py
@@ -149,6 +149,7 @@
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 @@
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 0000000..5aa51f8
--- /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 62b1b49..155555e 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1408,6 +1408,9 @@
@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 fef99e0..a6030b6 100644
--- a/tests/charts/api_tests.py
+++ b/tests/charts/api_tests.py
@@ -1369,7 +1369,7 @@
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 @@
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 912b23b..3bc230a 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -612,6 +612,7 @@
@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 @@
@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 9780b16..f243400 100644
--- a/tests/dashboard_tests.py
+++ b/tests/dashboard_tests.py
@@ -20,20 +20,21 @@
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 @@
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 @@
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 @@
@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 41a01fa..19885d9 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 @@
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 @@
# 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 @@
# 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 @@
# 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 @@
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 @@
# 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 @@
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 @@
# 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 0000000..0e3dc8c
--- /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 0000000..d103877
--- /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 1b7e0b3..335fe63 100644
--- a/tests/security_tests.py
+++ b/tests/security_tests.py
@@ -30,7 +30,6 @@
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 @@
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 @@
) # 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 @@
# 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 @@
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 @@
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 3959485..ee0fac5 100644
--- a/tests/sqllab_tests.py
+++ b/tests/sqllab_tests.py
@@ -253,7 +253,7 @@
# 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 @@
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 9398dab..0925ec1 100644
--- a/tests/superset_test_config.py
+++ b/tests/superset_test_config.py
@@ -73,7 +73,6 @@
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")
