From b5508135ce75f26fa7d71b3ddd1653c0a422e6d5 Mon Sep 17 00:00:00 2001 From: sgiehl Date: Thu, 20 Jul 2023 10:23:19 +0200 Subject: [PATCH] Validate report period for scheduled reports --- plugins/ScheduledReports/API.php | 20 +++++ plugins/ScheduledReports/Controller.php | 7 +- .../tests/Integration/ApiTest.php | 78 ++++++++++++++++++- 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/plugins/ScheduledReports/API.php b/plugins/ScheduledReports/API.php index a2f06cd480c..aa9d6489022 100644 --- a/plugins/ScheduledReports/API.php +++ b/plugins/ScheduledReports/API.php @@ -122,6 +122,10 @@ public function addReport( self::validateCommonReportAttributes($period, $hour, $description, $idSegment, $reportType, $reportFormat, $evolutionPeriodFor, $evolutionPeriodN); + if (null !== $periodParam) { + self::validatePeriodParam($periodParam); + } + // report parameters validations $parameters = self::validateReportParameters($reportType, $parameters); @@ -195,6 +199,10 @@ public function updateReport( self::validateCommonReportAttributes($period, $hour, $description, $idSegment, $reportType, $reportFormat, $evolutionPeriodFor, $evolutionPeriodN); + if (null !== $periodParam) { + self::validatePeriodParam($periodParam); + } + // report parameters validations $parameters = self::validateReportParameters($reportType, $parameters); @@ -621,6 +629,7 @@ public function sendReport($idReport, $period = false, $date = false, $force = f $report = reset($reports); if (!empty($period)) { + self::validatePeriodParam($period); $report['period_param'] = $period; } @@ -845,6 +854,17 @@ private static function validateReportPeriod($period) } } + private static function validatePeriodParam($period) + { + $periodValidator = new Period\PeriodValidator(); + $allowedPeriods = array_flip($periodValidator->getPeriodsAllowedForAPI()); + unset($allowedPeriods['range']); + + if (!array_key_exists($period, $allowedPeriods)) { + throw new Exception('Report period must be one of the following: ' . implode(', ', array_keys($allowedPeriods)) . ' (got ' . $period . ')'); + } + } + private static function validateReportHour($hour) { if (!is_numeric($hour) || $hour < 0 || $hour > 23) { diff --git a/plugins/ScheduledReports/Controller.php b/plugins/ScheduledReports/Controller.php index 05ee189874e..6c386fd88b5 100644 --- a/plugins/ScheduledReports/Controller.php +++ b/plugins/ScheduledReports/Controller.php @@ -12,6 +12,7 @@ use Piwik\API\Request; use Piwik\Common; use Piwik\Nonce; +use Piwik\Period\PeriodValidator; use Piwik\Piwik; use Piwik\Plugins\ImageGraph\ImageGraph; use Piwik\Plugins\LanguagesManager\LanguagesManager; @@ -46,7 +47,11 @@ public function index() $view->displayFormats = ScheduledReports::getDisplayFormats(); $view->paramPeriods = []; - foreach (Piwik::$idPeriods as $label => $id) { + + $periodValidator = new PeriodValidator(); + $allowedPeriods = $periodValidator->getPeriodsAllowedForAPI(); + + foreach ($allowedPeriods as $label) { if ($label === 'range') { continue; } diff --git a/plugins/ScheduledReports/tests/Integration/ApiTest.php b/plugins/ScheduledReports/tests/Integration/ApiTest.php index e5bf8b3f206..22727d922d8 100644 --- a/plugins/ScheduledReports/tests/Integration/ApiTest.php +++ b/plugins/ScheduledReports/tests/Integration/ApiTest.php @@ -479,7 +479,7 @@ public function testGetReportSubjectAndReportTitle($expectedReportSubject, $expe ); $getReportSubjectAndReportTitle->setAccessible(true); - list($reportSubject, $reportTitle) = $getReportSubjectAndReportTitle->invoke( APIScheduledReports::getInstance(), $websiteName, $reports); + [$reportSubject, $reportTitle] = $getReportSubjectAndReportTitle->invoke( APIScheduledReports::getInstance(), $websiteName, $reports); $this->assertEquals($expectedReportSubject, $reportSubject); $this->assertEquals($expectedReportTitle, $reportTitle); } @@ -584,6 +584,37 @@ public function test_addReport_validatesEvolutionPeriodForParam() ); } + public function testAddReportValidatesPeriodParam() + { + $invalidPeriod = 'tomorrow'; + + $this->expectException(\Exception::class); + $this->expectExceptionMessage( + 'Report period must be one of the following: day, week, month, year (got ' . $invalidPeriod . ')' + ); + + self::setSuperUser(); + + APIScheduledReports::getInstance()->addReport( + 1, + '', + Schedule::PERIOD_DAY, + 0, + ScheduledReports::EMAIL_TYPE, + ReportRenderer::HTML_FORMAT, + [ + 'VisitsSummary_get', + 'UserCountry_getCountry', + 'Referrers_getWebsites', + ], + [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_TABLES_ONLY], + false, + 'each', + null, + $invalidPeriod + ); + } + public function test_addReport_validatesEvolutionPeriodNParam() { $this->expectException(\Exception::class); @@ -636,6 +667,51 @@ public function test_addReport_throwsIfEvolutionPeriodNParamIsEach_AndLastNSuppl ); } + public function testUpdateReportValidatesPeriodParam() + { + $invalidPeriod = 'tomorrow'; + + $this->expectException(\Exception::class); + $this->expectExceptionMessage( + 'Report period must be one of the following: day, week, month, year (got ' . $invalidPeriod . ')' + ); + + self::setSuperUser(); + + $idReport = APIScheduledReports::getInstance()->addReport( + 1, + '', + Schedule::PERIOD_DAY, + 0, + ScheduledReports::EMAIL_TYPE, + ReportRenderer::HTML_FORMAT, + [ + 'VisitsSummary_get', + ], + [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_TABLES_ONLY] + ); + + APIScheduledReports::getInstance()->updateReport( + $idReport, + 1, + '', + Schedule::PERIOD_DAY, + 0, + ScheduledReports::EMAIL_TYPE, + ReportRenderer::HTML_FORMAT, + [ + 'VisitsSummary_get', + 'UserCountry_getCountry', + 'Referrers_getWebsites', + ], + [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_TABLES_ONLY], + false, + 'each', + null, + $invalidPeriod + ); + } + public function test_updateReport_validatesEvolutionPeriodForParam() { $this->expectException(\Exception::class);