You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/01/05 19:57:28 UTC
[superset] 08/14: fix: check that imports are ZIPs (#21875)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 92863659a675e3da8ba83bc836765a5edf6db519
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed Oct 26 12:15:46 2022 -0700
fix: check that imports are ZIPs (#21875)
---
superset/charts/api.py | 9 +++--
superset/databases/api.py | 9 +++--
superset/queries/saved_queries/api.py | 9 +++--
tests/unit_tests/conftest.py | 20 +++++++++++
tests/unit_tests/databases/api_test.py | 56 +++++++++++++++++++++++++++++++
tests/unit_tests/importexport/api_test.py | 3 +-
6 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/superset/charts/api.py b/superset/charts/api.py
index 260ae4442c..601bb9f841 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -19,7 +19,7 @@ import logging
from datetime import datetime
from io import BytesIO
from typing import Any, Optional
-from zipfile import ZipFile
+from zipfile import is_zipfile, ZipFile
from flask import g, redirect, request, Response, send_file, url_for
from flask_appbuilder.api import expose, protect, rison, safe
@@ -64,7 +64,10 @@ from superset.charts.schemas import (
screenshot_query_schema,
thumbnail_query_schema,
)
-from superset.commands.importers.exceptions import NoValidFilesFoundError
+from superset.commands.importers.exceptions import (
+ IncorrectFormatError,
+ NoValidFilesFoundError,
+)
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.extensions import event_logger
@@ -871,6 +874,8 @@ class ChartRestApi(BaseSupersetModelRestApi):
upload = request.files.get("formData")
if not upload:
return self.response_400()
+ if not is_zipfile(upload):
+ raise IncorrectFormatError("Not a ZIP file")
with ZipFile(upload) as bundle:
contents = get_contents_from_bundle(bundle)
diff --git a/superset/databases/api.py b/superset/databases/api.py
index 0de8bcf83e..186ce93e06 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -20,7 +20,7 @@ import logging
from datetime import datetime
from io import BytesIO
from typing import Any, Dict, List, Optional
-from zipfile import ZipFile
+from zipfile import is_zipfile, ZipFile
from flask import g, request, Response, send_file
from flask_appbuilder.api import expose, protect, rison, safe
@@ -29,7 +29,10 @@ from marshmallow import ValidationError
from sqlalchemy.exc import NoSuchTableError, OperationalError, SQLAlchemyError
from superset import app, event_logger
-from superset.commands.importers.exceptions import NoValidFilesFoundError
+from superset.commands.importers.exceptions import (
+ IncorrectFormatError,
+ NoValidFilesFoundError,
+)
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.databases.commands.create import CreateDatabaseCommand
@@ -825,6 +828,8 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
upload = request.files.get("formData")
if not upload:
return self.response_400()
+ if not is_zipfile(upload):
+ raise IncorrectFormatError("Not a ZIP file")
with ZipFile(upload) as bundle:
contents = get_contents_from_bundle(bundle)
diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py
index 04df2345c1..19d4191f25 100644
--- a/superset/queries/saved_queries/api.py
+++ b/superset/queries/saved_queries/api.py
@@ -19,14 +19,17 @@ import logging
from datetime import datetime
from io import BytesIO
from typing import Any
-from zipfile import ZipFile
+from zipfile import is_zipfile, ZipFile
from flask import g, request, Response, send_file
from flask_appbuilder.api import expose, protect, rison, safe
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import ngettext
-from superset.commands.importers.exceptions import NoValidFilesFoundError
+from superset.commands.importers.exceptions import (
+ IncorrectFormatError,
+ NoValidFilesFoundError,
+)
from superset.commands.importers.v1.utils import get_contents_from_bundle
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.databases.filters import DatabaseFilter
@@ -325,6 +328,8 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
upload = request.files.get("formData")
if not upload:
return self.response_400()
+ if not is_zipfile(upload):
+ raise IncorrectFormatError("Not a ZIP file")
with ZipFile(upload) as bundle:
contents = get_contents_from_bundle(bundle)
diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py
index 4987aaf0e0..43c4ac69ad 100644
--- a/tests/unit_tests/conftest.py
+++ b/tests/unit_tests/conftest.py
@@ -25,6 +25,7 @@ from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from sqlalchemy.orm.session import Session
+from superset import security_manager
from superset.app import SupersetApp
from superset.extensions import appbuilder
from superset.initialization import SupersetAppInitializer
@@ -61,6 +62,7 @@ def app() -> Iterator[SupersetApp]:
app.config.from_object("superset.config")
app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite://"
+ app.config["WTF_CSRF_ENABLED"] = False
app.config["TESTING"] = True
# ``superset.extensions.appbuilder`` is a singleton, and won't rebuild the
@@ -93,3 +95,21 @@ def app_context(app: SupersetApp) -> Iterator[None]:
"""
with app.app_context():
yield
+
+
+@pytest.fixture
+def full_api_access(mocker: MockFixture) -> Iterator[None]:
+ """
+ Allow full access to the API.
+
+ TODO (betodealmeida): we should replace this with user-fixtures, eg, ``admin`` or
+ ``gamma``, so that we have granular access to the APIs.
+ """
+ mocker.patch(
+ "flask_appbuilder.security.decorators.verify_jwt_in_request",
+ return_value=True,
+ )
+ mocker.patch.object(security_manager, "has_access", return_value=True)
+ mocker.patch.object(security_manager, "can_access_all_databases", return_value=True)
+
+ yield
diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
new file mode 100644
index 0000000000..2d20e5346f
--- /dev/null
+++ b/tests/unit_tests/databases/api_test.py
@@ -0,0 +1,56 @@
+# 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.
+
+# pylint: disable=unused-argument, import-outside-toplevel, line-too-long
+
+from io import BytesIO
+from typing import Any
+
+import pytest
+
+
+def test_non_zip_import(client: Any, full_api_access: None) -> None:
+ """
+ Test that non-ZIP imports are not allowed.
+ """
+ buf = BytesIO(b"definitely_not_a_zip_file")
+ form_data = {
+ "formData": (buf, "evil.pdf"),
+ }
+ response = client.post(
+ "/api/v1/database/import/",
+ data=form_data,
+ content_type="multipart/form-data",
+ )
+ assert response.status_code == 422
+ assert response.json == {
+ "errors": [
+ {
+ "message": "Not a ZIP file",
+ "error_type": "GENERIC_COMMAND_ERROR",
+ "level": "warning",
+ "extra": {
+ "issue_codes": [
+ {
+ "code": 1010,
+ "message": "Issue 1010 - Superset encountered an error while running a command.",
+ }
+ ]
+ },
+ }
+ ]
+ }
diff --git a/tests/unit_tests/importexport/api_test.py b/tests/unit_tests/importexport/api_test.py
index e5dee975d8..1ef4309fc2 100644
--- a/tests/unit_tests/importexport/api_test.py
+++ b/tests/unit_tests/importexport/api_test.py
@@ -14,7 +14,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-# pylint: disable=invalid-name, import-outside-toplevel
+
+# pylint: disable=invalid-name, import-outside-toplevel, unused-argument
import json
from io import BytesIO