Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use package permissions to protect views instead of more general ones #120

Merged
merged 5 commits into from
Feb 21, 2024
Merged

Use package permissions to protect views instead of more general ones #120

merged 5 commits into from
Feb 21, 2024

Conversation

pbauer
Copy link
Member

@pbauer pbauer commented Feb 19, 2024

This was previously part of #96

These permissions are so far only used in Volto via plone.restapi but for some reason not in Classic-UI.
We might also add checks for these permissions to the actions via browser/control.py.

@mister-roboto
Copy link

@pbauer thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@pbauer pbauer mentioned this pull request Feb 19, 2024
Open
@pbauer
Copy link
Member Author

pbauer commented Feb 19, 2024

Should we also change the rolemap-setup by adding profiles/default/rolemap.xml where we assign these to the respective roles and add a upgrade-step for that? It will make it easier to change them.

@pbauer
Copy link
Member Author

pbauer commented Feb 19, 2024

@jenkins-plone-org please run jobs

@pbauer pbauer requested a review from davisagli February 19, 2024 14:36
@jensens
Copy link
Member

jensens commented Feb 19, 2024

Should we also change the rolemap-setup by adding profiles/default/rolemap.xml where we assign these to the respective roles and add a upgrade-step for that? It will make it easier to change them.

Yes! Sure.

@pbauer
Copy link
Member Author

pbauer commented Feb 21, 2024

While implementing this I ran into something worth mentioning:

The permissions were previously setup and assigned to during startup even when plone.app.iterate is not installed.
By switching to rolemap.xml the permission are setup but not assigned to any roles unless the package is installed.
Existing sites will get the rolemap installed by the upgrade-step.

With this change a site in which plone.app.iterate was not installed has different permissions than before (before with roles, after without roles). I don't think that has a negative effect in any scenario but I thought it is worth spelling it out here.

plone.app.iterate is in the list of core-addons so the upgrade-step will be triggered by a plone-upgrade.

@pbauer pbauer requested a review from jensens February 21, 2024 06:04
@pbauer
Copy link
Member Author

pbauer commented Feb 21, 2024

@jenkins-plone-org please run jobs

pbauer added a commit to plone/Products.CMFPlone that referenced this pull request Feb 21, 2024
@pbauer
plone.app.iterate permissions now use rolemap. See plone/plone.app.it…
a8a38eb 
…erate#120
@pbauer pbauer mentioned this pull request Feb 21, 2024
Merged
@pbauer
Copy link
Member Author

pbauer commented Feb 21, 2024

Test and merge together with plone/Products.CMFPlone#3907

https://jenkins.plone.org/job/pull-request-6.0-3.8/3098/
https://jenkins.plone.org/job/pull-request-6.0-3.11/1052/
https://jenkins.plone.org/job/pull-request-6.1-3.8/159/
https://jenkins.plone.org/job/pull-request-6.1-3.12/172/

@jensens jensens merged commit 44d8322 into master Feb 21, 2024
12 checks passed
@jensens jensens deleted the use_package_permissions branch February 21, 2024 14:05
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Feb 21, 2024
@jensens
[fc] Repository: Products.CMFPlone
cd6f78c 
Branch: refs/heads/6.0.x
Date: 2024-02-21T08:32:55+01:00
Author: Philip Bauer (pbauer) <bauer@starzel.de>
Commit: plone/Products.CMFPlone@a8a38eb

plone.app.iterate permissions now use rolemap. See plone/plone.app.iterate#120

Files changed:
M Products/CMFPlone/tests/testSiteAdminRole.py
Repository: Products.CMFPlone

Branch: refs/heads/6.0.x
Date: 2024-02-21T08:56:09+01:00
Author: Philip Bauer (pbauer) <bauer@starzel.de>
Commit: plone/Products.CMFPlone@cca64cf

add changenote

Files changed:
A news/3907.bugfix
Repository: Products.CMFPlone

Branch: refs/heads/6.0.x
Date: 2024-02-21T15:05:01+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/Products.CMFPlone@996b9da

Merge pull request #3907 from plone/iterate_permissions_use_rolemap

Iterate permissions use rolemap

Files changed:
A news/3907.bugfix
M Products/CMFPlone/tests/testSiteAdminRole.py
mauritsvanrees pushed a commit to plone/Products.CMFPlone that referenced this pull request Feb 21, 2024
@pbauer @mauritsvanrees
plone.app.iterate permissions now use rolemap. See plone/plone.app.it…
255adbb 
…erate#120
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Feb 21, 2024
@mauritsvanrees
[fc] Repository: Products.CMFPlone
64b2757 
Branch: refs/heads/master
Date: 2024-02-21T16:47:10+01:00
Author: Philip Bauer (pbauer) <bauer@starzel.de>
Commit: plone/Products.CMFPlone@255adbb

plone.app.iterate permissions now use rolemap. See plone/plone.app.iterate#120

Files changed:
M Products/CMFPlone/tests/testSiteAdminRole.py
Repository: Products.CMFPlone

Branch: refs/heads/master
Date: 2024-02-21T16:47:25+01:00
Author: Philip Bauer (pbauer) <bauer@starzel.de>
Commit: plone/Products.CMFPlone@72df670

add changenote

Files changed:
A news/3907.bugfix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jensens jensens jensens approved these changes

@davisagli davisagli Awaiting requested review from davisagli

Assignees
No one assigned
Labels
04 type: enhancement 22 status: in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pbauer @mister-roboto @jensens