From 42861dd986c415f847985794a1e452eb85f7596c Mon Sep 17 00:00:00 2001
From: David Sevilla Martin <me@datitisev.me>
Date: Wed, 13 Nov 2019 19:43:16 -0500
Subject: [PATCH] Change implementation to add validation rules, of which
 'required' is shown in the frontend

---
 .../core/js/src/admin/components/MailPage.js  | 18 ++--
 .../core/src/Forum/ForumServiceProvider.php   |  2 +
 .../src/Forum/ValidateMailConfiguration.php   | 91 +++++++++++++++++++
 framework/core/src/Mail/DriverInterface.php   |  5 -
 framework/core/src/Mail/LogDriver.php         |  5 -
 .../core/src/Mail/MailServiceProvider.php     | 14 +--
 framework/core/src/Mail/MailgunDriver.php     | 13 +--
 framework/core/src/Mail/MandrillDriver.php    |  7 +-
 framework/core/src/Mail/NullDriver.php        |  5 -
 framework/core/src/Mail/SendmailDriver.php    |  5 -
 framework/core/src/Mail/SesDriver.php         | 11 +--
 framework/core/src/Mail/SmtpDriver.php        | 19 +---
 12 files changed, 116 insertions(+), 79 deletions(-)
 create mode 100644 framework/core/src/Forum/ValidateMailConfiguration.php

diff --git a/framework/core/js/src/admin/components/MailPage.js b/framework/core/js/src/admin/components/MailPage.js
index dcbbb8f13..5140240e0 100644
--- a/framework/core/js/src/admin/components/MailPage.js
+++ b/framework/core/js/src/admin/components/MailPage.js
@@ -6,6 +6,10 @@ import Select from '../../common/components/Select';
 import LoadingIndicator from '../../common/components/LoadingIndicator';
 import saveSettings from '../utils/saveSettings';
 
+// From https://www.30secondsofcode.org/snippet/deepFlatten
+// Array.prototype.flatMap is not supported in IE or Edge
+const deepFlatten = arr => [].concat(...arr.map(v => (Array.isArray(v) ? deepFlatten(v) : v)));
+
 export default class MailPage extends Page {
   init() {
     super.init();
@@ -15,7 +19,6 @@ export default class MailPage extends Page {
 
     this.driverFields = {};
     this.fields = ['mail_driver', 'mail_from'];
-    this.fieldsRequired = [];
     this.values = {};
 
     const settings = app.data.settings;
@@ -37,8 +40,6 @@ export default class MailPage extends Page {
         }
       }
 
-      this.fieldsRequired = response['data'].map(driver => driver['attributes']['fieldsRequired']).flat();
-
       this.loading = false;
       m.redraw();
     });
@@ -56,6 +57,7 @@ export default class MailPage extends Page {
     }
 
     const fields = this.driverFields[this.values.mail_driver()];
+    const fieldKeys = Object.keys(fields);
 
     return (
       <div className="MailPage">
@@ -88,18 +90,18 @@ export default class MailPage extends Page {
               ]
             })}
 
-            {Object.keys(fields).length > 0 && FieldSet.component({
+            {fieldKeys.length > 0 && FieldSet.component({
               label: app.translator.trans(`core.admin.email.${this.values.mail_driver()}_heading`),
               className: 'MailPage-MailSettings',
               children: [
-                fields.filter(field => this.fieldsRequired.includes(field) && !this.values[field]()).length > 0 && Alert.component({
+                fieldKeys.filter(field => fields[field] && fields[field].indexOf('required') !== -1 && !this.values[field]()).length > 0 && Alert.component({
                   children: app.translator.trans('core.admin.email.incomplete_configuration_text'),
                   dismissible: false,
                 }),
 
                 <div className="MailPage-MailSettings-input">
-                  {Object.keys(fields).map(field => [
-                    <label>{app.translator.trans(`core.admin.email.${field}_label`)} {this.fieldsRequired.includes(field) ? '*' : ''}</label>,
+                  {fieldKeys.map(field => [
+                    <label>{app.translator.trans(`core.admin.email.${field}_label`)} {(fields[field] || '').indexOf('required') !== -1 ? '*' : ''}</label>,
                     this.renderField(field),
                   ])}
                 </div>
@@ -125,7 +127,7 @@ export default class MailPage extends Page {
     const prop = this.values[name];
 
     if (typeof field === 'string') {
-      return <input className="FormControl" value={prop() || ''} oninput={m.withAttr('value', prop)} required={this.fieldsRequired.includes(field)} />;
+      return <input className="FormControl" value={prop() || ''} oninput={m.withAttr('value', prop)} required={(fields[field] || '').indexOf('required') !== -1} />;
     } else {
       return <Select value={prop()} options={field} onchange={prop} />;
     }
diff --git a/framework/core/src/Forum/ForumServiceProvider.php b/framework/core/src/Forum/ForumServiceProvider.php
index 0c2865eaf..9ac4ee464 100644
--- a/framework/core/src/Forum/ForumServiceProvider.php
+++ b/framework/core/src/Forum/ForumServiceProvider.php
@@ -168,6 +168,8 @@ class ForumServiceProvider extends AbstractServiceProvider
                     $this->app
                 );
                 $validator->whenSettingsSaving($event);
+
+                $this->app->make(ValidateMailConfiguration::class)->whenSettingsSaving($event);
             }
         );
     }
diff --git a/framework/core/src/Forum/ValidateMailConfiguration.php b/framework/core/src/Forum/ValidateMailConfiguration.php
new file mode 100644
index 000000000..b718cf7c2
--- /dev/null
+++ b/framework/core/src/Forum/ValidateMailConfiguration.php
@@ -0,0 +1,91 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * (c) Toby Zerner <toby.zerner@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Flarum\Forum;
+
+use Flarum\Foundation\Application;
+use Flarum\Mail\DriverInterface;
+use Flarum\Mail\NullDriver;
+use Flarum\Settings\Event\Saving;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\Settings\SettingsServiceProvider;
+use Illuminate\Contracts\Container\Container;
+use Illuminate\Support\Arr;
+use Illuminate\Validation\Validator;
+
+class ValidateMailConfiguration
+{
+    /**
+     * @var SettingsServiceProvider
+     */
+    protected $settings;
+
+    /**
+     * @var Application
+     */
+    protected $container;
+
+    /**
+     * @param Container $container
+     * @param SettingsRepositoryInterface $settings
+     */
+    public function __construct(Container $container, SettingsRepositoryInterface $settings)
+    {
+        $this->container = $container;
+        $this->settings = $settings;
+    }
+
+    public function whenSettingsSaving(Saving $event)
+    {
+        if (! isset($event->settings['mail_driver'])) {
+            return;
+        }
+
+        $driver = $this->getDriver($event->settings);
+
+        $this->getValidator($driver, $event->settings)->validate();
+    }
+
+    public function getWorkingDriver()
+    {
+        $settings = $this->settings->all();
+        $driver = $this->getDriver($settings);
+        $validator = $this->getValidator($driver, $settings);
+
+        return$validator->passes()
+            ? $driver
+            : $this->container->make(NullDriver::class);
+    }
+
+    /**
+     * @param DriverInterface $driver
+     * @param array $settings
+     * @return Validator
+     */
+    protected function getValidator($driver, $settings)
+    {
+        $rules = $driver->availableSettings();
+        $settings = Arr::only($settings, array_keys($rules));
+
+        return $this->container->make('validator')->make($settings, $rules);
+    }
+
+    protected function getDriver($settings)
+    {
+        $drivers = $this->container->make('mail.supported_drivers');
+        $specifiedDriver = Arr::get($settings, 'mail_driver');
+        $driverClass = Arr::get($drivers, $specifiedDriver);
+
+        return $specifiedDriver && $driverClass
+            ? $this->container->make($driverClass)
+            : $this->container->make(NullDriver::class);
+    }
+}
diff --git a/framework/core/src/Mail/DriverInterface.php b/framework/core/src/Mail/DriverInterface.php
index c1802d552..d4c2f18e6 100644
--- a/framework/core/src/Mail/DriverInterface.php
+++ b/framework/core/src/Mail/DriverInterface.php
@@ -31,11 +31,6 @@ interface DriverInterface
      */
     public function availableSettings(): array;
 
-    /**
-     * Provide a list of required settings for this driver.
-     */
-    public function requiredFields(): array;
-
     /**
      * Build a mail transport based on Flarum's current settings.
      */
diff --git a/framework/core/src/Mail/LogDriver.php b/framework/core/src/Mail/LogDriver.php
index 137b57951..afa2025b2 100644
--- a/framework/core/src/Mail/LogDriver.php
+++ b/framework/core/src/Mail/LogDriver.php
@@ -31,11 +31,6 @@ class LogDriver implements DriverInterface
         return [];
     }
 
-    public function requiredFields(): array
-    {
-        return [];
-    }
-
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new LogTransport($this->logger);
diff --git a/framework/core/src/Mail/MailServiceProvider.php b/framework/core/src/Mail/MailServiceProvider.php
index 1e492b49c..11f4bc096 100644
--- a/framework/core/src/Mail/MailServiceProvider.php
+++ b/framework/core/src/Mail/MailServiceProvider.php
@@ -9,6 +9,7 @@
 
 namespace Flarum\Mail;
 
+use Flarum\Forum\ValidateMailConfiguration;
 use Flarum\Foundation\AbstractServiceProvider;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Illuminate\Mail\Mailer;
@@ -30,18 +31,7 @@ class MailServiceProvider extends AbstractServiceProvider
         });
 
         $this->app->singleton('mail.driver', function () {
-            $settings = $this->app->make(SettingsRepositoryInterface::class);
-            $drivers = $this->app->make('mail.supported_drivers');
-
-            $driver = $this->app->make($drivers[$settings->get('mail_driver')]);
-
-            // check that all required fields have been filled
-            $settings = $this->app->make(SettingsRepositoryInterface::class);
-            $valid = $driver && array_reduce($driver->requiredFields(), function ($carry, $field) use ($settings) {
-                return $carry && ! empty($settings->get($field));
-            }, true);
-
-            return $valid ? $driver : $this->app->make(NullDriver::class);
+            return $this->app->make(ValidateMailConfiguration::class)->getWorkingDriver();
         });
 
         $this->app->alias('mail.driver', DriverInterface::class);
diff --git a/framework/core/src/Mail/MailgunDriver.php b/framework/core/src/Mail/MailgunDriver.php
index 2b11efcd9..2f4d0edd0 100644
--- a/framework/core/src/Mail/MailgunDriver.php
+++ b/framework/core/src/Mail/MailgunDriver.php
@@ -19,20 +19,11 @@ class MailgunDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            'mail_mailgun_secret' => '', // the secret key
-            'mail_mailgun_domain' => '', // the API base URL
-            'mail_mailgun_region' => [ // region's endpoint
-                'api.mailgun.net' => 'US',
-                'api.eu.mailgun.net' => 'EU',
-            ],
+            'mail_mailgun_secret' => 'required', // the secret key
+            'mail_mailgun_domain' => 'required|regex:/^(?!\-)(?:[a-zA-Z\d\-]{0,62}[a-zA-Z\d]\.){1,126}(?!\d+)[a-zA-Z\d]{1,63}$/', // the API base URL
         ];
     }
 
-    public function requiredFields(): array
-    {
-        return $this->availableSettings();
-    }
-
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new MailgunTransport(
diff --git a/framework/core/src/Mail/MandrillDriver.php b/framework/core/src/Mail/MandrillDriver.php
index d45236e60..4d0656a98 100644
--- a/framework/core/src/Mail/MandrillDriver.php
+++ b/framework/core/src/Mail/MandrillDriver.php
@@ -19,15 +19,10 @@ class MandrillDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            'mail_mandrill_secret' => '',
+            'mail_mandrill_secret' => 'required',
         ];
     }
 
-    public function requiredFields(): array
-    {
-        return $this->availableSettings();
-    }
-
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new MandrillTransport(
diff --git a/framework/core/src/Mail/NullDriver.php b/framework/core/src/Mail/NullDriver.php
index acddd51df..f99a77dc6 100644
--- a/framework/core/src/Mail/NullDriver.php
+++ b/framework/core/src/Mail/NullDriver.php
@@ -20,11 +20,6 @@ class NullDriver implements DriverInterface
         return [];
     }
 
-    public function requiredFields(): array
-    {
-        return [];
-    }
-
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new Swift_NullTransport();
diff --git a/framework/core/src/Mail/SendmailDriver.php b/framework/core/src/Mail/SendmailDriver.php
index 937d93452..da7213cf9 100644
--- a/framework/core/src/Mail/SendmailDriver.php
+++ b/framework/core/src/Mail/SendmailDriver.php
@@ -20,11 +20,6 @@ class SendmailDriver implements DriverInterface
         return [];
     }
 
-    public function requiredFields(): array
-    {
-        return [];
-    }
-
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new Swift_SendmailTransport;
diff --git a/framework/core/src/Mail/SesDriver.php b/framework/core/src/Mail/SesDriver.php
index 89f1cd61f..b62d66aac 100644
--- a/framework/core/src/Mail/SesDriver.php
+++ b/framework/core/src/Mail/SesDriver.php
@@ -19,17 +19,12 @@ class SesDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            'mail_ses_key' => '',
-            'mail_ses_secret' => '',
-            'mail_ses_region' => '',
+            'mail_ses_key' => 'required',
+            'mail_ses_secret' => 'required',
+            'mail_ses_region' => 'required',
         ];
     }
 
-    public function requiredFields(): array
-    {
-        return $this->availableSettings();
-    }
-
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         $config = [
diff --git a/framework/core/src/Mail/SmtpDriver.php b/framework/core/src/Mail/SmtpDriver.php
index 6818a3565..3c0f6f7bd 100644
--- a/framework/core/src/Mail/SmtpDriver.php
+++ b/framework/core/src/Mail/SmtpDriver.php
@@ -18,20 +18,11 @@ class SmtpDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            'mail_host' => '', // a hostname, IPv4 address or IPv6 wrapped in []
-            'mail_port' => '', // a number, defaults to 25
-            'mail_encryption' => '', // "tls" or "ssl"
-            'mail_username' => '',
-            'mail_password' => '',
-        ];
-    }
-
-    public function requiredFields(): array
-    {
-        return [
-            'mail_host',
-            'mail_username',
-            'mail_password',
+            'mail_host' => 'required', // a hostname, IPv4 address or IPv6 wrapped in []
+            'mail_port' => 'nullable|integer', // a number, defaults to 25
+            'mail_encryption' => 'nullable|in:tls,ssl', // "tls" or "ssl"
+            'mail_username' => 'required',
+            'mail_password' => 'required',
         ];
     }