Skip to content
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

Show only Slices and Dashboards users have access to #404

Merged
merged 2 commits into from
Apr 26, 2016

Conversation

mistercrunch
Copy link
Member

@michellethomas @williaster

  • Materialized the perm associated to the slice in the slice model to make queries faster when listing Dashboards and Slices.
  • Also added Many to many owners for slices and dashboards, should eventually define who can alter the objects, looking for the proper hook in FAB

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.95% when pulling dd824c8 on mistercrunch:security into c0fb9ee on airbnb:master.

@coveralls
Copy link

coveralls commented Apr 26, 2016

Coverage Status

Coverage decreased (-0.4%) to 80.362% when pulling dd824c83ea26f7a12ec6428ba957ab6108f8c195 on mistercrunch:security into c0fb9ee on airbnb:master.

* Many to many owners for slices and dashboards
* Slices are filtered to only slices that the user has access to
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.02% when pulling 5c79214 on mistercrunch:security into ab64a26 on airbnb:master.

@coveralls
Copy link

coveralls commented Apr 26, 2016

Coverage Status

Coverage increased (+0.2%) to 80.986% when pulling 5c79214 on mistercrunch:security into ab64a26 on airbnb:master.

@mistercrunch mistercrunch merged commit b634d03 into apache:master Apr 26, 2016
@mistercrunch mistercrunch deleted the security branch April 26, 2016 23:44
@x4base
Copy link
Contributor

x4base commented Apr 27, 2016

@mistercrunch
So right now whether a user can access a dashboard is determined by whether the role has the datasource access. It has nothing to do with whether the user is the owner of the slice or dashboard. Am I correct? Thanks

@mistercrunch
Copy link
Member Author

Admin and Alpha have access to all datasources, Gamma user need and extra role with specific datasource access to list and access dashboard and slices

@mistercrunch
Copy link
Member Author

I'm planning on making it such that only object owners can alter dashboards and slices. This PR added the possibility to add many owners to a dashboard or slice, but that is not taken into account yet. I opened issues to ask the Flask AppBuilder project on advice on how to implement that

@x4base
Copy link
Contributor

x4base commented Apr 28, 2016

So in your plan it is only for the access control of editing dashboards/slices, right? Do you think it is good to have the view access control on whether a user can see a certain dashboard/slice, maybe like #357 ? Is it in any of your plans?

@MrMauricioLeite
Copy link

Hi @x4base, I guess that @mistercrunch is going to finish the edit permissions first and them extend for view permissions once we have the edit fixed.

But that sure is needed to deploy here at our company too so I'm keeping this issue on my bookmarks.

@hamidmahmoodnbs
Copy link

hamidmahmoodnbs commented Oct 12, 2018

can anybody confirm that has this issue been fixed or not? #1434

I am facing the same issue with superset v 0.25.6. The issue still persists as the user seeing all the datasources in chart drop down that he has no access to. I have given user the gamma role and new role that only has permission to a single datasource.

@pbr0ck3r
Copy link

pbr0ck3r commented Nov 7, 2018

@hamidmahmoodnbs seems like the slices/dashboard/schema filtering works. But the databases themselves are not filtered based on roles/permissions. On load of the SQL editor all databases are returned. Even if you have a role such as database access on x.[id:5] you get that database and all of the others. Instead of only getting x. Seems like a security issue to me.

@pbr0ck3r
Copy link

pbr0ck3r commented Nov 7, 2018

@mistercrunch databases are not being filtered at all. Every user is returned all databases. Even if they only have a role such as database access x.[id:5] they still get all databases.

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.8.9 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.8.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants