feat: let admins notify all users about updated access codes

config/packages/messenger.yaml

@@ -21,7 +21,7 @@ framework:
             # Mail is deliberately NOT routed to async. Nothing starts a worker on the server --
             # deploy.php only stops them -- so a queued SendEmailMessage would be written to
             # messenger_messages and stay there, with no error anywhere and nobody notified.
-            # Sending happens in the request instead; BookingNotifier catches a transport
+            # Sending happens in the request instead; App\Email\Mailer catches a transport
             # failure so it can never fail the booking that triggered it. Route it back here
             # once a `messenger:consume async` worker actually runs.
             # Symfony\Component\Mailer\Messenger\SendEmailMessage: async

config/services.yaml

@@ -26,9 +26,8 @@ services:
         arguments:
             $logger: '@monolog.logger.audit'
 
-    # Booking notifications. The sender is also the To of every notification -- the actual
+    # Every mail goes out from this address, one per recipient -- see App\Email\Mailer.
-    # recipients are Bcc'd -- so it has to be an address that may both send and receive.
+    App\Email\Mailer:
-    App\Service\BookingNotifier:
         arguments:
             $sender: '%env(string:MAILER_FROM)%'
 

src/Controller/Admin/AccessCode/NotifyController.php

@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Controller\Admin\AccessCode;
+
+use App\Controller\Traits\ConfirmedActionTrait;
+use App\Htmx\HxRedirectResponse;
+use App\Repository\UserRepository;
+use App\Security\Role;
+use App\Service\AccessCodeNotifier;
+use App\Service\AuditLogger;
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Attribute\Route;
+use Symfony\Component\Security\Http\Attribute\IsGranted;
+
+/**
+ * Tells the whole staff that the codes changed. Confirmed first, because a mail to everybody
+ * cannot be taken back -- and the confirmation says how many people that is.
+ */
+#[IsGranted(Role::ADMIN)]
+final class NotifyController extends AbstractController
+{
+    use ConfirmedActionTrait;
+
+    private const CSRF_TOKEN_ID = 'notify-access-codes';
+
+    public function __construct(
+        private readonly AccessCodeNotifier $notifier,
+        private readonly UserRepository $users,
+        private readonly AuditLogger $audit,
+    ) {
+    }
+
+    #[Route('/admin/access-codes/notify', name: 'app_admin_access_code_notify', methods: ['GET', 'POST'])]
+    public function index(Request $request): Response
+    {
+        if (!$request->isMethod(Request::METHOD_POST)) {
+            return $this->render('admin/_modal_access_code_notify.html.twig', [
+                'recipients' => $this->users->countActive(),
+                'csrf_token_id' => self::CSRF_TOKEN_ID,
+            ]);
+        }
+
+        $this->denyUnlessCsrfValid($request, self::CSRF_TOKEN_ID);
+
+        $report = $this->notifier->codesUpdated();
+        $this->audit->accessCodesAnnounced($report->sent, $report->failed);
+
+        if (0 === $report->failed) {
+            $this->addFlash('success', \sprintf('Benachrichtigung an %d Mitarbeitende versendet.', $report->sent));
+        } else {
+            $this->addFlash('warning', \sprintf(
+                'Benachrichtigung an %d Mitarbeitende versendet, %d konnten nicht erreicht werden.',
+                $report->sent,
+                $report->failed,
+            ));
+        }
+
+        return new HxRedirectResponse($this->generateUrl('app_admin_access_code'));
+    }
+}

src/Email/Mailer.php

