Never send normal mail to unverified addresses

Summary:
Ref T12237. This tightens our delivery rules, which previously sent normal mail to unverified addresses:

  - We sent general mail to unverified addresses so that you wouldn't miss anything between the time you sign up (or have an account created) and the time you verify your address. This was imagined as a slight convenience for users.
  - We sent automatic reply mail to unverified addresses if they sent mail to us first, saying "we don't recognize that address". This was imagined as a convenience for users who accidentally send mail "From" the wrong address (personal vs work, for example).

I think both behaviors are probably a little better for users on the balance, but not having mail providers randomly shut us off without warning is better for me, personally -- so stop doing this stuff.

This creates a problem which we likely need to solve before the release is cut:

  - On installs which do not require mail verification, mail to you will now mostly-silently be dropped if you never bothered to verify your address.

I'd like to solve this by adding some kind of per-user alert that says "We recently tried to send you some mail but you haven't verified your address.", and giving them links to verify the address and review the mail. I'll pursue this after restoring mail service to `secure.phabricator.com`.

Test Plan:
  - Added a unit test.
  - Unverified my address, sent mail, saw it get dropped.
  - Reverified my address, sent mail, saw it go through.
  - Verified that important mail (password reset, invite, confirm-this-address) either uses "Force Delivery" (skips this check) or "Raw To Addresses" (also skips this check).
    - Verified that Phacility instance stuff is also covered: it uses the same invite flow.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12237

Differential Revision: https://secure.phabricator.com/D17329

src/applications/metamta/query/PhabricatorMetaMTAActor.php

@@ -20,12 +20,14 @@ final class PhabricatorMetaMTAActor extends Phobject {
   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 @@ final class PhabricatorMetaMTAActor extends Phobject {
     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 @@ final class PhabricatorMetaMTAActor extends Phobject {
       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 @@ final class PhabricatorMetaMTAActor extends Phobject {
       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));

src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php

@@ -89,6 +89,7 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
         $actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_NO_ADDRESS);
       } else {
         $actor->setEmailAddress($email->getAddress());
+        $actor->setIsVerified($email->getIsVerified());
       }
     }
   }
@@ -119,6 +120,13 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
       }
 
       $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);
     }
   }
 

src/applications/metamta/receiver/PhabricatorMailReceiver.php

@@ -153,6 +153,10 @@ abstract class PhabricatorMailReceiver extends Phobject {
         ->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,14 +178,14 @@ abstract class PhabricatorMailReceiver extends Phobject {
         if ($user) {
           return $user;
         }
-      }
 
         $reasons[] = pht(
-        "Phabricator is misconfigured, the application email ".
+          'Phabricator is misconfigured: the application email '.
-        "'%s' is set to user '%s' but that user does not exist.",
+          '"%s" is set to user "%s", but that user does not exist.',
           $application_email->getAddress(),
           $default_user_phid);
       }
+    }
 
     $reasons = implode("\n\n", $reasons);
 

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -951,6 +951,16 @@ final class PhabricatorMetaMTAMail
       }
     }
 
+    // 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;
   }
 

src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php

@@ -153,6 +153,22 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
     $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() {