explorerhq / explorerhq/sql-explorer
Permissions Refactor
- Dominant language
- Python
- Stars
- 2.9k
- Forks
- 373
- PR merge metrics
- No merged PRs in 30d
Description
This is a two part suggestion:
1) Use the Django auth backend for permissions. Permissions are already generated for the models in the app (add/change/delete), and you can add new ones. Specifically a 'view' permission could be added for the Query model, and these permissions could become the default way of handling permission instead of the lambdas. For more complex cases, Django has a tuple of auth backends, allowing for [custom backends](https://docs.djangoproject.com/en/1.8/topics/auth/customizing/#handling-authorization-in-custom-backends) which can grant permissions (if any backend says yes then it's allowed, unless it raises the `PermissionDenied` exception). This means users who currently have a complex function for permissions currently can port it to a custom backend like:
```
class ExplorerPermissionsBackend(object):
def has_perm(self, user_obj, perm, obj=None):
# This could be wrapped into a simple base model for users
if not perm.startswith("explorer."):
return False
if user_obj.username == "foobar" and perm == "explorer.change_query":
return True
else:
return False
```
This would also open up integrating an object-level permissions backend such as [django-guardian](https://github.com/lukaszb/django-guardian) to grant certain users permissions on individual queries.
2) The second suggestion dovetails nicely with the first, and that's to allow blacklisting tables for certain users. While it may be OK for super users to dump the whole contents of `auth_user`, it's a bad idea to leave that exposed to regular staff members. This can somewhat be accomplished with the database connection itself, but it's very coarse-grained.
Use `sqlparse` to find table names in the query (would need a list of table names, probably from the same method that gets them for the schema list) and check permissions against the auth backends like:
```
tables_to_permissions = {} # table name -> (app_label + ".change_" + model name)
# Ensure the user has permissions for all tables in the query
for table_name in query_table_names:
if not request.user.has_perm(tables_to_permissions[table_name]):
return # Permission denied, similar to keyword blacklisting
```
This would limit users to doing queries on the tables for the models they have the change permission for (unfortunately Django doesn't have a stock 'view' permission).
The code could also first look for a "view_" permission to help open up the possibility of opening up the interface to public consumption. In that situation the custom auth backend in 1 would return True for the "view_" permission for any model which should be queryable by anyone.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.