codeant-ai-for-open-source[bot] commented on code in PR #37973:
URL: https://github.com/apache/superset/pull/37973#discussion_r2806810158
##########
superset/mcp_service/auth.py:
##########
@@ -107,14 +116,28 @@ def get_user_from_request() -> User:
if hasattr(g, "user") and g.user:
return g.user
+ # Try API key authentication via FAB SecurityManager
+ api_key_enabled = current_app.config.get("FAB_API_KEY_ENABLED", False)
+ if api_key_enabled:
Review Comment:
**Suggestion:** The API key extraction unconditionally calls a helper that
likely relies on a Flask request context; when MCP tools run with only an
application context (as mcp_auth_hook does for internal/non-HTTP operations),
this will raise a RuntimeError ("working outside of request context") instead
of gracefully skipping API-key auth, breaking flows that previously worked when
API keys are enabled. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ MCP tools crash when FAB_API_KEY_ENABLED is enabled.
- ❌ FastMCP internal operations fail with RuntimeError, breaking tooling.
- ⚠️ Development flows without HTTP requests become unusable.
```
</details>
```suggestion
from flask import has_request_context
# Try API key authentication via FAB SecurityManager
api_key_enabled = current_app.config.get("FAB_API_KEY_ENABLED", False)
if api_key_enabled and has_request_context():
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable FAB API key auth by setting `FAB_API_KEY_ENABLED = True` in
`superset_config.py`, causing the API-key branch in `get_user_from_request()`
(`superset/mcp_service/auth.py:119-131`) to execute.
2. Start the standalone MCP server which wraps tools with `mcp_auth_hook()`
(defined in
`superset/mcp_service/auth.py`) so that each tool call enters
`_setup_user_context()` and
then `get_user_from_request()` inside a Flask *application* context only.
3. Trigger an MCP operation that is not handling an HTTP request (e.g.,
FastMCP tool
discovery or background invocation), so
`mcp_auth_hook._get_app_context_manager()` pushes
`app.app_context()` but no request context is created (no
`test_request_context()` or view
handling).
4. During this call, `get_user_from_request()` reaches the API-key block and
calls
`sm._extract_api_key_from_request()` at line 123, which accesses
`flask.request` and
raises `RuntimeError("Working outside of request context")`; this bubbles
out of
`_setup_user_context()` (which only treats "application context" errors as
expected) and
crashes the MCP tool execution instead of falling back to `MCP_DEV_USERNAME`
or returning
`None` for internal calls.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/auth.py
**Line:** 119:121
**Comment:**
*Logic Error: The API key extraction unconditionally calls a helper
that likely relies on a Flask request context; when MCP tools run with only an
application context (as mcp_auth_hook does for internal/non-HTTP operations),
this will raise a RuntimeError ("working outside of request context") instead
of gracefully skipping API-key auth, breaking flows that previously worked when
API keys are enabled.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37973&comment_hash=5b8a7c3eff24c00992cd61d553264a8dff522bf9d8cd4ab04c260485d0cee4d9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37973&comment_hash=5b8a7c3eff24c00992cd61d553264a8dff522bf9d8cd4ab04c260485d0cee4d9&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/apiKeys/ApiKeyCreateModal.tsx:
##########
@@ -0,0 +1,150 @@
+/**
+ * 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 { useState } from 'react';
+import { SupersetClient, t } from '@superset-ui/core';
+import { css, useTheme, Alert } from '@apache-superset/core/ui';
+import {
+ FormModal,
+ FormItem,
+ Input,
+ Button,
+} 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<string | null>(null);
+ const [copied, setCopied] = useState(false);
+
+ const handleFormSubmit = async (values: FormValues) => {
+ try {
+ const response = await SupersetClient.post({
+ endpoint: '/api/v1/security/api_keys/',
+ jsonPayload: values,
+ });
+ setCreatedKey(response.json.result.key);
+ addSuccessToast(t('API key created successfully'));
+ } catch (error) {
+ addDangerToast(t('Failed to create API key'));
+ }
+ };
+
+ const handleCopyKey = () => {
+ if (createdKey) {
+ navigator.clipboard.writeText(createdKey);
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
Review Comment:
**Suggestion:** The clipboard write operation is asynchronous but is neither
awaited nor error-handled, so if `navigator.clipboard.writeText` fails (e.g.,
due to denied permissions or unsupported environment) it will reject with an
unhandled promise while the UI still shows a successful "Copied!" state,
misleading the user and potentially causing runtime errors. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Clipboard copy failures silently ignored for API key modal.
- ⚠️ Users see "Copied!" despite clipboard write failure.
- ⚠️ Unhandled promise rejections clutter browser error logs.
```
</details>
```suggestion
const handleCopyKey = async () => {
if (!createdKey) {
return;
}
try {
await navigator.clipboard.writeText(createdKey);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch {
addDangerToast(t('Failed to copy API key to clipboard'));
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Log into Superset and open the API key management UI that renders
`ApiKeyCreateModal`
from `superset-frontend/src/features/apiKeys/ApiKeyCreateModal.tsx:33-150`.
2. Create a new API key via the modal so that `handleFormSubmit` at lines
50-58 sets
`createdKey` to a non-null value and the component re-renders into the "API
Key Created"
branch at lines 80-127.
3. In that success modal, click the "Copy" button wired to `handleCopyKey`
at lines 63-69,
in a browser/environment where `navigator.clipboard.writeText` rejects (for
example,
clipboard permission denied or clipboard API unsupported for the current
context).
4. Observe in browser dev tools that the Promise returned by
`navigator.clipboard.writeText` at line 65 is rejected without any
`await`/`catch`,
causing an unhandled promise rejection, while the UI still switches the
button label to
"Copied!" via `setCopied(true)` at line 66 even though the key was not
copied.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/apiKeys/ApiKeyCreateModal.tsx
**Line:** 63:67
**Comment:**
*Possible Bug: The clipboard write operation is asynchronous but is
neither awaited nor error-handled, so if `navigator.clipboard.writeText` fails
(e.g., due to denied permissions or unsupported environment) it will reject
with an unhandled promise while the UI still shows a successful "Copied!"
state, misleading the user and potentially causing runtime errors.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37973&comment_hash=c57258fd5dd40ae97540c5d12b55dac8fe82c5a1636c7ef1eafae699d46093c2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37973&comment_hash=c57258fd5dd40ae97540c5d12b55dac8fe82c5a1636c7ef1eafae699d46093c2&reaction=dislike'>👎</a>
--
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]