@@ -0,0 +1,80 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Email;
+
+use Psr\Log\LoggerInterface;
+use Symfony\Bridge\Twig\Mime\TemplatedEmail;
+use Symfony\Component\Mailer\MailerInterface;
+use Symfony\Component\Mime\Address;
+
+/**
+ * Every mail this application sends goes through here.
+ *
+ * The counterpart of myep's App\Email\Mailer, with three deliberate differences. Arguments are
+ * typed rather than an OptionsResolver array, since there are only four of them and all are
+ * required. Every mail is multipart, with a hand-written text part beside the MyE&P layout. And a
+ * failure is logged and counted rather than rethrown -- see below.
+ *
+ * **Sending never throws.** Mail here is always the side effect of something that has already
+ * happened -- a booking committed, a code rotated -- and the person who caused it should not be
+ * shown an error about a mail server. The caller gets a SendReport instead and decides whether
+ * the failures are worth mentioning.
+ *
+ * Every recipient gets their own mail, addressed only to them. Nobody sees who else is on the
+ * list, and a single bad address loses its own mail rather than everybody's.
+ *
+ * Sending is synchronous, inside the request: no messenger worker runs on the server (see
+ * messenger.yaml). At the fifty-odd accounts this application has, that is a few seconds at
+ * most. Routing SendEmailMessage to `async` once a worker runs needs no change here.
+ */
+final readonly class Mailer
+{
+    public function __construct(
+        private MailerInterface $mailer,
+        private LoggerInterface $logger,
+        private string $sender,
+    ) {
+    }
+
+    /**
+     * @param iterable<Address|string> $recipients
+     * @param string                   $template   the template pair's basename; both .html.twig
+     *                                             and .txt.twig of it are rendered
+     * @param array<string, mixed>     $context
+     */
+    public function send(iterable $recipients, string $subject, string $template, array $context = []): SendReport
+    {
+        $sent = 0;
+        $failed = 0;
+
+        foreach ($recipients as $recipient) {
+            $email = (new TemplatedEmail())
+                ->from(new Address($this->sender, 'E&P Platz'))
+                ->to($recipient)
+                ->subject($subject)
+                ->htmlTemplate($template.'.html.twig')
+                ->textTemplate($template.'.txt.twig')
+                ->context($context);
+
+            try {
+                $this->mailer->send($email);
+                ++$sent;
+            } catch (\Throwable $e) {
+                // Caught per recipient, so one unreachable address does not cost the others theirs.
+                // \Throwable rather than TransportExceptionInterface because the templates are
+                // rendered inside send(): a typo in one of them would otherwise surface as an
+                // error in whatever action triggered the mail.
+                ++$failed;
+                $this->logger->error('Email could not be sent.', [
+                    'subject' => $subject,
+                    'recipient' => $recipient instanceof Address ? $recipient->getAddress() : $recipient,
+                    'exception' => $e,
+                ]);
+            }
+        }
+
+        return new SendReport($sent, $failed);
+    }
+}

src/Email/SendReport.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Email;
+
+/**
+ * What came of a Mailer::send(): how many recipients the mail reached the transport for, and how
+ * many it did not. The failures themselves are in the log.
+ */
+final readonly class SendReport
+{
+    public function __construct(
+        public int $sent,
+        public int $failed,
+    ) {
+    }
+}

src/Repository/UserRepository.php

@@ -30,4 +30,20 @@ class UserRepository extends ServiceEntityRepository
     {
         return $this->findOneBy(['sub' => $sub]);
     }
+
+    /**
+     * Everybody who can currently sign in -- the audience of a mail meant for the whole staff.
+     * A locked account is left out: its holder could not follow the link in it anyway.
+     *
+     * @return list<User>
+     */
+    public function findActive(): array
+    {
+        return $this->findBy(['disabledAt' => null], ['name' => 'ASC']);
+    }
+
+    public function countActive(): int
+    {
+        return $this->count(['disabledAt' => null]);
+    }
 }

src/Service/AccessCodeNotifier.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service;
+
+use App\Email\Mailer;
+use App\Email\SendReport;
+use App\Entity\User;
+use App\Repository\UserRepository;
+use Symfony\Component\Mime\Address;
+
+/**
+ * Tells the whole staff that the access codes have changed -- and nothing more.
+ *
+ * **The mail carries no codes, and no door names either.** A mailbox is not a place a door code
+ * should end up: it is synced to phones, forwarded, and kept long after the code is rotated away.
+ * The door names are left out too, because some of them belong to codes only supervisors or
+ * administrators may read, and naming them to everybody would leak what this application
+ * otherwise keeps from them. So every recipient gets the same mail, and the link in it leads to
+ * /access-codes, which shows each of them exactly what they may see.
+ *
+ * Sent on an administrator's explicit request rather than on every save: a rotation is usually
+ * several edits in a row, and a corrected description or an admin-only code is no news to the
+ * staff.
+ */
+final readonly class AccessCodeNotifier
+{
+    public function __construct(
+        private Mailer $mailer,
+        private UserRepository $users,
+    ) {
+    }
+
+    public function codesUpdated(): SendReport
+    {
+        return $this->mailer->send(
+            array_map(
+                static fn (User $user): Address => new Address($user->getEmail(), $user->getName()),
+                $this->users->findActive(),
+            ),
+            'Neue Zugangscodes bei E&P',
+            'email/access_codes_updated',
+        );
+    }
+}

src/Service/AuditLogger.php

@@ -295,6 +295,18 @@ final readonly class AuditLogger
         ]);
     }
 
