make filters use security manager #5567
Conversation
timifasubaa
force-pushed
the
make_filter_use_security_manager
branch
from
497af61
to
287d156
Compare
timifasubaa
changed the title
timifasubaa
force-pushed
the
make_filter_use_security_manager
branch
from
287d156
to
a581bc6
Compare
Codecov Report
@@ Coverage Diff @@
## master #5567 +/- ##
=========================================
+ Coverage 63.36% 63.5% +0.13%
=========================================
Files 351 360 +9
Lines 22259 22888 +629
Branches 2470 2549 +79
=========================================
+ Hits 14104 14534 +430
- Misses 8140 8339 +199
Partials 15 15
Continue to review full report at Codecov.
|
superset/security.py
Outdated
| @@ -85,6 +85,15 @@ def get_schema_perm(self, database, schema): | |||
| if schema: | |||
| return '[{}].[{}]'.format(database, schema) | |||
|
|
|||
| def always_list_all_dashboards(self): | |||
There was a problem hiding this comment.
I agree with your comment that we should defer to the security manager to answer said question, however I’m not sure we need these always_ methods.
| @@ -248,15 +248,11 @@ def get_view_menus(self, permission_name): | |||
| vm.add(vm_name) | |||
| return vm | |||
|
|
|||
| def has_all_datasource_access(self): | |||
| return ( | |||
| self.has_role(['Admin', 'Alpha']) or | |||
There was a problem hiding this comment.
Does removing these role checks potentially break things?
There was a problem hiding this comment.
There are only three usage of the has_all_datasource_access and they are the ones I changed.
There was a problem hiding this comment.
Here is the link to where all_datasource_access is granted to Alpha.
Note that Admin is granted all the possible priviledges, which includes those contained in Alpha.
Also linked is the line in the security manager that checks the user has all_datasource_access
* make filters use security manager * remove the superset short-circuit
mistercrunch
added
🏷️ bot
This PR removes the redundant has_all_datasource_access method and uses the security manager's all_datasource_access method. It also makes superset filters configurable via the security manager.
Longer term, it will be nicer to make the whole method configurable so different deployments can decide their own mechanisms to filter the dashboards.
@john-bodley @michellethomas @mistercrunch