Mail drivers: Separate definition from validation

js/src/admin/components/MailPage.js

@@ -6,32 +6,32 @@ 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();
 
-    this.loading = true;
     this.saving = false;
+    this.refresh();
+  }
+
+  refresh() {
+    this.loading = true;
 
     this.driverFields = {};
     this.fields = ['mail_driver', 'mail_from'];
     this.values = {};
+    this.status = {sending: false, errors: {}};
 
     const settings = app.data.settings;
     this.fields.forEach(key => this.values[key] = m.prop(settings[key]));
 
     app.request({
       method: 'GET',
-      url: app.forum.attribute('apiUrl') + '/mail-drivers'
+      url: app.forum.attribute('apiUrl') + '/mail-settings'
     }).then(response => {
-      this.driverFields = response['data'].reduce(
-        (hash, driver) => ({...hash, [driver['id']]: driver['attributes']['fields']}),
-        {}
-      );
+      this.driverFields = response['data']['attributes']['fields'];
+      this.status.sending = response['data']['attributes']['sending'];
+      this.status.errors = response['data']['attributes']['errors'];
 
       for (const driver in this.driverFields) {
         for (const field in this.driverFields[driver]) {
@@ -46,7 +46,7 @@ export default class MailPage extends Page {
   }
 
   view() {
-    if (this.loading) {
+    if (this.loading || this.saving) {
       return (
         <div className="MailPage">
           <div className="container">
@@ -90,19 +90,20 @@ export default class MailPage extends Page {
               ]
             })}
 
+            {this.status.sending || Alert.component({
+              children: app.translator.trans('core.admin.email.not_sending_message'),
+              dismissible: false,
+            })}
+
             {fieldKeys.length > 0 && FieldSet.component({
               label: app.translator.trans(`core.admin.email.${this.values.mail_driver()}_heading`),
               className: 'MailPage-MailSettings',
               children: [
-                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">
                   {fieldKeys.map(field => [
-                    <label>{app.translator.trans(`core.admin.email.${field}_label`)} {(fields[field] || '').indexOf('required') !== -1 ? '*' : ''}</label>,
+                    <label>{app.translator.trans(`core.admin.email.${field}_label`)}</label>,
                     this.renderField(field),
+                    this.status.errors[field] && <p className='ValidationError'>{this.status.errors[field]}</p>,
                   ])}
                 </div>
               ]
@@ -112,7 +113,6 @@ export default class MailPage extends Page {
               type: 'submit',
               className: 'Button Button--primary',
               children: app.translator.trans('core.admin.email.submit_button'),
-              loading: this.saving,
               disabled: !this.changed()
             })}
           </form>
@@ -127,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={(fields[field] || '').indexOf('required') !== -1} />;
+      return <input className="FormControl" value={prop() || ''} oninput={m.withAttr('value', prop)} />;
     } else {
       return <Select value={prop()} options={field} onchange={prop} />;
     }
@@ -156,7 +156,7 @@ export default class MailPage extends Page {
       .catch(() => {})
       .then(() => {
         this.saving = false;
-        m.redraw();
+        this.refresh();
       });
   }
 }

less/admin/MailPage.less

@@ -8,14 +8,14 @@
     }
   }
 
-  fieldset {
-    margin-bottom: 30px;
+  fieldset, .Alert {
+    margin-bottom: 20px;
+  }
 
-    > ul {
-      list-style: none;
-      margin: 0;
-      padding: 0;
-    }
+  fieldset > ul {
+    list-style: none;
+    margin: 0;
+    padding: 0;
   }
 }
 

src/Api/Controller/ListMailDriversController.php

@@ -1,43 +0,0 @@
-<?php
-
-/*
- * This file is part of Flarum.
- *
- * For detailed copyright and license information, please view the
- * LICENSE file that was distributed with this source code.
- */
-
-namespace Flarum\Api\Controller;
-
-use Flarum\Api\Serializer\MailDriverSerializer;
-use Flarum\User\AssertPermissionTrait;
-use Psr\Http\Message\ServerRequestInterface;
-use Tobscure\JsonApi\Document;
-
-class ListMailDriversController extends AbstractListController
-{
-    use AssertPermissionTrait;
-
-    /**
-     * {@inheritdoc}
-     */
-    public $serializer = MailDriverSerializer::class;
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function data(ServerRequestInterface $request, Document $document)
-    {
-        $this->assertAdmin($request->getAttribute('actor'));
-
-        $drivers = self::$container->make('mail.supported_drivers');
-        array_walk($drivers, function (&$driver, $key) {
-            $driver = [
-                'id' => $key,
-                'driver' => self::$container->make($driver),
-            ];
-        });
-
-        return $drivers;
-    }
-}

src/Api/Controller/ShowMailSettingsController.php

@@ -0,0 +1,52 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Api\Controller;
+
+use Flarum\Api\Serializer\MailSettingsSerializer;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\User\AssertPermissionTrait;
+use Illuminate\Contracts\Validation\Factory;
+use Psr\Http\Message\ServerRequestInterface;
+use Tobscure\JsonApi\Document;
+
+class ShowMailSettingsController extends AbstractShowController
+{
+    use AssertPermissionTrait;
+
+    /**
+     * {@inheritdoc}
+     */
+    public $serializer = MailSettingsSerializer::class;
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function data(ServerRequestInterface $request, Document $document)
+    {
+        $this->assertAdmin($request->getAttribute('actor'));
+
+        $drivers = array_map(function ($driver) {
+            return self::$container->make($driver);
+        }, self::$container->make('mail.supported_drivers'));
+
+        $settings = self::$container->make(SettingsRepositoryInterface::class);
+        $configured = self::$container->make('flarum.mail.configured_driver');
+        $actual = self::$container->make('mail.driver');
+        $validator = self::$container->make(Factory::class);
+
+        $errors = $configured->validate($settings, $validator);
+
+        return [
+            'drivers' => $drivers,
+            'sending' => $actual->canSend(),
+            'errors' => $errors,
+        ];
+    }
+}

src/Api/Serializer/MailDriverSerializer.php

@@ -1,58 +0,0 @@
-<?php
-
-/*
- * This file is part of Flarum.
- *
- * For detailed copyright and license information, please view the
- * LICENSE file that was distributed with this source code.
- */
-
-namespace Flarum\Api\Serializer;
-
-use Flarum\Mail\DriverInterface;
-use InvalidArgumentException;
-
-class MailDriverSerializer extends AbstractSerializer
-{
-    /**
-     * {@inheritdoc}
-     */
-    protected $type = 'mail-drivers';
-
-    /**
-     * {@inheritdoc}
-     *
-     * @param \Flarum\Mail\DriverInterface $driver
-     * @throws InvalidArgumentException
-     */
-    protected function getDefaultAttributes($driver)
-    {
-        if (! ($driver['driver'] instanceof DriverInterface)) {
-            throw new InvalidArgumentException(
-                get_class($this).' can only serialize instances of '.DriverInterface::class
-            );
-        }
-
-        $settings = $driver['driver']->availableSettings();
-
-        if (key($settings) === 0) {
-            // BACKWARDS COMPATIBILITY: Support a simple list of fields (without
-            // type or additional metadata).
-            // Turns ["f1", "f2"] into {"f1": "", "f2": ""}
-            // @deprecated since 0.1.0-beta.12
-            $settings = array_reduce($settings, function ($memo, $key) {
-                return [$key => ''] + $memo;
-            }, []);
-        }
-
-        return [
-            'fields' => $settings,
-            'fieldsRequired' => $driver['driver']->requiredFields(),
-        ];
-    }
-
-    public function getId($model)
-    {
-        return $model['id'];
-    }
-}

src/Api/Serializer/MailSettingsSerializer.php

@@ -0,0 +1,46 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Api\Serializer;
+
+use Flarum\Mail\DriverInterface;
+use InvalidArgumentException;
+
+class MailSettingsSerializer extends AbstractSerializer
+{
+    /**
+     * {@inheritdoc}
+     */
+    protected $type = 'mail-settings';
+
+    /**
+     * {@inheritdoc}
+     *
+     * @param array $settings
+     * @throws InvalidArgumentException
+     */
+    protected function getDefaultAttributes($settings)
+    {
+        return [
+            'fields' => array_map([$this, 'serializeDriver'], $settings['drivers']),
+            'sending' => $settings['sending'],
+            'errors' => $settings['errors'],
+        ];
+    }
+
+    private function serializeDriver(DriverInterface $driver)
+    {
+        return $driver->availableSettings();
+    }
+
+    public function getId($model)
+    {
+        return 'global';
+    }
+}

src/Api/routes.php

@@ -307,10 +307,10 @@ return function (RouteCollection $map, RouteHandlerFactory $route) {
         $route->toController(Controller\ClearCacheController::class)
     );
 
-    // List available mail drivers and their configuration fields
+    // List available mail drivers, available fields and validation status
     $map->get(
-        '/mail-drivers',
-        'mailDrivers.index',
-        $route->toController(Controller\ListMailDriversController::class)
+        '/mail-settings',
+        'mailSettings.index',
+        $route->toController(Controller\ShowMailSettingsController::class)
     );
 };

src/Forum/ForumServiceProvider.php

@@ -168,8 +168,6 @@ class ForumServiceProvider extends AbstractServiceProvider
                     $this->app
                 );
                 $validator->whenSettingsSaving($event);
-
-                $this->app->make(ValidateMailConfiguration::class)->whenSettingsSaving($event);
             }
         );
     }

