Skip to content

Add Object permissions to project and allow set add_check globally or per-project#653

Open
mchehab wants to merge 5 commits into
getpatchwork:mainfrom
mchehab:object-permissions
Open

Add Object permissions to project and allow set add_check globally or per-project#653
mchehab wants to merge 5 commits into
getpatchwork:mainfrom
mchehab:object-permissions

Conversation

@mchehab

@mchehab mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Currently, patchwork doesn't allow granting permissions to add_checks without promoting an user to superuser.

This patch series:

  • add a button at project admin, allowing to select per-project add_check permissions;
  • change the logic which accepts adding check to use either a global add_check permission or per project, per-user permission;
  • now, having a maintainer status is orthogonal to add CI checks: maintainers that need to also update CI need to be granted with CI add checks permission.

@stephenfin

Copy link
Copy Markdown
Member

@mchehab Could you propose these against the main branch and I'll backport them myself?

mchehab and others added 4 commits June 7, 2026 17:53
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
@mchehab mchehab force-pushed the object-permissions branch from b99ac5a to 054de50 Compare June 7, 2026 15:59
@mchehab

mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

@mchehab Could you propose these against the main branch and I'll backport them myself?

Done. I'll keept django-guardian~=3.3.1 dependency, but feel free to use a newer one if you prefer.
I will dropped one patch (4e3d25d), which is not seem to be doing any difference.

@stephenfin stephenfin changed the base branch from stable/3.2 to main June 7, 2026 16:02
@mchehab

mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for changing the baseline. My user were unable to change the baseline indication at the PR.

@mchehab

mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

hmm... this one requires more work... it actually added "add <project>" instead of "add <checks>"

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
@mchehab mchehab changed the title Object permissions [RFC] Object permissions Jun 7, 2026
@mchehab

mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

I changed this one to RFC, as some work is still needed to be able to have per-project permissions.

@mchehab

mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Ah, the patch I dropped made some difference (or perhaps a re-run on manage.py migrate).

It is now shown:
image

so, besides add/change/delete/view project, there is an "add checks" over there.

@mchehab

mchehab commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Tests with https://github.com/mchehab/pw_tools:

User without global "add_check" permission:

$ ./pw-checks.py -p local set 20260606114703.5-1-ruoyuw560@gmail.com foo warning "" "No link"
ERROR: 147247: foo failed to set 'warning': 403 Client Error: Forbidden for url: http://127.0.0.1:8000/api/patches/147247/checks/

After granting patchwork | checks | add_check at Users admin:

$ ./pw-checks.py -p local set 20260606114703.5-1-ruoyuw560@gmail.com foo1 warning "" "No link"
INFO: 147247: foo1 set to 'warning'

After removing patchwork | checks | add_check at Users admin:

$ ./pw-checks.py -p local set 20260606114703.5-1-ruoyuw560@gmail.com foo4 warning "" "No link"
ERROR: 147247: foo4 failed to set 'warning': 403 Client Error: Forbidden for url: http://127.0.0.1:8000/api/patches/147247/checks/

After granding Projects -> <project_name> -> <Object permissions> add_check permission:

 $ ./pw-checks.py -p local set 20260606114703.5-1-ruoyuw560@gmail.com foo4 warning "" "No link"
INFO: 147247: foo4 set to 'warning'

In summary, both coarse grained and fine grained add_check permission worked.

@mchehab mchehab changed the title [RFC] Object permissions Add Object permissions to project and allow set add_check globally or per-project Jun 7, 2026

@matttbe matttbe left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking at this! I have a couple of questions/ suggestions of you don't mind.

Comment thread patchwork/api/check.py
# Notice that this is a global permission: it allows
# adding checks to any project inside Patchwork.
if user.has_perm('patchwork.add_check'):
patch._edited_by = user

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(patch is not defined in this first commit)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix on a next rebase (I'm currently in vacations - will likely do it next weekend).

Comment thread requirements-dev.txt
django-filter~=25.2.0
django-debug-toolbar~=6.3.0
django-dbbackup~=5.3.0
django-guardian~=3.3.1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move it to the test one, please? I guess that's why the CI is no longer passing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will do at the next rebase.

Comment thread patchwork/api/check.py
patch._edited_by = user
return True

# Being maintainer doesn't grant rights to create checks.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a behavioural change? If yes, could this be avoided not to make upgrade difficult? Especially on large instances with many users :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, after the change, enabling/disabling checks become an independent permission.
I suspect that, even on large instances, there are typically just one user that do CI check additions on a given project - usually a special account for it; the other maintainers shall not have any business adding CI checks.

You got a point though: it could be better to set it by default during migration. Not sure if is there a way to teach migrations to set it by default for maintainers to preserve existing behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants