aminghadersohi commented on code in PR #37973:
URL: https://github.com/apache/superset/pull/37973#discussion_r2911770428
##########
superset/mcp_service/auth.py:
##########
@@ -107,6 +116,33 @@ 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)
+ # Avoid circular import: superset/__init__.py imports create_app which
+ # depends on the MCP service module tree during app initialization.
+ from superset import is_feature_enabled
+
+ if is_feature_enabled("FAB_API_KEY_ENABLED") 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.
+ # No public API is exposed for this; see FAB SecurityManager.
+ api_key_string = sm._extract_api_key_from_request()
+ if api_key_string is not None:
+ 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,
+ )
+ return user_with_rels or user
Review Comment:
Good catch — added explicit warning logging when
`load_user_with_relationships` fails after API key validation, instead of
silently falling back. The fallback still returns the original user object for
resilience, but the warning makes it visible in logs so lazy-load issues can be
diagnosed.
##########
superset-frontend/src/features/apiKeys/ApiKeyList.tsx:
##########
@@ -0,0 +1,224 @@
+/**
+ * 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, 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<ApiKey[]>([]);
+ const [loading, setLoading] = useState(false);
+ const [showCreateModal, setShowCreateModal] = useState(false);
+
+ async function fetchApiKeys() {
+ setLoading(true);
+ try {
+ const response = await SupersetClient.get({
+ endpoint: '/api/v1/security/api_keys/',
+ });
+ setApiKeys(response.json.result || []);
+ } catch (error) {
+ addDangerToast(t('Failed to fetch API keys'));
+ } finally {
+ 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 <Tag color="error">{t('Revoked')}</Tag>;
+ }
+ if (key.expires_on && new Date(key.expires_on) < new Date()) {
+ return <Tag color="warning">{t('Expired')}</Tag>;
+ }
+ return <Tag color="success">{t('Active')}</Tag>;
+ };
+
+ const columns = [
+ {
+ title: t('Name'),
+ dataIndex: 'name',
+ key: 'name',
+ },
+ {
+ title: t('Key Prefix'),
+ dataIndex: 'key_prefix',
+ key: 'key_prefix',
+ render: (prefix: string) => (
+ <code
+ css={css`
+ background: ${theme.colorFillSecondary};
+ padding: 2px 6px;
+ border-radius: 3px;
+ `}
+ >
+ {prefix}...
+ </code>
+ ),
+ },
+ {
+ 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 && (
+ <Tooltip title={t('Revoke this API key')}>
+ <Button
+ type="link"
+ danger
+ onClick={() => handleRevokeKey(record.uuid)}
+ >
+ {t('Revoke')}
+ </Button>
+ </Tooltip>
+ )}
+ </>
+ ),
+ },
+ ];
+
+ return (
+ <div>
+ <div
+ css={css`
+ display: flex;
+ justify-content: space-between;
+ align-items: center;
+ margin-bottom: ${theme.sizeUnit * 4}px;
+ `}
+ >
+ <div>
+ <p
+ css={css`
+ margin-bottom: ${theme.sizeUnit * 2}px;
+ `}
+ >
+ {t('API keys allow scoped programmatic access to Superset.')}
+ </p>
+ <p
+ css={css`
+ margin-bottom: 0;
+ `}
+ >
+ {t('Keys are shown only once at creation. Store them securely.')}
+ </p>
+ </div>
+ <Button type="primary" onClick={() => setShowCreateModal(true)}>
+ {t('Create API Key')}
+ </Button>
+ </div>
+ <Table
+ columns={columns}
+ data={apiKeys}
+ loading={loading}
+ rowKey="uuid"
+ pagination={{ pageSize: 10 }}
+ />
Review Comment:
This is correct as-is — Superset's `Table` component (from
`@superset-ui/core/components`) wraps antd's Table with a custom `data` prop
(see
`superset-frontend/packages/superset-ui-core/src/components/Table/index.tsx:64`).
It's not using antd's `dataSource` directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
