pulp / pulp/pulpcore

As a developer, I can mark a Model as RBAC enabled and have exceptions raised for any quersets that don't use `with_perm`

Open
#2,067 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Feature
Dominant language
Python
Stars
598
Forks
168
Avg merge
1d 4h
Merged PRs (30d)
86

Description

Author: @bmbouter (bmbouter)

Redmine Issue: 9614, https://pulp.plan.io/issues/9614


This is build on the introduction of with_perm.

Motivation

Everytime a queryset is constructed that deals with an RBAC enabled object, we need to ensure that only those objects that user has permissions to operate on are available in the queryset results. For example, if I have the core.delete_task permission on some objects, but not others, I can't just run Task.objects.all().delete().

We deal with querysets in so many places, it would be great to have a safer way to be told if I've filtered each queryset at least in some way by permissions.

Proposal

Add an attribute on all models called RBAC_PROTECTED = False and have models opt-in to using this safety feature by setting it to True on their model definition.

Then modify the querset evaluation to raise an exception if that queryset never had a with_perm call occur. This would be an opt-in, model-by-model safety feature.

There are some situations when you are supposed to not need a with_perm call. For example if the viewset queries for all objects, and then passes the list of pks to the task in the backend to handle, the backend queryset construction already handled permissions but there is no call to with_perm there.

Let's add a queryset method called qs.with_no_perms(). With this I could call Task.objects.with_no_perms().all() and I would not receive the exception even without a call to with_perm.

Special considerations

There could be situations where a new querset is made as a new object, e.g. boolean or set operations. Let's get examples of these kinds of situations right:

  • qs.all() | qs.with_perm("core.task_show") -> unsafe
  • qs.none() | qs.with_perm("core.task_show") -> safe
  • qs.none() & qs.with_perm("core.task_show") ??
  • qs.all() & qs.with_perm("core.task_show") -> safe

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the existing with_perm implementation and the queryset evaluation path, then inspect how model definitions can opt into RBAC protection. Define and test the behavior for with_perm, with_no_perms(), and queryset combinations such as unions and intersections; done means unsafe evaluation raises an exception while explicitly safe cases do not.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
authorization, backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.