Gamma user can clone a dashboard and all of its charts without being owner of dashboard, chart and its underlying dataset

[mdeshmu]

Folks,

I am confused & annoyed with this behavior which I have described below.

We are using DASHBOARD_RBAC to give read-only dashboard access to users without giving them any access to charts, datasets, and databases. We have assigned only the default Gamma role to the user profile and to the dashboard roles.

see below, SERVICEACCOUNT user is not the owner of the dashboard. The dashboard has the Gamma role assigned.

see below, SERVICEACCOUNT user has only Gamma role assigned.

Here are the two issues we are observing:

1. In 2.1.0, 3.0.0rc1, and in the current master, a Gamma user can save a copy of a dashboard not owned by them. More importantly, they can make copies of all charts in the dashboard with the "also copy (duplicate) charts" checkbox. This is very undesired behavior and a maintenance nightmare for us.

see below, SERVICEACCOUNT user can use "save as" on the dashboard and duplicate all charts.

2. They can edit the chart from the dashboard and "save as" a new chart, even though the chart is not owned by them.

see below, SERVICEACCOUNT user can edit a chart from the dashboard and save it as a new chart from the chart builder.

see below, SERVICEACCOUNT user is not the owner of the chart.

The irony is, Gamma users can't list any charts from the Charts Menu (including cloned charts). An admin can see that charts are being cloned.

They can't see any dataset from the Datasets menu.

They can't see any dataset when trying to create a new chart.

This was reported by another user here as well: https://apache-superset.slack.com/archives/CCKHMGRRB/p1688356037634189

This behavior didn't exist in 1.5.3. Is this a deliberately added behavior or is it a bug with DASHBOARD_RBAC or somewhere else in the code?

Expected results

Gamma users who are not owners of the dashboard shouldn't be able to save a dashboard.
Gamma users who are not owners of the charts shouldn't be able to edit a chart from the dashboard and should not be able to save it as a new chart from the chart builder.

Actual results

Gamma users can save a dashboard and chart even if they are not owners.

Screenshots

Added above.

Environment

• browser type and version: Version 114.0.5735.134 (Official Build) (64-bit)
• superset version: 3.0.0rc1
• python version: 3.9.x
• node.js version: NA
• any feature flags active: This feature flags are set to true ->
  ALERT_REPORTS, DASHBOARD_CROSS_FILTERS, DASHBOARD_RBAC, GENERIC_CHART_AXES, ALLOW_FULL_CSV_EXPORT, DRILL_TO_DETAIL, HORIZONTAL_FILTER_BAR

Checklist

Make sure to follow these steps before submitting your issue - thank you!

• I have checked the superset logs for Python stack traces and included them here as text if there are any.
• [ x] I have reproduced the issue with at least the latest released version of the superset.
• [x ] I have checked the issue tracker for the same issue and haven't found one similar.

Additional context

NA

[john-bodley]

@betodealmeida , @mdeshmu, @michael-s-molina, @sfirke et al. I'm trying to get up to speed with the RBAC for dashboards. Per the documentation it states,

Granting a role access to a dashboard will bypass dataset level checks. Having dashboard access implicitly grants read access to all the featured charts in the dashboard, and thereby also all the associated datasets.

is the intended behavior that by adding an RBAC role than users of said role are only allowed to see the charts (and access the data backing said charts) in the controlled context of the dashboard, i.e., they should not be able to either explore it, save it (via save-as), or directly access the underlying dataset?

[betodealmeida]

is the intended behavior that by adding an RBAC role than users of said role are only allowed to see the charts (and access the data backing said charts) in the controlled context of the dashboard, i.e., they should not be able to either explore it, save it (via save-as), or directly access the underlying dataset?

I think in theory yes, that was in the intention, but in practice it might've been too complicated to implement it this way and the access just "cascades".

(I'm not super familiar with dashboard RBAC since we don't use it.)

[john-bodley]

As a follow up I gather creating a copy of the dashboard (see screenshot) should also likely be forbidden, i.e., said user is stepping beyond the intent of the dashboard owner(s) who provided the RBAC to the role.

Spelunking through the code I think this is likely one part of the logic which should change, i.e., access to the underly ing dataset should be context aware—granted only in the context of an accessible dashboard.

Sadly complicated logic doesn't bode well from a security/access perspective and thus I wonder in a future release whether we should consider reverting this feature given the complexity of how/where these controls are enforced—be that within the security manager, filters, etc.

[mdeshmu]

Thank you so much @john-bodley for looking into this issue.

[mattitoo]

Just to be sure about the intended behavior:
a) If the user has no permissions to the datasets, but the dashboard is visible due to RBAC, she/he should not be able to copy the dashboard via "Save as".
b) If the user has permission to the datasets, he/she should be able to make a copy via "Save as".
Is my thinking correct?

[mdeshmu]

@john-bodley Really appreciate your effort (series of PRs) to extensively address the issues I have raised here.

IMO, In this case, A user should not be able to open a chart in Explore by clicking on the chart title in the dashboard.

Do you already have an open PR to address this?

[john-bodley]

@mdeshmu #24789 partially addresses the issue, i.e., though the link will exist when they click through they'll be denied access to the underlying dataset. I'll likely work on a follow up PR which actually results in a redirect to ensure consistent behavior with the dashboard workflow when a user is denied access.

Note the challenge is currently we (many of the committers) don't have a good sense on what the actual desired UX was—the unit/integration tests which initially failed due to my change indicates that chart exploration was viable, albeit the issues surfaced in this issue. I'm hoping that @villebro can provide some additional context on the situation.

[mdeshmu]

IMO, In this case, A user should not be able to open a chart in Explore by clicking on the chart title in the dashboard.

Note the challenge is currently we (many of the committers) don't have a good sense on what the actual desired UX was—the unit/integration tests which initially failed due to my change indicates that chart exploration was viable, albeit the issues surfaced in this issue. I'm hoping that @villebro can provide some additional context on the situation.

@villebro can you please clarify what is the desired behavior?