fix(mailer): use noreply@bewelcome.org as From to pass DMARC

src/Service/Mailer.php

@@ -97,10 +97,11 @@ public function sendCommentReportedFeedbackEmail(Member $member, $parameters): b
         $feedbackCategory = $feedbackCategoryRepository->findOneBy(['name' => 'Comment_issue']);
 
         return $this->sendTemplateEmail(
-            new Address($member->getEmail()),
+            new Address(self::NO_REPLY_EMAIL_ADDRESS, 'BeWelcome'),
             new Address($feedbackCategory->getEmailToNotify(), 'Comment Issue'),
             'comment.feedback',
-            $parameters
+            $parameters,
+            $member->getEmail(),
         );
     }
 
@@ -147,18 +148,20 @@ public function sendNotificationEmail(Address $sender, Member $receiver, $parame
     }
 
     /**
-     * This feeds the feedback given by a user into the OTRS queues.
+     * Sends contact/feedback form submissions to the helpdesk queue.
+     * From: is always noreply@bewelcome.org to pass DMARC; the reporter's address goes in Reply-To.
      */
     public function sendFeedbackEmail($sender, Address $receiver, $parameters): bool
     {
         $parameters['subject'] = "Your feedback in '"
             . str_replace('_', ' ', ($parameters['IdCategory'])->getName()) . "'";
 
         return $this->sendTemplateEmail(
-            $sender,
+            self::NO_REPLY_EMAIL_ADDRESS,
             $receiver,
             'feedback',
-            $parameters
+            $parameters,
+            \is_string($sender) ? $sender : null,
         );
     }
 
@@ -298,7 +301,7 @@ private function getBeWelcomeAddress(Member $sender, $email): Address
      *
      * @return bool
      */
