Fixes bug where emails are sent to blocked addresses (#43)

models/MailBlocker.php

@@ -211,11 +211,11 @@ public static function checkAllForUser($user)
  /**
  * Checks if an email address has blocked a given template,
  * returns an array of blocked emails.
- * @param  string $template
- * @param  string $email
+ * @param ?string $template
+ * @param string|string[] $email
  * @return array
  */
- public static function checkForEmail($template, $email)
+ public static function checkForEmail(?string $template, $email): array
  {
  if (in_array($template, static::$safeTemplates)) {
  return [];
@@ -225,11 +225,7 @@ public static function checkForEmail($template, $email)
  return [];
  }
 
- if (!is_array($email)) {
- $email = [$email => null];
- }
-
- $emails = array_keys($email);
+ $emails = is_array($email) ? $email : [$email];
 
  return static::where(
  function ($query) use ($template) {
@@ -248,22 +244,40 @@ function ($query) use ($template) {
  * @param \Illuminate\Mail\Message $message
  * @return bool|null
  */
- public static function filterMessage($template, $message)
+ public static function filterMessage($template, $message): ?bool
  {
- $recipients = $message->getTo();
- $blockedAddresses = static::checkForEmail($template, $recipients);
+ $recipients = array_map(function ($address) {
+ return $address->getAddress();
+ }, $message->getTo());
+ $ccRecipients = array_map(function ($address) {
+ return $address->getAddress();
+ }, $message->getCc());
+ $bccRecipients = array_map(function ($address) {
+ return $address->getAddress();
+ }, $message->getBcc());
+
+ $allRecipients = array_merge($recipients, $ccRecipients, $bccRecipients);
+ $allRecipientsNoDuplicates = array_combine($allRecipients, $allRecipients);
+
+ $blockedAddresses = static::checkForEmail($template, $allRecipientsNoDuplicates);
 
  if (!count($blockedAddresses)) {
  return null;
  }
 
- foreach (array_keys($recipients) as $address) {
- if (in_array($address, $blockedAddresses)) {
- unset($recipients[$address]);
- }
- }
+ $recipients = array_filter($recipients, function ($address) use (&$blockedAddresses) {
+ return !in_array($address, $blockedAddresses);
+ });
+ $ccRecipients = array_filter($ccRecipients, function ($address) use (&$blockedAddresses) {
+ return !in_array($address, $blockedAddresses);
+ });
+ $bccRecipients = array_filter($bccRecipients, function ($address) use (&$blockedAddresses) {
+ return !in_array($address, $blockedAddresses);
+ });
 
  $message->to($recipients, null, true);
- return count($recipients) ? null : false;
+ $message->cc($ccRecipients, null, true);
+ $message->bcc($bccRecipients, null, true);
+ return (count($recipients) + count($ccRecipients) + count($bccRecipients)) ? null : false;
  }
 }

tests/unit/models/MailBlockerModelTest.php

@@ -0,0 +1,200 @@
+<?php
+
+namespace Winter\User\Tests\Unit\Models;
+
+use Illuminate\Mail\Message;
+use Mockery;
+use Symfony\Component\Mime\Email;
+use Winter\User\Models\MailBlocker;
+use Winter\User\Models\User;
+use Winter\User\Tests\UserPluginTestCase;
+
+class MailBlockerModelTest extends UserPluginTestCase
+{
+ public function testFilterMessage_to()
+ {
+ list(
+ $userMock1,
+ $userMock2,
+ $userMock3
+ ) = $this->getMockedUsers();
+
+ $this->mockBlockers($userMock1, $userMock2);
+
+ $message1 = $this->getMessage('test1', [$userMock1->email]);
+ $message2 = $this->getMessage('test2', [$userMock2->email]);
+ $message3 = $this->getMessage('test3', [$userMock3->email]);
+
+ $message12 = $this->getMessage('test12', [$userMock1->email, $userMock2->email]);
+ $message13 = $this->getMessage('test13', [$userMock1->email, $userMock3->email]);
+
+ $filterMessage1 = MailBlocker::filterMessage(null, $message1);
+ $filterMessage2 = MailBlocker::filterMessage(null, $message2);
+ $filterMessage3 = MailBlocker::filterMessage(null, $message3);
+ $filterMessage12 = MailBlocker::filterMessage(null, $message12);
+ $filterMessage13 = MailBlocker::filterMessage(null, $message13);
+
+ $this->assertFalse($filterMessage1);
+ $this->assertEmpty($message1->getTo());
+ $this->assertFalse($filterMessage2);
+ $this->assertEmpty($message2->getTo());
+ $this->assertNull($filterMessage3);
+ $this->assertEquals($userMock3->email, $message3->getTo()[0]->getAddress());
+ $this->assertFalse($filterMessage12);
+ $this->assertEmpty($message12->getTo());
+ $this->assertNull($filterMessage13);
+ $this->assertEquals(1, count($message13->getTo()));
+ $this->assertEquals($userMock3->email, $message13->getTo()[0]->getAddress());
+ }
+
+ public function testFilterMessage_ccAndBcc()
+ {
+ list(
+ $userMock1,
+ $userMock2,
+ $userMock3,
+ $userMock4
+ ) = $this->getMockedUsers();
+
+ $this->mockBlockers($userMock1, $userMock2);
+
+ $messageTo0Cc12 = $this->getMessage(
+ 'test_to0_cc12',
+ [],
+ [$userMock1->email,$userMock2->email]
+ );
+ $messageTo3Cc14 = $this->getMessage(
+ 'test_to3_cc14',
+ [$userMock3->email],
+ [$userMock1->email,$userMock4->email]
+ );
+ $messageTo3Bcc14 = $this->getMessage(
+ 'test_to3_bcc14',
+ [$userMock3->email],
+ [],
+ [$userMock1->email,$userMock4->email]
+ );
+
+ $filterMessageTo0Cc12 = MailBlocker::filterMessage(null, $messageTo0Cc12);
+ $filterMessageTo3Cc14 = MailBlocker::filterMessage(null, $messageTo3Cc14);
+ $filterMessageTo3Bcc14 = MailBlocker::filterMessage(null, $messageTo3Bcc14);
+
+ $this->assertFalse($filterMessageTo0Cc12);
+ $this->assertNull($filterMessageTo3Cc14);
+ $this->assertNull($filterMessageTo3Bcc14);
+ $this->assertEquals($userMock4->email, $messageTo3Cc14->getCc()[0]->getAddress());
+ $this->assertEmpty($messageTo0Cc12->getCc());
+ $this->assertEquals($userMock4->email, $messageTo3Bcc14->getBcc()[0]->getAddress());
+ }
+
+ public function testCheckForEmail()
+ {
+ list($userMock1, $userMock2, $userMock3) = $this->getMockedUsers();
+
+ $this->mockBlockers($userMock1, $userMock2);
+
+ $checkForEmail1 = MailBlocker::checkForEmail(null, $userMock1->email);
+ $checkForEmail2 = MailBlocker::checkForEmail(null, $userMock2->email);
+ $checkForEmail3 = MailBlocker::checkForEmail(null, $userMock3->email);
+ $checkForEmail12 = MailBlocker::checkForEmail(null, [$userMock1->email, $userMock2->email]);
+ $checkForEmail13 = MailBlocker::checkForEmail(null, [$userMock1->email, $userMock3->email]);
+
+ $this->assertEquals([$userMock1->email], $checkForEmail1);
+ $this->assertEquals([$userMock2->email], $checkForEmail2);
+ $this->assertEmpty($checkForEmail3);
+ $this->assertEquals([$userMock1->email, $userMock2->email], $checkForEmail12);
+ $this->assertEquals([$userMock1->email], $checkForEmail13);
+ }
+
+ /**
+ * Helper method that mocks 4 users, saves them and returns them
+ * @return array
+ */
+ private static function getMockedUsers(): array
+ {
+ $userMock1 = Mockery::mock(User::class)->makePartial();
+ $userMock2 = Mockery::mock(User::class)->makePartial();
+ $userMock3 = Mockery::mock(User::class)->makePartial();
+ $userMock4 = Mockery::mock(User::class)->makePartial();
+
+ $userMock1->shouldReceive('isActivatedByUser')->andReturn(true);
+ $userMock2->shouldReceive('isActivatedByUser')->andReturn(true);
+ $userMock3->shouldReceive('isActivatedByUser')->andReturn(true);
+ $userMock4->shouldReceive('isActivatedByUser')->andReturn(true);
+ $userMock1->shouldReceive('flushEventListeners')->andReturnNull();
+ $userMock2->shouldReceive('flushEventListeners')->andReturnNull();
+ $userMock3->shouldReceive('flushEventListeners')->andReturnNull();
+ $userMock4->shouldReceive('flushEventListeners')->andReturnNull();
+
+ $userMock1->fill([
+ 'name' => 'test1',
+ 'email' => '[email protected]',
+ 'password' => 'password',
+ ]);
+ $userMock2->fill([
+ 'name' => 'test2',
+ 'email' => '[email protected]',
+ 'password' => 'password',
+ ]);
+ $userMock3->fill([
+ 'name' => 'test3',
+ 'email' => '[email protected]',
+ 'password' => 'password',
+ ]);
+ $userMock4->fill([
+ 'name' => 'test3',
+ 'email' => '[email protected]',
+ 'password' => 'password',
+ ]);
+ $userMock1->save();
+ $userMock2->save();
+ $userMock3->save();
+ $userMock4->save();
+ return array($userMock1, $userMock2, $userMock3, $userMock4);
+ }
+
+ /**
+ * Helper method that mocks and saves 2 mail blockers for the two given users
+ * @param mixed $userMock1
+ * @param mixed $userMock2
+ * @return void
+ */
+ private static function mockBlockers(mixed $userMock1, mixed $userMock2): void
+ {
+ $mailBlockerMock1 = Mockery::mock(MailBlocker::class)->makePartial();
+ $mailBlockerMock2 = Mockery::mock(MailBlocker::class)->makePartial();
+ $mailBlockerMock1->shouldReceive('flushEventListeners')->andReturnNull();
+ $mailBlockerMock2->shouldReceive('flushEventListeners')->andReturnNull();
+
+ $mailBlockerMock1->email = $userMock1->email;
+ $mailBlockerMock1->template = '*';
+ $mailBlockerMock1->user_id = $userMock1->id;
+
+ $mailBlockerMock2->email = $userMock2->email;
+ $mailBlockerMock2->template = '*';
+ $mailBlockerMock2->user_id = $userMock2->id;
+
+ $mailBlockerMock1->save();
+ $mailBlockerMock2->save();
+ }
+
+ /**
+ * Helper to create and return a new Mail Message
+ * @param string $subject
+ * @param string[] $to
+ * @param string[] $cc
+ * @param string[] $bcc
+ * @return Message
+ */
+ private static function getMessage(string $subject, array $to = [], array $cc = [], array $bcc = []): Message
+ {
+ $message = new Message(new Email());
+ $message
+ ->subject($subject)
+ ->to($to)
+ ->cc($cc)
+ ->bcc($bcc)
+ ;
+ return $message;
+ }
+}

updates/version.yaml

@@ -83,3 +83,4 @@
  - v2.0.1/rename_indexes.php
 "2.1.0": "Enforce password length rules on sign in. Compatibility fixes."
 "2.2.0": "Add avatar removal. Password resets will activate users if User activation mode is enabled."
+"2.2.1": "Fixes a bug introduced by the adoption of symfony/mime required since Laravel 7.x where sending an email to a blocked email address would not be prevented."