From d598edc5f38beab44d0593f1c49957a37bdbbff9 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Thu, 12 Feb 2015 11:05:39 -0800 Subject: [PATCH] MetaMTA - update documentation and make config a tad easier Summary: Fixes T7088. Mainly this updates the documentation but I also snuck in tweaking how the domain reply handler is built. This does two main things -- makes the behavior consistent as some applications who didn't override this behavior would send out emails with reply tos AND makes it easier for us to deprecate the custom domain thing on a per application basis, which is just silly. On that note, the main documentation doesn't get into how this can be overridden, though I left in that mini blurb on the config setting itself. We could deprecate this harder and LOCK things if you want as well. Test Plan: read docs, looked good. reasoned through re-factor Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7088 Differential Revision: https://secure.phabricator.com/D11725 --- .../mail/PhabricatorAuditReplyHandler.php | 2 +- .../PhabricatorMetaMTAConfigOptions.php | 5 ++- .../mail/DifferentialReplyHandler.php | 2 +- .../fund/mail/FundInitiativeReplyHandler.php | 4 -- .../legalpad/mail/LegalpadReplyHandler.php | 5 --- .../mail/PhabricatorMacroReplyHandler.php | 2 +- .../maniphest/mail/ManiphestReplyHandler.php | 2 +- .../meta/query/PhabricatorAppSearchEngine.php | 19 +++++++++ .../query/PhabricatorApplicationQuery.php | 14 +++++++ .../PhabricatorMailReplyHandler.php | 11 ++++++ .../pholio/mail/PholioReplyHandler.php | 2 +- .../mail/PhortuneCartReplyHandler.php | 4 -- .../phriction/mail/PhrictionReplyHandler.php | 4 -- .../configuring_inbound_email.diviner | 39 +++++++++---------- 14 files changed, 70 insertions(+), 45 deletions(-) diff --git a/src/applications/audit/mail/PhabricatorAuditReplyHandler.php b/src/applications/audit/mail/PhabricatorAuditReplyHandler.php index 698be5b9ef..db254fb4d1 100644 --- a/src/applications/audit/mail/PhabricatorAuditReplyHandler.php +++ b/src/applications/audit/mail/PhabricatorAuditReplyHandler.php @@ -18,7 +18,7 @@ final class PhabricatorAuditReplyHandler extends PhabricatorMailReplyHandler { } public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig( + return $this->getCustomReplyHandlerDomainIfExists( 'metamta.diffusion.reply-handler-domain'); } diff --git a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php index fb4667a0ff..7952c4273d 100644 --- a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php +++ b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php @@ -243,10 +243,11 @@ EODOC $this->newOption( 'metamta.reply-handler-domain', 'string', - 'phabricator.example.com') + null) ->setDescription(pht( 'Domain used for reply email addresses. Some applications can '. - 'configure this domain.')), + 'override this configuration with a different domain.')) + ->addExample('phabricator.example.com', ''), $this->newOption('metamta.reply.show-hints', 'bool', true) ->setBoolOptions( array( diff --git a/src/applications/differential/mail/DifferentialReplyHandler.php b/src/applications/differential/mail/DifferentialReplyHandler.php index 92d888f3db..7dd64b51c5 100644 --- a/src/applications/differential/mail/DifferentialReplyHandler.php +++ b/src/applications/differential/mail/DifferentialReplyHandler.php @@ -25,7 +25,7 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler { } public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig( + return $this->getCustomReplyHandlerDomainIfExists( 'metamta.differential.reply-handler-domain'); } diff --git a/src/applications/fund/mail/FundInitiativeReplyHandler.php b/src/applications/fund/mail/FundInitiativeReplyHandler.php index 1665eb9729..a2bd2587df 100644 --- a/src/applications/fund/mail/FundInitiativeReplyHandler.php +++ b/src/applications/fund/mail/FundInitiativeReplyHandler.php @@ -17,10 +17,6 @@ final class FundInitiativeReplyHandler extends PhabricatorMailReplyHandler { return $this->getDefaultPublicReplyHandlerEmailAddress('I'); } - public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig('metamta.reply-handler-domain'); - } - public function getReplyHandlerInstructions() { if ($this->supportsReplies()) { // TODO: Implement. diff --git a/src/applications/legalpad/mail/LegalpadReplyHandler.php b/src/applications/legalpad/mail/LegalpadReplyHandler.php index 006b3efd80..08699d9bdd 100644 --- a/src/applications/legalpad/mail/LegalpadReplyHandler.php +++ b/src/applications/legalpad/mail/LegalpadReplyHandler.php @@ -17,11 +17,6 @@ final class LegalpadReplyHandler extends PhabricatorMailReplyHandler { return $this->getDefaultPublicReplyHandlerEmailAddress('L'); } - public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig( - 'metamta.reply-handler-domain'); - } - public function getReplyHandlerInstructions() { if ($this->supportsReplies()) { return pht('Reply to comment or !unsubscribe.'); diff --git a/src/applications/macro/mail/PhabricatorMacroReplyHandler.php b/src/applications/macro/mail/PhabricatorMacroReplyHandler.php index b447895d7f..c2cf90352b 100644 --- a/src/applications/macro/mail/PhabricatorMacroReplyHandler.php +++ b/src/applications/macro/mail/PhabricatorMacroReplyHandler.php @@ -18,7 +18,7 @@ final class PhabricatorMacroReplyHandler extends PhabricatorMailReplyHandler { } public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig( + return $this->getCustomReplyHandlerDomainIfExists( 'metamta.macro.reply-handler-domain'); } diff --git a/src/applications/maniphest/mail/ManiphestReplyHandler.php b/src/applications/maniphest/mail/ManiphestReplyHandler.php index c853c0f8ec..1c04093f4c 100644 --- a/src/applications/maniphest/mail/ManiphestReplyHandler.php +++ b/src/applications/maniphest/mail/ManiphestReplyHandler.php @@ -18,7 +18,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler { } public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig( + return $this->getCustomReplyHandlerDomainIfExists( 'metamta.maniphest.reply-handler-domain'); } diff --git a/src/applications/meta/query/PhabricatorAppSearchEngine.php b/src/applications/meta/query/PhabricatorAppSearchEngine.php index a25260b6f6..a9d262724e 100644 --- a/src/applications/meta/query/PhabricatorAppSearchEngine.php +++ b/src/applications/meta/query/PhabricatorAppSearchEngine.php @@ -32,6 +32,9 @@ final class PhabricatorAppSearchEngine $saved->setParameter( 'launchable', $this->readBoolFromRequest($request, 'launchable')); + $saved->setParameter( + 'appemails', + $this->readBoolFromRequest($request, 'appemails')); return $saved; } @@ -73,6 +76,11 @@ final class PhabricatorAppSearchEngine $query->withLaunchable($launchable); } + $appemails = $saved->getParameter('appemails'); + if ($appemails !== null) { + $query->withApplicationEmailSupport($appemails); + } + return $query; } @@ -129,6 +137,17 @@ final class PhabricatorAppSearchEngine '' => pht('Show All Applications'), 'true' => pht('Show Launchable Applications'), 'false' => pht('Show Non-Launchable Applications'), + ))) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Application Emails')) + ->setName('appemails') + ->setValue($this->getBoolFromQuery($saved, 'appemails')) + ->setOptions( + array( + '' => pht('Show All Applications'), + 'true' => pht('Show Applications w/ App Email Support'), + 'false' => pht('Show Applications w/o App Email Support'), ))); } diff --git a/src/applications/meta/query/PhabricatorApplicationQuery.php b/src/applications/meta/query/PhabricatorApplicationQuery.php index cd42089fc6..63de1174cf 100644 --- a/src/applications/meta/query/PhabricatorApplicationQuery.php +++ b/src/applications/meta/query/PhabricatorApplicationQuery.php @@ -10,6 +10,7 @@ final class PhabricatorApplicationQuery private $unlisted; private $classes; private $launchable; + private $applicationEmailSupport; private $phids; const ORDER_APPLICATION = 'order:application'; @@ -47,6 +48,11 @@ final class PhabricatorApplicationQuery return $this; } + public function withApplicationEmailSupport($appemails) { + $this->applicationEmailSupport = $appemails; + return $this; + } + public function withClasses(array $classes) { $this->classes = $classes; return $this; @@ -131,6 +137,14 @@ final class PhabricatorApplicationQuery } } + if ($this->applicationEmailSupport !== null) { + foreach ($apps as $key => $app) { + if ($app->supportsEmailIntegration() != + $this->applicationEmailSupport) { + unset($apps[$key]); + } + } + } switch ($this->order) { case self::ORDER_NAME: diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index be6511e6fc..c320de9b78 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -49,9 +49,20 @@ abstract class PhabricatorMailReplyHandler { abstract public function getPrivateReplyHandlerEmailAddress( PhabricatorObjectHandle $handle); public function getReplyHandlerDomain() { + return $this->getDefaultReplyHandlerDomain(); + } + protected function getCustomReplyHandlerDomainIfExists($config_key) { + $domain = PhabricatorEnv::getEnvConfig($config_key); + if ($domain) { + return $domain; + } + return $this->getDefaultReplyHandlerDomain(); + } + private function getDefaultReplyHandlerDomain() { return PhabricatorEnv::getEnvConfig( 'metamta.reply-handler-domain'); } + abstract public function getReplyHandlerInstructions(); abstract protected function receiveEmail( PhabricatorMetaMTAReceivedMail $mail); diff --git a/src/applications/pholio/mail/PholioReplyHandler.php b/src/applications/pholio/mail/PholioReplyHandler.php index 08ba11e393..ce7578ff92 100644 --- a/src/applications/pholio/mail/PholioReplyHandler.php +++ b/src/applications/pholio/mail/PholioReplyHandler.php @@ -18,7 +18,7 @@ final class PholioReplyHandler extends PhabricatorMailReplyHandler { } public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig( + return $this->getCustomReplyHandlerDomainIfExists( 'metamta.pholio.reply-handler-domain'); } diff --git a/src/applications/phortune/mail/PhortuneCartReplyHandler.php b/src/applications/phortune/mail/PhortuneCartReplyHandler.php index 92e3022d43..c48d1ce264 100644 --- a/src/applications/phortune/mail/PhortuneCartReplyHandler.php +++ b/src/applications/phortune/mail/PhortuneCartReplyHandler.php @@ -17,10 +17,6 @@ final class PhortuneCartReplyHandler extends PhabricatorMailReplyHandler { return $this->getDefaultPublicReplyHandlerEmailAddress('CART'); } - public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig('metamta.reply-handler-domain'); - } - public function getReplyHandlerInstructions() { if ($this->supportsReplies()) { // TODO: Implement. diff --git a/src/applications/phriction/mail/PhrictionReplyHandler.php b/src/applications/phriction/mail/PhrictionReplyHandler.php index 1f6564dbd7..b458e13dc0 100644 --- a/src/applications/phriction/mail/PhrictionReplyHandler.php +++ b/src/applications/phriction/mail/PhrictionReplyHandler.php @@ -20,10 +20,6 @@ final class PhrictionReplyHandler extends PhabricatorMailReplyHandler { PhrictionDocumentPHIDType::TYPECONST); } - public function getReplyHandlerDomain() { - return PhabricatorEnv::getEnvConfig('metamta.reply-handler-domain'); - } - public function getReplyHandlerInstructions() { if ($this->supportsReplies()) { // TODO: Implement. diff --git a/src/docs/user/configuration/configuring_inbound_email.diviner b/src/docs/user/configuration/configuring_inbound_email.diviner index cc6e85120f..c59199e2c0 100644 --- a/src/docs/user/configuration/configuring_inbound_email.diviner +++ b/src/docs/user/configuration/configuring_inbound_email.diviner @@ -2,8 +2,7 @@ @group config This document contains instructions for configuring inbound email, so users -may update Differential and Maniphest by replying to messages and create -Maniphest tasks via email. +may interact with some Phabricator applications via email. = Preamble = @@ -33,20 +32,13 @@ in Phabricator and users will not be able to take actions like claiming tasks or requesting changes to revisions. To change this behavior so that users can interact with objects in Phabricator -over email, set these configuration keys: - - - ##metamta.differential.reply-handler-domain##: enables email replies for - Differential. - - ##metamta.maniphest.reply-handler-domain##: enables email replies for - Maniphest. - -Set these keys to some domain which you configure according to the instructions -below, e.g. `phabricator.example.com`. You can set these both to the same -domain, and will generally want to. Once you set these keys, emails will use a -'Reply-To' like `T123+273+af310f9220ad@example.com`, which -- when +over email, change the configuration key `metamta.reply-handler-domain` to some +domain you configure according to the instructions below, e.g. +`phabricator.example.com`. Once you set this key, emails will use a +'Reply-To' like `T123+273+af310f9220ad@phabricator.example.com`, which -- when configured correctly, according to the instructions below -- will parse incoming -email and allow users to interact with Maniphest tasks and Differential -revisions over email. +email and allow users to interact with Differential revisions, Maniphest tasks, +etc. over email. If you don't want Phabricator to take up an entire domain (or subdomain) you can configure a general prefix so you can use a single mailbox to receive mail @@ -56,10 +48,15 @@ mail address. This works because everything up to the first (optional) '+' character in an email-address is considered the receiver, and everything after is essentially ignored. -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. +You can also set up application email addresses to allow users to create +application objects via email. For example, you could configure +`bugs@phabricator.example.com` to create a Maniphest task out of any email +which is sent to it. To do this, see application settings for a given +application at + +{nav icon=home, name=Home > +name=Applications > +icon=cog, name=Settings} = Security = @@ -93,8 +90,8 @@ 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. +If you enable application email addresses, those addresses also use 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,