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

Research how permissions is implemented in Django and DRF #149

Open
3 of 5 tasks
fyliu opened this issue Apr 14, 2023 · 8 comments
Open
3 of 5 tasks

Research how permissions is implemented in Django and DRF #149

fyliu opened this issue Apr 14, 2023 · 8 comments
Assignees
Labels
complexity: missing ethan feature: docs: team guide Same as PD team documentation? feature: infrastructure For changes on site technical architecture p-feature: permissions research Issue involving doing research role: back end s: PD team stakeholder: People Depot Team size: 2pt Can be done in 7-12 hours

Comments

@fyliu
Copy link
Member

fyliu commented Apr 14, 2023

Overview

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

Action Items

  • research permissions capabilities in Django and document it
    • Django comes with Group and Permission models in django.contrib.auth out of the box. How are they used?
    • Gather examples of implementations
  • research permissions capabilities in DRF and document it
    • Gather examples of implementations

Instructions

What we want is something like:

  • can it restrict access based on the user (admin has all permissions)
  • can it restrict based on the table (regular user can't modify project descriptions)
  • can it restrict specific rows of the table? (regular user can't modify user attributes of other users)
  • other things it can do
  • gather examples of how these scenarios are implemented
@fyliu fyliu added documentation Improvements or additions to documentation research Issue involving doing research p-feature: permissions labels Apr 14, 2023
@fyliu fyliu added this to the v0.01 - initial setup milestone Apr 14, 2023
@fyliu fyliu added feature: missing role: back end size: 2pt Can be done in 7-12 hours s: PD team stakeholder: People Depot Team labels Apr 14, 2023
@fyliu fyliu changed the title research how permissions is implemented in Django and DRF Research how permissions is implemented in Django and DRF Apr 14, 2023
@ExperimentsInHonesty ExperimentsInHonesty added feature: docs: PD team documentation documentation on PD team processes and architecture, etc. and removed documentation Improvements or additions to documentation feature: missing labels Jun 29, 2023
@fyliu fyliu added the feature: infrastructure For changes on site technical architecture label Aug 4, 2023
@anandramakris anandramakris self-assigned this Nov 2, 2023
@anandramakris
Copy link
Member

anandramakris commented Dec 8, 2023

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
Copy link
Member Author

fyliu commented Dec 16, 2023

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
Copy link
Member

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

@cnk
Copy link
Member

cnk commented Feb 20, 2024

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

  • Replaces authentication too which we don't want

django-role-permissions

  • Might work but it may be tough to prevent people from making a mess in the permissions files.
  • Is quite different from the built-in Django so will make it difficult to onboard people with other Django experience
  • I am not sure how this integrates with DRF

django-permission2

  • Also a bit different but closer to normal Django permisisons than django-role-permissions
  • Has some example logic files which cold be useful
  • Some docs though not fabulous
  • I am not sure how this integrates with DRF

django-rules

  • Very different but might work for us
  • Good docs - the README is extensive
  • There is information about how to use rules in DRF

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
Copy link
Member Author

fyliu commented Mar 4, 2024

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

@fyliu
Copy link
Member Author

fyliu commented Mar 4, 2024

(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
Copy link
Member Author

fyliu commented Mar 11, 2024

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

@fyliu fyliu added feature: docs: team guide Same as PD team documentation? and removed feature: docs: PD team documentation documentation on PD team processes and architecture, etc. labels Jun 3, 2024
@shmonks shmonks moved this to In progress (actively working) in P: PD: Project Board Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: missing ethan feature: docs: team guide Same as PD team documentation? feature: infrastructure For changes on site technical architecture p-feature: permissions research Issue involving doing research role: back end s: PD team stakeholder: People Depot Team size: 2pt Can be done in 7-12 hours
Projects
Status: 🏗In progress-actively working
Development

No branches or pull requests

5 participants