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

[slice] Adding slice owners to SliceFilter #5256

Conversation

john-bodley
Copy link
Member

Similar to #4520 this PR adds owners of a slice to the SliceFilter method which ensures that owners can view the slices they own irrelevant of the permission.

Note this really is a temporary partial fix as the current DashboardFilter, DatasourceFilter, and SliceFilter do not currently correctly apply the correct permission logic, i.e., for users without all-datasource-access the filters merely check if the corresponding datasource permission is explicitly defined for their roles, however it does not take into account indirect schema access, database access etc.

to: @graceguo-supercat @jeffreythewang @michellethomas @mistercrunch @timifasubaa

@graceguo-supercat
Copy link

The solution sounds good to me but you have a linting error.

@john-bodley john-bodley force-pushed the john-bodley-slice-filter-include-owner branch from cd8de1d to c1a35da Compare June 21, 2018 03:58
@jeffreythewang
Copy link
Contributor

Just wondering, is there a scenario where we want this functionality? For the dashboard case, it came up because people often created empty dashboards to which they later added charts to. If a user viewed a dashboard they owned, they would still not be able to see charts associated with datasources they don't have access to.

For charts, is there a similar scenario?

@john-bodley
Copy link
Member Author

@jefferythewang we had reports of some users not being able to see the slices they created. The SliceFilter logic isn’t correct as it only checks datasource access as opposed to schema and db access as well. If a user had access to a schema the in theory could create a slice using a datasource from said schema, however the slice would be filtered out because the datasource permission didn’t exist in the FAB security model.

@jeffreythewang
Copy link
Contributor

Ah I see. Yeah this looks good as a temporary fix.

@john-bodley
Copy link
Member Author

john-bodley commented Jun 21, 2018

The right solutuon is to fix the permission logic for slices and datasources. This is quite involved as it would require deprecating the slice/datasource perm field and instead use the datasource’s db, schema, and table name for permission checking.

In the case of custom SQL if there's no datasource permission which matches it should probably pass the SQL (not unlike SQL Lab) to determine the physical underlying tables/views and then check their permissions.

@mistercrunch
Copy link
Member

We should make sure there's no way for users to point the slice that they own to another datasource they don't have access to. I wonder if there's an endpoint out there that they could POST to and rewire their slice to another datasource...

@john-bodley
Copy link
Member Author

@mistercrunch I can dig into this. I think I just need to examine where slicemodelview is called.

@mistercrunch
Copy link
Member

mistercrunch commented Jun 22, 2018

One way is to raise an error on the pre_add and pre_edit of SliceModelView

@stale
Copy link

stale bot commented Apr 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 10, 2019
@stale stale bot closed this Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive Inactive for >= 30 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants