john-bodley commented on code in PR #24690:
URL: https://github.com/apache/superset/pull/24690#discussion_r1262838488
##########
superset/security/manager.py:
##########
@@ -2006,28 +2006,37 @@ def raise_for_dashboard_access(self, dashboard:
"Dashboard") -> None:
from superset import is_feature_enabled
from superset.dashboards.commands.exceptions import
DashboardAccessDeniedError
- if self.is_guest_user() and dashboard.embedded:
+ if self.is_guest_user():
+ # Guest user is currently used for embedded dashboards only. If
the guest user
+ # doesn't have access to the dashboard, ignore all other checks.
if self.has_guest_access(dashboard):
return
- else:
- if self.is_admin() or self.is_owner(dashboard):
- return
+ raise DashboardAccessDeniedError()
- # RBAC and legacy (datasource inferred) access controls.
- if is_feature_enabled("DASHBOARD_RBAC") and dashboard.roles:
- if dashboard.published and {role.id for role in
dashboard.roles} & {
- role.id for role in self.get_user_roles()
- }:
- return
- elif (
- not dashboard.published
- or not dashboard.datasources
- or any(
- self.can_access_datasource(datasource)
- for datasource in dashboard.datasources
- )
- ):
+ if self.is_admin() or self.is_owner(dashboard):
+ return
+
+ # RBAC and legacy (datasource inferred) access controls.
+ if is_feature_enabled("DASHBOARD_RBAC") and dashboard.roles:
+ if dashboard.published and {role.id for role in dashboard.roles} &
{
+ role.id for role in self.get_user_roles()
+ }:
return
+ elif (
+ # To understand why we rely on status and give access to draft
dashboards
Review Comment:
I second @michael-s-molina's comment. I think the security logic for
dashboard access is rather convoluted and difficult to grok which is definitely
undesirable from a security perspective, i.e., there likely are holes which can
be exploited.
I wonder if in 4.0 if dashboard RBAC should no longer be a feature but
rather the default which will likely simplify things even if (per
@michael-s-molina's comment) this breaks compatibility.
--
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]
