Merge pull request #1921 from flarum/ds/1763-handle-incomplete-email-configuration

Improve handling of incomplete mail configuration

framework/core/js/src/admin/components/MailPage.js

@@ -10,24 +10,28 @@ 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(
+      this.driverFields = response['data']['attributes']['fields'];
-        (hash, driver) => ({...hash, [driver['id']]: driver['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]) {
@@ -42,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">
@@ -52,6 +56,9 @@ export default class MailPage extends Page {
       );
     }
 
+    const fields = this.driverFields[this.values.mail_driver()];
+    const fieldKeys = Object.keys(fields);
+
     return (
       <div className="MailPage">
         <div className="container">
@@ -66,8 +73,10 @@ export default class MailPage extends Page {
               className: 'MailPage-MailSettings',
               children: [
                 <div className="MailPage-MailSettings-input">
-                  <label>{app.translator.trans('core.admin.email.from_label')}</label>
+                  <label>
-                  <input className="FormControl" value={this.values.mail_from() || ''} oninput={m.withAttr('value', this.values.mail_from)} />
+                    {app.translator.trans('core.admin.email.from_label')}
+                    <input className="FormControl" value={this.values.mail_from() || ''} oninput={m.withAttr('value', this.values.mail_from)} />
+                  </label>
                 </div>
               ]
             })}
@@ -77,20 +86,30 @@ export default class MailPage extends Page {
               className: 'MailPage-MailSettings',
               children: [
                 <div className="MailPage-MailSettings-input">
-                  <label>{app.translator.trans('core.admin.email.driver_label')}</label>
+                  <label>
-                  <Select value={this.values.mail_driver()} options={Object.keys(this.driverFields).reduce((memo, val) => ({...memo, [val]: val}), {})} onchange={this.values.mail_driver} />
+                    {app.translator.trans('core.admin.email.driver_label')}
+                    <Select value={this.values.mail_driver()} options={Object.keys(this.driverFields).reduce((memo, val) => ({...memo, [val]: val}), {})} onchange={this.values.mail_driver} />
+                  </label>
                 </div>
               ]
             })}
 
-            {Object.keys(this.driverFields[this.values.mail_driver()]).length > 0 && FieldSet.component({
+            {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: [
                 <div className="MailPage-MailSettings-input">
-                  {Object.keys(this.driverFields[this.values.mail_driver()]).map(field => [
+                  {fieldKeys.map(field => [
-                    <label>{app.translator.trans(`core.admin.email.${field}_label`)}</label>,
+                    <label>
-                    this.renderField(field),
+                      {app.translator.trans(`core.admin.email.${field}_label`)}
+                      {this.renderField(field)}
+                    </label>,
+                    this.status.errors[field] && <p className='ValidationError'>{this.status.errors[field]}</p>,
                   ])}
                 </div>
               ]
@@ -100,7 +119,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>
@@ -115,7 +133,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)}/>;
+      return <input className="FormControl" value={prop() || ''} oninput={m.withAttr('value', prop)} />;
     } else {
       return <Select value={prop()} options={field} onchange={prop} />;
     }
@@ -144,7 +162,7 @@ export default class MailPage extends Page {
       .catch(() => {})
       .then(() => {
         this.saving = false;
-        m.redraw();
+        this.refresh();
       });
   }
 }

framework/core/less/admin/MailPage.less

@@ -8,32 +8,25 @@
     }
   }
 
-  fieldset {
+  fieldset, .Alert {
-    margin-bottom: 30px;
+    margin-bottom: 20px;
+  }
 
-    > ul {
+  fieldset > ul {
-      list-style: none;
+    list-style: none;
-      margin: 0;
+    margin: 0;
-      padding: 0;
+    padding: 0;
-    }
   }
 }
 
 .MailPage-MailSettings-input {
 
   label {
-    margin-bottom: 5px;
+    display: block;
-  }
-
-  .FormControl {
     margin-bottom: 7px;
   }
 
   .Select {
     display: block;
   }
-
-  :last-child {
-    margin-bottom: 0;
-  }
 }

framework/core/less/common/ValidationError.less

@@ -0,0 +1,5 @@
+.ValidationError {
+  font-size: 0.9em;
+  font-weight: bold;
+  color: @validation-error-color;
+}

framework/core/less/common/common.less

@@ -26,3 +26,4 @@
 @import "Search";
 @import "Select";
 @import "Tooltip";
+@import "ValidationError";

framework/core/less/common/variables.less

@@ -68,15 +68,19 @@
 @hero-color:                       @control-color;
 @hero-muted-color:                 @control-color;
 
+@error-color:                      #d83e3e;
+
 @alert-bg:                         #fff2ae;
 @alert-color:                      #ad6c00;
 
-@alert-error-bg:                   #d83e3e;
+@alert-error-bg:                   @error-color;
 @alert-error-color:                #fff;
 
 @alert-success-bg:                 #B4F1AF;
 @alert-success-color:              #33722D;
 
+@validation-error-color:           @error-color;
+
 .define-header(@config-colored-header);
 .define-header(false) {
   @header-bg:                      @body-bg;

framework/core/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;
-    }
-}

