hackforla / peopledepot

A project to setup a datastore for people and projects at HackforLA. The link below takes you to the code documentation
https://hackforla.github.io/peopledepot/
GNU General Public License v2.0
7 stars 26 forks source link

Research how permissions is implemented in Django and DRF #149

Open fyliu opened 1 year ago

fyliu commented 1 year ago

Overview

We need to research the permissions capabilities provided by Django and DRF. See instructions below.

Action Items

Instructions

What we want is something like:

anandramakris commented 10 months ago

Seems to be that we can include permissions in a table by inserting a "permissions" line into the Meta class for each table.

For example, if we wanted to restrict modifying the project description, we would include the following:

class Project(AbstractBaseModel):
    class Meta:
        permissions = [
            ("change_description", "Can change project description")
        ]

To assign it to a user we do

from django.contrib.auth.models import Permission
permission = Permissions.object.get("change_description")
user.user_permissions.add(permission)

Lastly to make sure the restriction is in place, we will put permission_required("change_description") in front of the description change function.

fyliu commented 9 months ago

Thanks for finding out how it works in django.

I think maybe the issue description is too vague. We're looking for a solution that will allow us to assign permissions to users and user groups without having to change the source code. So, maybe an implementation is having a table that stores the user in relation to the permissions they have. The permissions need to be fairly fine-grained like they can update a row in a table but not create or delete it. I'm thinking of the example where a project manager can update the project's description but not create or delete the project.

Since we're building this backend for other teams to connect their frontends, we really only care about the API layer which DRF creates. But I wasn't sure if we need to do the same thing for django itself or if doing it in the DRF/API layer is sufficient, or if the DRF implementation also takes care of setting it in django. So this is an exploratory issue to try to figure that out.

The implementation I described above is just an example. I'm sure there are packages out there we can use and we wouldn't necessarily need to know the implementation in detail, just that it does what we need.

Bonnie and Nicole have been meeting to figure out exactly what permissions each user level should have. So I guess it'd be mostly like a user group sharing a set of permissions.

anandramakris commented 8 months ago

There are four default permissions in the Django admin site: add, create, delete, and view, which are specific to each table. The superuser, or any user with the "core | user | Can change user" permission, can assign any of these permissions to any user. This is done in the "Change user" view.

For your example - if we want a user to be able to change a project description but not add or delete the project, we give it the permission "core | project | Can change project" but not "core | project | Can add project" or "core | project | Can delete project".

There isn't a way in the interface to restrict specific rows.

It appears the way to do that is to include a readonly_fields attribute for each admin class in app/core/admin.py, then overwriting the get_readonly_fields method for the specific user/group.

See the documentation below: https://docs.djangoproject.com/en/4.2/ref/contrib/admin/#django.contrib.admin.ModelAdmin.readonly_fields Also see: https://stackoverflow.com/questions/11568921/restricting-django-admin-change-permissions

ExperimentsInHonesty commented 7 months ago
cnk commented 7 months ago

The default permission system in Django allows you to grant CRUD permissions at the model/table level but people depot is going to need at minimum object/field level permissions. Looking at the Django packages permissions page, we have a few options for object level permission add-ons

Django OSO

django-role-permissions

django-permission2

django-rules

django-guardian

Nice article with examples of the last 4 options: https://www.vinta.com.br/blog/controlling-access-a-django-permission-apps-comparison

fyliu commented 7 months ago

From @ethanstrominger Github Copilot solution for adding field-level permission to django.

fyliu commented 7 months ago

(https://github.com/hackforla/peopledepot/wiki/Github-Copilot-Suggestion-for-Field-Level-Security) for adding field-level permission to django.

I can see that this is trying to extend the permission to add the field. But it's checking permissions in a middleware, which allows access to the web server's request/response processing. I'm not sure if this will work for the API or not, but middlewares are made for processing webpage requests.

Maybe the Github Copilot prompt could be tweaked to request adapter code for django rest framework.


I tried adding a simpler logging middleware to see what request.resolver_match.url_name prints out since that's being used as a filter condition but I got an error

click to expand ``` web | Internal Server Error: /api/v1/stack-element-types/ web | Traceback (most recent call last): web | File "/usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py", line 56, in inner web | response = get_response(request) web | File "/usr/src/app/peopledepot/middleware.py", line 11, in __call__ web | f"Incoming request: {request.method} {request.path} {request.resolver_match.url_name}" web | AttributeError: 'NoneType' object has no attribute 'url_name' web | "POST /api/v1/stack-element-types/ HTTP/1.1" 500 60137 ```

resolver_match is set to None when the url is not resolved.

fyliu commented 6 months ago

@ethanstrominger generated some code that tries to make it rest_framework-friendly.