Fixes the permission issue with import-all. #379

decko · 2024-06-24T19:53:38Z

Closes #373

git-hyagi

LGTM!

lubosmj · 2024-06-25T12:01:51Z

pulp_ostree/tests/functional/api/test_import.py

+        permissions=[
+            "core.add_compositecontentguard",
+            "core.add_domain",
+            "core.add_headercontentguard",
+            "ostree.add_ostreedistribution",
+            "ostree.add_ostreeremote",
+            "ostree.add_ostreerepository",
+            "ostree.view_ostreerepository",
+            "ostree.change_ostreerepository",
+            "ostree.delete_ostreerepository",
+            "ostree.import_commits_ostreerepository",
+            "ostree.manage_roles_ostreerepository",
+            "ostree.modify_ostreerepository",
+            "ostree.repair_ostreerepository",
+            "ostree.sync_ostreerepository",
+            "ostree.view_ostreerepository",
+            "ostree.add_ostreerepository",
+            "ostree.view_ostreeremote",
+            "ostree.change_ostreeremote",
+            "ostree.delete_ostreeremote",
+            "ostree.manage_roles_ostreeremote",
+            "ostree.view_ostreeremote",
+            "ostree.add_ostreeremote",
+            "ostree.view_ostreedistribution",
+            "ostree.change_ostreedistribution",
+            "ostree.delete_ostreedistribution",
+            "ostree.manage_roles_ostreedistribution",
+            "ostree.view_ostreedistribution",
+            "ostree.add_ostreedistribution",
+            "core.replicate_upstreampulp",
+            "core.view_upload",
+            "core.change_upload",
+            "core.delete_upload",
+            "core.manage_roles_upload",
+            "core.view_upload",
+            "core.add_upload",
+            "core.view_task",
+            "core.change_task",
+            "core.delete_task",
+            "core.manage_roles_task",
+            "core.view_task",
+            "core.view_taskschedule",
+            "core.manage_roles_taskschedule",
+            "core.view_taskschedule",
+            "core.download_rbaccontentguard",
+            "core.view_rbaccontentguard",
+            "core.change_rbaccontentguard",
+            "core.delete_rbaccontentguard",
+            "core.manage_roles_rbaccontentguard",
+            "core.view_rbaccontentguard",
+            "core.add_rbaccontentguard",
+            "core.view_headercontentguard",
+            "core.change_headercontentguard",
+            "core.delete_headercontentguard",
+            "core.manage_roles_headercontentguard",
+            "core.view_headercontentguard",
+            "core.add_headercontentguard",
+            "core.view_group",
+            "core.change_group",
+            "core.delete_group",
+            "core.manage_roles_group",
+            "core.view_group",
+            "core.add_group",
+            "core.view_domain",
+            "core.change_domain",
+            "core.delete_domain",
+            "core.manage_roles_domain",
+            "core.view_domain",
+            "core.add_domain",
+            "core.view_contentredirectcontentguard",
+            "core.change_contentredirectcontentguard",
+            "core.delete_contentredirectcontentguard",
+            "core.manage_roles_contentredirectcontentguard",
+            "core.view_contentredirectcontentguard",
+            "core.add_contentredirectcontentguard",
+            "core.view_compositecontentguard",
+            "core.change_compositecontentguard",
+            "core.delete_compositecontentguard",
+            "core.manage_roles_compositecontentguard",
+            "core.view_compositecontentguard",
+            "core.add_compositecontentguard",
+        ],


Suggested change

permissions=[

"core.add_compositecontentguard",

"core.add_domain",

"core.add_headercontentguard",

"ostree.add_ostreedistribution",

"ostree.add_ostreeremote",

"ostree.add_ostreerepository",

"ostree.view_ostreerepository",

"ostree.change_ostreerepository",

"ostree.delete_ostreerepository",

"ostree.import_commits_ostreerepository",

"ostree.manage_roles_ostreerepository",

"ostree.modify_ostreerepository",

"ostree.repair_ostreerepository",

"ostree.sync_ostreerepository",

"ostree.view_ostreerepository",

"ostree.add_ostreerepository",

"ostree.view_ostreeremote",

"ostree.change_ostreeremote",

"ostree.delete_ostreeremote",

"ostree.manage_roles_ostreeremote",

"ostree.view_ostreeremote",

"ostree.add_ostreeremote",

"ostree.view_ostreedistribution",

"ostree.change_ostreedistribution",

"ostree.delete_ostreedistribution",

"ostree.manage_roles_ostreedistribution",

"ostree.view_ostreedistribution",

"ostree.add_ostreedistribution",

"core.replicate_upstreampulp",

"core.view_upload",

"core.change_upload",

"core.delete_upload",

"core.manage_roles_upload",

"core.view_upload",

"core.add_upload",

"core.view_task",

"core.change_task",

"core.delete_task",

"core.manage_roles_task",

"core.view_task",

"core.view_taskschedule",

"core.manage_roles_taskschedule",

"core.view_taskschedule",

"core.download_rbaccontentguard",

"core.view_rbaccontentguard",

"core.change_rbaccontentguard",

"core.delete_rbaccontentguard",

"core.manage_roles_rbaccontentguard",

"core.view_rbaccontentguard",

"core.add_rbaccontentguard",

"core.view_headercontentguard",

"core.change_headercontentguard",

"core.delete_headercontentguard",

"core.manage_roles_headercontentguard",

"core.view_headercontentguard",

"core.add_headercontentguard",

"core.view_group",

"core.change_group",

"core.delete_group",

"core.manage_roles_group",

"core.view_group",

"core.add_group",

"core.view_domain",

"core.change_domain",

"core.delete_domain",

"core.manage_roles_domain",

"core.view_domain",

"core.add_domain",

"core.view_contentredirectcontentguard",

"core.change_contentredirectcontentguard",

"core.delete_contentredirectcontentguard",

"core.manage_roles_contentredirectcontentguard",

"core.view_contentredirectcontentguard",

"core.add_contentredirectcontentguard",

"core.view_compositecontentguard",

"core.change_compositecontentguard",

"core.delete_compositecontentguard",

"core.manage_roles_compositecontentguard",

"core.view_compositecontentguard",

"core.add_compositecontentguard",

],

permissions=[

"ostree.add_ostreerepository",

],

We can use just one permission. The other remaining are automatically assigned to a user after the creation.

Or better, we could create a user with the ostree.ostreerepository_creator role.

Like this: https://github.com/pulp/pulp_container/blob/ce05037517d62b36a35e0a8c57efc7e0e46e5a45/pulp_container/tests/functional/api/test_rbac_repo_content.py#L41-L46.

I've tested with those permissions because we've created an ostree.admin role for services. But for sure we can trim down the permissions.

Also, I think we need this permission here https://github.com/pulp/pulp_ostree/pull/379/files#diff-933b628e96bbcca797e520be49a32612920c4b46068b3f45e12660ebcb68d2e9R163

Not really. Because that permission will be automatically assigned to a respective user once she creates a repository, see the creation hooks here:

pulp_ostree/pulp_ostree/app/viewsets.py

Lines 184 to 203 in cb092dd

"creation_hooks": [

{

"function": "add_roles_for_object_creator",

"parameters": {"roles": "ostree.ostreerepository_owner"},

},

],

"queryset_scoping": {"function": "scope_queryset"},

}

LOCKED_ROLES = {

"ostree.ostreerepository_creator": ["ostree.add_ostreerepository"],

"ostree.ostreerepository_owner": [

"ostree.view_ostreerepository",

"ostree.change_ostreerepository",

"ostree.delete_ostreerepository",

"ostree.modify_ostreerepository",

"ostree.sync_ostreerepository",

"ostree.manage_roles_ostreerepository",

"ostree.repair_ostreerepository",

"ostree.import_commits_ostreerepository",

],

.

Please don't use permissions, but roles.

Thanks folks. I've applied all your suggestions.

Closes pulp#373

lubosmj · 2024-06-25T20:42:33Z

Do we want to backport this to 2.4? 🤔

patchback · 2024-06-26T09:18:04Z

Backport to 2.4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/2.4/94ed3e570a8fdb60ad926f159bea50abe49ad49d/pr-379

Backported as #380

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

decko · 2024-06-26T13:34:25Z

Do we want to backport this to 2.4? 🤔

I believe it should be just to 2.3.

patchback · 2024-06-26T14:07:28Z

Backport to 2.3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/2.3/94ed3e570a8fdb60ad926f159bea50abe49ad49d/pr-379

Backported as #381

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

decko requested review from lubosmj and git-hyagi June 24, 2024 21:04

git-hyagi previously approved these changes Jun 25, 2024

View reviewed changes

lubosmj requested changes Jun 25, 2024

View reviewed changes

Fixes the permission issue with import-all.

fd13743

Closes pulp#373

decko dismissed git-hyagi’s stale review via fd13743 June 25, 2024 18:00

decko force-pushed the permission_issue branch from 4e7084d to fd13743 Compare June 25, 2024 18:00

decko requested review from lubosmj and git-hyagi June 25, 2024 18:01

lubosmj approved these changes Jun 25, 2024

View reviewed changes

lubosmj added backport-2.3 and removed backport-2.3 labels Jun 25, 2024

lubosmj added the backport-2.4 label Jun 26, 2024

lubosmj merged commit 94ed3e5 into pulp:main Jun 26, 2024
16 checks passed

patchback bot mentioned this pull request Jun 26, 2024

[PR #379/94ed3e57 backport][2.4] Fixes the permission issue with import-all. #380

Merged

lubosmj added the backport-2.3 label Jun 26, 2024

patchback bot mentioned this pull request Jun 26, 2024

[PR #379/94ed3e57 backport][2.3] Fixes the permission issue with import-all. #381

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes the permission issue with import-all. #379

Fixes the permission issue with import-all. #379

decko commented Jun 24, 2024

git-hyagi left a comment

lubosmj Jun 25, 2024 •

edited

Loading

lubosmj Jun 25, 2024

lubosmj Jun 25, 2024

decko Jun 25, 2024

decko Jun 25, 2024

lubosmj Jun 25, 2024

mdellweg Jun 25, 2024

decko Jun 25, 2024

lubosmj commented Jun 25, 2024

patchback bot commented Jun 26, 2024 •

edited

Loading

decko commented Jun 26, 2024 •

edited

Loading

patchback bot commented Jun 26, 2024 •

edited

Loading

	"creation_hooks": [
	{
	"function": "add_roles_for_object_creator",
	"parameters": {"roles": "ostree.ostreerepository_owner"},
	},
	],
	"queryset_scoping": {"function": "scope_queryset"},
	}
	LOCKED_ROLES = {
	"ostree.ostreerepository_creator": ["ostree.add_ostreerepository"],
	"ostree.ostreerepository_owner": [
	"ostree.view_ostreerepository",
	"ostree.change_ostreerepository",
	"ostree.delete_ostreerepository",
	"ostree.modify_ostreerepository",
	"ostree.sync_ostreerepository",
	"ostree.manage_roles_ostreerepository",
	"ostree.repair_ostreerepository",
	"ostree.import_commits_ostreerepository",
	],

Fixes the permission issue with import-all. #379

Fixes the permission issue with import-all. #379

Conversation

decko commented Jun 24, 2024

git-hyagi left a comment

Choose a reason for hiding this comment

lubosmj Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

lubosmj Jun 25, 2024

Choose a reason for hiding this comment

lubosmj Jun 25, 2024

Choose a reason for hiding this comment

decko Jun 25, 2024

Choose a reason for hiding this comment

decko Jun 25, 2024

Choose a reason for hiding this comment

lubosmj Jun 25, 2024

Choose a reason for hiding this comment

mdellweg Jun 25, 2024

Choose a reason for hiding this comment

decko Jun 25, 2024

Choose a reason for hiding this comment

lubosmj commented Jun 25, 2024

patchback bot commented Jun 26, 2024 • edited Loading

Backport to 2.4: 💚 backport PR created

decko commented Jun 26, 2024 • edited Loading

patchback bot commented Jun 26, 2024 • edited Loading

Backport to 2.3: 💚 backport PR created

lubosmj Jun 25, 2024 •

edited

Loading

patchback bot commented Jun 26, 2024 •

edited

Loading

decko commented Jun 26, 2024 •

edited

Loading

patchback bot commented Jun 26, 2024 •

edited

Loading