codeant-ai-for-open-source[bot] commented on code in PR #41305:
URL: https://github.com/apache/superset/pull/41305#discussion_r3456359446
##########
superset/security/api.py:
##########
@@ -204,6 +208,15 @@ def guest_token(self) -> Response:
body["rls"],
**({"datasets": body["datasets"]} if "datasets" in body else
{}),
)
+ logger.info(
+ "Guest token issued: %s",
+ build_guest_token_audit_payload(
+ issuer_user_id=get_user_id(),
+ source_ip=request.remote_addr,
+ body=body,
+ token=token,
+ ),
+ )
Review Comment:
**Suggestion:** This logs issuance metadata with the module logger instead
of the configured `event_logger`, so the data will not be written through
Superset's audit-event pipeline (for example DBEventLogger) and can be silently
lost when INFO logging is disabled. Attach this payload to the existing event
logging flow (`log_this_with_extra_payload` / event logger API) so it is
captured as a real audit event, not just a best-effort app log line.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Guest token audits lack issuer/resources/RLS metadata in DB.
- ❌ Incident investigations rely on possibly-disabled INFO application logs.
- ⚠️ Multi-tenant compliance auditing weakened for embedded dashboards.
```
</details>
<details>
<summary><b>Steps of Reproduction âś… </b></summary>
```mdx
1. Note that the guest-token issuance endpoint is implemented in
`superset/security/api.py:34-41` as `SecurityRestApi.guest_token`, decorated
with
`@event_logger.log_this` to send audit events through Superset's
`event_logger` pipeline.
2. Inside `guest_token`, observe at `superset/security/api.py:205-210` that
a JWT is
minted via `self.appbuilder.sm.create_guest_access_token(...)` and then, at
lines 211-219,
`logger.info("Guest token issued: %s",
build_guest_token_audit_payload(...))` is called to
log issuance metadata.
3. Inspect `superset/utils/log.py:38-67` where `event_logger.log_this` is
implemented: it
wraps the function in `log_context` and logs via
`self.log_with_context(...)`, but does
not read or incorporate messages emitted through the standard
`logging.getLogger(__name__)` logger used in `security/api.py`.
4. Configure Superset with the default `DBEventLogger` (see
`superset/utils/log.py:69-72`)
and run a POST request to `/api/v1/security/guest_token` (which routes to
`SecurityRestApi.guest_token`) to issue a guest token; observe that the DB
audit event
created by `event_logger.log_this` does not contain the rich payload from
`build_guest_token_audit_payload`, and if the module logger's level is
WARNING or higher,
the `logger.info` line—and therefore the issuance metadata—will not be
recorded anywhere.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=13e7c75225ec4cacbb0caaacab693bae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=13e7c75225ec4cacbb0caaacab693bae&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/security/api.py
**Line:** 211:219
**Comment:**
*Incomplete Implementation: This logs issuance metadata with the module
logger instead of the configured `event_logger`, so the data will not be
written through Superset's audit-event pipeline (for example DBEventLogger) and
can be silently lost when INFO logging is disabled. Attach this payload to the
existing event logging flow (`log_this_with_extra_payload` / event logger API)
so it is captured as a real audit event, not just a best-effort app log line.
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%2F41305&comment_hash=1e1be879da14dabec00637085158acf810cb7988c700af1618078138b0add06c&reaction=like'>đź‘Ť</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41305&comment_hash=1e1be879da14dabec00637085158acf810cb7988c700af1618078138b0add06c&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]
