Copy mail sender selection logic into MailReceivers

Summary: Copies sender identification logic into MailReceivers and makes it basically sane. The mess we run into after this try/catch is terrifying so I'm avoiding actually getting rid of any of it quite yet. Ref T1205.

Test Plan: Added a bit of test coverage. Used Receiver test console to verify some additional behaviors.

Reviewers: btrahan

Reviewed By: btrahan

CC: Afaque_Hussain, aran

Maniphest Tasks: T1205

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

src/applications/maniphest/mail/ManiphestCreateMailReceiver.php

@@ -20,4 +20,40 @@ final class ManiphestCreateMailReceiver extends PhabricatorMailReceiver {
     return false;
   }
 
+  public function loadSender(PhabricatorMetaMTAReceivedMail $mail) {
+    try {
+      // Try to load the sender normally.
+      return parent::loadSender($mail);
+    } catch (PhabricatorMetaMTAReceivedMailProcessingException $ex) {
+
+      // If we failed to load the sender normally, use this special legacy
+      // black magic.
+
+      // TODO: Deprecate and remove this.
+
+      $default_author_key = 'metamta.maniphest.default-public-author';
+      $default_author = PhabricatorEnv::getEnvConfig($default_author_key);
+
+      if (!strlen($default_author)) {
+        throw $ex;
+      }
+
+      $user = id(new PhabricatorUser())->loadOneWhere(
+        'username = %s',
+        $default_author);
+
+      if ($user) {
+        return $user;
+      }
+
+      throw new PhabricatorMetaMTAReceivedMailProcessingException(
+        MetaMTAReceivedMailStatus::STATUS_UNKNOWN_SENDER,
+        pht(
+          "Phabricator is misconfigured, the configuration key ".
+          "'metamta.maniphest.default-public-author' is set to user ".
+          "'%s' but that user does not exist.",
+          $default_author));
+    }
+  }
+
 }

src/applications/metamta/constants/MetaMTAReceivedMailStatus.php

@@ -7,5 +7,6 @@ final class MetaMTAReceivedMailStatus
   const STATUS_FROM_PHABRICATOR     = 'err:self';
   const STATUS_NO_RECEIVERS         = 'err:no-receivers';
   const STATUS_ABUNDANT_RECEIVERS   = 'err:multiple-receivers';
+  const STATUS_UNKNOWN_SENDER       = 'err:unknown-sender';
 
 }

src/applications/metamta/receiver/PhabricatorMailReceiver.php

