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

fix: solve customizer permissions on multisites #4033 #4034

Merged

Conversation

preda-bogdan
Copy link
Contributor

Summary

Changed capability for the page we register to point back to the Customizer.
The previous capability was activate_plugins, now it is set to manage_options only for this page.

Will affect the visual aspect of the product

NO

Screenshots

image

Test instructions

  1. Create a fresh multisite instance that is using Neve or use an existing one and enable Neve
  2. Create a user with the role of Administrator to use instead of the Super Admin account
  3. Login with the Administrator account
  4. Check that you can access the Customizer, previously it will not allow the user as it does not have the capability to activate plugins.

Check before Pull Request is ready:

Closes #4033.

@preda-bogdan preda-bogdan added the pr-checklist-skip Allow this Pull Request to skip checklist. label Jul 13, 2023
@pirate-bot pirate-bot added the pr-checklist-complete The Pull Request checklist is complete. (automatic label) label Jul 13, 2023
@pirate-bot
Copy link
Collaborator

Plugin build for 0b902b8 is ready 🛎️!

@rodica-andronache
Copy link
Contributor

@preda-bogdan accessing Customizer works well now 👍
I'm just not sure about how we need to handle access to the Neve Options page. Right now, that's not accessible for admins ( without super admin role ), meaning that those admins also don't have access to Neve Pro options once the plugin is installed https://vertis.d.pr/i/fkQpDM
I've checked what Astra is doing, and they are showing Astra's Options in this case https://vertis.d.pr/i/xrFCpV

I'm thinking that the same thing should happen in Neve too, meaning admins should have access to Neve Options page. Do you think that makes sense? If so, should this be done in this PR or should I create a new issue for it?
Thanks!

@preda-bogdan
Copy link
Contributor Author

@rodica-andronache Yes, it makes total sense. I will include the required changes in this PR and add more testing scenarios. Thank you!

@preda-bogdan
Copy link
Contributor Author

preda-bogdan commented Jul 13, 2023

@rodica-andronache I had a closer look at what the changes would look like and I think we should better open a new issue to implement this.
This was not previously supported (tested with v3.5.8), and it will require some more changes to work properly, here are the issues that need addressing:

  1. The Options page will be available if I change the capability but we will need to hide the Starter Sites section, and Plugins section as the user does not have any permissions to install plugins.
    image

  2. Another change that is required is to sync the changes with the Custom Layouts to allow them to be displayed and used.

  3. Settings that have an CTA to install should be hidden
    image
    image

In conclusion, I think this would need some further discussion so as to not create issues with these changes.

We can keep the scope of this PR to just fixing the Customiser access.

Let me know your input on this.

cc: @HardeepAsrani

@HardeepAsrani
Copy link
Member

@preda-bogdan Agreed. We can fix the Customizer access and work out better permissions in a separate issue. Will be good first take a look at this from a user point of view and see different users and what options they currently have and what will make sense for scenarios, ie superadmins and admins. Such as, we can’t allow admins to install new plugins but I think we can still allow them to activate it if they’re already installed. Or if we don’t want to show that part at all if they can’t install plugins. Let me know your thoughts.

@rodica-andronache
Copy link
Contributor

@preda-bogdan ok, thank you for looking into this! I've created a new issue and moved the current issue to Ready to merge

@cristian-ungureanu cristian-ungureanu merged commit c104fc9 into development Aug 2, 2023
17 checks passed
@cristian-ungureanu cristian-ungureanu deleted the fix/customizer_permission_multisite branch August 2, 2023 10:45
@pirate-bot
Copy link
Collaborator

🎉 This PR is included in version 3.6.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-checklist-complete The Pull Request checklist is complete. (automatic label) pr-checklist-skip Allow this Pull Request to skip checklist. released Indicate that an issue has been resolved and released in a particular version of the product.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customization access denied for non super admin user on multisite network
5 participants