From 7339288bda2baed511d745c734a61a028e371025 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Wed, 24 Aug 2022 02:34:44 +0600 Subject: [PATCH 1/9] Add exception classes for invalid metrics and dimensions. --- .../Invalid_Dimensions_Exception.php | 75 +++++++++++++++++++ .../Exception/Invalid_Metrics_Exception.php | 75 +++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php create mode 100644 includes/Core/REST_API/Exception/Invalid_Metrics_Exception.php diff --git a/includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php b/includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php new file mode 100644 index 00000000000..87e2acd018e --- /dev/null +++ b/includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php @@ -0,0 +1,75 @@ +getMessage(), + array( + 'status' => 400, // Bad request. + ) + ); + } +} diff --git a/includes/Core/REST_API/Exception/Invalid_Metrics_Exception.php b/includes/Core/REST_API/Exception/Invalid_Metrics_Exception.php new file mode 100644 index 00000000000..50c0079780c --- /dev/null +++ b/includes/Core/REST_API/Exception/Invalid_Metrics_Exception.php @@ -0,0 +1,75 @@ +getMessage(), + array( + 'status' => 400, // Bad request. + ) + ); + } +} From ed28b0e3cc733cabfffa470039bec2d7d1f4b0fa Mon Sep 17 00:00:00 2001 From: nfmohit Date: Wed, 24 Aug 2022 02:35:47 +0600 Subject: [PATCH 2/9] Update validation methods to use new exception classes. --- includes/Modules/Analytics.php | 86 ++++++++++------------------------ 1 file changed, 26 insertions(+), 60 deletions(-) diff --git a/includes/Modules/Analytics.php b/includes/Modules/Analytics.php index ef44e2c1ff6..b07785d5abf 100644 --- a/includes/Modules/Analytics.php +++ b/includes/Modules/Analytics.php @@ -23,6 +23,8 @@ use Google\Site_Kit\Core\Modules\Module_With_Owner; use Google\Site_Kit\Core\Modules\Module_With_Owner_Trait; use Google\Site_Kit\Core\REST_API\Exception\Invalid_Datapoint_Exception; +use Google\Site_Kit\Core\REST_API\Exception\Invalid_Metrics_Exception; +use Google\Site_Kit\Core\REST_API\Exception\Invalid_Dimensions_Exception; use Google\Site_Kit\Core\Authentication\Google_Proxy; use Google\Site_Kit\Core\Assets\Asset; use Google\Site_Kit\Core\Assets\Script; @@ -543,22 +545,7 @@ function ( $dimension_def ) { ); if ( ! empty( $dimensions ) ) { - $invalid_dimensions_error_message = $this->validate_report_dimensions( - array_map( - function ( $dimension_def ) { - return $dimension_def->getName(); - }, - $dimensions - ) - ); - - if ( isset( $invalid_dimensions_error_message ) ) { - return new WP_Error( - 'invalid_dimensions', - $invalid_dimensions_error_message, - array( 'status' => 400 ) - ); - } + $this->validate_report_dimensions( $dimensions ); $request_args['dimensions'] = $dimensions; } @@ -658,22 +645,7 @@ function ( $metric_def ) { ); if ( ! empty( $metrics ) ) { - $invalid_metrics_error_message = $this->validate_report_metrics( - array_map( - function ( $metric_def ) { - return $metric_def->getExpression(); - }, - $metrics - ) - ); - - if ( isset( $invalid_metrics_error_message ) ) { - return new WP_Error( - 'invalid_metrics', - $invalid_metrics_error_message, - array( 'status' => 400 ) - ); - } + $this->validate_report_metrics( $metrics ); $request->setMetrics( $metrics ); } @@ -1372,7 +1344,7 @@ public function check_service_entity_access() { * * @param array $metrics The metrics to validate. * - * @return null|WP_Error NULL if metrics are valid, otherwise WP_Error. + * @throws Invalid_Metrics_Exception Thrown if the metrics are invalid. */ protected function validate_report_metrics( $metrics ) { if ( false === $this->is_using_shared_credentials ) { @@ -1395,22 +1367,19 @@ protected function validate_report_metrics( $metrics ) { ) ); - $invalid_metrics = array_diff( $metrics, $valid_metrics ); + $invalid_metrics = array_diff( + array_map( + function ( $metric ) { + return $metric->getExpression(); + }, + $metrics + ), + $valid_metrics + ); if ( count( $invalid_metrics ) > 0 ) { - return sprintf( - /* translators: %s is replaced with a comma separated list of the invalid metrics. */ - _n( - 'Unsupported metric requested: %s', - 'Unsupported metrics requested: %s', - count( $invalid_metrics ), - 'google-site-kit' - ), - implode( ', ', $invalid_metrics ) - ); + throw new Invalid_Metrics_Exception( '', 0, null, $invalid_metrics ); } - - return null; } /** @@ -1420,7 +1389,7 @@ protected function validate_report_metrics( $metrics ) { * * @param array $dimensions The dimensions to validate. * - * @return null|WP_Error NULL if dimensions are valid, otherwise WP_Error. + * @throws Invalid_Dimensions_Exception Thrown if the dimensions are invalid. */ protected function validate_report_dimensions( $dimensions ) { if ( false === $this->is_using_shared_credentials ) { @@ -1440,22 +1409,19 @@ protected function validate_report_dimensions( $dimensions ) { ) ); - $invalid_dimensions = array_diff( $dimensions, $valid_dimensions ); + $invalid_dimensions = array_diff( + array_map( + function ( $dimension ) { + return $dimension->getName(); + }, + $dimensions + ), + $valid_dimensions + ); if ( count( $invalid_dimensions ) > 0 ) { - return sprintf( - /* translators: %s is replaced with a comma separated list of the invalid dimensions. */ - _n( - 'Unsupported dimension requested: %s', - 'Unsupported dimensions requested: %s', - count( $invalid_dimensions ), - 'google-site-kit' - ), - implode( ', ', $invalid_dimensions ) - ); + throw new Invalid_Dimensions_Exception( '', 0, null, $invalid_dimensions ); } - - return null; } } From e1d9a2bd747faf85e0244bea76503298f5ef4923 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Wed, 24 Aug 2022 02:42:21 +0600 Subject: [PATCH 3/9] Change shared credentials usage property only for the current request. --- includes/Core/Modules/Module.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/includes/Core/Modules/Module.php b/includes/Core/Modules/Module.php index 18fbcd89900..f742660cb00 100644 --- a/includes/Core/Modules/Module.php +++ b/includes/Core/Modules/Module.php @@ -338,6 +338,9 @@ final protected function execute_data_request( Data_Request $data ) { $datapoint = $this->get_datapoint_definition( "{$data->method}:{$data->datapoint}" ); $oauth_client = $this->get_oauth_client_for_datapoint( $datapoint ); + // Set request as using shared credentials to false. + $this->is_using_shared_credentials = false; + $this->validate_datapoint_scopes( $datapoint, $oauth_client ); $this->validate_base_scopes( $oauth_client ); From f0d5cd0e77e290eaec4d089140a4e8bc2382cf7a Mon Sep 17 00:00:00 2001 From: nfmohit Date: Wed, 24 Aug 2022 03:35:56 +0600 Subject: [PATCH 4/9] Update comment to be more useful. --- includes/Core/Modules/Module.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/Core/Modules/Module.php b/includes/Core/Modules/Module.php index f742660cb00..343be458fa3 100644 --- a/includes/Core/Modules/Module.php +++ b/includes/Core/Modules/Module.php @@ -338,7 +338,7 @@ final protected function execute_data_request( Data_Request $data ) { $datapoint = $this->get_datapoint_definition( "{$data->method}:{$data->datapoint}" ); $oauth_client = $this->get_oauth_client_for_datapoint( $datapoint ); - // Set request as using shared credentials to false. + // Always reset this property first to ensure it is only set true for the current request. $this->is_using_shared_credentials = false; $this->validate_datapoint_scopes( $datapoint, $oauth_client ); From 97474ec7dec07bc9a4289ce5804ad9d535a6b5cd Mon Sep 17 00:00:00 2001 From: nfmohit Date: Wed, 24 Aug 2022 03:36:25 +0600 Subject: [PATCH 5/9] Change namespaces for the new exception classes. --- .../Exception/Invalid_Dimensions_Exception.php | 10 +++++----- .../Exception/Invalid_Metrics_Exception.php | 10 +++++----- includes/Modules/Analytics.php | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) rename includes/Core/{REST_API => Validation}/Exception/Invalid_Dimensions_Exception.php (84%) rename includes/Core/{REST_API => Validation}/Exception/Invalid_Metrics_Exception.php (84%) diff --git a/includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php b/includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php similarity index 84% rename from includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php rename to includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php index 87e2acd018e..ccb285e5983 100644 --- a/includes/Core/REST_API/Exception/Invalid_Dimensions_Exception.php +++ b/includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php @@ -1,17 +1,17 @@ Date: Wed, 24 Aug 2022 03:40:33 +0600 Subject: [PATCH 6/9] Update params to expect an array of the appropriate instance. --- includes/Modules/Analytics.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/Modules/Analytics.php b/includes/Modules/Analytics.php index 1c7d4f603c9..bc9b2f43f52 100644 --- a/includes/Modules/Analytics.php +++ b/includes/Modules/Analytics.php @@ -1342,7 +1342,7 @@ public function check_service_entity_access() { * * @since n.e.x.t * - * @param array $metrics The metrics to validate. + * @param Google_Service_AnalyticsReporting_Metric[] $metrics The metrics to validate. * * @throws Invalid_Metrics_Exception Thrown if the metrics are invalid. */ @@ -1387,7 +1387,7 @@ function ( $metric ) { * * @since n.e.x.t * - * @param array $dimensions The dimensions to validate. + * @param Google_Service_AnalyticsReporting_Dimension[] $dimensions The dimensions to validate. * * @throws Invalid_Dimensions_Exception Thrown if the dimensions are invalid. */ From 89771bfeaf45944c9c208971994918b088694d24 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 24 Aug 2022 14:48:10 -0400 Subject: [PATCH 7/9] Refactor validation exceptions. --- .../Invalid_Dimensions_Exception.php | 75 ------------------- .../Exception/Invalid_Metrics_Exception.php | 75 ------------------- .../Invalid_Report_Dimensions_Exception.php | 24 ++++++ .../Invalid_Report_Metrics_Exception.php | 24 ++++++ includes/Modules/Analytics.php | 62 ++++++++++++--- 5 files changed, 100 insertions(+), 160 deletions(-) delete mode 100644 includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php delete mode 100644 includes/Core/Validation/Exception/Invalid_Metrics_Exception.php create mode 100644 includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php create mode 100644 includes/Core/Validation/Exception/Invalid_Report_Metrics_Exception.php diff --git a/includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php b/includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php deleted file mode 100644 index ccb285e5983..00000000000 --- a/includes/Core/Validation/Exception/Invalid_Dimensions_Exception.php +++ /dev/null @@ -1,75 +0,0 @@ -getMessage(), - array( - 'status' => 400, // Bad request. - ) - ); - } -} diff --git a/includes/Core/Validation/Exception/Invalid_Metrics_Exception.php b/includes/Core/Validation/Exception/Invalid_Metrics_Exception.php deleted file mode 100644 index 53935fd2acd..00000000000 --- a/includes/Core/Validation/Exception/Invalid_Metrics_Exception.php +++ /dev/null @@ -1,75 +0,0 @@ -getMessage(), - array( - 'status' => 400, // Bad request. - ) - ); - } -} diff --git a/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php b/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php new file mode 100644 index 00000000000..bae73ce761b --- /dev/null +++ b/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php @@ -0,0 +1,24 @@ +validate_report_dimensions( $dimensions ); + try { + $this->validate_report_dimensions( $dimensions ); + } catch ( Invalid_Report_Dimensions_Exception $exception ) { + return new WP_Error( + 'invalid_analytics_report_dimensions', + $exception->getMessage() + ); + } $request_args['dimensions'] = $dimensions; } @@ -645,7 +652,14 @@ function ( $metric_def ) { ); if ( ! empty( $metrics ) ) { - $this->validate_report_metrics( $metrics ); + try { + $this->validate_report_metrics( $metrics ); + } catch ( Invalid_Report_Metrics_Exception $exception ) { + return new WP_Error( + 'invalid_analytics_report_metrics', + $exception->getMessage() + ); + } $request->setMetrics( $metrics ); } @@ -1343,8 +1357,7 @@ public function check_service_entity_access() { * @since n.e.x.t * * @param Google_Service_AnalyticsReporting_Metric[] $metrics The metrics to validate. - * - * @throws Invalid_Metrics_Exception Thrown if the metrics are invalid. + * @throws Invalid_Report_Metrics_Exception Thrown if the metrics are invalid. */ protected function validate_report_metrics( $metrics ) { if ( false === $this->is_using_shared_credentials ) { @@ -1378,7 +1391,22 @@ function ( $metric ) { ); if ( count( $invalid_metrics ) > 0 ) { - throw new Invalid_Metrics_Exception( '', 0, null, $invalid_metrics ); + $message = sprintf( + /* translators: %s is replaced with a comma separated list of the invalid metrics. */ + _n( + 'Unsupported metric requested: %s', + 'Unsupported metrics requested: %s', + count( $invalid_metrics ), + 'google-site-kit' + ), + join( + /* translators: used between list items, there is a space after the comma. */ + __( ', ', 'google-site-kit' ), + $invalid_metrics + ) + ); + + throw new Invalid_Report_Metrics_Exception( $message ); } } @@ -1388,8 +1416,7 @@ function ( $metric ) { * @since n.e.x.t * * @param Google_Service_AnalyticsReporting_Dimension[] $dimensions The dimensions to validate. - * - * @throws Invalid_Dimensions_Exception Thrown if the dimensions are invalid. + * @throws Invalid_Report_Dimensions_Exception Thrown if the dimensions are invalid. */ protected function validate_report_dimensions( $dimensions ) { if ( false === $this->is_using_shared_credentials ) { @@ -1420,7 +1447,22 @@ function ( $dimension ) { ); if ( count( $invalid_dimensions ) > 0 ) { - throw new Invalid_Dimensions_Exception( '', 0, null, $invalid_dimensions ); + $message = sprintf( + /* translators: %s is replaced with a comma separated list of the invalid dimensions. */ + _n( + 'Unsupported dimension requested: %s', + 'Unsupported dimensions requested: %s', + count( $dimensions ), + 'google-site-kit' + ), + join( + /* translators: used between list items, there is a space after the comma. */ + __( ', ', 'google-site-kit' ), + $dimensions + ) + ); + + throw new Invalid_Report_Dimensions_Exception( $message ); } } From a89ee0953f9899a24cf96e42df38c5425f3377cc Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 24 Aug 2022 15:08:23 -0400 Subject: [PATCH 8/9] Fix dimensions referenced in exception. --- includes/Modules/Analytics.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/Modules/Analytics.php b/includes/Modules/Analytics.php index 50d9c7d174d..c6e678dd261 100644 --- a/includes/Modules/Analytics.php +++ b/includes/Modules/Analytics.php @@ -1452,13 +1452,13 @@ function ( $dimension ) { _n( 'Unsupported dimension requested: %s', 'Unsupported dimensions requested: %s', - count( $dimensions ), + count( $invalid_dimensions ), 'google-site-kit' ), join( /* translators: used between list items, there is a space after the comma. */ __( ', ', 'google-site-kit' ), - $dimensions + $invalid_dimensions ) ); From dbd3d9eeb3c5c7d28e472d0cb5aa2bbed70038b7 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Wed, 24 Aug 2022 15:09:33 -0400 Subject: [PATCH 9/9] Fix docblocks for exceptions. --- .../Exception/Invalid_Report_Dimensions_Exception.php | 2 +- .../Validation/Exception/Invalid_Report_Metrics_Exception.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php b/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php index bae73ce761b..6ab3a690ad7 100644 --- a/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php +++ b/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php @@ -1,6 +1,6 @@