How should Pulp handle different default permission classes?

Development
#1

At the moment pulp uses the default permission class from django rest framework as the base permission class for all of it’s viewsets. By default this is set to pulpcore.plugin.access_policy.AccessPolicyFromDB, however this can be set to any permission class that implements the DRF permission class interface. As a user, if I chose to switch the default access policy to something else, I would expect this to also disable the rest of Pulp’s RBAC system however that’s not the case.

Pulp still performs the following actions that are linked to AccessPolicyFromDB:

  1. Populates the Access Policy database table
  2. Creates default roles
  3. Runs creation_hooks and the legacy permissions_assignment functions for every access policy that’s still configured in the DB

1 and 2 aren’t particularly problematic, however 3 can potentially lead to issues. User’s who don’t want to use the default AccessPolicyFromDB may not want permissions to be automatically assigned to objects when they’re created. At best this is an annoyance and the user will end up with a lot of unused groups. At worst this can lead to security issues if the groups used by the permission assignment functions collide with user specified groups or if the user choses create their own permission class or to use one of the other DRF base classes such as DjangoObjectPermissions or DjangoModelPermissions.

It might be a good idea to tie the auto assignment to default permission class in some way so that users who chose to customize their own permission classes can define how permission_assignment and creation_hooks are handled, or ignore them entirely if they want to use a permission class such as IsAdminUser that completely ignores groups.

1 Like
#2

I fear, this is not true. At least the status view overwrites the authentication and permission classes. Also some pulp_container api views, as well as the registry views configure their own permission classes with a subclassed AccessPolicyFromDB. So at the current state you cannot expect to reconfigure the default class and have a working system. Given this fact, i expect the default permission class is really something not meant to be touched by sys admins.

However, i read a certain desire in this post, and we should consider it. So the question is “How can i opt out of the RBAC machinery and configure a simpler permission schema?”.

#3

The status endpoint makes sense. I didn’t realize that pulp container also overrides the default access policy as well.

#4

When we switched the default for DEFAULT_PERMISSION_CLASSES to be pulpcore.plugin.access_policy.AccessPolicyFromDB (see this issue) I believe the intention was to have users be able to fully control and configure this (including disabling AuthZ entirely).

In working towards that goal I think there are a variety of problems preventing full control/customization as desired:

  1. The status API is hard-coded, it likely should get an accesspolicy just like all the others.
  2. The pulp_container and all plugins should not override this in their code. I don’t understand the implications of this for pulp_container, but that’s the idea I think.
  3. The querset scoping needs to be able to be disabled somehow and ideally put into the hands of admins. Right now that happens here and there is no way to disable/modify that. We should fix this somehow…

My believe on the list of things you made @newswangerd is that I agree (1) and (2) will still occur and that’s fine. Regarding your (3) with the new-style RBAC machinery, permission assignment should be totally customizable, but if that’s cumbersome todo it’s probably not an issue anyway since if you’ve overridden the checks in-place it’s likely not an issue anyway.

#5

@bmbouter for creation hook problem, I was wondering if it would be a good idea to have the creation hooks loaded from the default permission classes. So AccessPolicyFromDB would provide a get_creation_hooks method that would handle loading the creation hooks from the database and AutoAddObjPermsMixin would load the default permission class and run get_creation_hooks if it exists on the class. If it doesn’t, it would just skip the permission assignment.

That way you avoid running the creation hooks when the default permission class is configured to one of the default DRF options and you give people who want to create custom permission classes the option to use the creation hook machinery. Would an approach like this also work for the queryset scoping problem?

I’m happy to contribute these changes to pulp core if you think they sound good.

#6

One additional thing to consider here is that Django allows you to combine multiple permission_classes.
But yes, maybe you want to run the creation_hooks for all of them.

Pulp Ansible Meeting Minutes