iTip fixes

README.md

@@ -167,6 +167,21 @@ INVITE_FROM_ADDRESS=no-reply@example.org
 > If the username, password or host contain any character considered special in a URI (such as `: / ? # [ ] @ ! $ & ' ( ) * + , ; =`), you MUST encode them.
 > See [here](https://symfony.com/doc/current/mailer.html#transport-setup) for more details.
 
+If `INVITE_FROM_ADDRESS` is empty, Davis does not send scheduling emails at all and logs a warning when the DAV endpoint starts.
+
+Invitations are only produced for events whose `ORGANIZER` matches the organiser's own address, and that address is the `email` of their principal — the *Email* field of the account in the dashboard. If it is empty or different from what the calendar client sends, the server has nothing to send an invitation about and stays silent. To find accounts in that state:
+
+```sql
+SELECT uri, email FROM principals WHERE email IS NULL OR email = '';
+```
+
+To check the mailer itself without creating an event:
+
+```shell
+php bin/console davis:mail:test you@example.org
+```
+
+
 **f. The reminder offset for all birthdays**
 
 You must specify a relative duration, as specified in [the RFC 5545 spec](https://www.rfc-editor.org/rfc/rfc5545.html#section-3.3.6)

config/services.yaml

@@ -30,6 +30,10 @@ services:
         arguments:
             $authRealm: "%env(AUTH_REALM)%"
 
+    App\Command\MailTestCommand:
+        arguments:
+            $inviteAddress: "%env(INVITE_FROM_ADDRESS)%"
+
     App\Services\IMAPAuth:
         arguments:
             $IMAPAuthUrl: "%env(IMAP_AUTH_URL)%"

src/Command/MailTestCommand.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace App\Command;
+
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
+use Symfony\Component\Mailer\MailerInterface;
+use Symfony\Component\Mime\Address;
+use Symfony\Component\Mime\Email;
+use Symfony\Component\Mime\Exception\RfcComplianceException;
+
+/**
+ * Invitations are sent while a calendar client is storing an event, so a broken MAILER_DSN shows
+ * up as a missing email and nothing else. This sends one message with the same settings.
+ */
+#[AsCommand(
+    name: 'davis:mail:test',
+    description: 'Send a test email to check MAILER_DSN and INVITE_FROM_ADDRESS',
+)]
+class MailTestCommand extends Command
+{
+    public function __construct(
+        private MailerInterface $mailer,
+        private ?string $inviteAddress = null,
+    ) {
+        parent::__construct();
+    }
+
+    protected function configure(): void
+    {
+        $this
+            ->addArgument('to', InputArgument::REQUIRED, 'The address to send the test email to')
+            ->setHelp('This command sends a test email through the configured mailer, the way scheduling invitations are sent.');
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output): int
+    {
+        $io = new SymfonyStyle($input, $output);
+        $to = $input->getArgument('to');
+
+        if (!$this->inviteAddress) {
+            $io->error('INVITE_FROM_ADDRESS is not set, so Davis does not send scheduling invitations at all.');
+
+            return self::FAILURE;
+        }
+
+        $io->text(sprintf('Sending a test email from <info>%s</info> to <info>%s</info>…', $this->inviteAddress, $to));
+
+        try {
+            $this->mailer->send(
+                (new Email())
+                    ->from(new Address($this->inviteAddress, 'Davis'))
+                    ->to(new Address($to))
+                    ->subject('Davis test email')
+                    ->text("This is a test email from Davis.\n\nIf you received it, scheduling invitations can be delivered with the current configuration.\n")
+            );
+        } catch (RfcComplianceException $e) {
+            $io->error(sprintf('"%s" is not a valid email address: %s', $to, $e->getMessage()));
+
+            return self::FAILURE;
+        } catch (TransportExceptionInterface $e) {
+            $io->error('The email could not be sent: '.$e->getMessage());
+            $io->note('Check MAILER_DSN. Davis logs the same error and keeps saving events when this happens.');
+
+            return self::FAILURE;
+        }
+
+        $io->success('The email was handed to the transport. Check the destination mailbox.');
+
+        return self::SUCCESS;
+    }
+}

