diff --git a/src/applications/metamta/query/PhabricatorMetaMTAActor.php b/src/applications/metamta/query/PhabricatorMetaMTAActor.php --- a/src/applications/metamta/query/PhabricatorMetaMTAActor.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAActor.php @@ -20,12 +20,14 @@ const REASON_FORCE_HERALD = 'force-herald'; const REASON_ROUTE_AS_NOTIFICATION = 'route-as-notification'; const REASON_ROUTE_AS_MAIL = 'route-as-mail'; + const REASON_UNVERIFIED = 'unverified'; private $phid; private $emailAddress; private $name; private $status = self::STATUS_DELIVERABLE; private $reasons = array(); + private $isVerified = false; public function setName($name) { $this->name = $name; @@ -45,6 +47,15 @@ return $this->emailAddress; } + public function setIsVerified($is_verified) { + $this->isVerified = $is_verified; + return $this; + } + + public function getIsVerified() { + return $this->isVerified; + } + public function setPHID($phid) { $this->phid = $phid; return $this; @@ -104,6 +115,7 @@ self::REASON_FORCE_HERALD => pht('Forced by Herald'), self::REASON_ROUTE_AS_NOTIFICATION => pht('Route as Notification'), self::REASON_ROUTE_AS_MAIL => pht('Route as Mail'), + self::REASON_UNVERIFIED => pht('Address Not Verified'), ); return idx($names, $reason, pht('Unknown ("%s")', $reason)); @@ -158,6 +170,8 @@ self::REASON_ROUTE_AS_MAIL => pht( 'This message was upgraded to email by outbound mail rules '. 'in Herald.'), + self::REASON_UNVERIFIED => pht( + 'This recipient does not have a verified primary email address.'), ); return idx($descriptions, $reason, pht('Unknown Reason ("%s")', $reason)); diff --git a/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php --- a/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php @@ -89,6 +89,7 @@ $actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_NO_ADDRESS); } else { $actor->setEmailAddress($email->getAddress()); + $actor->setIsVerified($email->getIsVerified()); } } } @@ -119,6 +120,13 @@ } $actor->setEmailAddress($xuser->getAccountID()); + + // NOTE: This effectively drops all outbound mail to unrecognized + // addresses unless "phabricator.allow-email-users" is set. See T12237 + // for context. + $allow_key = 'phabricator.allow-email-users'; + $allow_value = PhabricatorEnv::getEnvConfig($allow_key); + $actor->setIsVerified((bool)$allow_value); } } diff --git a/src/applications/metamta/receiver/PhabricatorMailReceiver.php b/src/applications/metamta/receiver/PhabricatorMailReceiver.php --- a/src/applications/metamta/receiver/PhabricatorMailReceiver.php +++ b/src/applications/metamta/receiver/PhabricatorMailReceiver.php @@ -153,6 +153,10 @@ ->loadOneOrCreate(); return $xuser->getPhabricatorUser(); } else { + // NOTE: Currently, we'll always drop this mail (since it's headed to + // an unverified recipient). See T12237. These details are still useful + // because they'll appear in the mail logs and Mail web UI. + $reasons[] = pht( 'Phabricator is also not configured to allow unknown external users '. 'to send mail to the system using just an email address.'); @@ -174,13 +178,13 @@ if ($user) { return $user; } - } - $reasons[] = pht( - "Phabricator is misconfigured, the application email ". - "'%s' is set to user '%s' but that user does not exist.", - $application_email->getAddress(), - $default_user_phid); + $reasons[] = pht( + 'Phabricator is misconfigured: the application email '. + '"%s" is set to user "%s", but that user does not exist.', + $application_email->getAddress(), + $default_user_phid); + } } $reasons = implode("\n\n", $reasons); diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -951,6 +951,16 @@ } } + // Unless delivery was forced earlier (password resets, confirmation mail), + // never send mail to unverified addresses. + foreach ($actors as $phid => $actor) { + if ($actor->getIsVerified()) { + continue; + } + + $actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNVERIFIED); + } + return $actors; } diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php --- a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php +++ b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php @@ -153,6 +153,22 @@ $this->assertTrue( in_array($phid, $mail->buildRecipientList()), 'Recipients restored after tag preference removed.'); + + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'userPHID = %s AND isPrimary = 1', + $phid); + + $email->setIsVerified(0)->save(); + + $this->assertFalse( + in_array($phid, $mail->buildRecipientList()), + pht('Mail not sent to unverified address.')); + + $email->setIsVerified(1)->save(); + + $this->assertTrue( + in_array($phid, $mail->buildRecipientList()), + pht('Mail sent to verified address.')); } public function testThreadIDHeaders() {