diff --git a/docs/static/feature-flags.json b/docs/static/feature-flags.json index 5d7a86994f11..516115ac0aeb 100644 --- a/docs/static/feature-flags.json +++ b/docs/static/feature-flags.json @@ -51,6 +51,12 @@ "lifecycle": "development", "description": "Enable Superset extensions for custom functionality without modifying core" }, + { + "name": "FAB_API_KEY_ENABLED", + "default": false, + "lifecycle": "development", + "description": "Enable API key authentication via FAB SecurityManager When enabled, users can create/manage API keys in the User Info page" + }, { "name": "GRANULAR_EXPORT_CONTROLS", "default": false, diff --git a/requirements/base.txt b/requirements/base.txt index 240cf7d4eb14..14f5d8a6106e 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -120,7 +120,7 @@ flask==2.3.3 # flask-session # flask-sqlalchemy # flask-wtf -flask-appbuilder==5.1.0 +flask-appbuilder==5.2.0 # via # apache-superset (pyproject.toml) # apache-superset-core diff --git a/requirements/development.txt b/requirements/development.txt index 2b9fffd474b0..50f5404f9c75 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -259,7 +259,7 @@ flask==2.3.3 # flask-sqlalchemy # flask-testing # flask-wtf -flask-appbuilder==5.1.0 +flask-appbuilder==5.2.0 # via # -c requirements/base-constraint.txt # apache-superset diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 18bc85af00c3..8f21d83738b6 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -53,6 +53,7 @@ export enum FeatureFlag { EnableTemplateProcessing = 'ENABLE_TEMPLATE_PROCESSING', EscapeMarkdownHtml = 'ESCAPE_MARKDOWN_HTML', EstimateQueryCost = 'ESTIMATE_QUERY_COST', + FabApiKeyEnabled = 'FAB_API_KEY_ENABLED', FilterBarClosedByDefault = 'FILTERBAR_CLOSED_BY_DEFAULT', GlobalAsyncQueries = 'GLOBAL_ASYNC_QUERIES', GlobalTaskFramework = 'GLOBAL_TASK_FRAMEWORK', diff --git a/superset-frontend/src/features/apiKeys/ApiKeyCreateModal.tsx b/superset-frontend/src/features/apiKeys/ApiKeyCreateModal.tsx new file mode 100644 index 000000000000..cc2cf7d4ac8e --- /dev/null +++ b/superset-frontend/src/features/apiKeys/ApiKeyCreateModal.tsx @@ -0,0 +1,175 @@ +/** + * 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 { useEffect, useRef, useState } from 'react'; +import { SupersetClient } from '@superset-ui/core'; +import { t } from '@apache-superset/core/translation'; +import { css, useTheme } from '@apache-superset/core/theme'; +import { Alert } from '@apache-superset/core/components'; +import { + FormModal, + FormItem, + Input, + Button, + Modal, +} from '@superset-ui/core/components'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; + +interface ApiKeyCreateModalProps { + show: boolean; + onHide: () => void; + onSuccess: () => void; +} + +interface FormValues { + name: string; +} + +export function ApiKeyCreateModal({ + show, + onHide, + onSuccess, +}: ApiKeyCreateModalProps) { + const theme = useTheme(); + const { addDangerToast, addSuccessToast } = useToasts(); + const [createdKey, setCreatedKey] = useState(null); + const [copied, setCopied] = useState(false); + const copyTimerRef = useRef | null>(null); + + useEffect( + () => () => { + if (copyTimerRef.current) { + clearTimeout(copyTimerRef.current); + } + }, + [], + ); + + const handleFormSubmit = async (values: FormValues) => { + try { + const response = await SupersetClient.post({ + endpoint: '/api/v1/security/api_keys/', + jsonPayload: values, + }); + const key = response.json?.result?.key; + if (!key) { + throw new Error('API response did not include a key'); + } + setCreatedKey(key); + addSuccessToast(t('API key created successfully')); + } catch (error) { + addDangerToast(t('Failed to create API key')); + throw error; + } + }; + + const handleCopyKey = async () => { + if (!createdKey) { + return; + } + try { + await navigator.clipboard.writeText(createdKey); + setCopied(true); + if (copyTimerRef.current) { + clearTimeout(copyTimerRef.current); + } + copyTimerRef.current = setTimeout(() => setCopied(false), 2000); + } catch { + addDangerToast(t('Failed to copy API key to clipboard')); + } + }; + + const handleClose = () => { + if (createdKey) { + onSuccess(); + } + setCreatedKey(null); + setCopied(false); + onHide(); + }; + + if (createdKey) { + return ( + + {t('Done')} + + } + > + +
+ + +
+ + ); + } + + return ( + {}} + formSubmitHandler={handleFormSubmit} + requiredFields={['name']} + > + + + + + ); +} diff --git a/superset-frontend/src/features/apiKeys/ApiKeyList.tsx b/superset-frontend/src/features/apiKeys/ApiKeyList.tsx new file mode 100644 index 000000000000..9b12962eb0a4 --- /dev/null +++ b/superset-frontend/src/features/apiKeys/ApiKeyList.tsx @@ -0,0 +1,236 @@ +/** + * 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 { useEffect, useRef, useState } from 'react'; +import { SupersetClient } from '@superset-ui/core'; +import { t } from '@apache-superset/core/translation'; +import { css, useTheme } from '@apache-superset/core/theme'; +import { + Button, + Table, + Modal, + Tag, + Tooltip, +} from '@superset-ui/core/components'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import { ApiKeyCreateModal } from './ApiKeyCreateModal'; + +export interface ApiKey { + uuid: string; + name: string; + key_prefix: string; + active: boolean; + created_on: string; + expires_on: string | null; + revoked_on: string | null; + last_used_on: string | null; + scopes: string | null; +} + +export function ApiKeyList() { + const theme = useTheme(); + const { addDangerToast, addSuccessToast } = useToasts(); + const [apiKeys, setApiKeys] = useState([]); + const [loading, setLoading] = useState(false); + const [showCreateModal, setShowCreateModal] = useState(false); + const fetchCounterRef = useRef(0); + + async function fetchApiKeys() { + fetchCounterRef.current += 1; + const thisRequest = fetchCounterRef.current; + setLoading(true); + try { + const response = await SupersetClient.get({ + endpoint: '/api/v1/security/api_keys/', + }); + // Only apply results if this is still the most recent request + if (thisRequest === fetchCounterRef.current) { + setApiKeys(response.json.result || []); + } + } catch (error) { + if (thisRequest === fetchCounterRef.current) { + addDangerToast(t('Failed to fetch API keys')); + } + } finally { + if (thisRequest === fetchCounterRef.current) { + setLoading(false); + } + } + } + + useEffect(() => { + fetchApiKeys(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + function handleRevokeKey(keyUuid: string) { + Modal.confirm({ + title: t('Revoke API Key'), + content: t( + 'Are you sure you want to revoke this API key? This action cannot be undone.', + ), + okText: t('Revoke'), + okType: 'danger', + cancelText: t('Cancel'), + onOk: async () => { + try { + await SupersetClient.delete({ + endpoint: `/api/v1/security/api_keys/${keyUuid}`, + }); + addSuccessToast(t('API key revoked successfully')); + fetchApiKeys(); + } catch (error) { + addDangerToast(t('Failed to revoke API key')); + } + }, + }); + } + + const formatDate = (dateString: string | null) => { + if (!dateString) return '-'; + return new Date(dateString).toLocaleDateString(undefined, { + year: 'numeric', + month: 'short', + day: 'numeric', + }); + }; + + const getStatusBadge = (key: ApiKey) => { + if (key.revoked_on) { + return {t('Revoked')}; + } + if (key.expires_on && new Date(key.expires_on) < new Date()) { + return {t('Expired')}; + } + if (!key.active) { + return {t('Inactive')}; + } + return {t('Active')}; + }; + + const columns = [ + { + title: t('Name'), + dataIndex: 'name', + key: 'name', + }, + { + title: t('Key Prefix'), + dataIndex: 'key_prefix', + key: 'key_prefix', + render: (prefix: string) => ( + + {prefix}... + + ), + }, + { + title: t('Created'), + dataIndex: 'created_on', + key: 'created_on', + render: formatDate, + }, + { + title: t('Last Used'), + dataIndex: 'last_used_on', + key: 'last_used_on', + render: formatDate, + }, + { + title: t('Status'), + key: 'status', + render: (_: unknown, record: ApiKey) => getStatusBadge(record), + }, + { + title: t('Actions'), + key: 'actions', + render: (_: unknown, record: ApiKey) => ( + <> + {!record.revoked_on && record.active && ( + + + + )} + + ), + }, + ]; + + return ( +
+
+
+

+ {t('API keys allow scoped programmatic access to Superset.')} +

+

+ {t('Keys are shown only once at creation. Store them securely.')} +

+
+ +
+ + {showCreateModal && ( + { + setShowCreateModal(false); + }} + onSuccess={() => { + fetchApiKeys(); + }} + /> + )} + + ); +} diff --git a/superset-frontend/src/pages/UserInfo/index.tsx b/superset-frontend/src/pages/UserInfo/index.tsx index 92a54ad0e7de..a5d70f6ff868 100644 --- a/superset-frontend/src/pages/UserInfo/index.tsx +++ b/superset-frontend/src/pages/UserInfo/index.tsx @@ -19,7 +19,11 @@ import { useCallback, useEffect, useState } from 'react'; import { t } from '@apache-superset/core/translation'; -import { SupersetClient } from '@superset-ui/core'; +import { + SupersetClient, + FeatureFlag, + isFeatureEnabled, +} from '@superset-ui/core'; import { css, useTheme, styled } from '@apache-superset/core/theme'; import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu'; import { useToasts } from 'src/components/MessageToasts/withToasts'; @@ -30,6 +34,7 @@ import { UserInfoResetPasswordModal, } from 'src/features/userInfo/UserInfoModal'; import { Icons, Collapse } from '@superset-ui/core/components'; +import { ApiKeyList } from 'src/features/apiKeys/ApiKeyList'; const StyledHeader = styled.div` ${({ theme }) => css` @@ -159,7 +164,16 @@ export function UserInfo({ user }: { user: UserWithPermissionsAndRoles }) { {t('Your user information')} - + {t('User info')}} key="userInfo" @@ -205,6 +219,14 @@ export function UserInfo({ user }: { user: UserWithPermissionsAndRoles }) { + {isFeatureEnabled(FeatureFlag.FabApiKeyEnabled) && ( + {t('API Keys')}} + key="apiKeys" + > + + + )} {modalState.resetPassword && ( diff --git a/superset/config.py b/superset/config.py index 4e79aff9395e..0d9c2ed2e67b 100644 --- a/superset/config.py +++ b/superset/config.py @@ -563,6 +563,10 @@ class D3TimeFormat(TypedDict, total=False): # in addition to relative timeshifts (e.g., "1 day ago") # @lifecycle: development "DATE_RANGE_TIMESHIFTS_ENABLED": False, + # Enable API key authentication via FAB SecurityManager + # When enabled, users can create/manage API keys in the User Info page + # @lifecycle: development + "FAB_API_KEY_ENABLED": False, # Enable granular export controls (can_export_data, can_export_image, # can_copy_clipboard) instead of the single can_csv permission # @lifecycle: development @@ -1638,6 +1642,13 @@ def allowed_schemas_for_csv_upload( # pylint: disable=unused-argument FAB_ADD_SECURITY_VIEW_MENU_VIEW = False FAB_ADD_SECURITY_PERMISSION_VIEWS_VIEW = False +# API Key Authentication via FAB SecurityManager +# FAB reads this config directly to register the ApiKeyApi blueprint. +# The FAB_API_KEY_ENABLED feature flag (in DEFAULT_FEATURE_FLAGS) controls +# the frontend UI visibility independently. +FAB_API_KEY_ENABLED = False +FAB_API_KEY_PREFIXES = ["sst_"] + # The link to a page containing common errors and their resolutions # It will be appended at the bottom of sql_lab errors. TROUBLESHOOTING_LINK = "" diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py index 41484d444cf6..3579d2e8a557 100644 --- a/superset/mcp_service/auth.py +++ b/superset/mcp_service/auth.py @@ -19,20 +19,36 @@ Authentication and authorization hooks for MCP tools. This module provides: -- User authentication from JWT or configured dev user +- User authentication from JWT, API key, or configured dev user - RBAC permission checking aligned with Superset's REST API permissions - Dataset access validation - Session lifecycle management The RBAC enforcement mirrors Flask-AppBuilder's @protect() decorator, ensuring MCP tools respect the same permission model as the REST API. + +Supports multiple authentication methods: +1. API Key authentication via FAB SecurityManager (configurable prefix) +2. JWT token authentication (via FastMCP BearerAuthProvider) +3. Development mode (MCP_DEV_USERNAME configuration) + +API Key Authentication: +- Users create API keys via FAB's /api/v1/security/api_keys/ endpoints +- Keys use configurable prefixes (FAB_API_KEY_PREFIXES, default: ["sst_"]) +- Keys are validated by FAB's SecurityManager.validate_api_key() +- Keys inherit the user's roles and permissions via FAB's RBAC + +Configuration: +- FAB_API_KEY_ENABLED: Flask config key to enable API key auth (default: False) +- FAB_API_KEY_PREFIXES: Key prefixes (default: ["sst_"]) +- MCP_DEV_USERNAME: Fallback username for development """ import logging from contextlib import AbstractContextManager from typing import Any, Callable, TYPE_CHECKING, TypeVar -from flask import g +from flask import g, has_request_context from flask_appbuilder.security.sqla.models import Group, User if TYPE_CHECKING: @@ -177,8 +193,9 @@ def get_user_from_request() -> User: Get the current user for the MCP tool request. Priority order: - 1. g.user if already set (by Preset workspace middleware) - 2. MCP_DEV_USERNAME from configuration (for development/testing) + 1. g.user if already set (by Preset workspace middleware or FastMCP auth) + 2. API key from Authorization header (via FAB SecurityManager) + 3. MCP_DEV_USERNAME from configuration (for development/testing) Returns: User object with roles and groups eagerly loaded @@ -192,6 +209,55 @@ def get_user_from_request() -> User: if hasattr(g, "user") and g.user: return g.user + # Try API key authentication via FAB SecurityManager + # Only attempt when in a request context (not for MCP internal operations + # like tool discovery that run with only an application context) + # Use the Flask config key FAB_API_KEY_ENABLED (not the feature flag), + # because the config key controls whether FAB registers the API key + # endpoints and validation logic. The feature flag with the same name + # in DEFAULT_FEATURE_FLAGS only controls the frontend UI visibility. + if current_app.config.get("FAB_API_KEY_ENABLED", False) and has_request_context(): + sm = current_app.appbuilder.sm + # _extract_api_key_from_request is FAB's internal method for reading + # the Bearer token from the Authorization header and matching prefixes. + # Not all FAB versions include this method, so guard with hasattr. + if not hasattr(sm, "_extract_api_key_from_request"): + logger.debug( + "FAB SecurityManager does not have _extract_api_key_from_request; " + "API key authentication is not available in this FAB version" + ) + else: + api_key_string = sm._extract_api_key_from_request() + if api_key_string is not None: + if not hasattr(sm, "validate_api_key"): + logger.warning( + "FAB SecurityManager does not have validate_api_key; " + "cannot validate API key" + ) + raise PermissionError( + "API key validation is not available in this FAB version." + ) + user = sm.validate_api_key(api_key_string) + if user: + # Reload user with all relationships eagerly loaded to avoid + # detached-instance errors during later permission checks. + user_with_rels = load_user_with_relationships( + username=user.username, + ) + if user_with_rels is None: + logger.warning( + "Failed to reload API key user %s with relationships; " + "using original user object which may have lazy-loaded " + "relationships", + user.username, + ) + return user + return user_with_rels + raise PermissionError( + "Invalid or expired API key. " + "Create a new key at /api/v1/security/api_keys/." + ) + # Fall back to configured username for development/single-user deployments username = current_app.config.get("MCP_DEV_USERNAME") @@ -209,11 +275,13 @@ def get_user_from_request() -> User: f"JWT keys configured={jwt_configured})" ) details.append("MCP_DEV_USERNAME is not configured") + configured_prefixes = current_app.config.get("FAB_API_KEY_PREFIXES", ["sst_"]) + prefix_example = configured_prefixes[0] if configured_prefixes else "sst_" raise ValueError( "No authenticated user found. Tried:\n" + "\n".join(f" - {d}" for d in details) - + "\n\nEither pass a valid JWT bearer token or configure " - "MCP_DEV_USERNAME for development." + + f"\n\nEither pass a valid API key (Bearer {prefix_example}...), " + "JWT token, or configure MCP_DEV_USERNAME for development." ) # Use helper function to load user with all required relationships diff --git a/tests/unit_tests/mcp_service/test_auth_api_key.py b/tests/unit_tests/mcp_service/test_auth_api_key.py new file mode 100644 index 000000000000..131bdb57f081 --- /dev/null +++ b/tests/unit_tests/mcp_service/test_auth_api_key.py @@ -0,0 +1,227 @@ +# 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. + +"""Tests for API key authentication in get_user_from_request().""" + +from unittest.mock import MagicMock, patch + +import pytest +from flask import g + +from superset.mcp_service.auth import get_user_from_request + + +@pytest.fixture +def mock_user(): + user = MagicMock() + user.username = "api_key_user" + return user + + +@pytest.fixture +def _enable_api_keys(app): + """Enable FAB API key auth and clear MCP_DEV_USERNAME so the API key + path is exercised instead of falling through to the dev-user fallback.""" + app.config["FAB_API_KEY_ENABLED"] = True + old_dev = app.config.pop("MCP_DEV_USERNAME", None) + yield + app.config.pop("FAB_API_KEY_ENABLED", None) + if old_dev is not None: + app.config["MCP_DEV_USERNAME"] = old_dev + + +@pytest.fixture +def _disable_api_keys(app): + app.config["FAB_API_KEY_ENABLED"] = False + old_dev = app.config.pop("MCP_DEV_USERNAME", None) + yield + app.config.pop("FAB_API_KEY_ENABLED", None) + if old_dev is not None: + app.config["MCP_DEV_USERNAME"] = old_dev + + +# -- Valid API key -> user loaded -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_valid_api_key_returns_user(app, mock_user) -> None: + """A valid API key should authenticate and return the user.""" + mock_sm = MagicMock() + mock_sm._extract_api_key_from_request.return_value = "sst_abc123" + mock_sm.validate_api_key.return_value = mock_user + + with app.test_request_context(headers={"Authorization": "Bearer sst_abc123"}): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + with patch( + "superset.mcp_service.auth.load_user_with_relationships", + return_value=mock_user, + ): + result = get_user_from_request() + + assert result.username == "api_key_user" + mock_sm.validate_api_key.assert_called_once_with("sst_abc123") + + +# -- Invalid API key -> PermissionError -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_invalid_api_key_raises(app) -> None: + """An invalid API key should raise PermissionError.""" + mock_sm = MagicMock() + mock_sm._extract_api_key_from_request.return_value = "sst_bad_key" + mock_sm.validate_api_key.return_value = None + + with app.test_request_context(headers={"Authorization": "Bearer sst_bad_key"}): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + with pytest.raises(PermissionError, match="Invalid or expired API key"): + get_user_from_request() + + +# -- API key disabled -> falls through to next auth method -- + + +@pytest.mark.usefixtures("_disable_api_keys") +def test_api_key_disabled_skips_auth(app) -> None: + """When FAB_API_KEY_ENABLED is False, API key auth is skipped entirely.""" + mock_sm = MagicMock() + + with app.test_request_context(headers={"Authorization": "Bearer sst_abc123"}): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + # Without API key auth or MCP_DEV_USERNAME, should raise ValueError + # about no authenticated user (not about invalid API key) + with pytest.raises(ValueError, match="No authenticated user found"): + get_user_from_request() + + # SecurityManager API key methods should never be called + mock_sm._extract_api_key_from_request.assert_not_called() + + +# -- No request context -> API key auth skipped -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_no_request_context_skips_api_key_auth(app) -> None: + """Without a request context, API key auth should be skipped + (e.g., during MCP tool discovery with only an app context).""" + mock_sm = MagicMock() + + with app.app_context(): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + # Explicitly mock has_request_context to False because the test + # framework's app fixture may implicitly provide a request context. + with patch("superset.mcp_service.auth.has_request_context", return_value=False): + with pytest.raises(ValueError, match="No authenticated user found"): + get_user_from_request() + + mock_sm._extract_api_key_from_request.assert_not_called() + + +# -- g.user already set -> API key auth skipped (JWT precedence) -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_existing_g_user_takes_precedence(app, mock_user) -> None: + """If g.user is already set (e.g., by JWT middleware), API key auth + should not be attempted.""" + mock_sm = MagicMock() + + with app.test_request_context(headers={"Authorization": "Bearer sst_abc123"}): + g.user = mock_user + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + result = get_user_from_request() + + assert result.username == "api_key_user" + mock_sm._extract_api_key_from_request.assert_not_called() + + +# -- FAB version without _extract_api_key_from_request -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_fab_without_extract_method_skips_gracefully(app) -> None: + """If FAB SecurityManager lacks _extract_api_key_from_request, + API key auth should be skipped with a debug log, not crash.""" + mock_sm = MagicMock(spec=[]) # empty spec = no attributes + + with app.test_request_context(): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + with pytest.raises(ValueError, match="No authenticated user found"): + get_user_from_request() + + +# -- FAB version without validate_api_key -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_fab_without_validate_method_raises(app) -> None: + """If FAB has _extract_api_key_from_request but not validate_api_key, + should raise PermissionError about unavailable validation.""" + mock_sm = MagicMock(spec=["_extract_api_key_from_request"]) + mock_sm._extract_api_key_from_request.return_value = "sst_abc123" + + with app.test_request_context(headers={"Authorization": "Bearer sst_abc123"}): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + with pytest.raises( + PermissionError, match="API key validation is not available" + ): + get_user_from_request() + + +# -- Relationship reload fallback -- + + +@pytest.mark.usefixtures("_enable_api_keys") +def test_relationship_reload_failure_returns_original_user(app, mock_user) -> None: + """If load_user_with_relationships fails, the original user from + validate_api_key should be returned as fallback.""" + mock_sm = MagicMock() + mock_sm._extract_api_key_from_request.return_value = "sst_abc123" + mock_sm.validate_api_key.return_value = mock_user + + with app.test_request_context(headers={"Authorization": "Bearer sst_abc123"}): + g.user = None + app.appbuilder = MagicMock() + app.appbuilder.sm = mock_sm + + with patch( + "superset.mcp_service.auth.load_user_with_relationships", + return_value=None, + ): + result = get_user_from_request() + + assert result is mock_user diff --git a/tests/unit_tests/security/api_test.py b/tests/unit_tests/security/api_test.py index 869b33084782..9e0a25131c38 100644 --- a/tests/unit_tests/security/api_test.py +++ b/tests/unit_tests/security/api_test.py @@ -14,6 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import Any + import pytest from superset.extensions import csrf @@ -24,9 +26,10 @@ [{"WTF_CSRF_ENABLED": True}], indirect=True, ) -def test_csrf_not_exempt(app_context: None) -> None: +def test_csrf_exempt_blueprints(app_context: None) -> None: """ - Test that REST API is not exempt from CSRF. + Test that only FAB security API blueprints (which use token-based auth) + are exempt from CSRF protection. """ assert {blueprint.name for blueprint in csrf._exempt_blueprints} == { "GroupApi", @@ -39,3 +42,21 @@ def test_csrf_not_exempt(app_context: None) -> None: "PermissionApi", "ViewMenuApi", } + + +@pytest.mark.parametrize( + "app", + [ + { + "WTF_CSRF_ENABLED": True, + "FAB_API_KEY_ENABLED": True, + } + ], + indirect=True, +) +def test_csrf_exempt_blueprints_with_api_key(app: Any, app_context: None) -> None: + """ + Test that ApiKeyApi blueprint is CSRF-exempt when FAB_API_KEY_ENABLED + config is enabled. + """ + assert "ApiKeyApi" in {blueprint.name for blueprint in csrf._exempt_blueprints}