Add RBAC to repository versions #212
Conversation
|
Attached issue: https://pulp.plan.io/issues/8017 |
goosemania
force-pushed
the
story8017
branch
2 times, most recently
from
26bf50e
to
89b2d7b
Compare
pulp_container/app/viewsets.py
Outdated
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.delete_containerrepository", | ||
| # }, | ||
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.sync_containerrepository", | ||
| # }, | ||
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.build_image_containerrepository", | ||
| # }, | ||
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.modify_content_containerrepository", | ||
| # }, |
There was a problem hiding this comment.
This is option two.
A default policy can't be changed easily and there are many statements for the same action/principal/effect.
A good thing is that a user will see those perms in detail if they try to read the access policy via API.
There was a problem hiding this comment.
Option three is to put all conditions in one loooong string and have one item for a destroy action.
There was a problem hiding this comment.
Option 4:
{
"action": ["destroy"],
"principal": "authenticated",
"effect": "allow",
"condition":
"has_repo_attr_model_or_obj_perms:container.delete_containerrepository"
" or has_repo_attr_model_or_obj_perms:container.delete_version_containerrepository",
},
|
@ipanova , @dkliban , @mdellweg , please choose which option out of three is a preference for the container team and I will go with that. In no particular order, the options (see details in my other comments) are:
|
pulp_container/app/viewsets.py
Outdated
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.delete_containerrepository", | ||
| # }, | ||
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.sync_containerrepository", | ||
| # }, | ||
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.build_image_containerrepository", | ||
| # }, | ||
| # { | ||
| # "action": ["destroy"], | ||
| # "principal": "authenticated", | ||
| # "effect": "allow", | ||
| # "condition": | ||
| # "has_repo_attr_model_or_obj_perms:container.modify_content_containerrepository", | ||
| # }, |
There was a problem hiding this comment.
Option 4:
{
"action": ["destroy"],
"principal": "authenticated",
"effect": "allow",
"condition":
"has_repo_attr_model_or_obj_perms:container.delete_containerrepository"
" or has_repo_attr_model_or_obj_perms:container.delete_version_containerrepository",
},
goosemania
force-pushed
the
story8017
branch
6 times, most recently
from
8161903
to
b286cb8
Compare
|
Needs a rebase, waiting on rbac branch to be fixed |
| monitor_task(response.task) | ||
|
|
||
|
|
||
| class PushRepoVersionTestCase(unittest.TestCase): |
There was a problem hiding this comment.
I wonder if I should move push tests to the module where push content tests are, to reduce the setup time for tests.
There was a problem hiding this comment.
push content tests are already quite long, i believe eventually we goona split those.
My personal preference would be to keep tests organized by 'topic' even though we are going to sacrifice on the setup time.
closes #8017 https://pulp.plan.io/issues/8017 Required PR: pulp/pulpcore#1100
|
Rebased. Green. |
| repo = repo_version.repository.cast() | ||
| if not (self.request.user.has_perm(perm) or self.request.user.has_perm(perm, repo)): | ||
| raise Http404(_("detail not found")) | ||
| return qs |
| "condition": [ | ||
| "has_repo_attr_model_or_obj_perms:container.delete_containerrepository_versions", # noqa | ||
| "has_repo_attr_model_or_obj_perms:container.view_containerrepository", | ||
| ], |
| del_user(cls.user_helpless) | ||
|
|
||
| @classmethod | ||
| def _pull(cls, image_path, user=None): |
There was a problem hiding this comment.
but i was thinking to move this into the registry base test so we do not copy paste them every time when we write a new module with pull/push tests.
closes #8017
https://pulp.plan.io/issues/8017