@@ -6,6 +6,83 @@ abstract class PhabricatorMailReceiver {
   abstract public function isEnabled();
   abstract public function canAcceptMail(PhabricatorMetaMTAReceivedMail $mail);
 
+  /**
+   * Identifies the sender's user account for a piece of received mail. Note
+   * that this method does not validate that the sender is who they say they
+   * are, just that they've presented some credential which corresponds to a
+   * recognizable user.
+   */
+  public function loadSender(PhabricatorMetaMTAReceivedMail $mail) {
+    $raw_from = $mail->getHeader('From');
+    $from = self::getRawAddress($raw_from);
+
+    $reasons = array();
+
+    // Try to find a user with this email address.
+    $user = PhabricatorUser::loadOneWithEmailAddress($from);
+    if ($user) {
+      return $user;
+    } else {
+      $reasons[] = pht(
+        "The email was sent from '%s', but this address does not correspond ".
+        "to any user account.",
+        $raw_from);
+    }
+
+    // If we missed on "From", try "Reply-To" if we're configured for it.
+    $reply_to_key = 'metamta.insecure-auth-with-reply-to';
+    $allow_reply_to = PhabricatorEnv::getEnvConfig($reply_to_key);
+    if ($allow_reply_to) {
+      $raw_reply_to = $mail->getHeader('Reply-To');
+      $reply_to = self::getRawAddress($raw_reply_to);
+
+      $user = PhabricatorUser::loadOneWithEmailAddress($reply_to);
+      if ($user) {
+        return $user;
+      } else {
+        $reasons[] = pht(
+          "Phabricator is configured to try to authenticate users using ".
+          "'Reply-To', but the reply to address ('%s') does not correspond ".
+          "to any user account.",
+          $raw_reply_to);
+      }
+    } else {
+      $reasons[] = pht(
+        "Phabricator is not configured to authenticate users using ".
+        "'Reply-To' (`metamta.insecure-auth-with-reply-to`), so the ".
+        "'Reply-To' header was not examined.");
+    }
+
+    // If we don't know who this user is, load or create an external user
+    // account for them if we're configured for it.
+    $email_key = 'phabricator.allow-email-users';
+    $allow_email_users = PhabricatorEnv::getEnvConfig($email_key);
+    if ($allow_email_users) {
+      $xuser = id(new PhabricatorExternalAccount())->loadOneWhere(
+        'accountType = %s AND accountDomain IS NULL and accountID = %s',
+        'email',
+        $from);
+      if (!$xuser) {
+        $xuser = id(new PhabricatorExternalAccount())
+          ->setAccountID($from)
+          ->setAccountType('email')
+          ->setDisplayName($from)
+          ->save();
+      }
+      return $xuser->getPhabricatorUser();
+    } else {
+      $reasons[] = pht(
+        "Phabricator is not configured to allow unknown external users to ".
+        "send mail to the system using just an email address ".
+        "(`phabricator.allow-email-users`), so an implicit external acount ".
+        "could not be created.");
+    }
+
+    throw new PhabricatorMetaMTAReceivedMailProcessingException(
+      MetaMTAReceivedMailStatus::STATUS_UNKNOWN_SENDER,
+      pht('Unknown sender: %s', implode(' ', $reasons)));
+  }
+
   /**
    * Determine if two inbound email addresses are effectively identical. This
    * method strips and normalizes addresses so that equivalent variations are
@@ -22,15 +99,12 @@ abstract class PhabricatorMailReceiver {
    * @return  bool    True if addresses match.
    */
   public static function matchAddresses($u, $v) {
-    $u = id(new PhutilEmailAddress($u))->getAddress();
+    $u = self::getRawAddress($u);
-    $v = id(new PhutilEmailAddress($v))->getAddress();
+    $v = self::getRawAddress($v);
 
     $u = self::stripMailboxPrefix($u);
     $v = self::stripMailboxPrefix($v);
 
-    $u = trim(phutil_utf8_strtolower($u));
-    $v = trim(phutil_utf8_strtolower($v));
-
     return ($u === $v);
   }
 
@@ -69,4 +143,22 @@ abstract class PhabricatorMailReceiver {
   }
 
 
+  /**
+   * Reduce an email address to its canonical form. For example, an adddress
+   * like:
+   *
+   *  "Abraham Lincoln" < ALincoln@example.com >
+   *
+   * ...will be reduced to:
+   *
+   *  alincoln@example.com
+   *
+   * @param   string  Email address in noncanonical form.
+   * @return  string  Canonical email address.
+   */
+  public static function getRawAddress($address) {
+    $address = id(new PhutilEmailAddress($address))->getAddress();
+    return trim(phutil_utf8_strtolower($address));
+  }
+
 }

src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php

@@ -175,6 +175,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
       $this->dropMailAlreadyReceived();
 
       $receiver = $this->loadReceiver();
+      $sender = $receiver->loadSender($this);
 
     } catch (PhabricatorMetaMTAReceivedMailProcessingException $ex) {
       $this

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

@@ -63,5 +63,28 @@ final class PhabricatorMetaMTAReceivedMailTestCase extends PhabricatorTestCase {
       $mail->getStatus());
   }
 
+  public function testDropUnknownSenderMail() {
+    $env = PhabricatorEnv::beginScopedEnv();
+    $env->overrideEnvConfig(
+      'metamta.maniphest.public-create-email',
+      'bugs@example.com');
+    $env->overrideEnvConfig('phabricator.allow-email-users', false);
+    $env->overrideEnvConfig('metamta.maniphest.default-public-author', null);
+
+    $mail = new PhabricatorMetaMTAReceivedMail();
+    $mail->setHeaders(
+      array(
+        'Message-ID' => 'test@example.com',
+        'To'         => 'bugs@example.com',
+        'From'       => 'does+not+exist@example.com',
+      ));
+    $mail->save();
+
+    $mail->processReceivedMail();
+
+    $this->assertEqual(
+      MetaMTAReceivedMailStatus::STATUS_UNKNOWN_SENDER,
+      $mail->getStatus());
+  }
 
 }