+    /**
+     * The staff told that the codes changed. Counts only: the recipients are everybody, and
+     * which addresses failed is in the framework log, where a failure belongs.
+     */
+    public function accessCodesAnnounced(int $sent, int $failed): void
+    {
+        $this->logger->info('Access code update announced.', [
+            'sent' => $sent,
+            'failed' => $failed,
+        ]);
+    }
+
     /**
      * @return array<string, mixed>
      */

src/Service/BookingNotifier.php

@@ -4,14 +4,11 @@ declare(strict_types=1);
 
 namespace App\Service;
 
+use App\Email\Mailer;
 use App\Entity\Booking;
 use App\Enum\ResourceType;
 use App\Model\SeriesResult;
 use App\Repository\ResourceTypeSettingsRepository;
-use Psr\Log\LoggerInterface;
-use Symfony\Bridge\Twig\Mime\TemplatedEmail;
-use Symfony\Component\Mailer\MailerInterface;
-use Symfony\Component\Mime\Address;
 
 /**
  * Tells whoever an administrator named that something was booked.
@@ -22,25 +19,14 @@ use Symfony\Component\Mime\Address;
  * would otherwise never notify anybody.
  *
  * **Nothing here may break a booking.** It runs after the transaction has committed, so a
- * failure cannot roll anything back, and a transport that is down is logged rather than thrown:
+ * failure cannot roll anything back -- and Mailer never throws, so the person who booked is not
- * the booking is made, and the person who made it should not be shown an error about somebody
+ * shown an error about somebody else's mail server.
- * else's mail server.
- *
- * Every recipient gets their own mail, addressed only to them. Nobody sees who else is on the
- * list, and a single bad address loses its own mail rather than everybody's.
- *
- * Each mail is multipart: the MyE&P layout for clients that render HTML, and a hand-written text
- * part for those that do not. The text part is written rather than generated because there is no
- * html-to-markdown converter installed, and three facts and a name read better as plain lines
- * than as a flattened table anyway.
  */
 final readonly class BookingNotifier
 {
     public function __construct(
-        private MailerInterface $mailer,
+        private Mailer $mailer,
         private ResourceTypeSettingsRepository $settings,
-        private LoggerInterface $logger,
-        private string $sender,
     ) {
     }
 
@@ -80,42 +66,10 @@ final readonly class BookingNotifier
     }
 
     /**
-     * @param string               $template the template pair's basename; both .html.twig and
-     *                                       .txt.twig of it are rendered
      * @param array<string, mixed> $context
      */
     private function send(ResourceType $type, string $subject, string $template, array $context): void
     {
-        $recipients = $this->settings->recipientsFor($type);
+        $this->mailer->send($this->settings->recipientsFor($type), $subject, $template, $context);
-
-        if ([] === $recipients) {
-            return;
-        }
-
-        foreach ($recipients as $recipient) {
-            // A fresh message per recipient: ->to() on a reused one appends rather than replaces,
-            // which would put the whole list back on everybody's copy.
-            $email = (new TemplatedEmail())
-                ->from(new Address($this->sender, 'E&P Platz'))
-                ->to($recipient)
-                ->subject($subject)
-                ->htmlTemplate($template.'.html.twig')
-                ->textTemplate($template.'.txt.twig')
-                ->context($context);
-
-            try {
-                $this->mailer->send($email);
-            } catch (\Throwable $e) {
-                // Caught per recipient, so one unreachable address does not cost the others theirs.
-                // \Throwable rather than TransportExceptionInterface because the templates are
-                // rendered inside send(): a typo in one of them would otherwise surface as a failed
-                // booking, which is exactly what this class promises never to do.
-                $this->logger->error('Booking notification could not be sent.', [
-                    'subject' => $subject,
-                    'recipient' => $recipient,
-                    'exception' => $e,
-                ]);
-            }
-        }
     }
 }

templates/admin/_modal_access_code_notify.html.twig

@@ -0,0 +1,15 @@
+{% extends 'htmx_confirmation_modal.html.twig' %}
+
+{% block title %}Mitarbeitende benachrichtigen?{% endblock %}
+
+{% block content %}
+    <p>
+        {{ recipients }} Mitarbeitende erhalten eine E-Mail, dass sich die Zugangscodes geändert
+        haben, mit einem Link zu dieser Anwendung.
+    </p>
+    <p class="pt-2 text-sm text-gray-500">
+        Die Codes selbst stehen aus Sicherheitsgründen nicht in der E-Mail.
+    </p>
+{% endblock %}
+
+{% block button_confirm %}Senden{% endblock %}

templates/admin/access_codes.html.twig

@@ -20,10 +20,16 @@
         <div>
             <h1>Zugangscodes verwalten</h1>
         </div>
