-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
/api/v1/chart/{pk}/data does not respect Guest Token resource limitations #26201
Comments
This is on 3.0.1. We'll be upgrading over to 3.0.2 and I'm willing to try to test it off of master at some point in the current/next week, but I didn't see anything in the recent changelogs that would indicate this would be fixed |
same issue is faced by me @jfrag1 and I don't think their is fix for it. It is serious vulnerability |
I'm not able to reproduce this issue, requesting the One thing to note is I have gamma permissions for the |
Ideally the guest role (in your case Gamma) can have any permission but based on the resource it got generated on, it should only access those datasets which were used in that particular resource. |
It does do that for the most part, but the admin role includes the |
Hi @jfrag1 let me simplify this : |
@jfrag1 have you checked it? |
I used access-token to fetch the chart data but no matter which pk for the chart I changed, the api always response 404 |
You shouldn’t grant the embed_dashboard role all dataset access, it doesn’t need it. Guest tokens are granted access to all charts on the dashboard it’s created on. It works similar to how the DASHBOARD_RBAC feature flag works. A limited role like gamma is ideal since it gives the guest user enough base permissions to view the dashboard/charts in general but doesn’t grant access to any resources. You could make an argument that a guest user should never be granted any admin-like/global resource access like all_dataset_access even if their role includes it, as I’m not sure of any use cases for having such a powerful guest user. That guardrail wouldn’t necessarily be trivial to implement, though. |
hi @jfrag1 that is just an example, we can give 5 dataset access let us say which include dataset_1 and dataset_2. In above answer I think you are saying just like dashboard_rbac the guest user have automatically dataset access of those dataset which are being used in dashboard. |
@jfrag1 thanks for the help I got your point Summary on conversation
which is correct I think we can close this issue as well (For resourse access we can raise a new issue) |
@lindenh I'm going to close this issue. I hope the conversation here can help with resolving any issues you may still have. Please leave a comment here if you feel the issue hasn't been properly addressed. |
Hi, I am getting an error in an ebedded dashboard using @superset-ui/embedded-sdk in a React app: In chrome at network tab I see passed ok the following methods,
The embedded get the titles and format same as viewed in Superset without data only showing this error: Error: ORA-00936: missing expression I comment that in Superset the dashboard is loading succesfully. Someone with same error. Thanks in advanced |
When creating a guest token with resources to allow access to (namely dashboards), access to charts should be limited to those on that dashboard.
How to reproduce the bug
Expected results
Some unauthorized error.
Actual results
Data is correctly pulled back from any chart.
Screenshots
If applicable, add screenshots to help explain your problem.
Environment
(please complete the following information):
superset version
python --version
node -v
Checklist
Make sure to follow these steps before submitting your issue - thank you!
Additional context
This could also be considered a security issue, since RLS is also applied to guest tokens. In our case, we have some dashboards that do not need RLS and some that absolutely need them. The only way (without changing the dataset) to disable RLS for embedded dashboards is by creating a guest token without them, there are no settings on dashboards/charts to ignore it.
The workaround we're doing is to add
false as rls_required
to our datasets, and either having RLS clauses per user or having the clause "rls_required=false" which will error out charts without that column. This isn't a desirable workaround.The text was updated successfully, but these errors were encountered: