Merge pull request #37 from matomo-org/32-BanIp-500-ErrorFix

Changed to PHP from twig for ban ip email template
matomo-org · Dec 8, 2021 · 8bce999 · 8bce999
2 parents 12b8a97 + acbaa4d
commit 8bce999
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 27 deletions.
diff --git a/BanIpNotificationEmail.php b/BanIpNotificationEmail.php
@@ -8,11 +8,11 @@
 
 namespace Piwik\Plugins\TrackingSpamPrevention;
 
+use Piwik\Common;
 use Piwik\Log;
 use Piwik\Mail;
 use Piwik\Piwik;
 use Piwik\SettingsPiwik;
-use Piwik\View;
 
 class BanIpNotificationEmail
 {
@@ -24,16 +24,13 @@ public function send($ip, $email, $maxActionsAllowed, $locationData, $nowDateTim
 
         $mail = new Mail();
         $mail->addTo($email);
-        $mail->setSubject(Piwik::translate('TrackingSpamPrevention_BanIpNotificationMailSubject'));
+        $mail->setSubject('An IP was banned as too many actions were tracked.');
         $mail->setDefaultFromPiwik();
 
-        $view = new View('@TrackingSpamPrevention/notificationBanIpEmail.twig');
-        $view->instanceId = SettingsPiwik::getPiwikInstanceId();
-        $view->maxActionsAllowed = $maxActionsAllowed;
-        $view->ipBanned = $ip;
-        $view->ipHeader = \Piwik\IP::getIpFromHeader();
-        $view->nowDataTime = $nowDateTime;
-        $view->geoIpInfo = $locationData;
+        $mailBody = 'This is for your information. The following IP was banned because visit tried to track more than ' . Common::sanitizeInputValue($maxActionsAllowed) . ' actions:';
+        $mailBody .= PHP_EOL.PHP_EOL.'"' . Common::sanitizeInputValue($ip) . '"'.PHP_EOL;
+        $instanceId = SettingsPiwik::getPiwikInstanceId();
+
 
         if (!empty($_GET)) {
             $get = $_GET;
@@ -53,17 +50,29 @@ public function send($ip, $email, $maxActionsAllowed, $locationData, $nowDateTim
             $post = [];
         }
 
-        $view->getRequest = $get;
-        $view->postRequest = $post;
-        $mail->setBodyText($view->render());
+        if (!empty($instanceId)) {
+            $mailBody .= PHP_EOL.'Instance ID: ' . Common::sanitizeInputValue($instanceId);
+        }
+        $mailBody .= PHP_EOL.'Current date (UTC): ' . Common::sanitizeInputValue($nowDateTime) . '
+IP as detected in header: ' . Common::sanitizeInputValue(\Piwik\IP::getIpFromHeader()) . '
+GET request info: ' . json_encode($get) . '
+POST request info: ' . json_encode($post). PHP_EOL;
+
+        if (!empty($locationData)) {
+            $mailBody .= 'Geo IP info: ' . json_encode($locationData);
+        }
+
+        $mail->setBodyText($mailBody);
 
         $testMode = (defined('PIWIK_TEST_MODE') && PIWIK_TEST_MODE);
         if ($testMode) {
-            Log::info($mail->getSubject() .':' . $mail->getBodyText());
+            Log::info($mail->getSubject() . ':' . $mail->getBodyText());
         } else {
             $mail->send();
         }
 
+        $a=$mail->getBodyText();
+
         return $mail->getBodyText();
     }
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 ## Changelog
 
+#4.1.1
+- Fixed IP ban notification email leading to internal sever error
+
 # 4.1.0
 - Exclude user agents from load testing services
 - Exclude user agents from server side tracking SDK by enabling an option

diff --git a/README.md b/README.md
@@ -32,7 +32,7 @@ It cannot detect a headless browser when the user agent is customised. Often, we
 
 When enabled, you can configure how many actions a visit should max have. 
 
-Most sites, under normal circumstances, may have no more than 100 or 200 or 300 actions within one visit. In many cases it might be therefore safe to assume that if someone has caused more than the configured amount of actions, it might be a spammer, or a bot, or something else unnatural causing these actions. 
+Most sites never have more than around 100 to 300 actions within one visit under normal circumstances. In many cases it might therefore be safe to assume that if someone has caused more actions than the configured amount of actions, it might be actually tracking spam or a bot or something else like non-human activity is causing these actions. 
 
 Matomo will in this case stop recording further actions for that visit to have less inaccurate data and to reduce server load. The IP address of this visit will then be blocked for up to 24 hours.
 

diff --git a/plugin.json b/plugin.json
@@ -1,7 +1,7 @@
 {
     "name": "TrackingSpamPrevention",
     "description": "This plugin offers various options to prevent spammers and bots from making your data inaccurate so you can rely on your data again.",
-    "version": "4.1.0",
+    "version": "4.1.1",
     "theme": false,
     "require": {
         "matomo": ">=4.0.5,<5.0.0-b1"

diff --git a/templates/notificationBanIpEmail.twig b/templates/notificationBanIpEmail.twig
diff --git a/tests/Integration/NotificationEmailTest.php b/tests/Integration/NotificationEmailTest.php
@@ -37,15 +37,26 @@ public function test_send_noValidEmail()
 
     public function test_send_ValidEmail()
     {
-        $this->assertEquals('This is for your information. The following IP was banned because visit tried to track more than 112 actions:
+        $this->assertEquals(
+            'This is for your information. The following IP was banned because visit tried to track more than 112 actions:
 
 "10.10.10.10"
 
 Current date (UTC): 2020-12-14 01:42:27
 IP as detected in header: 127.0.0.1
 GET request info: []
 POST request info: []
-Geo IP info: {"test":"foo","bar":"baz"}', trim($this->email->send('10.10.10.10', '[email protected]', 112, ['test' => 'foo', 'bar' => 'baz'], '2020-12-14 01:42:27')));
+Geo IP info: {"test":"foo","bar":"baz"}',
+            trim(
+                $this->email->send(
+                    '10.10.10.10',
+                    '[email protected]',
+                    112,
+                    ['test' => 'foo', 'bar' => 'baz'],
+                    '2020-12-14 01:42:27'
+                )
+            )
+        );
     }