-        <button type="button" class="button button--small button--primary"
+        <div class="flex flex-wrap gap-2">
-                hx-get="{{ path('app_admin_access_code_new') }}"
+            <button type="button" class="button button--small button--neutral"
-                hx-target="body"
+                    hx-get="{{ path('app_admin_access_code_notify') }}"
-                hx-swap="beforeend">Code hinzufügen</button>
+                    hx-target="body"
+                    hx-swap="beforeend">Mitarbeitende benachrichtigen</button>
+            <button type="button" class="button button--small button--primary"
+                    hx-get="{{ path('app_admin_access_code_new') }}"
+                    hx-target="body"
+                    hx-swap="beforeend">Code hinzufügen</button>
+        </div>
     </div>
 
     {% if rows is empty %}

templates/email/access_codes_updated.html.twig

@@ -0,0 +1,14 @@
+{# No codes and no door names -- see AccessCodeNotifier for why. #}
+{% extends 'email/layout.html.twig' %}
+
+{% block body %}
+    <h1>Neue Zugangscodes</h1>
+
+    <p>
+        Die Zugangscodes wurden aktualisiert. Die aktuell gültigen Codes findest du in E&amp;P Platz.
+    </p>
+
+    <p>
+        <a href="{{ url('app_access_code') }}" class="button">Zugangscodes ansehen</a>
+    </p>
+{% endblock %}

templates/email/access_codes_updated.txt.twig

@@ -0,0 +1,7 @@
+{# No codes and no door names -- see AccessCodeNotifier for why. #}
+Die Zugangscodes wurden aktualisiert. Die aktuell gültigen Codes findest du in E&P Platz:
+
+{{ url('app_access_code') }}
+
+--
+E&P Platz

tests/Controller/Admin/AccessCodeAdminControllerTest.php

@@ -56,6 +56,10 @@ class AccessCodeAdminControllerTest extends WebTestCase
         self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN);
         $this->em->clear();
         self::assertNotNull($this->em->find(AccessCode::class, $code->getId()));
+
+        $this->client->request('POST', '/admin/access-codes/notify', ['_token' => 'whatever'], server: ['HTTP_HX-Request' => 'true']);
+        self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN);
+        self::assertEmailCount(0);
     }
 
     /**
@@ -219,6 +223,59 @@ class AccessCodeAdminControllerTest extends WebTestCase
         self::assertNotNull($this->em->find(AccessCode::class, $code->getId()));
     }
 
+    /**
+     * Everybody who can sign in gets a mail of their own, and none of them contains a code -- not
+     * even one only the recipient's own role could read.
+     */
+    public function testNotifyingTheStaffMailsEveryActiveUserWithoutTheCodes(): void
+    {
+        $this->code('Aufzug', '4711');
+        $this->code('Tresor', '9876');
+        $admin = $this->admin();
+        $this->user('[email protected]');
+        $locked = $this->user('[email protected]');
+        $locked->disable('ausgeschieden');
+        $this->em->flush();
+        $expected = $this->em->getRepository(User::class)->count(['disabledAt' => null]);
+        $this->client->loginUser($admin);
+        $uri = '/admin/access-codes/notify';
+
+        $modal = $this->client->request('GET', $uri, server: ['HTTP_HX-Request' => 'true']);
+        self::assertStringContainsString(\sprintf('%d Mitarbeitende', $expected), $modal->text());
+
+        $this->client->request('POST', $uri, ['_token' => $this->confirmationToken($uri)], server: ['HTTP_HX-Request' => 'true']);
+
+        self::assertSame('/admin/access-codes', $this->client->getResponse()->headers->get('HX-Redirect'));
+        self::assertEmailCount($expected);
+
+        $recipients = [];
+        foreach (self::getMailerMessages() as $email) {
+            self::assertCount(1, $email->getTo());
+            $recipients[] = $email->getTo()[0]->getAddress();
+
+            foreach ([$email->getTextBody(), $email->getHtmlBody()] as $body) {
+                self::assertStringContainsString('https://localhost/access-codes', $body);
+                self::assertStringNotContainsString('4711', $body);
+                self::assertStringNotContainsString('9876', $body);
+                self::assertStringNotContainsString('Aufzug', $body);
+            }
+        }
+
+        self::assertContains('[email protected]', $recipients);
+        self::assertContains('[email protected]', $recipients);
+        self::assertNotContains('[email protected]', $recipients);
+    }
+
+    public function testNotifyingTheStaffNeedsAValidToken(): void
+    {
+        $this->client->loginUser($this->admin());
+
+        $this->client->request('POST', '/admin/access-codes/notify', ['_token' => 'whatever'], server: ['HTTP_HX-Request' => 'true']);
+
+        self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN);
+        self::assertEmailCount(0);
+    }
+
     /**
      * Opens the modal, then posts the given fields with its CSRF token -- the round trip a
      * browser makes.

tests/Email/MailerTest.php

@@ -0,0 +1,103 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Email;
+
+use App\Email\Mailer;
+use Monolog\Handler\TestHandler;
+use Monolog\Logger;
+use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\Twig\Mime\TemplatedEmail;
+use Symfony\Component\Mailer\Envelope;
+use Symfony\Component\Mailer\Exception\TransportException;
+use Symfony\Component\Mailer\MailerInterface;
+use Symfony\Component\Mime\Address;
+use Symfony\Component\Mime\RawMessage;
+
+/**
+ * The two promises every caller relies on: one mail per recipient, addressed to them alone, and
+ * a failure that costs only its own recipient and is never thrown.
+ */
+class MailerTest extends TestCase
+{
+    public function testEveryRecipientGetsTheirOwnMail(): void
+    {
+        $transport = $this->transport();
+        $mailer = new Mailer($transport, new Logger('test'), '[email protected]');
+
+        $report = $mailer->send(
+            ['[email protected]', new Address('[email protected]', 'B')],
+            'Betreff',
+            'email/some',
+            ['key' => 'value'],
+        );
+
+        self::assertSame(2, $report->sent);
+        self::assertSame(0, $report->failed);
+        self::assertCount(2, $transport->sent);
+
+        foreach ($transport->sent as $i => $email) {
+            self::assertCount(1, $email->getTo());
+            self::assertSame(['[email protected]', '[email protected]'][$i], $email->getTo()[0]->getAddress());
+            self::assertSame('[email protected]', $email->getFrom()[0]->getAddress());
+            self::assertSame('E&P Platz', $email->getFrom()[0]->getName());
+            self::assertSame('Betreff', $email->getSubject());
+            self::assertSame('email/some.html.twig', $email->getHtmlTemplate());
+            self::assertSame('email/some.txt.twig', $email->getTextTemplate());
+            self::assertSame(['key' => 'value'], $email->getContext());
+        }
+    }
+
+    public function testAFailedRecipientIsLoggedAndTheOthersStillGetTheirs(): void
+    {
+        $transport = $this->transport(failFor: '[email protected]');
+        $log = new TestHandler();
+        $mailer = new Mailer($transport, new Logger('test', [$log]), '[email protected]');
+
+        $report = $mailer->send(['[email protected]', '[email protected]', '[email protected]'], 'Betreff', 'email/some');
+
+        self::assertSame(2, $report->sent);
+        self::assertSame(1, $report->failed);
+        self::assertCount(2, $transport->sent);
+        self::assertTrue($log->hasErrorThatContains('Email could not be sent.'));
+        self::assertSame('[email protected]', $log->getRecords()[0]->context['recipient']);
+    }
+
+    public function testNoRecipientsSendsNothing(): void
+    {
+        $transport = $this->transport();
+        $mailer = new Mailer($transport, new Logger('test'), '[email protected]');
+
+        $report = $mailer->send([], 'Betreff', 'email/some');
+
+        self::assertSame(0, $report->sent);
+        self::assertSame([], $transport->sent);
+    }
+
+    /**
+     * @return MailerInterface&object{sent: list<TemplatedEmail>}
+     */
+    private function transport(?string $failFor = null): MailerInterface
+    {
+        return new class($failFor) implements MailerInterface {
+            /** @var list<TemplatedEmail> */
+            public array $sent = [];
+
+            public function __construct(private readonly ?string $failFor)
+            {
+            }
+
+            public function send(RawMessage $message, ?Envelope $envelope = null): void
+            {
+                \assert($message instanceof TemplatedEmail);
+
+                if ($message->getTo()[0]->getAddress() === $this->failFor) {
+                    throw new TransportException('550 mailbox unavailable');
+                }
+
+                $this->sent[] = $message;
+            }
+        };
+    }
+}

tests/Service/AuditLoggerTest.php

@@ -180,6 +180,14 @@ class AuditLoggerTest extends TestCase
         self::assertSame(7, $records[2]->context['access_code_id']);
     }
 
+    public function testAnAnnouncementIsRecordedWithItsCounts(): void
+    {
+        $this->audit->accessCodesAnnounced(50, 1);
+
+        self::assertSame('Access code update announced.', $this->record()->message);
+        self::assertSame(['sent' => 50, 'failed' => 1], $this->record()->context);
+    }
+
     private function record(): LogRecord
     {
         return $this->handler->getRecords()[0];