bito-code-review[bot] commented on code in PR #39302:
URL: https://github.com/apache/superset/pull/39302#discussion_r3294409129
##########
superset/security/manager.py:
##########
@@ -2799,6 +2799,24 @@ def raise_for_access( # noqa: C901
self.get_datasource_access_error_object(datasource)
)
+ # When the guest token carries a dataset allowlist, restrict access
+ # to only those dataset IDs even if the chart/dashboard check above
+ # would otherwise grant it. Tokens without the ``datasets`` claim
+ # retain the existing behaviour (all dashboard datasets
accessible).
+ if guest_user := self.get_current_guest_user_if_guest():
+ allowed_datasets: Optional[list[int]] =
guest_user.guest_token.get(
+ "datasets"
+ )
+ if allowed_datasets is not None:
+ if not isinstance(allowed_datasets, list):
+ raise SupersetSecurityException(
+ self.get_datasource_access_error_object(datasource)
+ )
+ if datasource.id not in allowed_datasets:
+ raise SupersetSecurityException(
+ self.get_datasource_access_error_object(datasource)
+ )
Review Comment:
<!-- Bito Reply -->
The suggestion in the comment is valid and should be applied. The current
code checks if `allowed_datasets` is a list but does not verify that all
elements are integers. This could lead to incorrect access control decisions if
the dataset IDs in the token are not integers. The suggested fix adds a check
to ensure all elements in `allowed_datasets` are integers, which addresses the
input validation issue and prevents potential access control errors.
--
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]