-    private function sendTemplateEmail($sender, $receiver, string $template, array $parameters): bool
+    private function sendTemplateEmail($sender, $receiver, string $template, array $parameters, ?string $replyTo = null): bool
     {
         $currentLocale = $this->translator->getLocale();
         $success = true;
@@ -337,6 +340,10 @@ private function sendTemplateEmail($sender, $receiver, string $template, array $
         }
         $email->from($sender);
 
+        if (null !== $replyTo) {
+            $email->replyTo(new Address($replyTo));
+        }
+
         try {
             $this->mailer->send($email);
         } catch (TransportExceptionInterface $e) {

templates/emails/feedback.html.twig

@@ -1,5 +1,8 @@
 {% if member is not null %}
-    Member {{ member.username }} has sent some feedback.<br>
+    Member {{ member.username }} ({{ FeedbackEmail }}) has sent some feedback.<br>
+    <br>
+{% elseif FeedbackEmail is not null %}
+    From: {{ FeedbackEmail }}<br>
     <br>
 {% endif %}
 {{ FeedbackQuestion|raw }}

tests/Service/MailerTest.php

@@ -0,0 +1,162 @@
+<?php
+
+namespace App\Tests\Service;
+
+use App\Entity\FeedbackCategory;
+use App\Entity\Member;
+use App\Logger\Logger;
+use App\Service\Mailer;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\EntityRepository;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Mailer\MailerInterface;
+use Symfony\Component\Mime\Address;
+use Symfony\Component\Mime\Email;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+use Symfony\Contracts\Translation\TranslatorInterface;
+
+class MailerTest extends TestCase
+{
+    private MailerInterface $innerMailer;
+    private Mailer $mailer;
+
+    protected function setUp(): void
+    {
+        $translator = $this->createStub(TranslatorInterface::class);
+        $translator->method('getLocale')->willReturn('en');
+        $translator->method('trans')->willReturnArgument(0);
+
+        $this->innerMailer = $this->createMock(MailerInterface::class);
+
+        $this->mailer = new Mailer(
+            $this->createStub(EntityManagerInterface::class),
+            $this->createStub(UrlGeneratorInterface::class),
+            $translator,
+            $this->innerMailer,
+            $this->createStub(Logger::class),
+        );
+    }
+
+    public function testSendFeedbackEmailUsesNoreplyAsFrom(): void
+    {
+        $captured = null;
+        $this->innerMailer
+            ->expects($this->once())
+            ->method('send')
+            ->with($this->callback(static function (Email $email) use (&$captured) {
+                $captured = $email;
+
+                return true;
+            }));
+
+        $category = $this->createStub(FeedbackCategory::class);
+        $category->method('getName')->willReturn('Safety_Concern');
+
+        $this->mailer->sendFeedbackEmail(
+            'reporter@yahoo.fr',
+            new Address('abuse@bewelcome.org'),
+            [
+                'IdCategory' => $category,
+                'FeedbackQuestion' => 'Test feedback',
+                'member' => null,
+                'no_reply_needed' => false,
+                'browser' => 'Firefox',
+                'host' => 'bewelcome.org',
+                'version' => '1.0',
+            ]
+        );
+
+        $this->assertNotNull($captured);
+
+        $from = $captured->getFrom();
+        $this->assertCount(1, $from);
+        $this->assertSame('noreply@bewelcome.org', $from[0]->getAddress(), 'From must be noreply to pass DMARC');
+
+        $replyTo = $captured->getReplyTo();
+        $this->assertCount(1, $replyTo);
+        $this->assertSame('reporter@yahoo.fr', $replyTo[0]->getAddress(), 'Reply-To must be the reporter so staff can reply');
+    }
+
+    public function testSendFeedbackEmailNullFallbackUsesNoreplyWithNoReplyTo(): void
+    {
+        $captured = null;
+        $this->innerMailer
+            ->expects($this->once())
+            ->method('send')
+            ->with($this->callback(static function (Email $email) use (&$captured) {
+                $captured = $email;
+
+                return true;
+            }));
+
+        $category = $this->createStub(FeedbackCategory::class);
+        $category->method('getName')->willReturn('General');
+
+        // AboutModel falls back to 'feedback@bewelcome.org' when FeedbackEmail is null
+        $this->mailer->sendFeedbackEmail(
+            'feedback@bewelcome.org',
+            new Address('feedback@bewelcome.org'),
+            [
+                'IdCategory' => $category,
+                'FeedbackQuestion' => 'Test feedback',
+                'member' => null,
+                'no_reply_needed' => true,
+                'browser' => 'Chrome',
+                'host' => 'bewelcome.org',
+                'version' => '1.0',
+            ]
+        );
+
+        $from = $captured->getFrom();
+        $this->assertSame('noreply@bewelcome.org', $from[0]->getAddress());
+    }
+
+    public function testSendCommentReportedFeedbackEmailUsesNoreplyAsFrom(): void
+    {
+        $captured = null;
+        $this->innerMailer
+            ->expects($this->once())
+            ->method('send')
+            ->with($this->callback(static function (Email $email) use (&$captured) {
+                $captured = $email;
+
+                return true;
+            }));
+
+        $member = $this->createStub(Member::class);
+        $member->method('getEmail')->willReturn('member@gmail.com');
+
+        $category = $this->createStub(FeedbackCategory::class);
+        $category->method('getEmailToNotify')->willReturn('account@bewelcome.org');
+
+        $repo = $this->createStub(EntityRepository::class);
+        $repo->method('findOneBy')->willReturn($category);
+
+        $em = $this->createStub(EntityManagerInterface::class);
+        $em->method('getRepository')->willReturn($repo);
+
+        $translator = $this->createStub(TranslatorInterface::class);
+        $translator->method('getLocale')->willReturn('en');
+        $translator->method('trans')->willReturnArgument(0);
+
+        $mailer = new Mailer(
+            $em,
+            $this->createStub(UrlGeneratorInterface::class),
+            $translator,
+            $this->innerMailer,
+            $this->createStub(Logger::class),
+        );
+
+        $mailer->sendCommentReportedFeedbackEmail($member, ['subject' => 'Comment feedback']);
+
+        $this->assertNotNull($captured);
+
+        $from = $captured->getFrom();
+        $this->assertCount(1, $from);
+        $this->assertSame('noreply@bewelcome.org', $from[0]->getAddress(), 'From must be noreply to pass DMARC');
+
+        $replyTo = $captured->getReplyTo();
+        $this->assertCount(1, $replyTo);
+        $this->assertSame('member@gmail.com', $replyTo[0]->getAddress(), 'Reply-To must be the member so staff can reply');
+    }
+}