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.

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

Lack of upgrade step message shown to a Site Administrator #3906

Open
wesleybl opened this issue Feb 15, 2024 · 6 comments
Open

Lack of upgrade step message shown to a Site Administrator #3906

wesleybl opened this issue Feb 15, 2024 · 6 comments

Comments

@wesleybl
Copy link
Member

Describe the bug
On sites that lack the Plone upgrade step, the message The site configuration is outdated and needs to be upgraded. Please continue with the upgrade. for a user with the Site Administrator role.

However, when clicking the link, a blank screen appears. I don't think the Site Administrator has permission to do this.

To Reproduce
Steps to reproduce the behavior:

  1. Make logging into a portal with lack Plone upgrade step with a user with the Site Administrator role.
  2. Go to Site Setup (http://localhost:3000/controlpanel)
  3. See the upgrade message

Expected behavior
Do not show the message to a Site Administrator.

Screenshots
If applicable, add screenshots to help explain your problem.
download (10)

Software (please complete the following information):

  • OS: Ubuntu
  • Browser [e.g. chrome, safari]: Chrome
  • Volto Version : 18.0.0-alpha.10
  • Plone Version: 6.0.7
  • Plone REST API Version 9.4.1
@kanhaiya04
Copy link

We can incorporate the condition userHasRoles(user, ['Manager']) when rendering the message.

I will create the PR soon if this approach suits well.

@davisagli
Copy link
Sponsor Member

@kanhaiya04 That doesn't sound like the right solution to me.

@wesleybl I think we should give users Site Administrators permission to run upgrade steps, rather than removing the message for them. The intent of the Site Administrator role is to allow all actions that affect a single Plone site, but prevent actions that could affect another Plone site in the same Zope instance. Does this sound right for your use case?

Also, whatever we decide, we should check for a permission rather than a role. This keeps things flexible, so that the permission can be assigned to different roles in the policy for a specific deployment. For example, if @wesleybl disagrees that Site Administrators should be able to do this for his deployment, he can simply remove the permission from the Site Administrator role.

For me, checking roles instead of permissions is always a sign of an incorrect design.

@wesleybl
Copy link
Member Author

@davisagli I don't think the Site Administrator should have permission to run upgrade step. Whoever run the upgrade step must know what they are doing. Generally we give permission to Site Administrator, "common users", who do not have this knowledge. Upgrade steps can be time-consuming and must be performed in specific windows. Upgrade steps may require permissions that only the Manager has, such as creating an index in the catalog.

I also agree that verification should be by permission.

@davisagli
Copy link
Sponsor Member

@wesleybl I think a reasonable argument can be made for enabling it or disabling it for Site Administrators in different scenarios. It sounds like we at least agree on adding a permission to control it, which provides that flexibility. I'm okay with keeping the permission disabled by default.

@davisagli
Copy link
Sponsor Member

This is a medium-level task that requires changes in a number of places:

  1. Define the permission in CMFPlone
  2. Check the permission in the plone.restapi service that does upgrades
  3. Check the permission in the Classic UI view that does upgrades
  4. Check the permission in the volto component that shows this message

I'll move this issue to CMFPlone since the scope goes beyond just volto.

@davisagli davisagli transferred this issue from plone/volto Feb 20, 2024
@wesleybl
Copy link
Member Author

I think a reasonable argument can be made for enabling it or disabling it for Site Administrators in different scenarios.

@davisagli I think that whoever created an update step doesn't think "will the Site Administrator have permission to do this?"

Those who would give permission to the Site Administrator and those who create the upgrade are different people. Giving him permission and him receiving Unauthorized is not a smart solution.

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

No branches or pull requests

3 participants