Add support for Role Based Access Control#39
Conversation
aKlimau
force-pushed
the
add-rbac-support
branch
2 times, most recently
from
a73be60 to
d786edd
Compare
| class Meta: | ||
| default_related_name = "%(app_label)s_%(model_name)s" | ||
| permissions = [ | ||
| ("sync_rustrepository", "Can start a sync task"), |
There was a problem hiding this comment.
We don't do syncing yet (or maybe ever - the stub task code exists and I haven't deleted it yet but it's nonfunctional)
I need to think a bit more whether we should go ahead and remove this though, since it seems like changes require migrations?
There was a problem hiding this comment.
Yes changes to "permissions" field in model would require a migration
There was a problem hiding this comment.
It's worth asking @dkliban if there would be any need for sync in the future for e.g. project lightwell. I'm otherwise content to just have pull through caching.
|
|
||
|
|
||
| @pytest.fixture | ||
| def rust_content_factory(rust_content_api_client, pulpcore_bindings, monitor_task): |
There was a problem hiding this comment.
Just use or slightly modify https://github.com/pulp/pulp_rust/blob/main/pulp_rust/tests/functional/utils.py#L91
You should use the cargo publish API, not try to create content directly. To be honest, if we can disable that completely we probably should. As it stands, this is basically a re-implementation of the cargo publish API endpoint, just inside of our tests.
aKlimau
force-pushed
the
add-rbac-support
branch
from
d786edd to
cfd23e9
Compare
There was a problem hiding this comment.
@aKlimau Could you do as Gerrod suggested and use ReadOnlyContentViewSet here?
| "condition": [ | ||
| "has_required_repo_perms_on_upload:rust.modify_rustrepository", | ||
| "has_required_repo_perms_on_upload:rust.view_rustrepository", | ||
| "has_upload_param_model_or_domain_or_obj_perms:core.change_upload", |
There was a problem hiding this comment.
Not 100% sure if this means this block of permissions can go, as well - I think it would?
There was a problem hiding this comment.
I also deleted test_upload.py for that reason, since it was relying on modifying content directly
| alice, bob, charlie = gen_users("rustrepository") | ||
|
|
||
| with bob: | ||
| repo = rust_repo_api_client.create({"name": str(uuid.uuid4())}) |
There was a problem hiding this comment.
Can rust_repo_factory() not be used here?
|
Apart from the notes, this looks good |
aKlimau
force-pushed
the
add-rbac-support
branch
2 times, most recently
from
48f69ca to
ca333b0
Compare
| "effect": "allow", | ||
| "condition": [ | ||
| "has_model_or_domain_or_obj_perms:rust.change_rustdistribution", | ||
| "has_model_or_domain_or_obj_perms:rust.view_rustdistribution", |
There was a problem hiding this comment.
Is this right? I read this as "if you have view permissions, you can update and set labels"
There was a problem hiding this comment.
Never mind, they are AND'd, not OR'd
| } | ||
|
|
||
| @transaction.atomic | ||
| def create(self, request): |
There was a problem hiding this comment.
This can be deleted, right?
| creator = gen_user(model_roles=["rust.rustrepository_creator"]) | ||
|
|
||
| with creator: | ||
| repo = rust_repo_api_client.create({"name": str(uuid.uuid4())}) |
There was a problem hiding this comment.
Can rust_repo_factory() be used here?
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
Add RBAC to pulp_rust following the same pattern as pulp_python. All standard REST API viewsets now have access policies with creator/owner/viewer locked roles.
Also fixed the sync dispatch call args so that RBAC could be tested with sync.
closes #23
📜 Checklist
See: Pull Request Walkthrough