search

plone / plone.app.content

Various views for Plone, such as folder_contents, as well as general content infrastructure, such as base classes and name choosers.
https://pypi.org/project/plone.app.content/
7 stars 32 forks source link

Allow access to content_status_modify page for all with View permission. #235

Closed mauritsvanrees closed 2 years ago

mauritsvanrees commented 2 years ago

This was the case when it was still a skin script. The internal logic of the page makes sure you can only really change anything when you have the proper permission.

Fixes https://github.com/plone/Products.CMFPlone/issues/3338, where an Editor could no longer submit a page.

mister-roboto commented 2 years ago

@mauritsvanrees thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

mauritsvanrees commented 2 years ago

@jenkins-plone-org please run jobs

1letter commented 2 years ago

@mauritsvanrees I'm not sure that the zope2.View Permission for this view is right the solution. Now can everyone call this View, also a anonymous user. Perhaps, we should add a condition in the BrowserView like if Anonymous then return?

mauritsvanrees commented 2 years ago

In default Plone this would be fine. But there may be workflows out there that allow anonymous visitors to create content and then submit it. That should still work. I think the old Products.Poi issue tracker worked like that.

Try it as anonymous user on the Plone 5 demo site. The link tries to retract a published page. This even results in a status message "Item state changed", even though the state has not actually changed. So there is room for improvement, also in 5.2.

But it looks safe to me. But if someone else from the @plone/security-team could have a look for a second opinion, that would be good.

mauritsvanrees commented 2 years ago

I feel confident on this one. Let's merge.