diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php @@ -6,6 +6,9 @@ private $priority; private $options = array(); + private $supportsInbound = true; + private $supportsOutbound = true; + final public function getAdapterType() { return $this->getPhobjectClassConstant('ADAPTERTYPE'); } @@ -67,6 +70,24 @@ return $this->priority; } + final public function setSupportsInbound($supports_inbound) { + $this->supportsInbound = $supports_inbound; + return $this; + } + + final public function getSupportsInbound() { + return $this->supportsInbound; + } + + final public function setSupportsOutbound($supports_outbound) { + $this->supportsOutbound = $supports_outbound; + return $this; + } + + final public function getSupportsOutbound() { + return $this->supportsOutbound; + } + final public function getOption($key) { if (!array_key_exists($key, $this->options)) { throw new Exception( diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php --- a/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php +++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php @@ -17,9 +17,12 @@ // inbound mail from any of them. Test the signature to see if it matches // any configured Mailgun mailer. - $mailers = PhabricatorMetaMTAMail::newMailersWithTypes( + $mailers = PhabricatorMetaMTAMail::newMailers( array( - PhabricatorMailImplementationMailgunAdapter::ADAPTERTYPE, + 'inbound' => true, + 'types' => array( + PhabricatorMailImplementationMailgunAdapter::ADAPTERTYPE, + ), )); foreach ($mailers as $mailer) { $api_key = $mailer->getOption('api-key'); diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAPostmarkReceiveController.php b/src/applications/metamta/controller/PhabricatorMetaMTAPostmarkReceiveController.php --- a/src/applications/metamta/controller/PhabricatorMetaMTAPostmarkReceiveController.php +++ b/src/applications/metamta/controller/PhabricatorMetaMTAPostmarkReceiveController.php @@ -12,9 +12,12 @@ */ public function handleRequest(AphrontRequest $request) { // Don't process requests if we don't have a configured Postmark adapter. - $mailers = PhabricatorMetaMTAMail::newMailersWithTypes( + $mailers = PhabricatorMetaMTAMail::newMailers( array( - PhabricatorMailImplementationPostmarkAdapter::ADAPTERTYPE, + 'inbound' => true, + 'types' => array( + PhabricatorMailImplementationPostmarkAdapter::ADAPTERTYPE, + ), )); if (!$mailers) { return new Aphront404Response(); diff --git a/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php b/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php --- a/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php +++ b/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php @@ -11,9 +11,12 @@ // SendGrid doesn't sign payloads so we can't be sure that SendGrid // actually sent this request, but require a configured SendGrid mailer // before we activate this endpoint. - $mailers = PhabricatorMetaMTAMail::newMailersWithTypes( + $mailers = PhabricatorMetaMTAMail::newMailers( array( - PhabricatorMailImplementationSendGridAdapter::ADAPTERTYPE, + 'inbound' => true, + 'types' => array( + PhabricatorMailImplementationSendGridAdapter::ADAPTERTYPE, + ), )); if (!$mailers) { return new Aphront404Response(); diff --git a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php --- a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php @@ -168,7 +168,8 @@ $mailer_key = $args->getArg('mailer'); if ($mailer_key !== null) { - $mailers = PhabricatorMetaMTAMail::newMailers(); + $mailers = PhabricatorMetaMTAMail::newMailers(array()); + $mailers = mpull($mailers, null, 'getKey'); if (!isset($mailers[$mailer_key])) { throw new PhutilArgumentUsageException( @@ -178,6 +179,13 @@ implode(', ', array_keys($mailers)))); } + if (!$mailers[$mailer_key]->getSupportsOutbound()) { + throw new PhutilArgumentUsageException( + pht( + 'Mailer ("%s") is not configured to support outbound mail.', + $mailer_key)); + } + $mail->setTryMailers(array($mailer_key)); } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -494,7 +494,10 @@ throw new Exception(pht('Trying to send an already-sent mail!')); } - $mailers = self::newMailers(); + $mailers = self::newMailers( + array( + 'outbound' => true, + )); $try_mailers = $this->getParam('mailers.try'); if ($try_mailers) { @@ -505,21 +508,15 @@ return $this->sendWithMailers($mailers); } - public static function newMailersWithTypes(array $types) { - $mailers = self::newMailers(); - $types = array_fuse($types); - - foreach ($mailers as $key => $mailer) { - $mailer_type = $mailer->getAdapterType(); - if (!isset($types[$mailer_type])) { - unset($mailers[$key]); - } - } - - return array_values($mailers); - } + public static function newMailers(array $constraints) { + PhutilTypeSpec::checkMap( + $constraints, + array( + 'types' => 'optional list', + 'inbound' => 'optional bool', + 'outbound' => 'optional bool', + )); - public static function newMailers() { $mailers = array(); $config = PhabricatorEnv::getEnvConfig('cluster.mailers'); @@ -565,10 +562,45 @@ $options = idx($spec, 'options', array()) + $defaults; $mailer->setOptions($options); + $mailer->setSupportsInbound(idx($spec, 'inbound', true)); + $mailer->setSupportsOutbound(idx($spec, 'outbound', true)); + $mailers[] = $mailer; } } + // Remove mailers with the wrong types. + if (isset($constraints['types'])) { + $types = $constraints['types']; + $types = array_fuse($types); + foreach ($mailers as $key => $mailer) { + $mailer_type = $mailer->getAdapterType(); + if (!isset($types[$mailer_type])) { + unset($mailers[$key]); + } + } + } + + // If we're only looking for inbound mailers, remove mailers with inbound + // support disabled. + if (!empty($constraints['inbound'])) { + foreach ($mailers as $key => $mailer) { + if (!$mailer->getSupportsInbound()) { + unset($mailers[$key]); + } + } + } + + // If we're only looking for outbound mailers, remove mailers with outbound + // support disabled. + if (!empty($constraints['outbound'])) { + foreach ($mailers as $key => $mailer) { + if (!$mailer->getSupportsOutbound()) { + unset($mailers[$key]); + } + } + } + $sorted = array(); $groups = mgroup($mailers, 'getPriority'); krsort($groups); @@ -589,9 +621,24 @@ public function sendWithMailers(array $mailers) { if (!$mailers) { + $any_mailers = self::newMailers(); + + // NOTE: We can end up here with some custom list of "$mailers", like + // from a unit test. In that case, this message could be misleading. We + // can't really tell if the caller made up the list, so just assume they + // aren't tricking us. + + if ($any_mailers) { + $void_message = pht( + 'No configured mailers support sending outbound mail.'); + } else { + $void_message = pht( + 'No mailers are configured.'); + } + return $this ->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID) - ->setMessage(pht('No mailers are configured.')) + ->setMessage($void_message) ->save(); } diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php --- a/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php +++ b/src/applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php @@ -118,11 +118,80 @@ $this->assertTrue($saw_a1 && $saw_a2); } - private function newMailersWithConfig(array $config) { + public function testMailerConstraints() { + $config = array( + array( + 'key' => 'X1', + 'type' => 'test', + ), + array( + 'key' => 'X1-in', + 'type' => 'test', + 'outbound' => false, + ), + array( + 'key' => 'X1-out', + 'type' => 'test', + 'inbound' => false, + ), + array( + 'key' => 'X1-void', + 'type' => 'test', + 'inbound' => false, + 'outbound' => false, + ), + ); + + $mailers = $this->newMailersWithConfig( + $config, + array()); + $this->assertEqual(4, count($mailers)); + + $mailers = $this->newMailersWithConfig( + $config, + array( + 'inbound' => true, + )); + $this->assertEqual(2, count($mailers)); + + $mailers = $this->newMailersWithConfig( + $config, + array( + 'outbound' => true, + )); + $this->assertEqual(2, count($mailers)); + + $mailers = $this->newMailersWithConfig( + $config, + array( + 'inbound' => true, + 'outbound' => true, + )); + $this->assertEqual(1, count($mailers)); + + $mailers = $this->newMailersWithConfig( + $config, + array( + 'types' => array('test'), + )); + $this->assertEqual(4, count($mailers)); + + $mailers = $this->newMailersWithConfig( + $config, + array( + 'types' => array('duck'), + )); + $this->assertEqual(0, count($mailers)); + } + + private function newMailersWithConfig( + array $config, + array $constraints = array()) { + $env = PhabricatorEnv::beginScopedEnv(); $env->overrideEnvConfig('cluster.mailers', $config); - $mailers = PhabricatorMetaMTAMail::newMailers(); + $mailers = PhabricatorMetaMTAMail::newMailers($constraints); unset($env); return $mailers; diff --git a/src/docs/user/configuration/configuring_outbound_email.diviner b/src/docs/user/configuration/configuring_outbound_email.diviner --- a/src/docs/user/configuration/configuring_outbound_email.diviner +++ b/src/docs/user/configuration/configuring_outbound_email.diviner @@ -82,6 +82,10 @@ - `priority`: Optional string. Advanced option which controls load balancing and failover behavior. See below for details. - `options`: Optional map. Additional options for the mailer type. + - `inbound`: Optional bool. Use `false` to prevent this mailer from being + used to receive inbound mail. + - `outbound`: Optional bool. Use `false` to prevent this mailer from being + used to send outbound mail. The `type` field can be used to select these third-party mailers: diff --git a/src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php b/src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php --- a/src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php +++ b/src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php @@ -43,6 +43,8 @@ 'type' => 'string', 'priority' => 'optional int', 'options' => 'optional wild', + 'inbound' => 'optional bool', + 'outbound' => 'optional bool', )); } catch (Exception $ex) { throw $this->newException(