cloudflare / Cloudflare-WordPress

A Cloudflare plugin for WordPress
https://www.cloudflare.com/wordpress/
BSD 3-Clause "New" or "Revised" License
215 stars 84 forks source link

Empty settings page for users with custom role having the `manage_options` capability #544

Open rvdsteege opened 6 months ago

rvdsteege commented 6 months ago

Confirmation

WordPress version

6.4.3

Cloudflare-WordPress version

4.12.6

PHP version

7.4.33

Expected result

A user with a custom role which has the manage_options capability, being able to purge the cache through the Settings → Cloudflare page.

Actual result

When clicking the "Cloudflare" admin menu item, an empty page is displayed and CONFIG_FETCH_ERROR and ZONES_FETCH_ERRORS errors occur in the console.

Steps to reproduce

  1. Create a user with custom role, having the manage_options capability (e.g. using the Members plugin; https://wordpress.org/plugins/members/)
  2. Visit Settings → Cloudflare

Additional factoids

It appears that the changes from https://github.com/cloudflare/Cloudflare-WordPress/pull/529 are causing the issues (released in version 4.12.3). The "Cloudflare" admin menu item requires the manage_options capability and the WordPress AJAX action cloudflare_proxy — which seems needed to load the settings page — is checking for the administrator role.

https://github.com/cloudflare/Cloudflare-WordPress/blob/dd13e1509194ee0a15c4f737082d39cdc226ad71/src/WordPress/Hooks.php#L82-L87

https://github.com/cloudflare/Cloudflare-WordPress/blob/dd13e1509194ee0a15c4f737082d39cdc226ad71/src/WordPress/Proxy.php#L56-L60

It might be better to check against the manage_options capability in the proxy too, so both will be checking the same requirement to access the settings page.


Also, as mentioned in the WordPress developer documentation at https://developer.wordpress.org/reference/functions/current_user_can/, checking against a role instead of a capability using current_user_can() is discouraged:

While checking against particular roles in place of a capability is supported in part, this practice is discouraged as it may produce unreliable results.

https://github.com/cloudflare/Cloudflare-WordPress/blob/dd13e1509194ee0a15c4f737082d39cdc226ad71/src/WordPress/WordPressAPI.php#L159-L165

https://github.com/cloudflare/Cloudflare-WordPress/blob/58db13b91fbd5e8613a8599d58cf05d04914d7e6/src/WordPress/WordPressWrapper.php#L39-L42

References

https://github.com/cloudflare/Cloudflare-WordPress/pull/529

github-actions[bot] commented 5 days ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

remcotolsma commented 5 days ago

Is there someone from the @Cloudflare team who can review the PR and merge it if possible?