src/Forum/ValidateMailConfiguration.php

@@ -1,91 +0,0 @@
-<?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);
-    }
-}

src/Mail/DriverInterface.php

@@ -10,6 +10,8 @@
 namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
+use Illuminate\Support\MessageBag;
 use Swift_Transport;
 
 /**
@@ -31,6 +33,26 @@ interface DriverInterface
      */
     public function availableSettings(): array;
 
+    /**
+     * Ensure the given settings are enough to send emails.
+     *
+     * This method is responsible for determining whether the user-provided
+     * values stored in Flarum's settings are "valid" as far as a simple
+     * inspection of these values can determine it. Of course, this does not
+     * mean that the mail server or API will actually accept e.g. credentials.
+     *
+     * Any errors must be wrapped in a {@see \Illuminate\Support\MessageBag}.
+     * If there are no errors, an empty instance can be returned. In the
+     * presence of validation problems with the configured mail driver, Flarum
+     * will fall back to its no-op {@see \Flarum\Mail\NullDriver}.
+     */
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag;
+
+    /**
+     * Does this driver actually send out emails?
+     */
+    public function canSend(): bool;
+
     /**
      * Build a mail transport based on Flarum's current settings.
      */

src/Mail/LogDriver.php

@@ -10,7 +10,9 @@
 namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
 use Illuminate\Mail\Transport\LogTransport;
