mirror of
https://github.com/apache/superset.git
synced 2026-07-20 13:45:47 +00:00
fix(extensions): sync PR review fixes to test/chatbot-local
- ExtensionsList.tsx: unique Tooltip ids per row; onKeyDown Enter/Space for star and delete role=button spans; data-test attrs; remove stale loading dep - ExtensionsList.test.tsx: 10 unit tests (import validation, delete confirm, star toggle, keyboard a11y, file upload) - api.py: path-traversal validation, upload size limit, LOCAL_EXTENSIONS 409 - test_api.py: 19 Python unit tests for POST and DELETE endpoints Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -16,73 +16,284 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { render, waitFor } from 'spec/helpers/testing-library';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { fireEvent, render, screen, waitFor, within } from 'spec/helpers/testing-library';
|
||||
import { SupersetClient } from '@superset-ui/core';
|
||||
import ExtensionsList from './ExtensionsList';
|
||||
import fetchMock from 'fetch-mock';
|
||||
|
||||
beforeAll(() => fetchMock.unmockGlobal());
|
||||
// ---------------------------------------------------------------------------
|
||||
// Module-level mocks
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Mock initial state for the store
|
||||
const mockInitialState = {
|
||||
extensions: {
|
||||
loading: false,
|
||||
resourceCount: 2,
|
||||
resourceCollection: [
|
||||
{
|
||||
id: 1,
|
||||
name: 'Test Extension 1',
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
name: 'Test Extension 2',
|
||||
enabled: false,
|
||||
},
|
||||
],
|
||||
bulkSelectEnabled: false,
|
||||
jest.mock('src/views/CRUD/hooks', () => ({
|
||||
useListViewResource: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('src/components', () => ({
|
||||
ListView: ({ columns, data }: any) => (
|
||||
<table>
|
||||
<tbody>
|
||||
{(data ?? []).map((row: any) =>
|
||||
columns.map((col: any) => (
|
||||
<td key={`${row.id}-${col.id}`}>
|
||||
{col.Cell ? col.Cell({ row: { original: row } }) : row[col.accessor]}
|
||||
</td>
|
||||
)),
|
||||
)}
|
||||
</tbody>
|
||||
</table>
|
||||
),
|
||||
}));
|
||||
|
||||
// Stub SubMenu so tests aren't coupled to the navigation menu rendering chain.
|
||||
jest.mock('src/features/home/SubMenu', () => ({
|
||||
__esModule: true,
|
||||
default: ({ buttons }: any) => (
|
||||
<div data-test="submenu">
|
||||
{(buttons ?? []).map((btn: any, i: number) => (
|
||||
// eslint-disable-next-line react/no-array-index-key
|
||||
<button key={i} type="button" onClick={btn.onClick}>
|
||||
{btn.name}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
||||
// withToasts is the outermost HOC — pass through so callers can inject toast fns.
|
||||
jest.mock('src/components/MessageToasts/withToasts', () => (C: any) => C);
|
||||
|
||||
jest.mock('src/views/contributions', () => ({
|
||||
CHATBOT_LOCATION: 'superset.chatbot',
|
||||
}));
|
||||
|
||||
jest.mock('src/core/views', () => ({
|
||||
getRegisteredViewIds: jest.fn(() => []),
|
||||
subscribeToRegistry: jest.fn(() => () => undefined),
|
||||
getRegistryVersion: jest.fn(() => 0),
|
||||
}));
|
||||
|
||||
jest.mock('src/core/extensions', () => ({
|
||||
notifyExtensionSettingsChanged: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('@superset-ui/core', () => {
|
||||
const actual = jest.requireActual('@superset-ui/core');
|
||||
return {
|
||||
...actual,
|
||||
SupersetClient: {
|
||||
get: jest.fn(),
|
||||
post: jest.fn(),
|
||||
put: jest.fn(),
|
||||
delete: jest.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const { useListViewResource } = jest.requireMock('src/views/CRUD/hooks');
|
||||
const mockGet = SupersetClient.get as jest.Mock;
|
||||
const mockPost = SupersetClient.post as jest.Mock;
|
||||
const mockPut = SupersetClient.put as jest.Mock;
|
||||
const mockDelete = SupersetClient.delete as jest.Mock;
|
||||
|
||||
const EXTENSIONS = [
|
||||
{
|
||||
id: 'acme.chatbot',
|
||||
name: 'chatbot',
|
||||
publisher: 'acme',
|
||||
enabled: true,
|
||||
deletable: true,
|
||||
},
|
||||
};
|
||||
{
|
||||
id: 'acme.widget',
|
||||
name: 'widget',
|
||||
publisher: 'acme',
|
||||
enabled: true,
|
||||
deletable: false,
|
||||
},
|
||||
];
|
||||
|
||||
const mockFetchData = jest.fn();
|
||||
const mockRefreshData = jest.fn();
|
||||
|
||||
function setupHook(extensions = EXTENSIONS) {
|
||||
useListViewResource.mockReturnValue({
|
||||
state: {
|
||||
loading: false,
|
||||
resourceCount: extensions.length,
|
||||
resourceCollection: extensions,
|
||||
},
|
||||
fetchData: mockFetchData,
|
||||
refreshData: mockRefreshData,
|
||||
});
|
||||
}
|
||||
|
||||
const defaultProps = {
|
||||
addDangerToast: jest.fn(),
|
||||
addSuccessToast: jest.fn(),
|
||||
};
|
||||
|
||||
const renderWithStore = (props = {}) =>
|
||||
render(<ExtensionsList {...defaultProps} {...props} />, {
|
||||
function renderList(props = {}) {
|
||||
return render(<ExtensionsList {...defaultProps} {...props} />, {
|
||||
useRedux: true,
|
||||
useQueryParams: true,
|
||||
useRouter: true,
|
||||
useTheme: true,
|
||||
initialState: mockInitialState,
|
||||
});
|
||||
}
|
||||
|
||||
test('renders extensions list with basic structure', async () => {
|
||||
renderWithStore();
|
||||
function uploadFile(input: HTMLInputElement, file: File) {
|
||||
Object.defineProperty(input, 'files', { value: [file], configurable: true });
|
||||
fireEvent.change(input);
|
||||
}
|
||||
|
||||
// Check that the component renders
|
||||
expect(document.body).toBeInTheDocument();
|
||||
// ---------------------------------------------------------------------------
|
||||
// Setup / teardown
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockGet.mockResolvedValue({
|
||||
json: { result: { active_chatbot_id: null, enabled: {} } },
|
||||
});
|
||||
setupHook();
|
||||
});
|
||||
|
||||
test('displays extension names in the list', async () => {
|
||||
renderWithStore();
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('renders the import button in the submenu', () => {
|
||||
renderList();
|
||||
expect(screen.getByTestId('submenu')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('renders extension names in the table', async () => {
|
||||
renderList();
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('chatbot')).toBeInTheDocument();
|
||||
expect(screen.getByText('widget')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
test('renders delete button only for deletable extensions', async () => {
|
||||
renderList();
|
||||
await waitFor(() => {
|
||||
// Only acme.chatbot has deletable: true
|
||||
expect(screen.getAllByTestId('delete-extension')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
test('clicking delete opens confirmation dialog', async () => {
|
||||
renderList();
|
||||
await waitFor(() => screen.getByText('chatbot'));
|
||||
|
||||
await userEvent.click(screen.getByTestId('delete-extension'));
|
||||
|
||||
await waitFor(() => {
|
||||
// These texts should appear somewhere in the rendered component
|
||||
expect(document.body).toHaveTextContent(/Extensions/);
|
||||
expect(screen.getByText(/are you sure you want to delete/i)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
test('calls toast functions when provided', () => {
|
||||
test('typing DELETE in confirmation modal triggers delete API call', async () => {
|
||||
mockDelete.mockResolvedValue({});
|
||||
renderList();
|
||||
await waitFor(() => screen.getByText('chatbot'));
|
||||
|
||||
await userEvent.click(screen.getByTestId('delete-extension'));
|
||||
|
||||
const confirmInput = await screen.findByTestId('delete-modal-input');
|
||||
fireEvent.change(confirmInput, { target: { value: 'DELETE' } });
|
||||
|
||||
const modal = screen.getByRole('dialog');
|
||||
const confirmBtn = within(modal)
|
||||
.getAllByRole('button', { name: /^delete$/i })
|
||||
.pop()!;
|
||||
await userEvent.click(confirmBtn);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockDelete).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ endpoint: '/api/v1/extensions/acme/chatbot' }),
|
||||
);
|
||||
expect(mockRefreshData).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
test('star button shown only for extensions registered as chatbot views', async () => {
|
||||
const { getRegisteredViewIds } = jest.requireMock('src/core/views');
|
||||
(getRegisteredViewIds as jest.Mock).mockReturnValue(['acme.chatbot']);
|
||||
|
||||
renderList();
|
||||
await waitFor(() => screen.getByText('chatbot'));
|
||||
|
||||
expect(screen.getAllByTestId('set-default-chatbot')).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('clicking star calls PUT settings with the extension id', async () => {
|
||||
const { getRegisteredViewIds } = jest.requireMock('src/core/views');
|
||||
(getRegisteredViewIds as jest.Mock).mockReturnValue(['acme.chatbot']);
|
||||
mockPut.mockResolvedValue({ json: {} });
|
||||
|
||||
renderList();
|
||||
await waitFor(() => screen.getByText('chatbot'));
|
||||
|
||||
await userEvent.click(screen.getByTestId('set-default-chatbot'));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockPut).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
endpoint: '/api/v1/extensions/settings',
|
||||
jsonPayload: expect.objectContaining({
|
||||
active_chatbot_id: 'acme.chatbot',
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test('pressing Enter on star span triggers set-default action', async () => {
|
||||
const { getRegisteredViewIds } = jest.requireMock('src/core/views');
|
||||
(getRegisteredViewIds as jest.Mock).mockReturnValue(['acme.chatbot']);
|
||||
mockPut.mockResolvedValue({ json: {} });
|
||||
|
||||
renderList();
|
||||
await waitFor(() => screen.getByText('chatbot'));
|
||||
|
||||
fireEvent.keyDown(screen.getByTestId('set-default-chatbot'), { key: 'Enter' });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockPut).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
test('uploading a non-.supx file shows danger toast without calling API', async () => {
|
||||
const addDangerToast = jest.fn();
|
||||
const addSuccessToast = jest.fn();
|
||||
renderList({ addDangerToast });
|
||||
|
||||
renderWithStore({
|
||||
addDangerToast,
|
||||
addSuccessToast,
|
||||
});
|
||||
const input = document.querySelector<HTMLInputElement>('input[type="file"]')!;
|
||||
uploadFile(input, new File(['x'], 'evil.zip', { type: 'application/zip' }));
|
||||
|
||||
// The component should accept these props without error
|
||||
expect(addDangerToast).toBeDefined();
|
||||
expect(addSuccessToast).toBeDefined();
|
||||
expect(addDangerToast).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/\.supx/i),
|
||||
);
|
||||
expect(mockPost).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('uploading a .supx file calls POST endpoint and refreshes list', async () => {
|
||||
mockPost.mockResolvedValue({});
|
||||
renderList();
|
||||
|
||||
const input = document.querySelector<HTMLInputElement>('input[type="file"]')!;
|
||||
uploadFile(input, new File(['PK'], 'my.supx', { type: 'application/octet-stream' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockPost).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ endpoint: '/api/v1/extensions/' }),
|
||||
);
|
||||
expect(mockRefreshData).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -246,7 +246,7 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({
|
||||
</Tooltip>
|
||||
{isChatbot && (
|
||||
<Tooltip
|
||||
id="set-default-chatbot-tooltip"
|
||||
id={`set-chatbot-tooltip-${id}`}
|
||||
title={
|
||||
isDefault
|
||||
? t('Remove default')
|
||||
@@ -257,8 +257,14 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({
|
||||
<span
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
data-test="set-default-chatbot"
|
||||
className="action-button"
|
||||
onClick={() => setDefaultChatbot(id)}
|
||||
onKeyDown={(e: React.KeyboardEvent) => {
|
||||
if (e.key === 'Enter' || e.key === ' ') {
|
||||
setDefaultChatbot(id);
|
||||
}
|
||||
}}
|
||||
>
|
||||
{isDefault ? (
|
||||
<Icons.StarFilled iconSize="l" />
|
||||
@@ -281,15 +287,21 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({
|
||||
>
|
||||
{(confirmDelete: () => void) => (
|
||||
<Tooltip
|
||||
id="delete-extension-tooltip"
|
||||
id={`delete-extension-tooltip-${id}`}
|
||||
title={t('Delete')}
|
||||
placement="bottom"
|
||||
>
|
||||
<span
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
data-test="delete-extension"
|
||||
className="action-button"
|
||||
onClick={confirmDelete}
|
||||
onKeyDown={(e: React.KeyboardEvent) => {
|
||||
if (e.key === 'Enter' || e.key === ' ') {
|
||||
confirmDelete();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Icons.DeleteOutlined iconSize="l" />
|
||||
</span>
|
||||
@@ -302,14 +314,7 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({
|
||||
},
|
||||
},
|
||||
],
|
||||
[
|
||||
loading,
|
||||
settings,
|
||||
chatbotIds,
|
||||
toggleEnabled,
|
||||
setDefaultChatbot,
|
||||
handleDelete,
|
||||
],
|
||||
[settings, chatbotIds, toggleEnabled, setDefaultChatbot, handleDelete],
|
||||
);
|
||||
|
||||
const menuData: SubMenuProps = {
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
import mimetypes
|
||||
import re
|
||||
from io import BytesIO
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
@@ -37,6 +38,19 @@ from superset.extensions.utils import (
|
||||
)
|
||||
from superset.utils.core import check_is_safe_zip
|
||||
|
||||
# Allowlist for publisher and name path parameters — alphanumeric, hyphens,
|
||||
# underscores only. Rejects path-traversal attempts (../), URL-encoded slashes,
|
||||
# and any other characters that could escape EXTENSIONS_PATH.
|
||||
_SEGMENT_RE = re.compile(r"^[A-Za-z0-9_-]+$")
|
||||
|
||||
# Default 10 MB server-side upload limit; can be overridden via config.
|
||||
_DEFAULT_MAX_UPLOAD_BYTES = 10 * 1024 * 1024
|
||||
|
||||
|
||||
def _validate_segment(value: str) -> bool:
|
||||
"""Return True if *value* is a safe publisher or name segment."""
|
||||
return bool(_SEGMENT_RE.match(value))
|
||||
|
||||
|
||||
class ExtensionsRestApi(BaseApi):
|
||||
allow_browser_login = True
|
||||
@@ -180,7 +194,8 @@ class ExtensionsRestApi(BaseApi):
|
||||
500:
|
||||
$ref: '#/components/responses/500'
|
||||
"""
|
||||
# Reconstruct composite ID from publisher and name
|
||||
if not _validate_segment(publisher) or not _validate_segment(name):
|
||||
return self.response(400, message="Invalid publisher or name.")
|
||||
composite_id = f"{publisher}.{name}"
|
||||
extensions = get_extensions()
|
||||
extension = extensions.get(composite_id)
|
||||
@@ -249,7 +264,19 @@ class ExtensionsRestApi(BaseApi):
|
||||
if not upload.filename or not upload.filename.endswith(".supx"):
|
||||
return self.response(400, message="File must have a .supx extension.")
|
||||
|
||||
stream = BytesIO(upload.read())
|
||||
max_bytes: int = current_app.config.get(
|
||||
"EXTENSIONS_MAX_UPLOAD_SIZE", _DEFAULT_MAX_UPLOAD_BYTES
|
||||
)
|
||||
raw = upload.read(max_bytes + 1)
|
||||
if len(raw) > max_bytes:
|
||||
return self.response(
|
||||
400,
|
||||
message=(
|
||||
f"File exceeds the maximum allowed size of {max_bytes} bytes."
|
||||
),
|
||||
)
|
||||
|
||||
stream = BytesIO(raw)
|
||||
if not is_zipfile(stream):
|
||||
return self.response(400, message="File is not a valid ZIP archive.")
|
||||
|
||||
@@ -262,7 +289,22 @@ class ExtensionsRestApi(BaseApi):
|
||||
except Exception as ex: # pylint: disable=broad-except
|
||||
return self.response(400, message=f"Invalid extension bundle: {ex}")
|
||||
|
||||
# Reject bundles whose manifest id collides with a LOCAL_EXTENSIONS entry.
|
||||
local_ids = {
|
||||
Path(p).name for p in current_app.config.get("LOCAL_EXTENSIONS", [])
|
||||
}
|
||||
if extension.manifest.id in local_ids:
|
||||
return self.response(
|
||||
409,
|
||||
message=(
|
||||
f"Extension '{extension.manifest.id}' is already installed as a "
|
||||
"local extension. Remove it from LOCAL_EXTENSIONS before uploading."
|
||||
),
|
||||
)
|
||||
|
||||
# Persist to EXTENSIONS_PATH so the extension survives restarts.
|
||||
# Destination path is derived from the validated manifest id, not the
|
||||
# uploaded filename, so the upload filename cannot escape EXTENSIONS_PATH.
|
||||
dest_dir = Path(extensions_path)
|
||||
dest_dir.mkdir(parents=True, exist_ok=True)
|
||||
dest_file = dest_dir / f"{extension.manifest.id}.supx"
|
||||
@@ -304,6 +346,9 @@ class ExtensionsRestApi(BaseApi):
|
||||
if not security_manager.is_admin():
|
||||
return self.response(403, message="Admin access required.")
|
||||
|
||||
if not _validate_segment(publisher) or not _validate_segment(name):
|
||||
return self.response(400, message="Invalid publisher or name.")
|
||||
|
||||
composite_id = f"{publisher}.{name}"
|
||||
extensions = get_extensions()
|
||||
extension = extensions.get(composite_id)
|
||||
@@ -429,7 +474,8 @@ class ExtensionsRestApi(BaseApi):
|
||||
500:
|
||||
$ref: '#/components/responses/500'
|
||||
"""
|
||||
# Reconstruct composite ID from publisher and name
|
||||
if not _validate_segment(publisher) or not _validate_segment(name):
|
||||
return self.response(400, message="Invalid publisher or name.")
|
||||
composite_id = f"{publisher}.{name}"
|
||||
extensions = get_extensions()
|
||||
extension = extensions.get(composite_id)
|
||||
|
||||
381
tests/unit_tests/extensions/test_api.py
Normal file
381
tests/unit_tests/extensions/test_api.py
Normal file
@@ -0,0 +1,381 @@
|
||||
# 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.
|
||||
|
||||
"""Unit tests for the extensions REST API (POST and DELETE endpoints)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from superset.extensions.api import _validate_segment
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _validate_segment helper
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_validate_segment_accepts_alphanumeric() -> None:
|
||||
assert _validate_segment("acme") is True
|
||||
assert _validate_segment("my-ext") is True
|
||||
assert _validate_segment("my_ext") is True
|
||||
assert _validate_segment("Ext123") is True
|
||||
|
||||
|
||||
def test_validate_segment_rejects_traversal() -> None:
|
||||
assert _validate_segment("..") is False
|
||||
assert _validate_segment("../etc") is False
|
||||
assert _validate_segment("acme/bad") is False
|
||||
assert _validate_segment("acme%2Fbad") is False
|
||||
assert _validate_segment("") is False
|
||||
|
||||
|
||||
def test_validate_segment_rejects_dots() -> None:
|
||||
assert _validate_segment("acme.corp") is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers for building fake .supx payloads
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_supx(manifest_id: str = "acme.chatbot") -> bytes:
|
||||
"""Return minimal valid .supx (zip) bytes with a manifest."""
|
||||
buf = io.BytesIO()
|
||||
manifest_json = (
|
||||
f'{{"id": "{manifest_id}", "name": "Chatbot", "version": "1.0.0",'
|
||||
f'"publisher": "acme", "description": "test"}}'
|
||||
)
|
||||
with zipfile.ZipFile(buf, "w") as zf:
|
||||
zf.writestr("manifest.json", manifest_json)
|
||||
return buf.getvalue()
|
||||
|
||||
|
||||
def _make_fake_extension(manifest_id: str = "acme.chatbot") -> MagicMock:
|
||||
ext = MagicMock()
|
||||
ext.manifest.id = manifest_id
|
||||
ext.source_base_path = "upload://"
|
||||
ext.frontend = {}
|
||||
ext.backend = {}
|
||||
ext.version = "1.0.0"
|
||||
ext.name = "Chatbot"
|
||||
return ext
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# POST /api/v1/extensions/ — upload and install
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPostEndpoint:
|
||||
def _post(self, client: Any, data: dict[str, Any], full_api_access: None) -> Any:
|
||||
return client.post(
|
||||
"/api/v1/extensions/",
|
||||
data=data,
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
|
||||
def test_non_admin_rejected(
|
||||
self, client: Any, full_api_access: None, mocker: Any
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=False
|
||||
)
|
||||
resp = client.post("/api/v1/extensions/", data={})
|
||||
assert resp.status_code == 403
|
||||
|
||||
def test_missing_extensions_path_returns_400(
|
||||
self, client: Any, full_api_access: None, mocker: Any
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict("flask.current_app.config", {"EXTENSIONS_PATH": None})
|
||||
resp = client.post("/api/v1/extensions/", data={})
|
||||
assert resp.status_code == 400
|
||||
assert "EXTENSIONS_PATH" in resp.json["message"]
|
||||
|
||||
def test_missing_bundle_field_returns_400(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)}
|
||||
)
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert "bundle" in resp.json["message"]
|
||||
|
||||
def test_wrong_extension_rejected(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)}
|
||||
)
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={"bundle": (io.BytesIO(b"data"), "evil.zip")},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert ".supx" in resp.json["message"]
|
||||
|
||||
def test_oversize_upload_rejected(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config",
|
||||
{"EXTENSIONS_PATH": str(tmp_path), "EXTENSIONS_MAX_UPLOAD_SIZE": 10},
|
||||
)
|
||||
big = io.BytesIO(b"x" * 20)
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={"bundle": (big, "big.supx")},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert "maximum" in resp.json["message"]
|
||||
|
||||
def test_not_a_zip_returns_400(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)}
|
||||
)
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={"bundle": (io.BytesIO(b"not a zip"), "ext.supx")},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert "ZIP" in resp.json["message"]
|
||||
|
||||
def test_zip_slip_rejected(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
"""check_is_safe_zip raises on path-traversal entries inside the zip."""
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)}
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.check_is_safe_zip",
|
||||
side_effect=Exception("zip-slip detected"),
|
||||
)
|
||||
supx = _make_supx()
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={"bundle": (io.BytesIO(supx), "ext.supx")},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert "zip-slip" in resp.json["message"]
|
||||
|
||||
def test_local_extensions_collision_returns_409(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config",
|
||||
{
|
||||
"EXTENSIONS_PATH": str(tmp_path),
|
||||
"LOCAL_EXTENSIONS": ["/opt/superset/ext/acme.chatbot"],
|
||||
},
|
||||
)
|
||||
fake_ext = _make_fake_extension("acme.chatbot")
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_bundle_files_from_zip", return_value=[]
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_loaded_extension", return_value=fake_ext
|
||||
)
|
||||
supx = _make_supx("acme.chatbot")
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={"bundle": (io.BytesIO(supx), "ext.supx")},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 409
|
||||
assert "local extension" in resp.json["message"]
|
||||
|
||||
def test_happy_path_returns_201(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config",
|
||||
{"EXTENSIONS_PATH": str(tmp_path), "LOCAL_EXTENSIONS": []},
|
||||
)
|
||||
fake_ext = _make_fake_extension("acme.chatbot")
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_bundle_files_from_zip", return_value=[]
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_loaded_extension", return_value=fake_ext
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.build_extension_data",
|
||||
return_value={"id": "acme.chatbot"},
|
||||
)
|
||||
supx = _make_supx("acme.chatbot")
|
||||
resp = client.post(
|
||||
"/api/v1/extensions/",
|
||||
data={"bundle": (io.BytesIO(supx), "ext.supx")},
|
||||
content_type="multipart/form-data",
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
assert resp.json["result"]["id"] == "acme.chatbot"
|
||||
assert (tmp_path / "acme.chatbot.supx").exists()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# DELETE /api/v1/extensions/<publisher>/<name>
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDeleteEndpoint:
|
||||
def test_non_admin_rejected(
|
||||
self, client: Any, full_api_access: None, mocker: Any
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=False
|
||||
)
|
||||
resp = client.delete("/api/v1/extensions/acme/chatbot")
|
||||
assert resp.status_code == 403
|
||||
|
||||
def test_path_traversal_publisher_rejected(
|
||||
self, client: Any, full_api_access: None, mocker: Any
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
# Use percent-encoded dots so Flask routing passes the segment to the
|
||||
# handler as the string ".." — literal slashes in the path would be
|
||||
# intercepted by the router before reaching the view.
|
||||
resp = client.delete("/api/v1/extensions/%2E%2E/passwd")
|
||||
assert resp.status_code == 400
|
||||
assert "Invalid" in resp.json["message"]
|
||||
|
||||
def test_invalid_name_returns_400(
|
||||
self, client: Any, full_api_access: None, mocker: Any
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
resp = client.delete("/api/v1/extensions/acme/bad.name")
|
||||
assert resp.status_code == 400
|
||||
assert "Invalid" in resp.json["message"]
|
||||
|
||||
def test_unknown_extension_returns_404(
|
||||
self, client: Any, full_api_access: None, mocker: Any
|
||||
) -> None:
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch("superset.extensions.api.get_extensions", return_value={})
|
||||
resp = client.delete("/api/v1/extensions/acme/chatbot")
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_local_extension_cannot_be_deleted(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
local_base = str(tmp_path / "local-ext" / "dist")
|
||||
fake_ext = _make_fake_extension("acme.chatbot")
|
||||
fake_ext.source_base_path = local_base
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_extensions",
|
||||
return_value={"acme.chatbot": fake_ext},
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config",
|
||||
{"LOCAL_EXTENSIONS": [str(tmp_path / "local-ext")]},
|
||||
)
|
||||
resp = client.delete("/api/v1/extensions/acme/chatbot")
|
||||
assert resp.status_code == 400
|
||||
assert "LOCAL_EXTENSIONS" in resp.json["message"]
|
||||
|
||||
def test_happy_path_deletes_file(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
supx_file = tmp_path / "acme.chatbot.supx"
|
||||
supx_file.write_bytes(b"fake")
|
||||
|
||||
fake_ext = _make_fake_extension("acme.chatbot")
|
||||
fake_ext.source_base_path = "upload://"
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_extensions",
|
||||
return_value={"acme.chatbot": fake_ext},
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config",
|
||||
{
|
||||
"LOCAL_EXTENSIONS": [],
|
||||
"EXTENSIONS_PATH": str(tmp_path),
|
||||
},
|
||||
)
|
||||
resp = client.delete("/api/v1/extensions/acme/chatbot")
|
||||
assert resp.status_code == 200
|
||||
assert not supx_file.exists()
|
||||
|
||||
def test_supx_file_missing_returns_404(
|
||||
self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path
|
||||
) -> None:
|
||||
fake_ext = _make_fake_extension("acme.chatbot")
|
||||
fake_ext.source_base_path = "upload://"
|
||||
mocker.patch(
|
||||
"superset.extensions.api.security_manager.is_admin", return_value=True
|
||||
)
|
||||
mocker.patch(
|
||||
"superset.extensions.api.get_extensions",
|
||||
return_value={"acme.chatbot": fake_ext},
|
||||
)
|
||||
mocker.patch.dict(
|
||||
"flask.current_app.config",
|
||||
{"LOCAL_EXTENSIONS": [], "EXTENSIONS_PATH": str(tmp_path)},
|
||||
)
|
||||
resp = client.delete("/api/v1/extensions/acme/chatbot")
|
||||
assert resp.status_code == 404
|
||||
Reference in New Issue
Block a user