Refactor permissions

• Custom ACLs (spec here)
• Common management for permission list and permission check
• Handling of Guardian with user assigned permissions

I need to provide a diagram

changed the description

added Proposition need spec + 1 deleted label

added P1 label

assigned to @calummackervoy

changed the description

I had a chat with @sylvain yesterday and the verdict was: The role of a permissions class is:

• to control access to resources
• to output the permissions available for a client to prevent unnecessary 403s
• to filter results in a list so that a user only sees resources they can access

The proposed scope of the refactor is:

• DjangoLDP requires Django-Guardian
• Guardian used to check permissions in all views
• Currently models include anon_perms, auth_perms and owner_perms. This would be redundant and user_permissions would be derived instead by the per-user and per-group permissions via Guardian
• overriding user_permissions method in a PermissionsClass blocks the 'output' function of a PermissionsClass, because the permissions are dynamic and not returned in the document. These should be instead managed by Guardian object-level permissions, with permissions removed/added from a user dynamically by responding to signals
• Models are currently registered with the admin site automatically, and now they will be registered automatically with Guardian active
• Documentation to explain the DjangoLDP Permissions system, and how to use it with a custom model

Currently lists are filtered for permissions by the LDListMixin in serializers.py: https://git.happy-dev.fr/startinblox/djangoldp-packages/djangoldp/blob/master/djangoldp/serializers.py#L78 This contradicts the feeling of the DRF documentation which recommends overriding the view's get_queryset. Is this necessary, should it be changed, or just documented?

• A custom FilterBackend would be appropriate for this
• Guardian provides a method get_objects_for_user for this purpose

@bleme @balessan @christophehenry @jbpasquier what are your thoughts? In the past I've used custom PermissionClasses on models, I have a general understanding of permissions, and this is about the extent of my expertise (I've not used Guardian before). It's possible I've overlooked something

I've attached a summary of my research into Django-Guardian, for those who aren't familiar with it django_guardian_notes.pdf

Actually I'm not sure it's a good idea to get rid of anon_perms, auth_perms and owner_perms. They are currently used to conveniently define permission rules, and are an easy alternative to nominative permissions. We should probably keep them for the moment, until we've defined an easy way to manage permissions in a solid way. We may also push new standards if we feel we need them. See https://git.happy-dev.fr/startinblox/solid-spec/issues/6 for example.

overriding methods is possible, but it is then the responsibility of the developer to keep the consistency between the 3 methods.

Have you had a look at filters in DRF? We might want to use them: https://www.django-rest-framework.org/api-guide/permissions/#limitations-of-object-level-permissions

The rest sounds perfect to me.

In !116 (merged) I've extended DjangoLDP to support Django-Guardian.. @sylvain I've assigned this to you is that correct?

After this MR I'll change the other DjangoLDP packages to use the permissions system, and after this I'll open a new MR for using the Django Filters in the serializer (or maybe this is not a priority)

I'm synchronising with Alice to start a DjangoLDP guide which I think will help people who are new to the project

mentioned in merge request djangoldp-circle!19 (merged)

I've opened an MR which implements the refactor in DjangoLDP Circle (djangoldp-circle!19 (merged)), and I've written a guide for DjangoLDP permissions and how to extend them (https://git.startinblox.com/djangoldp-packages/djangoldp/wikis/guides/custom-permissions)

@calummackervoy @jbpasquier : is that question closed or do we have things to add ?

EDIT: hmm actually let me review the refactors after federation, an improvement might be needed ^^

the changes are merged, and circle and project packages are both refactored to use the new permissions system.. I don't think that others need to be changed for now, so we can close this?

Ok, got it.

removed P1 Proposition need spec + 1 deleted label

mentioned in issue #235

mentioned in issue #244 (closed)

Investigating a bug with nested field permissions, the proposed solution here to use agents may not be adequate

Please read the description here: #244 (comment 28382)

• here @sylvain mentions shape-masks.. could using these on nested fields be a solution (or are they only for reading data)?
• https://git.happy-dev.fr/startinblox/solid-spec/issues/6 may be related
• Possibly we could auto-create custom Guardian permissions for specific nested fields, although this could become complicated quickly Edit: I think this would be inefficient and complicated

There may be a better solution which I'm not seeing. Many DRF users will expect to be able to write custom permissions classes which currently is very difficult with DjangoLDP

Many DRF users will expect to be able to write custom permissions classes which currently is very difficult with DjangoLDP

Why? We already handle custom permissions classes, only some mess because of Guardians there.

About the Circle problem, I think that the simplest way to handle that is to create some custom guardians permission and not use the default add. Like this, we can base our custom permissions on them and don't impact nested fields, or at least only those we want.