Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
<element value="jetpack_network_settings_page" />
<element value="jetpack_network_sites_page" />
<element value="jetpack_reconnect" />
<element value="jetpack_view_analytics" />
<element value="view_stats" />
<element value="view_woocommerce_reports" />
</property>
</properties>
</rule>
Expand Down
20 changes: 10 additions & 10 deletions projects/packages/premium-analytics/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,16 @@ Two local REST surfaces; almost all data comes from WordPress.com via one agnost
- `<prefix>` must be allowlisted in `PREFIX_CONFIG` or the route 404s. This is the security
boundary — the blog token is only forwarded for these.

| Prefix | Capability | Writes (POST) |
| ----------------------------------------------------------------- | ------------------ | ------------------------------- |
| `analytics` (Woo store reports) | `manage_options` | — |
| `stats` | `view_stats` | `stats/referrers/spam/` |
| `wordads` | `activate_wordads` | — |
| `subscribers` / `site-has-never-published-post` / `jetpack-stats` | `view_stats` | — |
| `jetpack-stats-dashboard` | `view_stats` | whole prefix (busts read cache) |
| `commercial-classification` | `view_stats` | exact path |
| `upgrades` (not under `/sites/`) | `view_stats` | — |
| `posts` (pattern-constrained: only `<id>/likes`) | `view_stats` | — |
| Prefix | Capability | Writes (POST) |
| ----------------------------------------------------------------- | -------------------------- | ------------------------------- |
| `analytics` (Woo store reports) | `view_woocommerce_reports` | — |
| `stats` | `view_stats` | `stats/referrers/spam/` |
| `wordads` | `activate_wordads` | — |
| `subscribers` / `site-has-never-published-post` / `jetpack-stats` | `view_stats` | — |
| `jetpack-stats-dashboard` | `view_stats` | whole prefix (busts read cache) |
| `commercial-classification` | `view_stats` | exact path |
| `upgrades` (not under `/sites/`) | `view_stats` | — |
| `posts` (pattern-constrained: only `<id>/likes`) | `view_stats` | — |