src/Controller/DAVController.php

@@ -276,7 +276,11 @@ private function initServer(string $authMethod, string $authRealm = User::DEFAUL
             $this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
             $this->server->addPlugin(new \Sabre\CalDAV\Subscriptions\Plugin());
             if ($this->inviteAddress) {
-                $this->server->addPlugin(new DavisIMipPlugin($this->mailer, $this->inviteAddress, $this->publicDir));
+                $this->server->addPlugin(new DavisIMipPlugin($this->mailer, $this->inviteAddress, $this->publicDir, $this->logger));
+            } else {
+                // Without it the scheduling plugin above still answers, but no invitation ever
+                // leaves the server and nothing says so.
+                $this->logger->warning('CalDAV scheduling is enabled but INVITE_FROM_ADDRESS is not set: no invitation email will be sent.');
             }
         }
 

src/Form/UserType.php

@@ -13,6 +13,7 @@
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Component\Validator\Constraints as Assert;
 
 class UserType extends AbstractType
 {
@@ -27,10 +28,23 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
             ->add('displayName', TextType::class, [
                 'label' => 'form.displayName',
                 'mapped' => false,
+                'constraints' => [
+                    new Assert\Length(max: 255),
+                ],
             ])
+            // The field is unmapped — it belongs to the Principal, not the User — so the entity's
+            // own constraints never run on it and the rules have to live here. An empty address
+            // keeps the principal out of its own `calendar-user-address-set`, which means sabre
+            // never emits a scheduling message and invitations are silently never sent.
             ->add('email', EmailType::class, [
                 'label' => 'form.email',
                 'mapped' => false,
+                'help' => 'form.email.help',
+                'constraints' => [
+                    new Assert\NotBlank(),
+                    new Assert\Email(),
+                    new Assert\Length(max: 255),
+                ],
             ])
             ->add('password', RepeatedType::class, [
                 'type' => PasswordType::class,

src/Plugins/DavisIMipPlugin.php

@@ -5,12 +5,15 @@
 use DantSu\OpenStreetMapStaticAPI\LatLng;
 use DantSu\OpenStreetMapStaticAPI\Markers;
 use DantSu\OpenStreetMapStaticAPI\OpenStreetMap;
+use Psr\Log\LoggerInterface;
 use Sabre\CalDAV\Schedule\IMipPlugin as SabreBaseIMipPlugin;
 use Sabre\DAV;
 use Sabre\VObject\ITip;
 use Symfony\Bridge\Twig\Mime\TemplatedEmail;
+use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
 use Symfony\Component\Mailer\MailerInterface;
 use Symfony\Component\Mime\Address;
+use Symfony\Component\Mime\Exception\RfcComplianceException;
 
 /**
  * iMIP handler.
@@ -34,14 +37,20 @@ final class DavisIMipPlugin extends SabreBaseIMipPlugin
      */
     protected $publicDir;
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
     /**
      * Creates the email handler.
      */
-    public function __construct(MailerInterface $mailer, string $senderEmail, string $publicDir)
+    public function __construct(MailerInterface $mailer, string $senderEmail, string $publicDir, LoggerInterface $logger)
     {
         $this->mailer = $mailer;
         $this->senderEmail = $senderEmail;
         $this->publicDir = $publicDir;
+        $this->logger = $logger;
     }
 
     /**
@@ -52,6 +61,8 @@ public function schedule(ITip\Message $itip)
         // Not sending any emails if the system considers the update
         // insignificant.
         if (!$itip->significantChange) {
+            $this->logger->debug('iMIP: no email for an insignificant change', ['method' => $itip->method, 'recipient' => $itip->recipient]);
+
             if (empty($itip->scheduleStatus)) {
                 $itip->scheduleStatus = '1.0;We got the message, but it\'s not significant enough to warrant an email';
             }
@@ -63,6 +74,9 @@ public function schedule(ITip\Message $itip)
 
         if ('mailto' !== parse_url($itip->sender, PHP_URL_SCHEME)
             || 'mailto' !== parse_url($itip->recipient, PHP_URL_SCHEME)) {
+            // iTIP allows other schemes; this plugin only knows how to send email.
+            $this->logger->warning('iMIP: not an email exchange, no invitation sent', ['sender' => $itip->sender, 'recipient' => $itip->recipient]);
+
             return;
         }
 
@@ -90,6 +104,9 @@ public function schedule(ITip\Message $itip)
         }
 
         $subject = 'CalDAV message';
+        // Anything other than the three methods below leaves the template without an action to
+        // render, so the message is not one this plugin knows how to word.
+        $action = null;
         switch (strtoupper($itip->method)) {
             case 'REPLY':
                 // In the case of a reply, we need to find the `PARTSTAT` from
@@ -109,6 +126,7 @@ public function schedule(ITip\Message $itip)
                         $action = 'TENTATIVE';
                         break;
                     default:
+                        $this->logger->warning('iMIP: unsupported PARTSTAT in a REPLY, no email sent', ['partstat' => $partstat, 'recipient' => $itip->recipient]);
                         $itip->scheduleStatus = '5.0;Email not delivered. We didn\'t understand this PARTSTAT.';
 
                         return;
@@ -123,6 +141,11 @@ public function schedule(ITip\Message $itip)
                 $subject = '"'.$summary.'" has been canceled.';
                 $action = 'CANCEL';
                 break;
+            default:
+                $this->logger->warning('iMIP: unsupported iTIP method, no email sent', ['method' => $itip->method, 'recipient' => $itip->recipient]);
+                $itip->scheduleStatus = '5.0;Email not delivered. We don\'t know how to send this iTIP method.';
+
+                return;
         }
 
         // Construct objects for the mail template
@@ -192,14 +215,21 @@ public function schedule(ITip\Message $itip)
                 $latLng = new LatLng($coordinates['latitude'], $coordinates['longitude']);
 
                 // https://github.com/DantSu/php-osm-static-api
-                $locationImageDataAsBase64 = (new OpenStreetMap($latLng, $zoom, $width, $height))
-                    ->addMarkers(
-                        (new Markers($this->publicDir.'/images/marker.png'))
-                            ->setAnchor(Markers::ANCHOR_CENTER, Markers::ANCHOR_BOTTOM)
-                            ->addMarker(new LatLng($coordinates['latitude'], $coordinates['longitude']))
-                    )
-                    ->getImage()
-                    ->getBase64PNG();
+                // This fetches map tiles over the network. The map is decoration on an invitation,
+                // so a tile server that is unreachable or slow must not take the invitation — and
+                // with it the client's PUT — down with it.
+                try {
+                    $locationImageDataAsBase64 = (new OpenStreetMap($latLng, $zoom, $width, $height))
+                        ->addMarkers(
+                            (new Markers($this->publicDir.'/images/marker.png'))
+                                ->setAnchor(Markers::ANCHOR_CENTER, Markers::ANCHOR_BOTTOM)
+                                ->addMarker(new LatLng($coordinates['latitude'], $coordinates['longitude']))
+                        )
+                        ->getImage()
+                        ->getBase64PNG();
+                } catch (\Throwable $e) {
+                    $this->logger->warning('iMIP: the location map could not be rendered, sending the invitation without it', ['exception' => $e->getMessage()]);
+                }
 
                 $locationLink =
                     'https://www.openstreetmap.org'.
@@ -224,11 +254,24 @@ public function schedule(ITip\Message $itip)
             $mailSenderName = $senderEmail.' '.static::MESSAGE_ORIGIN_INDICATOR;
         }
 
-        $message = (new TemplatedEmail())
-            ->from(new Address($this->senderEmail, $mailSenderName))
-            ->to(new Address($recipientEmail, $recipientName ?? ''))
-            ->replyTo(new Address($senderEmail, $mailSenderName))
-            ->subject($subject);
+        try {
+            $message = (new TemplatedEmail())
+                ->from(new Address($this->senderEmail, $mailSenderName))
+                ->to(new Address($recipientEmail, $recipientName ?? ''))
+                ->replyTo(new Address($senderEmail, $mailSenderName))
+                ->subject($subject);
+        } catch (RfcComplianceException $e) {
+            // An attendee address comes from the organiser's client and is not validated anywhere
+            // before this point. Letting it through would abort the PUT that triggered it.
+            $this->logger->error('iMIP: an address in the invitation is not a valid email address, no email sent', [
+                'sender' => $senderEmail,
+                'recipient' => $recipientEmail,
+                'exception' => $e->getMessage(),
+            ]);
+            $itip->scheduleStatus = '5.3;Email not delivered. One of the addresses is not a valid email address.';
+
+            return;
+        }
 
         // Keep holiday auto-replies from bouncing back at invitations.
         $message->getHeaders()->addTextHeader('X-Auto-Response-Suppress', 'OOF, DR, RN, NRN, AutoReply');
@@ -259,7 +302,23 @@ public function schedule(ITip\Message $itip)
             $message->attach($itip->message->serialize(), 'invite.ics', 'text/calendar; method='.(string) $itip->method.'; charset=UTF-8');
         }
 
-        $this->mailer->send($message);
+        try {
+            $this->mailer->send($message);
+        } catch (TransportExceptionInterface $e) {
+            // The scheduling message is a side effect of storing the event: if the mail cannot be
+            // sent, the event still has to be saved. Throwing here would surface as a 500 on the
+            // client's PUT and lose it.
+            $this->logger->error('iMIP: the invitation could not be sent', [
+                'recipient' => $recipientEmail,
+                'method' => $itip->method,
+                'exception' => $e->getMessage(),
+            ]);
+            $itip->scheduleStatus = '5.1;Email not delivered. The mail transport is unavailable.';
+
+            return;
+        }
+
+        $this->logger->info('iMIP: invitation sent', ['recipient' => $recipientEmail, 'method' => $itip->method]);
 
         if (false === $deliveredLocally) {
             $itip->scheduleStatus = '1.1;Scheduling message is sent via iMip.';

tests/Functional/Controllers/UserControllerTest.php

@@ -91,6 +91,36 @@ public function testUserNew(): void
         $this->assertAnySelectorTextContains('h5', 'New test User');
     }
 
+    /**
+     * The address is what sabre puts in the principal's `calendar-user-address-set`, and it only
+     * emits a scheduling message when the event's organiser matches one of those. An account saved
+     * without an address therefore never sends an invitation, and nothing says so at the time.
+     */
+    public function testUserCreationRequiresAnEmailAddress(): void
+    {
+        $client = static::createClient();
+        $client->loginUser(new AdminUser('admin', 'test'));
+
+        foreach (['', 'not-an-address'] as $email) {
+            $crawler = $client->request('GET', '/users/new');
+            $form = $crawler->selectButton('user_save')->form();
+
+            $client->submit($form, [
+                'user[username]' => 'no_email_user',
+                'user[displayName]' => 'No email',
+                'user[email]' => $email,
+                'user[password][first]' => 'coucou',
+                'user[password][second]' => 'coucou',
+            ]);
+
+            $this->assertResponseIsSuccessful(sprintf('"%s" should be refused by the form', $email));
+            $this->assertSelectorExists('.invalid-feedback, .form-error-message');
+        }
+
+        $em = static::getContainer()->get('doctrine.orm.entity_manager');
+        $this->assertNull($em->getRepository(User::class)->findOneBy(['username' => 'no_email_user']));
+    }
+
     public function testUserDelete(): void
     {
         $user = new AdminUser('admin', 'test');