[BUGFIX]: Fixing permission rollup database/schema to table in SupersetFilter

[fabianmenges]

Currently the SupersetFilter (base class that filters on slices, datasources and dashboards) does not respect access to a database or schema. This can have the effect that if a user is allowed to use SQL lab, the user is not able to see the datasource she created when clicking on Visualize.

[xrmx]

We can probably add some tests to avoid regression in the future

[mistercrunch]

I also would like to see some tests in this area. I think it's also pretty important to do a single database call to list Slices and Dashboards

[mistercrunch]

It should be possible to avoid doing 2 database roundtrips when loading the slices or dashboard lists. If that doesn't work the second best option is to @memoize this method.

[fabianmenges]

I can probably refactor and get rid of one call.

[fabianmenges]

So this is not easy, for the DatasourceFilter I can refactor to something like this:

        permissions = self.get_all_permissions()
        return query.filter(or_(
          self.model.perm.in_(permissions(DATASOURCE_ACCESS)),
          self.model.schema_perm.in_(permissions(SCHEMA_ACCESS)),
          self.model.database.perm.in_(permissions(DATABASE_ACCESS)),
        ))

But that does not work for slices, since slices don't have a foreign key relationship to the datasource they reference (because we have different tables for Druid and SQL datasources).

If I'm memoizing the call (lets say for datasources_with_access), I have to handle cache invalidation when a new datasources is added. Cache invalidation does not seem to be implemented right now so I don't really want to pen that can of worms for this fix.

I think the best I can do is refactor the DatasourceFilter and leave the SliceFilter and DasboardFilter as is.

[mistercrunch]

It should be possible even without the FK, may be tricky though. You'll probably have to left join to LEFT JOIN to both tables and look for whether the it joins to either.

[john-bodley]

@fabianmenges per my comment in #4737 I'm concerned that this could make the IN clause contain thousands of perm strings depending on the number of entities in database. That said if a role has a whitelist of databases one must enumerate all the datasources in it.

A possible performance improvement would for the case of a role where it contains either all_datasource_access or all_database_access to return None (or True) to signify that no additional filter is required (as opposed from listing every single datasource) for a given database. One would have to augment the logic in query.filter(self.model.perm.in_(perms)) to be database specific and included only when neither of all_datasource_access or all_database_access are present.