1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00

Allow mailers to be explicitly marked as inbound or outbound

Summary:
See PHI785. Ref T13164. In this case, an install wants to receive mail via Mailgun, but not configure it (DKIM + SPF) for outbound mail.

Allow individual mailers to be marked as not supporting inbound or outbound mail.

Test Plan:
  - Added and ran unit tests.
  - Went through some mail pathways locally, but I don't have every inbound/outbound configured so this isn't totally conclusive.
  - Hit `bin/mail send-test` with a no-outbound mailer.
  - I'll hold this until after the release cut so it can soak on `secure` for a bit.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19546
This commit is contained in:
epriestley 2018-07-27 12:42:25 -07:00
parent 9e451879d9
commit 690a460c8e
9 changed files with 185 additions and 25 deletions

View file

@ -6,6 +6,9 @@ abstract class PhabricatorMailImplementationAdapter extends Phobject {
private $priority; private $priority;
private $options = array(); private $options = array();
private $supportsInbound = true;
private $supportsOutbound = true;
final public function getAdapterType() { final public function getAdapterType() {
return $this->getPhobjectClassConstant('ADAPTERTYPE'); return $this->getPhobjectClassConstant('ADAPTERTYPE');
} }
@ -67,6 +70,24 @@ abstract class PhabricatorMailImplementationAdapter extends Phobject {
return $this->priority; 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) { final public function getOption($key) {
if (!array_key_exists($key, $this->options)) { if (!array_key_exists($key, $this->options)) {
throw new Exception( throw new Exception(

View file

@ -17,9 +17,12 @@ final class PhabricatorMetaMTAMailgunReceiveController
// inbound mail from any of them. Test the signature to see if it matches // inbound mail from any of them. Test the signature to see if it matches
// any configured Mailgun mailer. // any configured Mailgun mailer.
$mailers = PhabricatorMetaMTAMail::newMailersWithTypes( $mailers = PhabricatorMetaMTAMail::newMailers(
array( array(
'inbound' => true,
'types' => array(
PhabricatorMailImplementationMailgunAdapter::ADAPTERTYPE, PhabricatorMailImplementationMailgunAdapter::ADAPTERTYPE,
),
)); ));
foreach ($mailers as $mailer) { foreach ($mailers as $mailer) {
$api_key = $mailer->getOption('api-key'); $api_key = $mailer->getOption('api-key');

View file

@ -12,9 +12,12 @@ final class PhabricatorMetaMTAPostmarkReceiveController
*/ */
public function handleRequest(AphrontRequest $request) { public function handleRequest(AphrontRequest $request) {
// Don't process requests if we don't have a configured Postmark adapter. // Don't process requests if we don't have a configured Postmark adapter.
$mailers = PhabricatorMetaMTAMail::newMailersWithTypes( $mailers = PhabricatorMetaMTAMail::newMailers(
array( array(
'inbound' => true,
'types' => array(
PhabricatorMailImplementationPostmarkAdapter::ADAPTERTYPE, PhabricatorMailImplementationPostmarkAdapter::ADAPTERTYPE,
),
)); ));
if (!$mailers) { if (!$mailers) {
return new Aphront404Response(); return new Aphront404Response();

View file

@ -11,9 +11,12 @@ final class PhabricatorMetaMTASendGridReceiveController
// SendGrid doesn't sign payloads so we can't be sure that SendGrid // SendGrid doesn't sign payloads so we can't be sure that SendGrid
// actually sent this request, but require a configured SendGrid mailer // actually sent this request, but require a configured SendGrid mailer
// before we activate this endpoint. // before we activate this endpoint.
$mailers = PhabricatorMetaMTAMail::newMailersWithTypes( $mailers = PhabricatorMetaMTAMail::newMailers(
array( array(
'inbound' => true,
'types' => array(
PhabricatorMailImplementationSendGridAdapter::ADAPTERTYPE, PhabricatorMailImplementationSendGridAdapter::ADAPTERTYPE,
),
)); ));
if (!$mailers) { if (!$mailers) {
return new Aphront404Response(); return new Aphront404Response();

View file

@ -168,7 +168,8 @@ final class PhabricatorMailManagementSendTestWorkflow
$mailer_key = $args->getArg('mailer'); $mailer_key = $args->getArg('mailer');
if ($mailer_key !== null) { if ($mailer_key !== null) {
$mailers = PhabricatorMetaMTAMail::newMailers(); $mailers = PhabricatorMetaMTAMail::newMailers(array());
$mailers = mpull($mailers, null, 'getKey'); $mailers = mpull($mailers, null, 'getKey');
if (!isset($mailers[$mailer_key])) { if (!isset($mailers[$mailer_key])) {
throw new PhutilArgumentUsageException( throw new PhutilArgumentUsageException(
@ -178,6 +179,13 @@ final class PhabricatorMailManagementSendTestWorkflow
implode(', ', array_keys($mailers)))); 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)); $mail->setTryMailers(array($mailer_key));
} }

View file

@ -494,7 +494,10 @@ final class PhabricatorMetaMTAMail
throw new Exception(pht('Trying to send an already-sent mail!')); 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'); $try_mailers = $this->getParam('mailers.try');
if ($try_mailers) { if ($try_mailers) {
@ -505,21 +508,15 @@ final class PhabricatorMetaMTAMail
return $this->sendWithMailers($mailers); return $this->sendWithMailers($mailers);
} }
public static function newMailersWithTypes(array $types) { public static function newMailers(array $constraints) {
$mailers = self::newMailers(); PhutilTypeSpec::checkMap(
$types = array_fuse($types); $constraints,
array(
'types' => 'optional list<string>',
'inbound' => 'optional bool',
'outbound' => 'optional bool',
));
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() {
$mailers = array(); $mailers = array();
$config = PhabricatorEnv::getEnvConfig('cluster.mailers'); $config = PhabricatorEnv::getEnvConfig('cluster.mailers');
@ -565,10 +562,45 @@ final class PhabricatorMetaMTAMail
$options = idx($spec, 'options', array()) + $defaults; $options = idx($spec, 'options', array()) + $defaults;
$mailer->setOptions($options); $mailer->setOptions($options);
$mailer->setSupportsInbound(idx($spec, 'inbound', true));
$mailer->setSupportsOutbound(idx($spec, 'outbound', true));
$mailers[] = $mailer; $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(); $sorted = array();
$groups = mgroup($mailers, 'getPriority'); $groups = mgroup($mailers, 'getPriority');
krsort($groups); krsort($groups);
@ -589,9 +621,24 @@ final class PhabricatorMetaMTAMail
public function sendWithMailers(array $mailers) { public function sendWithMailers(array $mailers) {
if (!$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 return $this
->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID) ->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID)
->setMessage(pht('No mailers are configured.')) ->setMessage($void_message)
->save(); ->save();
} }

View file

@ -118,11 +118,80 @@ final class PhabricatorMailConfigTestCase
$this->assertTrue($saw_a1 && $saw_a2); $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 = PhabricatorEnv::beginScopedEnv();
$env->overrideEnvConfig('cluster.mailers', $config); $env->overrideEnvConfig('cluster.mailers', $config);
$mailers = PhabricatorMetaMTAMail::newMailers(); $mailers = PhabricatorMetaMTAMail::newMailers($constraints);
unset($env); unset($env);
return $mailers; return $mailers;

View file

@ -82,6 +82,10 @@ The supported keys for each mailer are:
- `priority`: Optional string. Advanced option which controls load balancing - `priority`: Optional string. Advanced option which controls load balancing
and failover behavior. See below for details. and failover behavior. See below for details.
- `options`: Optional map. Additional options for the mailer type. - `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: The `type` field can be used to select these third-party mailers:

View file

@ -43,6 +43,8 @@ final class PhabricatorClusterMailersConfigType
'type' => 'string', 'type' => 'string',
'priority' => 'optional int', 'priority' => 'optional int',
'options' => 'optional wild', 'options' => 'optional wild',
'inbound' => 'optional bool',
'outbound' => 'optional bool',
)); ));
} catch (Exception $ex) { } catch (Exception $ex) {
throw $this->newException( throw $this->newException(