Permissions refactor

Hi @jbpasquier could I have a specification of permission requirements for this package as well please ? It's the last one :)

I should be able to remove a lot of the complexity which is great but I'd like to write some tests to make sure it still works as expected

in particular there is this concept of origin in get_model_or_obj which doesn't seem to be given to the function from anywhere (in the package)... I want to get rid of get_model_or_obj in the refactor

@jbpasquier can I have this tomorrow if possible please ? :) it's the last package to open an MR for the merge djangoldp!193 (merged) which we need for Energie Partagée

I assume we can release with a custom branch of DjangoLDP if it's not possible

This one is a bit more complex one.

The same permission class must be used for every models, whatever a community contain, it should be allowed to get the same permission behavior without any further complexity.

The current implementation is:

• Affiliations to a community are public (Job offers, members, circles, projects...) - as long as the resource itself is public
• Members can add job offers, circles, projects...
• Admins can add members
• Admins can remove a membership
• Admins can't remove admins
• Any external model can use CommunityPermissions, they'll only need a foreignkey to a community.

I wanted to provide a CommunityModel for external ones, but at this time abstract models were broken.

To be noted, middle-term objective on a community is to let admins provide some of their powers to members or to group of members within their community.

OK thanks

To be noted, middle-term objective on a community is to let admins provide some of their powers to members or to group of members within their community.

Possibly related: djangoldp#355

Hi! Sorry about the delay getting back to you I got caught up in EP/DPoP stuff

Affiliations to a community are public (Job offers, members, circles, projects...) - as long as the resource itself is public

I don't understand this part.. referencing the models I have Community and CommunityMember which aren't attached to Circle or other models, and then I have CommunityCircle which is, but does not have a connection to CommunityMember

In terms of endpoints I've drafted

• list communities - public
• list community members - public
• create community - authenticated
• update, delete, control community - community admin only
• create, update, delete, control community member - community admin only
• Admins can't remove admins (or themselves if they're the last admin)
• community projects - apply Project permissions (same for JobOffers and Circles)

middle-term objective on a community is to let admins provide some of their powers

Possibly related: djangoldp#355

admin only == admin of the community, right?

create community - ?? => Any auth user

Other items are OK.

Updated spec, should superusers have all permissions or not ?

For example

• update, delete, control community - community admin only

Super users of Django should never have any special permission reflected on app side, it breaks the app logic.

mentioned in issue djangoldp-circle#9

@jbpasquier my last failing tests are to do with the superuser permissions

my code in DjangoLDP returns the superuser permissions configured on the model correctly. LDPPermissions.get_object_permissions then makes a call which ultimately checks each auth backend (for FNK this will be Django's ModelBackend and Guardian's ObjectPermissionBackend)

The trouble is that Django's authentication backend has a hardcoded check on user.is_superuser to give back all permissions https://github.com/django/django/blob/stable/2.2.x/django/contrib/auth/backends.py#L56

As this is a decision on Django's side, I guess we'd need to open an issue with them to change the behaviour... the workarounds on DjangoLDP side all seem dirty and with Django having this philosophy we'll never know if there's another hardcoded check somewhere else, so we'll end up in workaround hell. We could fork Django for this purpose but I imagine this would be expensive to maintain

What do you think ? Personally I think the paths of least resistance are to change the requirements of Hubl or to accept it for now and open an issue with Django

Sounds bad. :-(

Did you tried to use an Hubl when you're super-user? It's unusable, every single permissions check are broken.

For sure, we can change other sides to deal with them:

• Custom permissions instead of solid compatible ones
• Per field permissions
• Don't make anyone super-user (That's definitely the worst possible solution, users will have multiple account, they'll login and never logout, it'll lead to inconsistency)

I'm struggling finding another approach. :(

Does our @balessan joker have any idea? :-)

Ping @balessan this is the blocking issue for releasing DjangoLDP 2.1

I am sorry guys I do not get the issue clearly.

Custom permissions instead of solid compatible ones

@jbpasquier : custom permissions specific to super-users right ? @calummackervoy is that doable ? That sounds like the best alternative.

Custom permissions instead of solid compatible ones

@jbpasquier I don't get what you meant by this, you mean let the serialized permissions differ from the runtime permissions ?

There are other workarounds - like having Hubl provide a DjangoLDP package which skips out the Django auth backends (it will prevent Group and Role permissions from working), or possibly there's a solution overriding the auth backend, but my feeling on everything which ignores the Django approach to super user is

we'll end up in workaround hell

I use the Hubl app on a superuser account and for my day-to-day I don't notice anything odd about it, but we're obviously having different experiences

We may prefer to approach it from the Django Admin side and look into providing access to the admin panel for users which are not superuser, but are in a custom Group for Hubl servers.. or something down this track ?

I don't like any of these solutions though really, because I feel like we're trying to find a way to "make a user a superuser, but not a superuser when they're using the front-end app" and this just seems inconsistent ?

I don't get what you meant by this, you mean let the serialized permissions differ from the runtime permissions ?

I meant to create custom permission, like superuser_view which would complete default ones.

Not a valid solution from my point of view though.

I use the Hubl app on a superuser account and for my day-to-day I don't notice anything odd about it, but we're obviously having different experiences

For eg, you can't leave a circle on a server on which you're superuser, you can only remove them as if you were administrator of the circle.

I don't like any of these solutions though really, because I feel like we're trying to find a way to "make a user a superuser, but not a superuser when they're using the front-end app" and this just seems inconsistent ?

I do agree. The thing is that we don't differ the Application from the Data Holder on the actual implementation of DjangoLDP. Being super-administrator on Django does not mean that you're super-administrator on the Application and should not get reflected at all.

When I login to some_api/admin/, I don't use the usual way to manage things, I bypass my own permission set to allow me to modify whatever I want, the same way I could change things directly on the database but in any case, I don't want to see that from the API.

This is not SOLID-compliant for sure, but this is something that somehow we need.

---

After a call with @balessan here's another suggested approach.

We target per field permissions. But, as this will not come right now and is subject to investigation (Trust thing), we should put a temporary workaround on the DjangoLDP serializer to make it lie and not reflect super user permission on the permission graph.

Even as dirty as it looks, this will do the trick on front-end and may not have any impact with the Django approach. In fact, it's not an issue if showed permission differ from real ones.

What do you think about this?

OK I'm happy with this too

I think the fastest workaround is the one which breaks Group permissions - if superuser_perms = [] then don't check Django's auth backend for permissions. Briefly looking at Guardian's backend for get_all_permissions it looks like it will be fine on that side

Found a better hack for this for info

We know that LDPPermissions.get_container_permissions will give the superusers their perms if they're configured, so LDPPermissions can safely pretend that they're not a superuser when passing the user onto the configured auth backends

Implemented and tested and looks good for both scenarios (superusers are special or they're not)

assigned to @calummackervoy

mentioned in merge request !2 (merged)

closed via merge request !2 (merged)