From f9599f44997bbd520e80229780fef1ff997b333c Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 4 Jul 2011 09:45:42 -0700 Subject: [PATCH] Allow configuration of a task-creation email address Summary: This lets you configure an email address which will create tasks when emails are sent to it. It's pretty basic but should get us most of the way there. Test Plan: Configured an address and created a task via email. Replied to a task via email to check that I didn't break that. Reviewed By: tuomaspelkonen Reviewers: davidreuss, jungejason, tuomaspelkonen, aran CC: aran, epriestley, tuomaspelkonen Differential Revision: 590 --- conf/default.conf.php | 10 ++ .../taskedit/ManiphestTaskEditController.php | 9 ++ .../controller/taskedit/__init__.php | 1 + .../replyhandler/ManiphestReplyHandler.php | 109 +++++++++++------- .../PhabricatorMetaMTAReceivedMail.php | 77 ++++++++++--- .../metamta/storage/receivedmail/__init__.php | 2 + .../configuring_inbound_email.diviner | 11 +- 7 files changed, 160 insertions(+), 59 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index 7472c1ecb0..00adb39311 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -201,6 +201,16 @@ return array( // single public email address, so objects can not be replied to blindly. 'metamta.public-replies' => false, + // You can configure an email address like "bugs@phabricator.example.com" + // which will automatically create Maniphest tasks when users send email + // to it. This relies on the "From" address to authenticate users, so it is + // is not completely secure. To set this up, enter a complete email + // address like "bugs@phabricator.example.com" and then configure mail to + // that address so it routed to Phabricator (if you've already configured + // reply handlers, you're probably already done). See "Configuring Inbound + // Email" in the documentation for more information. + 'metamta.maniphest.public-create-email' => null, + // -- Auth ------------------------------------------------------------------ // diff --git a/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php b/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php index 9f5f278935..e529d4f60b 100644 --- a/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/taskedit/ManiphestTaskEditController.php @@ -285,11 +285,20 @@ class ManiphestTaskEditController extends ManiphestController { } } + $email_create = PhabricatorEnv::getEnvConfig( + 'metamta.maniphest.public-create-email'); + $email_hint = null; + if (!$task->getID() && $email_create) { + $email_hint = 'You can also create tasks by sending an email to: '. + ''.phutil_escape_html($email_create).''; + } + $form ->appendChild( id(new AphrontFormTextAreaControl()) ->setLabel('Description') ->setName('description') + ->setCaption($email_hint) ->setValue($task->getDescription())) ->appendChild( id(new AphrontFormSubmitControl()) diff --git a/src/applications/maniphest/controller/taskedit/__init__.php b/src/applications/maniphest/controller/taskedit/__init__.php index d524917ca9..a12c754262 100644 --- a/src/applications/maniphest/controller/taskedit/__init__.php +++ b/src/applications/maniphest/controller/taskedit/__init__.php @@ -19,6 +19,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/transaction phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/form/base'); diff --git a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php index 0e0297c197..374da6c953 100644 --- a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php +++ b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php @@ -52,31 +52,85 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler { public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + // NOTE: We'll drop in here on both the "reply to a task" and "create a + // new task" workflows! Make sure you test both if you make changes! + $task = $this->getMailReceiver(); + + $is_new_task = !$task->getID(); + $user = $this->getActor(); $body = $mail->getCleanTextBody(); $body = trim($body); - $lines = explode("\n", trim($body)); - $first_line = head($lines); + $xactions = array(); - $command = null; - $matches = null; - if (preg_match('/^!(\w+)/', $first_line, $matches)) { - $lines = array_slice($lines, 1); - $body = implode("\n", $lines); - $body = trim($body); + $template = new ManiphestTransaction(); + $template->setAuthorPHID($user->getPHID()); - $command = $matches[1]; + if ($is_new_task) { + // If this is a new task, create a "User created this task." transaction + // and then set the title and description. + $xaction = clone $template; + $xaction->setTransactionType(ManiphestTransactionType::TYPE_STATUS); + $xaction->setNewValue(ManiphestTaskStatus::STATUS_OPEN); + $xactions[] = $xaction; + + $task->setAuthorPHID($user->getPHID()); + $task->setTitle(nonempty($mail->getSubject(), 'Untitled Task')); + $task->setDescription($body); + + } else { + $lines = explode("\n", trim($body)); + $first_line = head($lines); + + $command = null; + $matches = null; + if (preg_match('/^!(\w+)/', $first_line, $matches)) { + $lines = array_slice($lines, 1); + $body = implode("\n", $lines); + $body = trim($body); + + $command = $matches[1]; + } + + $ttype = ManiphestTransactionType::TYPE_NONE; + $new_value = null; + switch ($command) { + case 'close': + $ttype = ManiphestTransactionType::TYPE_STATUS; + $new_value = ManiphestTaskStatus::STATUS_CLOSED_RESOLVED; + break; + case 'claim': + $ttype = ManiphestTransactionType::TYPE_OWNER; + $new_value = $user->getPHID(); + break; + case 'unsubscribe': + $ttype = ManiphestTransactionType::TYPE_CCS; + $ccs = $task->getCCPHIDs(); + foreach ($ccs as $k => $phid) { + if ($phid == $user->getPHID()) { + unset($ccs[$k]); + } + } + $new_value = array_values($ccs); + break; + } + + $xaction = clone $template; + $xaction->setTransactionType($ttype); + $xaction->setNewValue($new_value); + $xaction->setComments($body); + + $xactions[] = $xaction; } - $xactions = array(); + // TODO: We should look at CCs on the mail and add them as CCs. $files = $mail->getAttachments(); if ($files) { - $file_xaction = new ManiphestTransaction(); - $file_xaction->setAuthorPHID($user->getPHID()); + $file_xaction = clone $template; $file_xaction->setTransactionType(ManiphestTransactionType::TYPE_ATTACH); $phid_type = PhabricatorPHIDConstants::PHID_TYPE_FILE; @@ -89,37 +143,6 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler { $xactions[] = $file_xaction; } - $ttype = ManiphestTransactionType::TYPE_NONE; - $new_value = null; - switch ($command) { - case 'close': - $ttype = ManiphestTransactionType::TYPE_STATUS; - $new_value = ManiphestTaskStatus::STATUS_CLOSED_RESOLVED; - break; - case 'claim': - $ttype = ManiphestTransactionType::TYPE_OWNER; - $new_value = $user->getPHID(); - break; - case 'unsubscribe': - $ttype = ManiphestTransactionType::TYPE_CCS; - $ccs = $task->getCCPHIDs(); - foreach ($ccs as $k => $phid) { - if ($phid == $user->getPHID()) { - unset($ccs[$k]); - } - } - $new_value = array_values($ccs); - break; - } - - $xaction = new ManiphestTransaction(); - $xaction->setAuthorPHID($user->getPHID()); - $xaction->setTransactionType($ttype); - $xaction->setNewValue($new_value); - $xaction->setComments($body); - - $xactions[] = $xaction; - $editor = new ManiphestTransactionEditor(); $editor->setParentMessageID($mail->getMessageID()); $editor->applyTransactions($task, $xactions); diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index 1ddc36c6e5..bbeca66523 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -50,15 +50,50 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return idx($this->headers, 'message-id'); } + public function getSubject() { + return idx($this->headers, 'subject'); + } + public function processReceivedMail() { $to = idx($this->headers, 'to'); + $to = $this->getRawEmailAddress($to); - // Accept a match either at the beginning of the address or after an open - // angle bracket, as in: - // "some display name" + $from = idx($this->headers, 'from'); + + $create_task = PhabricatorEnv::getEnvConfig( + 'metamta.maniphest.public-create-email'); + + if ($create_task && $to == $create_task) { + $user = $this->lookupPublicUser(); + if (!$user) { + // TODO: We should probably bounce these since from the user's + // perspective their email vanishes into a black hole. + return $this->setMessage("Invalid public user '{$from}'.")->save(); + } + + $this->setAuthorPHID($user->getPHID()); + + $receiver = new ManiphestTask(); + $receiver->setAuthorPHID($user->getPHID()); + $receiver->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); + + $editor = new ManiphestTransactionEditor(); + $handler = $editor->buildReplyHandler($receiver); + + $handler->setActor($user); + $handler->receiveEmail($this); + + $this->setRelatedPHID($receiver->getPHID()); + $this->setMessage('OK'); + + return $this->save(); + } + + // We've already stripped this, so look for an object address which has + // a format like: D291+291+b0a41ca848d66dcc@example.com $matches = null; $ok = preg_match( - '/(?:^|<)((?:D|T)\d+)\+([\w]+)\+([a-f0-9]{16})@/U', + '/^((?:D|T)\d+)\+([\w]+)\+([a-f0-9]{16})@/U', $to, $matches); @@ -75,18 +110,8 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return $this->setMessage("Public replies not enabled.")->save(); } - // Strip the email address out of the 'from' if necessary, since it might - // have some formatting like '"Abraham Lincoln" '. - $from = idx($this->headers, 'from'); - $matches = null; - $ok = preg_match('/<(.*)>/', $from, $matches); - if ($ok) { - $from = $matches[1]; - } + $user = $this->lookupPublicUser(); - $user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $from); if (!$user) { return $this->setMessage("Invalid public user '{$from}'.")->save(); } @@ -181,5 +206,27 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return substr($hash, 0, 16); } + /** + * Strip an email address down to the actual user@domain.tld part if + * necessary, since sometimes it will have formatting like + * '"Abraham Lincoln" '. + */ + private function getRawEmailAddress($address) { + $matches = null; + $ok = preg_match('/<(.*)>/', $address, $matches); + if ($ok) { + $address = $matches[1]; + } + return $address; + } + + private function lookupPublicUser() { + $from = idx($this->headers, 'from'); + $from = $this->getRawEmailAddress($from); + + return id(new PhabricatorUser())->loadOneWhere( + 'email = %s', + $from); + } } diff --git a/src/applications/metamta/storage/receivedmail/__init__.php b/src/applications/metamta/storage/receivedmail/__init__.php index afd38aa144..6254939d3f 100644 --- a/src/applications/metamta/storage/receivedmail/__init__.php +++ b/src/applications/metamta/storage/receivedmail/__init__.php @@ -7,7 +7,9 @@ phutil_require_module('phabricator', 'applications/differential/mail/base'); +phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/editor/transaction'); +phutil_require_module('phabricator', 'applications/maniphest/storage/task'); phutil_require_module('phabricator', 'applications/metamta/parser'); phutil_require_module('phabricator', 'applications/metamta/storage/base'); phutil_require_module('phabricator', 'applications/people/storage/user'); diff --git a/src/docs/configuration/configuring_inbound_email.diviner b/src/docs/configuration/configuring_inbound_email.diviner index 981b6a9512..e19f8885c9 100644 --- a/src/docs/configuration/configuring_inbound_email.diviner +++ b/src/docs/configuration/configuring_inbound_email.diviner @@ -2,7 +2,8 @@ @group config This document contains instructions for configuring inbound email, so users -may update Differential and Maniphest by replying to messages. +may update Differential and Maniphest by replying to messages and create +Maniphest tasks via email. = Preamble = @@ -42,6 +43,11 @@ configured correctly, according to the instructions below -- will parse incoming email and allow users to interact with Maniphest tasks and Differential revisions over email. +You can also set up a task creation email address, like ##bugs@example.com##, +which will create a Maniphest task out of any email which is set to it. To do +this, set ##metamta.maniphest.public-create-email## in your configuration. This +has some mild security implications, see below. + = Security = The email reply channel is "somewhat" authenticated. Each reply-to address is @@ -74,6 +80,9 @@ practically, is a reasonable setting for many installs. The reply-to address will still contain a hash unique to the object it represents, so users who have not received an email about an object can not blindly interact with it. +If you enable ##metamta.maniphest.public-create-email##, that address also uses +the weaker "From" authentication mechanism. + NOTE: Phabricator does not currently attempt to verify "From" addresses because this is technically complex, seems unreasonably difficult in the general case, and no installs have had a need for it yet. If you have a specific case where a