Prevent delivery of email to disabled objects

Summary: See T625. Facebook's REST-based MTA layer had a check for this so I
overlooked it in porting it out. We should not attempt to deliver email to
disabled users.

Test Plan:
Used MetaMTA console to send email to:

  - No users: received "no To" exception.
  - A disabled user: received "all To disabled" exception.
  - A valid user: received email.
  - A valid user and a disabled user: received email to valid user only.

(Note that you can't easily send to disabled users directly since they don't
appear in the typeahead, but you can prefill it and then disable the user by
hitting "Send".)

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: skrul, aran, epriestley

Differential Revision: 1120

src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php

@@ -296,18 +296,23 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
             $mailer->addReplyTo($value, $reply_to_name);
             break;
           case 'to':
-            $emails = array();
+            $emails = $this->getDeliverableEmailsFromHandles($value, $handles);
-            foreach ($value as $phid) {
+            if ($emails) {
-              $emails[] = $handles[$phid]->getEmail();
-            }
               $mailer->addTos($emails);
+            } else {
+              if ($value) {
+                throw new Exception(
+                  "All 'To' objects are undeliverable (e.g., disabled users).");
+              } else {
+                throw new Exception("No 'To' specified!");
+              }
+            }
             break;
           case 'cc':
-            $emails = array();
+            $emails = $this->getDeliverableEmailsFromHandles($value, $handles);
-            foreach ($value as $phid) {
+            if ($emails) {
-              $emails[] = $handles[$phid]->getEmail();
-            }
               $mailer->addCCs($emails);
+            }
             break;
           case 'headers':
             foreach ($value as $header_key => $header_value) {
@@ -444,4 +449,22 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
     return base64_encode($base);
   }
 
+  private function getDeliverableEmailsFromHandles(
+    array $phids,
+    array $handles) {
+
+    $emails = array();
+    foreach ($phids as $phid) {
+      if ($handles[$phid]->isDisabled()) {
+        continue;
+      }
+      if (!$handles[$phid]->isComplete()) {
+        continue;
+      }
+      $emails[] = $handles[$phid]->getEmail();
+    }
+
+    return $emails;
+  }
+
 }

src/applications/people/storage/user/PhabricatorUser.php

@@ -43,19 +43,27 @@ class PhabricatorUser extends PhabricatorUserDAO {
   private $preferences = null;
 
   protected function readField($field) {
-    if ($field === 'profileImagePHID') {
+    switch ($field) {
+      case 'profileImagePHID':
         return nonempty(
           $this->profileImagePHID,
           PhabricatorEnv::getEnvConfig('user.default-profile-image-phid'));
-    }
+      case 'timezoneIdentifier':
-    if ($field === 'timezoneIdentifier') {
         // If the user hasn't set one, guess the server's time.
         return nonempty(
           $this->timezoneIdentifier,
           date_default_timezone_get());
-    }
+      // Make sure these return booleans.
+      case 'isAdmin':
+        return (bool)$this->isAdmin;
+      case 'isDisabled':
+        return (bool)$this->isDisabled;
+      case 'isSystemAgent':
+        return (bool)$this->isSystemAgent;
+      default:
         return parent::readField($field);
     }
+  }
 
   public function getConfiguration() {
     return array(

src/applications/phid/handle/PhabricatorObjectHandle.php

@@ -29,6 +29,7 @@ class PhabricatorObjectHandle {
   private $alternateID;
   private $status = 'open';
   private $complete;
+  private $disabled;
 
   public function setURI($uri) {
     $this->uri = $uri;
@@ -135,11 +136,20 @@ class PhabricatorObjectHandle {
     return idx($map, $this->getType());
   }
 
+
+  /**
+   * Set whether or not the underlying object is complete. See
+   * @{method:getComplete} for an explanation of what it means to be complete.
+   *
+   * @param bool True if the handle represents a complete object.
+   * @return this
+   */
   public function setComplete($complete) {
     $this->complete = $complete;
     return $this;
   }
 
+
   /**
    * Determine if the handle represents an object which was completely loaded
    * (i.e., the underlying object exists) vs an object which could not be
@@ -156,6 +166,34 @@ class PhabricatorObjectHandle {
     return $this->complete;
   }
 
+
+  /**
+   * Set whether or not the underlying object is disabled. See
+   * @{method:getDisabled} for an explanation of what it means to be disabled.
+   *
+   * @param bool True if the handle represents a disabled object.
+   * @return this
+   */
+  public function setDisabled($disabled) {
+    $this->disabled = $disabled;
+    return $this;
+  }
+
+
+  /**
+   * Determine if the handle represents an object which has been disabled --
+   * for example, disabled users, archived projects, etc. These objects are
+   * complete and exist, but should be excluded from some system interactions
+   * (for instance, they usually should not appear in typeaheads, and should
+   * not have mail/notifications delivered to or about them).
+   *
+   * @return bool True if the handle represents a disabled object.
+   */
+  public function isDisabled() {
+    return $this->disabled;
+  }
+
+
   public function renderLink() {
 
     switch ($this->getType()) {

src/applications/phid/handle/data/PhabricatorObjectHandleData.php

@@ -158,6 +158,7 @@ class PhabricatorObjectHandleData {
                 $user->getUsername().' ('.$user->getRealName().')');
               $handle->setAlternateID($user->getID());
               $handle->setComplete(true);
+              $handle->setDisabled($user->getIsDisabled());
 
               $img_uri = idx($images, $user->getProfileImagePHID());
               if ($img_uri) {