bito-code-review[bot] commented on code in PR #44494:
URL: https://github.com/apache/superset/pull/44494#discussion_r4072426793
##########
superset/commands/chart/update.py:
##########
@@ -181,6 +185,12 @@ def validate(self) -> None: # noqa: C901
exceptions.append(ex)
raise_if_managed_externally(self._model, ChartForbiddenError)
else:
+ # ``raise_for_access`` admits a guest for every chart on the
+ # dashboard its token embeds, and a guest holds no write
+ # capability, so deny explicitly. The regular update path gets this
+ # from ``is_editor``, which never treats a guest as an editor.
+ if security_manager.is_guest_user():
+ raise ChartForbiddenError()
try:
security_manager.raise_for_access(chart=self._model)
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to use `raise_for_editorship` instead of
`raise_for_access` is based on the concern that `raise_for_access` is too
permissive, as it admits viewers and datasource-access users. However, as you
noted, `raise_for_access` is the intended gate for this path, and the specific
risk of guest users is addressed by the explicit
`security_manager.is_guest_user()` check added in the diff. Given that the
guest token is now explicitly denied and the regular update path correctly
handles editor permissions, the current implementation appears to be a correct
and secure approach for this specific requirement.
**superset/commands/chart/update.py**
```
+ # ``raise_for_access`` admits a guest for every chart on the
+ # dashboard its token embeds, and a guest holds no write
+ # capability, so deny explicitly. The regular update path gets
this
+ # from ``is_editor``, which never treats a guest as an editor.
+ if security_manager.is_guest_user():
+ raise ChartForbiddenError()
+ try:
+ security_manager.raise_for_access(chart=self._model)
```
--
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]
