Skip to content

Show only Slices and Dashboards users have access to#404

Merged
mistercrunch merged 2 commits intoapache:masterfrom
mistercrunch:security
Apr 26, 2016
Merged

Show only Slices and Dashboards users have access to#404
mistercrunch merged 2 commits intoapache:masterfrom
mistercrunch:security

Conversation

@mistercrunch
Copy link
Copy Markdown
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
Copy Markdown

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

@coveralls
Copy link
Copy Markdown

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
Copy Markdown

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

@coveralls
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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 First shipped in 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 First shipped in 0.8.9

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants