codeant-ai-for-open-source[bot] commented on code in PR #40699:
URL: https://github.com/apache/superset/pull/40699#discussion_r3345968901
##########
superset/dashboards/api.py:
##########
@@ -2134,6 +2135,56 @@ def delete_embedded(self, dashboard: Dashboard) ->
Response:
DeleteEmbeddedDashboardCommand(dashboard).run()
return self.response(200, message="OK")
+ @expose("/<id_or_slug>/embedded/revoke", methods=("POST",))
+ @protect()
+ @safe
+ @permission_name("set_embedded")
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: (
+ f"{self.__class__.__name__}.revoke_embedded"
+ ),
+ log_to_statsd=False,
+ )
+ @with_dashboard
+ def revoke_embedded(self, dashboard: Dashboard) -> Response:
+ """Revoke outstanding guest tokens for the dashboard's embedded config.
+ ---
+ post:
+ summary: Revoke active guest sessions for the dashboard
+ description: >-
+ Rejects guest tokens for this dashboard that were issued before
now,
+ terminating outstanding embedded guest sessions without affecting
+ other dashboards.
+ parameters:
+ - in: path
+ schema:
+ type: string
+ name: id_or_slug
+ description: The dashboard id or slug
+ responses:
+ 200:
+ description: Successfully revoked active guest tokens
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ message:
+ type: string
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not dashboard.embedded:
+ return self.response_404()
Review Comment:
**Suggestion:** Returning 404 when a dashboard exists but has no embedded
config creates an API contract mismatch with related embedded-management
endpoints (for example, delete is a no-op 200 on missing config). This makes
revoke non-idempotent and causes clients that safely retry/cleanup to fail
unnecessarily; return 200 for the no-config case instead. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Revoke endpoint non-idempotent for dashboards without embedded config.
- ⚠️ Programmatic cleanup tools must special-case embedded revocation
failures.
- ⚠️ Inconsistent semantics with delete_embedded complicate embedded API
clients.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Locate the delete endpoint `delete_embedded` in
`superset/dashboards/api.py` (around
lines 2100–2137 in the current file), which always calls
`DeleteEmbeddedDashboardCommand(dashboard).run()` and then unconditionally
returns
`self.response(200, message="OK")` regardless of the embedded configuration
state.
2. Inspect `DeleteEmbeddedDashboardCommand` in
`superset/commands/dashboard/delete.py`
(lines 4–12), where `run()` calls
`EmbeddedDashboardDAO.delete(self._dashboard.embedded)`.
When `dashboard.embedded` is empty (no embedded config), this passes an
empty list to
`BaseDAO.delete`.
3. Inspect `BaseDAO.delete` in `superset/daos/base.py` (lines 498–518),
which simply
routes to `soft_delete(items)` or `hard_delete(items)` without
special-casing empty lists;
passing `items=[]` becomes a no-op and does not raise, so `delete_embedded`
returns HTTP
200 even when no embedded config exists (idempotent behavior).
4. Compare with the new revoke endpoint `revoke_embedded` in
`superset/dashboards/api.py`
(lines 39–87 in the 2100–2219 block), which contains `if not
dashboard.embedded: return
self.response_404()` (diff lines 2182–2183). For any existing dashboard
without an
embedded configuration (e.g. a newly created dashboard that behaves like
`world_health`
before configuration in
`tests/integration_tests/dashboards/api_tests.py:3112–3117`), a
`POST /api/v1/dashboard/<id_or_slug>/embedded/revoke` call will return 404
instead of 200,
unlike `DELETE /api/v1/dashboard/<id_or_slug>/embedded` which is a no-op 200
in the same
state. This creates an API contract mismatch and breaks idempotent client
flows that
expect revocation/cleanup endpoints to succeed with 200 even when there is
nothing to
revoke.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0daa334aa3444c5eb387e478fc98d9bc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0daa334aa3444c5eb387e478fc98d9bc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/dashboards/api.py
**Line:** 2182:2183
**Comment:**
*Api Mismatch: Returning 404 when a dashboard exists but has no
embedded config creates an API contract mismatch with related
embedded-management endpoints (for example, delete is a no-op 200 on missing
config). This makes revoke non-idempotent and causes clients that safely
retry/cleanup to fail unnecessarily; return 200 for the no-config case instead.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40699&comment_hash=cf92adf8c86e3c619ff59e820d108234eed83ac3b4e1bf90ad505540ab164d1e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40699&comment_hash=cf92adf8c86e3c619ff59e820d108234eed83ac3b4e1bf90ad505540ab164d1e&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]