framework/core/src/Api/Controller/ShowMailSettingsController.php

@@ -0,0 +1,56 @@
+<?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);
+
+        if (method_exists($configured, 'validate')) {
+            $errors = $configured->validate($settings, $validator);
+        } else {
+            $errors = new \Illuminate\Support\MessageBag;
+        }
+
+        return [
+            'drivers' => $drivers,
+            'sending' => method_exists($actual, 'canSend') ? $actual->canSend() : true,
+            'errors' => $errors,
+        ];
+    }
+}

framework/core/src/Api/Serializer/MailDriverSerializer.php

@@ -1,57 +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,
-        ];
-    }
-
-    public function getId($model)
-    {
-        return $model['id'];
-    }
-}

framework/core/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';
+    }
+}

framework/core/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',
+        '/mail-settings',
-        'mailDrivers.index',
+        'mailSettings.index',
-        $route->toController(Controller\ListMailDriversController::class)
+        $route->toController(Controller\ShowMailSettingsController::class)
     );
 };

framework/core/src/Mail/DriverInterface.php

@@ -31,6 +31,28 @@ 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}.
+     */
+    // TODO: Uncomment for beta.13
+    //public function validate(SettingsRepositoryInterface $settings, Factory $validator): MessageBag;
+
+    /**
+     * Does this driver actually send out emails?
+     */
+    // TODO: Uncomment for beta.13
+    //public function canSend(): bool;
+
     /**
      * Build a mail transport based on Flarum's current settings.
      */

framework/core/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);

framework/core/src/Mail/MailServiceProvider.php

@@ -11,7 +11,9 @@ namespace Flarum\Mail;
 
 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
@@ -30,14 +32,33 @@ class MailServiceProvider extends AbstractServiceProvider
         });
 
         $this->app->singleton('mail.driver', function () {
+            $configured = $this->app->make('flarum.mail.configured_driver');
             $settings = $this->app->make(SettingsRepositoryInterface::class);
-            $drivers = $this->app->make('mail.supported_drivers');
+            $validator = $this->app->make(Factory::class);
 
-            return $this->app->make($drivers[$settings->get('mail_driver')]);
+            if (method_exists($configured, 'validate')) {
+                return $configured->validate($settings, $validator)->any()
+                    ? $this->app->make(NullDriver::class)
+                    : $configured;
+            } else {
+                return $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(

framework/core/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
@@ -28,6 +30,20 @@ class MailgunDriver implements DriverInterface
         ];
     }
 
+    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(

framework/core/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
@@ -23,6 +25,18 @@ class MandrillDriver implements DriverInterface
         ];
     }
 
+    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(

framework/core/src/Mail/NullDriver.php

@@ -0,0 +1,39 @@
+<?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\Mail;
+
+use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Validation\Factory;
+use Illuminate\Support\MessageBag;
+use Swift_NullTransport;
+use Swift_Transport;
+
+class NullDriver implements DriverInterface
+{
+    public function availableSettings(): array
+    {
+        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();
+    }
+}

framework/core/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;

framework/core/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
@@ -25,6 +27,20 @@ class SesDriver implements DriverInterface
         ];
     }
 
+    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
     {
         $config = [

framework/core/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;
 
@@ -21,11 +23,27 @@ class SmtpDriver implements DriverInterface
             'mail_host' => '', // a hostname, IPv4 address or IPv6 wrapped in []
             'mail_port' => '', // a number, defaults to 25
             'mail_encryption' => '', // "tls" or "ssl"
-            'mail_username' => '', // required
+            'mail_username' => '',
-            'mail_password' => '', // required
+            '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
     {
         $transport = new Swift_SmtpTransport(