Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.58k stars 797 forks source link

WPCOM endpoint wpcom/v2/admin-menu not setting is_admin() = true -> causing issues #27442

Open Ninodevo opened 1 year ago

Ninodevo commented 1 year ago

Impacted plugin

Jetpack

Steps to Reproduce

The used plugin here is only an example that causes the issue it could also be other plugins with similar implementations.

Example 1:

  1. Install Meta Slider plugin on a WPCOM site and Activate the plugin
  2. Call the route -> https://public-api.wordpress.com/wpcom/v2/sites/{blog_id}/admin-menu (Authorization: Bearer {token})

Example 2:

  1. Install Meta Slider plugin on a WPCOM site and Activate the plugin
  2. Go to Calypso admin of the site
  3. View the error log of the site in Grafana

A clear and concise description of what you expected to happen.

Example 1: I expect to get response 200 and an admin menu item.

Example 2: I expect to not see any fatal errors related to the MetaSlider plugin.

What actually happened

Example 1:

I received error 500:

Screenshot 2022-11-16 at 14 56 52

Example 2:

I see a fatal error: PHP Fatal error: Uncaught Error: Call to a member function add_page() on null... 2e365-pb

Browser

Google Chrome/Chromium

Other information

I believe that the issue is not in the plugin but in how our Jetpack WPCOM endpoint wpcom/v2/admin-menu is implemented -> wpcom-rest-api-v2-endpoint-admin-menu.php

Here is the code 2e366-pb from the MetaSlider plugin (how the plugin is written is not important only the code I will highlight). On line 116 is_admin() check is used to init a class property and on line 282 add_action('admin_menu', array($this, 'register_admin_pages'), 9553); a hook is added to admin_menu whose callback uses the initialized class property in is_admin. The hook admin_menu is only called in the admin so that should be ok.

In the Jetpack WPCOM endpoint wpcom/v2/admin-menu the get_item function includes the wp-admin/menu file on line 91 which subsequently includes the wp-admin/includes/menu.php file and that triggers the admin_menu hook but is_admin returns false.


The endpoint wpcom/v2/admin-menu -> triggers admin_menu hook but is_admin is false which shouldn't happen

I believe is_admin should be true if we are using the wpcom/v2/admin-menu endpoint or the implementation of the route should be in some other way (not including the WP admin files).

The plugin author should be able to use the is_admin function in regard to the admin_menu hook (which should only be fired in the admin). This plugin is only an example if there are similar implementations of using is_admin in combination of the admin menu hooks those will fail.

Platform (Simple, Atomic, or both?)

Atomic

Reproducibility

Consistent

Severity

Some (< 50%)

Available workarounds?

No response

Workaround details

No response

xpurichan commented 3 months ago

Hey @jeherve, I'm wondering if you can share any insight for this issue being looked at?

jeherve commented 3 months ago

@xpurichan I don't believe anyone is looking into this issue at the moment.

@davemart-in Do you think that's something you could look at as part of the Untangling Calypso project?

davemart-in commented 3 months ago

Yup! @jeherve I just moved it to our board. Thanks for the ping!

davemart-in commented 4 weeks ago

Removing this from the logical flows board. It's no longer a great fit.