+use Illuminate\Support\MessageBag;
 use Psr\Log\LoggerInterface;
 use Swift_Transport;
 
@@ -31,6 +33,16 @@ class LogDriver implements DriverInterface
         return [];
     }
 
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return new MessageBag;
+    }
+
+    public function canSend(): bool
+    {
+        return false;
+    }
+
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new LogTransport($this->logger);

src/Mail/MailServiceProvider.php

@@ -9,10 +9,11 @@
 
 namespace Flarum\Mail;
 
-use Flarum\Forum\ValidateMailConfiguration;
 use Flarum\Foundation\AbstractServiceProvider;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
 use Illuminate\Mail\Mailer;
+use Illuminate\Support\Arr;
 use Swift_Mailer;
 
 class MailServiceProvider extends AbstractServiceProvider
@@ -31,11 +32,29 @@ class MailServiceProvider extends AbstractServiceProvider
         });
 
         $this->app->singleton('mail.driver', function () {
-            return $this->app->make(ValidateMailConfiguration::class)->getWorkingDriver();
+            $configured = $this->app->make('flarum.mail.configured_driver');
+            $settings = $this->app->make(SettingsRepositoryInterface::class);
+            $validator = $this->app->make(Factory::class);
+
+            return $configured->validate($settings, $validator)->any()
+                ? $this->app->make(NullDriver::class)
+                : $configured;
         });
 
         $this->app->alias('mail.driver', DriverInterface::class);
 
