Fix Discord Transport when Fields to Embed Left Empty (#16439)

* Add PHPUnit test for LibreNMS Discord transport

* Style

* Adding comments for methods and others.

* Refactor title, color, embedFields and footer to its own methods.
Renaming and comments.

* testDiscordDelivery includes img and html tags.

* testDiscordDelivery tests INI options

* renamed getter methods for clarity.
Refactor to includeINIFields() method.

* refactor attribute $discord_message;

* Discord.php: Bugfix "Error: Invalid Field" when discord-embed-fields is left empty.
Discord.php: Removed DEFAULT_EMBEDS (defaults are not working with text fields).

DiscordTest.php: transport config tests improved.

* Transport.md: Documented options, images and ebed fields. Better examples.

* typos

* styleCI and PHPStan

* StyleCI

* StyleCI

LibreNMS/Alert/Transport/Discord.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Discord.php
  *
@@ -35,76 +36,141 @@ use LibreNMS\Util\Http;
 
 class Discord extends Transport
 {
-    public const DEFAULT_EMBEDS = 'hostname,name,timestamp,severity';
     private array $embedFieldTranslations = [
         'name' => 'Rule Name',
     ];
 
+    private array $discord_message = [];
+
+    /**
+     * Composes a Discord JSON message and delivers it using HTTP POST
+     * https://discord.com/developers/docs/resources/message#create-message
+     *
+     * @param  array  $alert_data
+     * @return bool
+     */
     public function deliverAlert(array $alert_data): bool
     {
-        $added_fields = $this->parseUserOptions($this->config['options']);
+        $this->discord_message = [
-
-        $discord_title = '#' . $alert_data['uid'] . ' ' . $alert_data['title'];
-        $discord_msg = $alert_data['msg'];
-        $color = hexdec(preg_replace('/[^\dA-Fa-f]/', '', self::getColorForState($alert_data['state'])));
-
-        // Special handling for the elapsed text in the footer if the elapsed is not set.
-        $footer_text = $alert_data['elapsed'] ? 'alert took ' . $alert_data['elapsed'] : '';
-
-        $data = [
             'embeds' => [
                 [
-                    'title' => $discord_title,
+                    'title' => $this->getTitle($alert_data),
-                    'color' => $color,
+                    'color' => $this->getColorOfAlertState($alert_data),
-                    'description' => $discord_msg,
+                    'description' => $this->getDescription($alert_data),
-                    'fields' => $this->createDiscordFields($alert_data),
+                    'fields' => $this->getEmbedFields($alert_data),
                     'footer' => [
-                        'text' => $footer_text,
+                        'text' => $this->getFooter($alert_data),
                     ],
                 ],
             ],
         ];
-        if (! empty($added_fields)) {
-            $data = array_merge($data, $added_fields);
-        }
 
-        $data = $this->embedGraphs($data);
+        $this->includeINIFields();
+        $this->embedGraphs();
+        $this->stripHTMLTagsFromDescription();
 
-        // remove all remaining HTML tags
+        $res = Http::client()->post($this->config['url'], $this->discord_message);
-        $data['embeds'][0]['description'] = strip_tags($data['embeds'][0]['description']);
-
-        $res = Http::client()->post($this->config['url'], $data);
 
         if ($res->successful()) {
             return true;
         }
 
-        throw new AlertTransportDeliveryException($alert_data, $res->status(), $res->body(), $discord_msg, $data);
+        throw new AlertTransportDeliveryException($alert_data, $res->status(), $res->body(), $alert_data['msg'], $this->discord_message);
     }
 
-    private function embedGraphs(array $data): array
+    private function getTitle(array $alert_data): string
     {
+        return '#' . $alert_data['uid'] . ' ' . $alert_data['title'];
+    }
+
+    private function stripHTMLTagsFromDescription(): array
+    {
+        $this->discord_message['embeds'][0]['description'] = strip_tags($this->discord_message['embeds'][0]['description']);
+
+        return $this->discord_message;
+    }
+
+    private function getColorOfAlertState(array $alert_data): int
+    {
+        $hexColor = self::getColorForState($alert_data['state']);
+        $sanitized = preg_replace('/[^\dA-Fa-f]/', '', $hexColor);
+
+        return hexdec($sanitized);
+    }
+
+    private function getDescription(array $alert_data): string
+    {
+        return $alert_data['msg'];
+    }
+
+    private function getFooter(array $alert_data): string
+    {
+        return $alert_data['elapsed'] ? 'alert took ' . $alert_data['elapsed'] : '';
+    }
+
+    private function includeINIFields(): array
+    {
+        $ini_fileds = $this->parseUserOptions($this->config['options']);
+
+        if (! empty($ini_fileds)) {
+            $this->discord_message = array_merge($this->discord_message, $ini_fileds);
+        }
+
+        return $this->discord_message;
+    }
+
+    /**
+     * Convert an html <img src=""> tag to a json Discord message Embed Image Structure
+     * https://discord.com/developers/docs/resources/message#embed-object-embed-image-structure
+     *
+     * @return array
+     */
+    private function embedGraphs(): array
+    {
+        $regex = '#<img class="librenms-graph" src="(.*?)"\s*/>#';
         $count = 1;
-        $data['embeds'][0]['description'] = preg_replace_callback('#<img class="librenms-graph" src="(.*?)" />#', function ($match) use (&$data, &$count) {
+
-            $data['embeds'][] = [
+        $this->discord_message['embeds'][0]['description'] = preg_replace_callback($regex, function ($match) use (&$count) {
+            $this->discord_message['embeds'][] = [
                 'image' => [
                     'url' => $match[1],
                 ],
             ];
 
             return '[Image ' . ($count++) . ']';
-        }, $data['embeds'][0]['description']);
+        }, $this->discord_message['embeds'][0]['description']);
 
-        return $data;
+        return $this->discord_message;
     }
 
-    public function createDiscordFields(array $alert_data): array
+    /**
+     * Converts comma-separated values into an array of name-value pairs.
+     * https://discord.com/developers/docs/resources/message#embed-object-embed-field-structure
+     *
+     * * @param  array  $alert_data  Array containing the values.
+     * @return array An array of name-value pairs.
+     *
+     * @example
+     * Example with 'hostname,sysDescr' as fields:
+     * $result will be:
+     * [
+     *     ['name' => 'Hostname', 'value' => 'server1'],
+     *     ['name' => 'SysDescr', 'value' => 'Linux server description'],
+     * ]
+     */
+    public function getEmbedFields(array $alert_data): array
     {
         $result = [];
 
-        $fields = explode(',', $this->config['discord-embed-fields'] ?? self::DEFAULT_EMBEDS);
+        if (empty($this->config['discord-embed-fields'])) {
+            return $result;
+        }
+
+        $fields = explode(',', $this->config['discord-embed-fields']);
 
         foreach ($fields as $field) {
+            $field = trim($field);
+
             $result[] = [
                 'name' => $this->embedFieldTranslations[$field] ?? ucfirst($field),
                 'value' => $alert_data[$field] ?? 'Error: Invalid Field',
@@ -133,9 +199,8 @@ class Discord extends Transport
                 [
                     'title' => 'Fields to embed in the alert',
                     'name' => 'discord-embed-fields',
-                    'descr' => 'Comma seperated list of fields from the alert to attach to the Discord message',
+                    'descr' => 'Comma seperated list from the alert to embed i.e. hostname,name,timestamp,severity',
                     'type' => 'text',
-                    'default' => self::DEFAULT_EMBEDS,
                 ],
             ],
             'validation' => [

doc/Alerting/Transports.md

@@ -269,20 +269,23 @@ Here an example using 3 numbers, any amount of numbers is supported:
 ## Discord
 
 The Discord transport will POST the alert message to your Discord
-Incoming WebHook. Simple html tags are stripped from  the message.
+Incoming WebHook. The only required value is Discord URL, without this no call to Discord will be made. 
 
-The only required value is for url, without this no call to Discord
+Graphs can be included in the template using: ```<img class="librenms-graph" src=""/>```. The rest of the html tags are stripped from the message.
-will be made. The Options field supports the JSON/Form Params listed
+
-in the Discord Docs below.
+
+ The Options field supports JSON/Form Params listed
+in the 
+[Discord Docs](https://discordapp.com/developers/docs/resources/webhook#execute-webhook). Fields to embed is a comma separated list from the [Alert Data](https://github.com/librenms/librenms/blob/master/LibreNMS/Alert/AlertData.php)).
 
-[Discord Docs](https://discordapp.com/developers/docs/resources/webhook#execute-webhook)
 
 **Example:**
 
 | Config | Example |
 | ------ | ------- |
 | Discord URL | <https://discordapp.com/api/webhooks/4515489001665127664/82-sf4385ysuhfn34u2fhfsdePGLrg8K7cP9wl553Fg6OlZuuxJGaa1d54fe> |
-| Options | username=myname |
+| Options | username=myname</br>content=Some content</br>tts=false |
+| Fields to embed | hostname,name,timestamp,severity |
 
 ## Elasticsearch
 

tests/Unit/Alert/Transports/DiscordTest.php

@@ -0,0 +1,237 @@
+<?php
+
+/**
+ * DiscordTest.php
+ *
+ * -Description-
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * @link       https://www.librenms.org
+ *
+ * @copyright  2022 Juan Diego Iannelli
+ * @author     Juan Diego Iannelli <jdibach@gmail.com>
+ */
+
+namespace LibreNMS\Tests\Unit\Alert\Transports;
+
+use App\Models\AlertTransport;
+use App\Models\Device;
+use Illuminate\Http\Client\Request;
+use Illuminate\Support\Facades\Http;
+use LibreNMS\Alert\AlertData;
+use LibreNMS\Alert\Transport;
+use LibreNMS\Tests\TestCase;
+
+use function PHPUnit\Framework\assertEquals;
+
+class DiscordTest extends TestCase
+{
+    public function testDiscordNoConfigDelivery(): void
+    {
+        Http::fake();
+
+        $transport = new Transport\Discord(new AlertTransport([
+            'transport_config' => [
+                'url' => '',
+                'options' => '',
+                'discord-embed-fields' => '',
+            ],
+        ]));
+
+        /** @var Device $mock_device */
+        $mock_device = Device::factory()->make(['hostname' => 'my-hostname.com']);
+
+        $transport->deliverAlert(AlertData::testData($mock_device));
+
+        Http::assertSent(function (Request $request) {
+            assertEquals('', $request->url());
+            assertEquals('POST', $request->method());
+            assertEquals('application/json', $request->header('Content-Type')[0]);
+            assertEquals(
+                [
+                    'embeds' => [
+                        [
+                            'title' => '#000 Testing transport from LibreNMS',
+                            'color' => 16711680,
+                            'description' => 'This is a test alert',
+                            'fields' => [],
+                            'footer' => [
+                                'text' => 'alert took 11s',
+                            ],
+                        ],
+                    ],
+                ],
+                $request->data()
+            );
+
+            return true;
+        });
+    }
+
+    public function testBadOptionsDelivery(): void
+    {
+        Http::fake();
+
+        $transport = new Transport\Discord(new AlertTransport([
+            'transport_config' => [
+                'url' => 'https://discord.com/api/webhooks/number/id',
+                'options' => 'multi-line options not in INIFormat' . PHP_EOL . 'are ignored',
+                'discord-embed-fields' => '',
+            ],
+        ]));
+
+        /** @var Device $mock_device */
+        $mock_device = Device::factory()->make(['hostname' => 'my-hostname.com']);
+
+        $transport->deliverAlert(AlertData::testData($mock_device));
+
+        Http::assertSent(function (Request $request) {
+            assertEquals('https://discord.com/api/webhooks/number/id', $request->url());
+            assertEquals('POST', $request->method());
+            assertEquals('application/json', $request->header('Content-Type')[0]);
+            assertEquals(
+                [
+                    'embeds' => [
+                        [
+                            'title' => '#000 Testing transport from LibreNMS',
+                            'color' => 16711680,
+                            'description' => 'This is a test alert',
+                            'fields' => [],
+                            'footer' => [
+                                'text' => 'alert took 11s',
+                            ],
+                        ],
+                    ],
+                ],
+                $request->data()
+            );
+
+            return true;
+        });
+    }
+
+    public function testBadEmbedFieldsDelivery(): void
+    {
+        Http::fake();
+
+        $transport = new Transport\Discord(new AlertTransport([
+            'transport_config' => [
+                'url' => 'https://discord.com/api/webhooks/number/id',
+                'options' => '',
+                'discord-embed-fields' => 'hostname severity',
+            ],
+        ]));
+
+        /** @var Device $mock_device */
+        $mock_device = Device::factory()->make(['hostname' => 'my-hostname.com']);
+
+        $transport->deliverAlert(AlertData::testData($mock_device));
+
+        Http::assertSent(function (Request $request) {
+            assertEquals('https://discord.com/api/webhooks/number/id', $request->url());
+            assertEquals('POST', $request->method());
+            assertEquals('application/json', $request->header('Content-Type')[0]);
+            assertEquals(
+                [
+                    'embeds' => [
+                        [
+                            'title' => '#000 Testing transport from LibreNMS',
+                            'color' => 16711680,
+                            'description' => 'This is a test alert',
+                            'fields' => [
+                                [
+                                    'name' => 'Hostname severity',
+                                    'value' => 'Error: Invalid Field',
+                                ],
+                            ],
+                            'footer' => [
+                                'text' => 'alert took 11s',
+                            ],
+                        ],
+                    ],
+                ],
+                $request->data()
+            );
+
+            return true;
+        });
+    }
+
+    public function testDiscordDelivery(): void
+    {
+        Http::fake();
+
+        $transport = new Transport\Discord(new AlertTransport([
+            'transport_config' => [
+                'url' => 'https://discord.com/api/webhooks/number/id',
+                'options' => 'tts=true' . PHP_EOL . 'content=This is a text',
+                'discord-embed-fields' => 'hostname,severity,wrongfield',
+            ],
+        ]));
+
+        /** @var Device $mock_device */
+        $mock_device = Device::factory()->make(['hostname' => 'my-hostname.com']);
+
+        $alert_data = AlertData::testData($mock_device);
+
+        $alert_data['msg'] = 'This test alert should not have image <img class="librenms-graph" src="google.jpeg" /> or <h2>html tags</h2></br>';
+
+        $transport->deliverAlert($alert_data);
+
+        Http::assertSent(function (Request $request) {
+            assertEquals($request->url(), 'https://discord.com/api/webhooks/number/id');
+            assertEquals($request->method(), 'POST');
+            assertEquals($request->header('Content-Type')[0], 'application/json');
+            assertEquals(
+                [
+                    'tts' => 'true',
+                    'content' => 'This is a text',
+                    'embeds' => [
+                        [
+                            'title' => '#000 Testing transport from LibreNMS',
+                            'color' => 16711680,
+                            'description' => 'This test alert should not have image [Image 1] or html tags',
+                            'fields' => [
+                                [
+                                    'name' => 'Hostname',
+                                    'value' => 'my-hostname.com',
+                                ],
+                                [
+                                    'name' => 'Severity',
+                                    'value' => 'critical',
+                                ],
+                                [
+                                    'name' => 'Wrongfield',
+                                    'value' => 'Error: Invalid Field',
+                                ],
+                            ],
+                            'footer' => [
+                                'text' => 'alert took 11s',
+                            ],
+                        ],
+                        [
+                            'image' => [
+                                'url' => 'google.jpeg',
+                            ],
+                        ],
+                    ],
+                ],
+                $request->data()
+            );
+
+            return true;
+        });
+    }
+}