Commit 2a93210226b for woocommerce
commit 2a93210226b0b04e71937e249ba83b8fa6fccedc
Author: Hannah Tinkler <hannah.tinkler@gmail.com>
Date: Tue Sep 8 14:08:36 2026 +0100
Require a push notifications role on the status endpoint (#68337)
* Require a push notifications role on the status endpoint
The endpoint accepted any logged in user, so a subscriber or customer on a
store with open registration could read whether the store is connected to
Jetpack, whether Jetpack Sync is installed and enabled, and whether push
notifications were filtered off. That is merchant configuration, and it does
not need to be readable by accounts that cannot use push notifications.
Requests signed with the Jetpack blog token still pass, since WPCOM reads this
endpoint to decide how to reach a store and that token identifies no user. The
endpoint also stays reachable while the module is disabled, so the role check
is reinstated without the enablement check that the other endpoints apply.
* Rename the status endpoint permission callback to name who it allows
* Prove the status endpoint authorization through the registered route
The blog token test called the permission callback directly, so it could not
catch a wrong callback name in register_routes(). It now dispatches a request
through the registered route, which needs the controller subclass wired up by
hand because the container cannot resolve a class declared in the test.
The rejection test also asserts the driver configuration is absent from the
response body, rather than inferring it from the status code, and an
administrator now has a test of its own. Every success path used the shop
manager fixture before, leaving the other allowed role uncovered.
* Assert explicit status codes in the status endpoint rejection tests
rest_authorization_required_code() hides which of the two rejections each
test covers, so the tests read the same whether the request was rejected
for being logged out or for holding no push-notifications role.
diff --git a/plugins/woocommerce/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestController.php b/plugins/woocommerce/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestController.php
index a64ff978d90..cc33d9613f4 100644
--- a/plugins/woocommerce/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestController.php
+++ b/plugins/woocommerce/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestController.php
@@ -97,7 +97,7 @@ class PushNotificationStatusRestController extends RestApiControllerBase {
array(
'methods' => WP_REST_Server::READABLE,
'callback' => fn ( WP_REST_Request $request ) => $this->run( $request, 'get_status' ),
- 'permission_callback' => array( $this, 'authorize_as_from_wpcom_or_logged_in_user' ),
+ 'permission_callback' => array( $this, 'authorize_as_from_wpcom_or_allowed_user' ),
),
// A sibling of the endpoint array, not a key inside it. WP_REST_Server
// promotes only non-numeric top-level keys into its route options, and
diff --git a/plugins/woocommerce/src/Internal/PushNotifications/ROADMAP.md b/plugins/woocommerce/src/Internal/PushNotifications/ROADMAP.md
index e6c74d6dddc..908283b007a 100644
--- a/plugins/woocommerce/src/Internal/PushNotifications/ROADMAP.md
+++ b/plugins/woocommerce/src/Internal/PushNotifications/ROADMAP.md
@@ -151,7 +151,7 @@ This library will be in the `src/Internal` directory and is not intended to be u
- **Get driver status:**
- Endpoint: `GET /wp-json/wc-push-notifications/status`
- - Auth: Jetpack blog token, or any logged in user. No role is required, because WPCOM reads this endpoint and signs those requests with the blog token, which identifies no user.
+ - Auth: Jetpack blog token, or a user holding one of the roles allowed to use push notifications. The blog token identifies no user, so a role check on its own would reject the requests WPCOM makes to this endpoint.
- Returns the installed notification drivers, each with `connected` (its underlying connection is present), `enabled` (the driver itself isn't disabled), and `available` (`connected && enabled`, i.e. configured and usable, which is not a statement about delivery) flags, plus the `preferred_driver`, the first available driver in precedence order. `preferred_driver` is the site's preference, not a statement about what is delivering notifications to a given app, which also depends on the app version and on whether its token registered successfully. The `jetpack-sync` driver is listed only when the Jetpack Sync package is installed; the `remote-push-notification-proxy` driver ships with core and is always listed. Stays reachable even when push notifications are disabled, so clients can read the driver state and fall back to Jetpack Sync when the proxy isn't available.
```json
diff --git a/plugins/woocommerce/src/Internal/PushNotifications/Traits/AuthorizesPushNotificationRequests.php b/plugins/woocommerce/src/Internal/PushNotifications/Traits/AuthorizesPushNotificationRequests.php
index cc18e69af35..89c3a7c5602 100644
--- a/plugins/woocommerce/src/Internal/PushNotifications/Traits/AuthorizesPushNotificationRequests.php
+++ b/plugins/woocommerce/src/Internal/PushNotifications/Traits/AuthorizesPushNotificationRequests.php
@@ -29,49 +29,58 @@ trait AuthorizesPushNotificationRequests {
* @return bool|WP_Error
*/
public function authorize_as_authenticated( WP_REST_Request $request ) {
- if ( ! get_current_user_id() ) {
- return new WP_Error(
- 'woocommerce_rest_cannot_view',
- __( 'Sorry, you are not allowed to do that.', 'woocommerce' ),
- array( 'status' => rest_authorization_required_code() )
- );
- }
+ $authorized = $this->authorize_as_authenticated_ignoring_enablement( $request );
- $has_valid_role = array_reduce(
- PushNotifications::ROLES_WITH_PUSH_NOTIFICATIONS_ENABLED,
- fn ( $carry, $role ) => $this->check_permission( $request, $role ) === true ? true : $carry,
- false
- );
-
- if ( ! $has_valid_role ) {
- return false;
+ if ( true !== $authorized ) {
+ return $authorized;
}
return wc_get_container()->get( PushNotifications::class )->should_be_enabled();
}
/**
- * Checks the caller is either WPCOM or a logged in user, with no role
- * requirement and without requiring the module to be enabled.
+ * Checks the caller is either WPCOM or an allowed user, without requiring
+ * the module to be enabled.
*
* WPCOM reads this endpoint to decide how to reach a store, and signs those
* requests with the Jetpack blog token, which identifies no user. Requiring a
- * role would reject them. The response describes driver configuration only,
- * so it carries nothing specific to the calling user or to the merchant.
+ * role would reject them.
*
+ * @param WP_REST_Request $request The request object.
+ * @phpstan-param WP_REST_Request<array<string, mixed>> $request
* @return bool|WP_Error
*
* @since 11.2.0
*/
- public function authorize_as_from_wpcom_or_logged_in_user() {
- if ( $this->is_signed_with_blog_token() || get_current_user_id() ) {
+ public function authorize_as_from_wpcom_or_allowed_user( WP_REST_Request $request ) {
+ if ( $this->is_signed_with_blog_token() ) {
return true;
}
- return new WP_Error(
- 'woocommerce_rest_cannot_view',
- __( 'Sorry, you are not allowed to do that.', 'woocommerce' ),
- array( 'status' => rest_authorization_required_code() )
+ return $this->authorize_as_authenticated_ignoring_enablement( $request );
+ }
+
+ /**
+ * Checks the user is authenticated and holds at least one role allowed to
+ * interact with push notifications.
+ *
+ * @param WP_REST_Request $request The request object.
+ * @phpstan-param WP_REST_Request<array<string, mixed>> $request
+ * @return bool|WP_Error
+ */
+ private function authorize_as_authenticated_ignoring_enablement( WP_REST_Request $request ) {
+ if ( ! get_current_user_id() ) {
+ return new WP_Error(
+ 'woocommerce_rest_cannot_view',
+ __( 'Sorry, you are not allowed to do that.', 'woocommerce' ),
+ array( 'status' => rest_authorization_required_code() )
+ );
+ }
+
+ return array_reduce(
+ PushNotifications::ROLES_WITH_PUSH_NOTIFICATIONS_ENABLED,
+ fn ( $carry, $role ) => $this->check_permission( $request, $role ) === true ? true : $carry,
+ false
);
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestControllerTest.php b/plugins/woocommerce/tests/php/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestControllerTest.php
index 9cb65a85c4b..9258f11b275 100644
--- a/plugins/woocommerce/tests/php/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestControllerTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/PushNotifications/Controllers/PushNotificationStatusRestControllerTest.php
@@ -43,6 +43,13 @@ class PushNotificationStatusRestControllerTest extends WC_Unit_Test_Case {
*/
private static $fixture_subscriber_id;
+ /**
+ * Administrator fixture user ID.
+ *
+ * @var int
+ */
+ private static $fixture_administrator_id;
+
/**
* Shop manager user ID for testing.
*
@@ -57,14 +64,22 @@ class PushNotificationStatusRestControllerTest extends WC_Unit_Test_Case {
*/
private $subscriber_id;
+ /**
+ * Administrator user ID for testing.
+ *
+ * @var int
+ */
+ private $administrator_id;
+
/**
* Create immutable users shared by the test class.
*
* @param WP_UnitTest_Factory $factory WordPress unit test factory.
*/
public static function wpSetUpBeforeClass( $factory ): void {
- self::$fixture_user_id = $factory->user->create( array( 'role' => 'shop_manager' ) );
- self::$fixture_subscriber_id = $factory->user->create( array( 'role' => 'subscriber' ) );
+ self::$fixture_user_id = $factory->user->create( array( 'role' => 'shop_manager' ) );
+ self::$fixture_subscriber_id = $factory->user->create( array( 'role' => 'subscriber' ) );
+ self::$fixture_administrator_id = $factory->user->create( array( 'role' => 'administrator' ) );
}
/**
@@ -75,22 +90,49 @@ class PushNotificationStatusRestControllerTest extends WC_Unit_Test_Case {
$this->reset_push_notifications_cache();
- $this->user_id = self::$fixture_user_id;
- $this->subscriber_id = self::$fixture_subscriber_id;
+ $this->user_id = self::$fixture_user_id;
+ $this->subscriber_id = self::$fixture_subscriber_id;
+ $this->administrator_id = self::$fixture_administrator_id;
}
/**
- * Register the controller's routes using the container so init() auto-wires
- * the push-notifications dependencies.
+ * Register the controller's routes, defaulting to the container instance so
+ * init() auto-wires the push-notifications dependencies.
+ *
+ * @param PushNotificationStatusRestController|null $controller Controller to register, or null for the container instance.
*/
- private function register_routes(): void {
- $controller = wc_get_container()->get( PushNotificationStatusRestController::class );
+ private function register_routes( ?PushNotificationStatusRestController $controller = null ): void {
+ $controller ??= wc_get_container()->get( PushNotificationStatusRestController::class );
$this->server = $this->create_rest_server_with_routes(
array( array( $controller, 'register_routes' ) ),
true
);
}
+ /**
+ * Builds a controller that reports every request as signed with the Jetpack
+ * blog token, wired up by hand because the container cannot resolve a
+ * subclass declared here.
+ *
+ * @return PushNotificationStatusRestController
+ */
+ private function create_blog_token_signed_controller(): PushNotificationStatusRestController {
+ $controller = new class() extends PushNotificationStatusRestController {
+ /**
+ * Stands in for a request WPCOM signed with the Jetpack blog token.
+ *
+ * @return bool
+ */
+ protected function is_signed_with_blog_token(): bool {
+ return true;
+ }
+ };
+
+ $controller->init( wc_get_container()->get( DriverAvailabilityService::class ) );
+
+ return $controller;
+ }
+
/**
* Tear down test.
*/
@@ -115,13 +157,13 @@ class PushNotificationStatusRestControllerTest extends WC_Unit_Test_Case {
$request = new WP_REST_Request( 'GET', '/wc-push-notifications/status' );
$response = $this->server->dispatch( $request );
- $this->assertSame( rest_authorization_required_code(), $response->get_status() );
+ $this->assertSame( WP_Http::UNAUTHORIZED, $response->get_status() );
}
/**
- * @testdox GET should accept a logged in user who holds no push-notifications role.
+ * @testdox GET should reject a logged in user who holds no push-notifications role.
*/
- public function test_get_status_accepts_users_without_role() {
+ public function test_get_status_rejects_users_without_role() {
wp_set_current_user( $this->subscriber_id );
$this->mock_jetpack_connection_manager_is_connected( true );
$this->register_routes();
@@ -129,30 +171,46 @@ class PushNotificationStatusRestControllerTest extends WC_Unit_Test_Case {
$request = new WP_REST_Request( 'GET', '/wc-push-notifications/status' );
$response = $this->server->dispatch( $request );
+ $this->assertSame( WP_Http::FORBIDDEN, $response->get_status() );
+
+ $data = $response->get_data();
+ $this->assertArrayNotHasKey( 'installed_drivers', $data );
+ $this->assertArrayNotHasKey( 'preferred_driver', $data );
+ }
+
+ /**
+ * @testdox GET should accept an administrator.
+ */
+ public function test_get_status_accepts_administrators() {
+ wp_set_current_user( $this->administrator_id );
+ $this->mock_jetpack_connection_manager_is_connected( true );
+ $this->register_routes();
+
+ $request = new WP_REST_Request( 'GET', '/wc-push-notifications/status' );
+ $response = $this->server->dispatch( $request );
+
$this->assertSame( WP_Http::OK, $response->get_status() );
+ $this->assertArrayHasKey( 'installed_drivers', $response->get_data() );
}
/**
* WPCOM signs its requests with the Jetpack blog token, which identifies no
- * user, so the endpoint has to authorize them without one.
+ * user, so the endpoint has to authorize them without one. Dispatched through
+ * the registered route so that the route's permission callback is the one
+ * under test.
*
* @testdox GET should accept a blog token signed request that carries no user.
*/
public function test_get_status_accepts_a_blog_token_signed_request_without_a_user() {
wp_set_current_user( 0 );
+ $this->mock_jetpack_connection_manager_is_connected( true );
+ $this->register_routes( $this->create_blog_token_signed_controller() );
- $controller = new class() extends PushNotificationStatusRestController {
- /**
- * Stands in for a request WPCOM signed with the Jetpack blog token.
- *
- * @return bool
- */
- protected function is_signed_with_blog_token(): bool {
- return true;
- }
- };
+ $request = new WP_REST_Request( 'GET', '/wc-push-notifications/status' );
+ $response = $this->server->dispatch( $request );
- $this->assertTrue( $controller->authorize_as_from_wpcom_or_logged_in_user() );
+ $this->assertSame( WP_Http::OK, $response->get_status() );
+ $this->assertArrayHasKey( 'installed_drivers', $response->get_data() );
}
/**