+        $this->app->singleton('flarum.mail.configured_driver', function () {
+            $drivers = $this->app->make('mail.supported_drivers');
+            $settings = $this->app->make(SettingsRepositoryInterface::class);
+            $driverName = $settings->get('mail_driver');
+
+            $driverClass = Arr::get($drivers, $driverName);
+
+            return $driverClass
+                ? $this->app->make($driverClass)
+                : $this->app->make(NullDriver::class);
+        });
+
         $this->app->singleton('swift.mailer', function ($app) {
             return new Swift_Mailer(
                 $app->make('mail.driver')->buildTransport(

src/Mail/MailgunDriver.php

@@ -11,7 +11,9 @@ namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
 use GuzzleHttp\Client;
+use Illuminate\Contracts\Validation\Factory;
 use Illuminate\Mail\Transport\MailgunTransport;
+use Illuminate\Support\MessageBag;
 use Swift_Transport;
 
 class MailgunDriver implements DriverInterface
@@ -19,11 +21,29 @@ class MailgunDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            '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
+            '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',
+            ],
         ];
     }
 
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return $validator->make($settings->all(), [
+            'mail_mailgun_secret' => 'required',
+            'mail_mailgun_domain' => 'required|regex:/^(?!\-)(?:[a-zA-Z\d\-]{0,62}[a-zA-Z\d]\.){1,126}(?!\d+)[a-zA-Z\d]{1,63}$/',
+            'mail_mailgun_region' => 'required|in:api.mailgun.net,api.eu.mailgun.net',
+        ])->errors();
+    }
+
+    public function canSend(): bool
+    {
+        return true;
+    }
+
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new MailgunTransport(

src/Mail/MandrillDriver.php

@@ -11,7 +11,9 @@ namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
 use GuzzleHttp\Client;
+use Illuminate\Contracts\Validation\Factory;
 use Illuminate\Mail\Transport\MandrillTransport;
+use Illuminate\Support\MessageBag;
 use Swift_Transport;
 
 class MandrillDriver implements DriverInterface
@@ -19,10 +21,22 @@ class MandrillDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            'mail_mandrill_secret' => 'required',
+            'mail_mandrill_secret' => '',
         ];
     }
 
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return $validator->make($settings->all(), [
+            'mail_mandrill_secret' => 'required',
+        ])->errors();
+    }
+
+    public function canSend(): bool
+    {
+        return true;
+    }
+
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new MandrillTransport(

src/Mail/NullDriver.php

@@ -10,6 +10,8 @@
 namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
+use Illuminate\Support\MessageBag;
 use Swift_NullTransport;
 use Swift_Transport;
 
@@ -20,6 +22,16 @@ class NullDriver implements DriverInterface
         return [];
     }
 
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return new MessageBag;
+    }
+
+    public function canSend(): bool
+    {
+        return false;
+    }
+
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new Swift_NullTransport();

src/Mail/SendmailDriver.php

@@ -10,6 +10,8 @@
 namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
+use Illuminate\Support\MessageBag;
 use Swift_SendmailTransport;
 use Swift_Transport;
 
@@ -20,6 +22,16 @@ class SendmailDriver implements DriverInterface
         return [];
     }
 
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return new MessageBag;
+    }
+
+    public function canSend(): bool
+    {
+        return true;
+    }
+
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport
     {
         return new Swift_SendmailTransport;

src/Mail/SesDriver.php

@@ -11,7 +11,9 @@ namespace Flarum\Mail;
 
 use Aws\Ses\SesClient;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
 use Illuminate\Mail\Transport\SesTransport;
+use Illuminate\Support\MessageBag;
 use Swift_Transport;
 
 class SesDriver implements DriverInterface
@@ -19,10 +21,24 @@ class SesDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
+            'mail_ses_key' => '',
+            'mail_ses_secret' => '',
+            'mail_ses_region' => '',
+        ];
+    }
+
+    public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return $validator->make($settings->all(), [
             'mail_ses_key' => 'required',
             'mail_ses_secret' => 'required',
             'mail_ses_region' => 'required',
-        ];
+        ])->errors();
+    }
+
+    public function canSend(): bool
+    {
+        return true;
     }
 
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport

src/Mail/SmtpDriver.php

@@ -10,6 +10,8 @@
 namespace Flarum\Mail;
 
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
+use Illuminate\Support\MessageBag;
 use Swift_SmtpTransport;
 use Swift_Transport;
 
@@ -18,12 +20,28 @@ class SmtpDriver implements DriverInterface
     public function availableSettings(): array
     {
         return [
-            '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_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 validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag
+    {
+        return $validator->make($settings->all(), [
+            'mail_host' => 'required',
+            'mail_port' => 'nullable|integer',
+            'mail_encryption' => 'nullable|in:tls,ssl',
             'mail_username' => 'required',
             'mail_password' => 'required',
-        ];
+        ])->errors();
+    }
+
+    public function canSend(): bool
+    {
+        return true;
     }
 
     public function buildTransport(SettingsRepositoryInterface $settings): Swift_Transport