`manage_options` is always accepted too. `POST` is rejected (`405 rest_read_only`) outside the
Writes column. Query params pass through except control params (`endpoint`, `version`,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Allow users with the view_stats capability to open the Analytics dashboard, and shop managers to read the store reports.
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ class Api_Proxy_Controller extends WP_REST_Controller {
* @var array<string, array<string, mixed>>
*/
private const PREFIX_CONFIG = array(
'analytics' => array( 'capability' => 'manage_options' ),
// Gated like WooCommerce's own Analytics screens, which shop managers can read;
// woocommerce-analytics made the same move away from manage_options (WOOA7S-551).
'analytics' => array( 'capability' => 'view_woocommerce_reports' ),
'stats' => array(
'capability' => 'view_stats',
'writes' => array( 'stats/referrers/spam/' ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace Automattic\Jetpack\PremiumAnalytics\REST;

use Automattic\Jetpack\PremiumAnalytics\Capabilities;
use Automattic\Jetpack\PremiumAnalytics\Notices;
use WP_REST_Request;
use WP_REST_Server;
Expand Down Expand Up @@ -99,7 +100,7 @@ public function register_routes(): void {
* @return bool
*/
public function check_permission(): bool {
return current_user_can( 'manage_options' ) || current_user_can( 'view_stats' );
return Capabilities::current_user_can_view_analytics();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

defined( 'ABSPATH' ) || exit;

use Automattic\Jetpack\PremiumAnalytics\Capabilities;
use Automattic\Jetpack\PremiumAnalytics\Reports\Export\Logging\Logger_Interface;
use Automattic\Jetpack\PremiumAnalytics\Reports\Export\Support\Logger_Trait;
use Automattic\Jetpack\PremiumAnalytics\Reports\Export\Support\Utilities;
Expand Down Expand Up @@ -135,14 +136,14 @@ public function register_routes(): void {
/**
* Check if user has permission to export reports.
*
* Must match the capability the analytics proxy enforces (`manage_options` for the
* `analytics` prefix in Api_Proxy_Controller); otherwise the route would advertise
* access the async data fetch cannot honor, scheduling a job that then fails.
* Must match the capability the analytics proxy enforces (the `analytics` prefix
* in Api_Proxy_Controller); otherwise the route would advertise access the async
* data fetch cannot honor, scheduling a job that then fails.
*
* @return bool True if user has permission.
*/
public function check_permission(): bool {
return current_user_can( 'manage_options' );
return Capabilities::current_user_can_view_store_reports();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ private static function apply_options( $options ) {
* @return void
*/
private static function boot_shared_services() {
// Must be hooked before admin_menu and rest_api_init check the capability.
Capabilities::register();

// Emit WooCommerce store events into the Woo pipeline (ClickHouse + proxy).
WooCommerce_Analytics_Tracker::configure();

Expand Down Expand Up @@ -310,7 +313,7 @@ public static function register_admin_menu() {
add_menu_page(
esc_html( $menu_title ),
esc_html( $menu_title ),
'manage_options',
Capabilities::VIEW_ANALYTICS,
'jetpack-premium-analytics-wp-admin',
$render_callback,
'dashicons-chart-bar',
Expand Down
108 changes: 108 additions & 0 deletions projects/packages/premium-analytics/src/class-capabilities.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php
/**
* Who may see the Premium Analytics dashboard.
*
* Jetpack Stats lets a site grant non-administrators access through the
* `view_stats` meta capability, and this dashboard replaces that UI, so it has
* to honour the same grant. `add_menu_page()` takes a single capability string,
* so the "manage_options OR view_stats" rule lives in a meta capability of our
* own rather than being spelled out at each call site.
*
* A class rather than a function file so every consumer reaches it through the
* autoloader: gating lives in files loaded on several different paths, and a
* `require_once` in each of them is a dependency to keep in sync (and an
* unmeasurable line of coverage) for no benefit.
*
* @package automattic/jetpack-premium-analytics
*/

namespace Automattic\Jetpack\PremiumAnalytics;

/**
* The dashboard's capability rules.
*
* @since $$next-version$$
*/
class Capabilities {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it how we are centralizing the capabilities check 👍


/**
* Meta capability for reading the dashboard.
*/
const VIEW_ANALYTICS = 'jetpack_view_analytics';

/**
* Hooks the dashboard's meta capability mapping.
*
* Called from WordPress-aware entry points, never at load time: this class is
* autoloaded in contexts where WordPress — and add_filter() — isn't there.
* Idempotent, so overlapping callers are free to call it.
*
* @return void
*/
public static function register() {
add_filter( 'map_meta_cap', array( __CLASS__, 'map_meta_caps' ), 10, 3 );
}

/**
* Unhooks the mapping registered by register().
*
* Test tear-down needs this to drop the one filter: remove_all_filters(
* 'map_meta_cap' ) would also take out Stats' own `view_stats` mapping.
*
* @return void
*/
public static function unregister() {
remove_filter( 'map_meta_cap', array( __CLASS__, 'map_meta_caps' ), 10 );
}

/**
* Maps the dashboard capability to the primitives that grant it.
*
* `view_stats` alone would track Stats more closely, but it only means
* anything once the Stats package has hooked its own `map_meta_cap` — which
* `Analytics::init_wpcom_simple()` never does. Without the `manage_options`
* arm, an unmapped `view_stats` would take the dashboard away from
* administrators too, which is worse than the gap this closes.
*
* @param string[] $caps Primitive capabilities required of the user.
* @param string $cap Capability being checked.
* @param int $user_id User being checked.
* @return string[] Primitives for the dashboard capability; anything else untouched.
*/
public static function map_meta_caps( $caps, $cap, $user_id ) {
if ( self::VIEW_ANALYTICS !== $cap ) {
return $caps;
}

if ( user_can( $user_id, 'manage_options' ) || user_can( $user_id, 'view_stats' ) ) {
return array( 'read' );
}

return array( 'do_not_allow' );
}

/**
* Whether the current user may read the dashboard.
*
* @return bool
*/
public static function current_user_can_view_analytics() {
return current_user_can( self::VIEW_ANALYTICS );
}

/**
* Whether the current user may read the store reports.
*
* "Store reports" is everything the proxy serves from its `analytics` prefix —
* WooCommerce's own reporting data. Mirrors the capability
* {@see \Automattic\Jetpack\PremiumAnalytics\REST\Api_Proxy_Controller} enforces
* there (Capabilities_Test pins the two together); surfaces backed by the prefix
* are hidden from readers who fail it, since all they could collect is 403s.
*
* @return bool
*/
public static function current_user_can_view_store_reports() {
// The proxy accepts manage_options for every prefix.
return current_user_can( 'manage_options' ) || current_user_can( 'view_woocommerce_reports' );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ public static function boot_routes() {
require_once __DIR__ . '/dashboard-layout.php';
require_once __DIR__ . '/dashboard-sections.php';

// These routes are gated on the dashboard capability, and WPCOM Simple boots
// this class standalone, without going through Analytics::init().
Capabilities::register();

register_widget_modules_rest_route();
register_dashboard_default_layout_route();
register_dashboard_sections_rest_routes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ function register_dashboard_default_layout_route() {
array(
'methods' => \WP_REST_Server::READABLE,
'callback' => __NAMESPACE__ . '\\get_dashboard_default_layout_response',
'permission_callback' => static function () {
return current_user_can( 'manage_options' );
},
'permission_callback' => array( Capabilities::class, 'current_user_can_view_analytics' ),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This route's permission check was widened from manage_options to Capabilities::current_user_can_view_analytics() (manage_options OR view_stats), but the handler it guards (get_dashboard_default_layout_for()) doesn't re-check the store-report capability when the requested dashboard resolves to woocommerce/store.

A view_stats-only reader — exactly the population this PR admits to the dashboard but explicitly not to store data (see test_omits_woocommerce_dashboard_section_from_a_view_stats_reader) — can call GET /wpcom/v2/dashboards/traffic/default-layout?name=woocommerce/store. WP's get_param() resolves query-string values ahead of the URL route capture (the same shadowing risk Api_Proxy_Controller::validate_data_endpoint()'s ?endpoint= comment already defends against), so $request['name'] resolves to 'woocommerce/store' regardless of the URL path, and the Store tab's default widget layout (jpa/store-performance, jpa/orders-over-time, etc.) gets seeded with no current_user_can_view_store_reports() check.

Impact is limited — only default widget-type identifiers leak (strings already public in the plugin source/JS bundles), not real store data, since the proxy call for actual data stays correctly gated on view_woocommerce_reports. But it's inconsistent with the stated design goal in current_user_can_view_store_reports()'s own docblock ("surfaces backed by the prefix are hidden from readers who fail it"), and no test currently covers this route with a $name-shadowed request.

Suggest gating on Capabilities::current_user_can_view_store_reports() when the resolved section is woocommerce/store, mirroring what is_woocommerce_dashboard_section_available_to_current_user() and the /sections/{section}/default-layout route already do via Dashboard_Section::is_available().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 66833cd

'args' => array(
'name' => array(
'description' => __( 'Dashboard identifier as produced by the build pipeline.', 'jetpack-premium-analytics-pkg' ),
Expand Down
18 changes: 16 additions & 2 deletions projects/packages/premium-analytics/src/dashboard-sections.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,20 @@ function is_woocommerce_dashboard_section_available() {
return (bool) apply_filters( WOOCOMMERCE_DASHBOARD_SECTION_AVAILABLE_FILTER, $is_available );
}

/**
* Whether the current user should be shown the WooCommerce dashboard section.
*
* The sibling is_woocommerce_dashboard_section_available() answers "is
* WooCommerce here"; this adds "and may this reader see store data".
*
* @since $$next-version$$
*
* @return bool
*/
function is_woocommerce_dashboard_section_available_to_current_user() {
return is_woocommerce_dashboard_section_available() && Capabilities::current_user_can_view_store_reports();
}

/**
* Returns the default widget layout for the WooCommerce dashboard section.
*
Expand Down Expand Up @@ -108,7 +122,7 @@ function register_default_dashboard_sections() {
'woocommerce/store' => array(
'label' => __( 'Store', 'jetpack-premium-analytics-pkg' ),
'order' => 40,
'is_available' => __NAMESPACE__ . '\\is_woocommerce_dashboard_section_available',
'is_available' => __NAMESPACE__ . '\\is_woocommerce_dashboard_section_available_to_current_user',
'default_layout' => __NAMESPACE__ . '\\get_woocommerce_dashboard_section_default_layout',
),
);
Expand Down Expand Up @@ -139,7 +153,7 @@ function bootstrap_dashboard_sections() {
* @return bool
*/
function check_dashboard_sections_permission() {
return current_user_can( 'manage_options' );
return Capabilities::current_user_can_view_analytics();
}

/**
Expand Down
58 changes: 58 additions & 0 deletions projects/packages/premium-analytics/src/widget-availability.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@
*/
const WOOCOMMERCE_BOOKINGS_WIDGET_CATEGORIES = array( 'bookings' );

/**
* Widget categories whose data counts as a store report.
*
* The membership rule is the data source, not the subject matter: every
* category here reaches WPCOM through the proxy's `analytics` prefix, which
* {@see \Automattic\Jetpack\PremiumAnalytics\REST\Api_Proxy_Controller} gates
* on `view_woocommerce_reports`. That is why `visitors` is in the list: its
* widgets read `sessions/…` from the same prefix.
*/
const STORE_REPORT_WIDGET_CATEGORIES = array( 'store', 'orders', 'coupons', 'bookings', 'visitors' );

/**
* Removes developer-only candidates in production.
*
Expand Down Expand Up @@ -154,3 +165,50 @@ function filter_registrable_widget_types_by_plugin( $widget_candidates ) {
}

add_filter( REGISTRABLE_WIDGET_TYPES_FILTER, __NAMESPACE__ . '\\filter_registrable_widget_types_by_plugin' );

/**
* Removes candidates the reader could not load data for anyway.
*
* Split from the hook callback so both branches are testable without a user.
*
* @since $$next-version$$
*
* @param array $widget_candidates Manifest candidates, each with a `category`.
* @param bool $can_view_store_reports Whether the reader may see the store reports.
* @return array The candidates, minus the store-report categories for readers who can't.
*/
function remove_capability_gated_widget_types( $widget_candidates, $can_view_store_reports ) {
if ( $can_view_store_reports ) {
return $widget_candidates;
}

return array_values(
array_filter(
$widget_candidates,
static function ( $widget ) {
return ! in_array( $widget['category'] ?? '', STORE_REPORT_WIDGET_CATEGORIES, true );
}
)
);
}

/**
* Registry-time callback: hides widgets whose data the reader cannot fetch.
*
* A `view_stats` reader reaching one of them would only collect 403s from the
* proxy's `analytics` prefix, so the types are never registered for them. The
* registry is request-scoped, so filtering on the current user is safe here.
*
* @since $$next-version$$
*
* @param array $widget_candidates Manifest candidates.
* @return array The candidates, minus the store-report categories for readers who can't see them.
*/
function filter_registrable_widget_types_by_capability( $widget_candidates ) {
return remove_capability_gated_widget_types(
$widget_candidates,
Capabilities::current_user_can_view_store_reports()
);
}

add_filter( REGISTRABLE_WIDGET_TYPES_FILTER, __NAMESPACE__ . '\\filter_registrable_widget_types_by_capability' );
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ function register_widget_modules_rest_route() {
array(
'methods' => \WP_REST_Server::READABLE,
'callback' => __NAMESPACE__ . '\\get_widget_modules_response',
'permission_callback' => static function () {
return current_user_can( 'manage_options' );
},
'permission_callback' => array( Capabilities::class, 'current_user_can_view_analytics' ),
)
);
}
Expand Down
Loading
Loading