Page MenuHomePhabricator
Files F18736026

D19952.id47626.diff
No OneTemporary
Actions

D19952.id47626.diff
View Options

diff --git a/src/applications/audit/mail/PhabricatorAuditMailReceiver.php b/src/applications/audit/mail/PhabricatorAuditMailReceiver.php
--- a/src/applications/audit/mail/PhabricatorAuditMailReceiver.php
+++ b/src/applications/audit/mail/PhabricatorAuditMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)preg_replace('/^COMMIT/', '', $pattern);
+ $id = (int)preg_replace('/^COMMIT/i', '', $pattern);
return id(new DiffusionCommitQuery())
->setViewer($viewer)
diff --git a/src/applications/calendar/mail/PhabricatorCalendarEventMailReceiver.php b/src/applications/calendar/mail/PhabricatorCalendarEventMailReceiver.php
--- a/src/applications/calendar/mail/PhabricatorCalendarEventMailReceiver.php
+++ b/src/applications/calendar/mail/PhabricatorCalendarEventMailReceiver.php
@@ -13,7 +13,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'E');
+ $id = (int)substr($pattern, 1);
return id(new PhabricatorCalendarEventQuery())
->setViewer($viewer)
diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
--- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
+++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
@@ -394,6 +394,10 @@
'metamta.insecure-auth-with-reply-to' => pht(
'Authenticating users based on "Reply-To" is no longer supported.'),
+
+ 'phabricator.allow-email-users' => pht(
+ 'Public email is now accepted if the associated address has a '.
+ 'default author, and rejected otherwise.'),
);
return $ancient_config;
diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php
--- a/src/applications/config/option/PhabricatorCoreConfigOptions.php
+++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php
@@ -234,14 +234,6 @@
$this->newOption('phabricator.cache-namespace', 'string', 'phabricator')
->setLocked(true)
->setDescription(pht('Cache namespace.')),
- $this->newOption('phabricator.allow-email-users', 'bool', false)
- ->setBoolOptions(
- array(
- pht('Allow'),
- pht('Disallow'),
- ))
- ->setDescription(
- pht('Allow non-members to interact with tasks over email.')),
$this->newOption('phabricator.silent', 'bool', false)
->setLocked(true)
->setBoolOptions(
diff --git a/src/applications/conpherence/mail/ConpherenceThreadMailReceiver.php b/src/applications/conpherence/mail/ConpherenceThreadMailReceiver.php
--- a/src/applications/conpherence/mail/ConpherenceThreadMailReceiver.php
+++ b/src/applications/conpherence/mail/ConpherenceThreadMailReceiver.php
@@ -13,7 +13,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'Z');
+ $id = (int)substr($pattern, 1);
return id(new ConpherenceThreadQuery())
->setViewer($viewer)
diff --git a/src/applications/countdown/mail/PhabricatorCountdownMailReceiver.php b/src/applications/countdown/mail/PhabricatorCountdownMailReceiver.php
--- a/src/applications/countdown/mail/PhabricatorCountdownMailReceiver.php
+++ b/src/applications/countdown/mail/PhabricatorCountdownMailReceiver.php
@@ -13,7 +13,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)substr($pattern, 4);
+ $id = (int)substr($pattern, 1);
return id(new PhabricatorCountdownQuery())
->setViewer($viewer)
diff --git a/src/applications/differential/mail/DifferentialCreateMailReceiver.php b/src/applications/differential/mail/DifferentialCreateMailReceiver.php
--- a/src/applications/differential/mail/DifferentialCreateMailReceiver.php
+++ b/src/applications/differential/mail/DifferentialCreateMailReceiver.php
@@ -9,14 +9,16 @@
protected function processReceivedMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
+ PhutilEmailAddress $target) {
+
+ $author = $this->getAuthor();
$attachments = $mail->getAttachments();
$files = array();
$errors = array();
if ($attachments) {
$files = id(new PhabricatorFileQuery())
- ->setViewer($sender)
+ ->setViewer($author)
->withPHIDs($attachments)
->execute();
foreach ($files as $index => $file) {
@@ -37,7 +39,7 @@
array(
'diff' => $file->loadFileData(),
));
- $call->setUser($sender);
+ $call->setUser($author);
try {
$result = $call->execute();
$diffs[$file->getName()] = $result['uri'];
@@ -56,7 +58,7 @@
array(
'diff' => $body,
));
- $call->setUser($sender);
+ $call->setUser($author);
try {
$result = $call->execute();
$diffs[pht('Mail Body')] = $result['uri'];
@@ -108,10 +110,10 @@
}
id(new PhabricatorMetaMTAMail())
- ->addTos(array($sender->getPHID()))
+ ->addTos(array($author->getPHID()))
->setSubject($subject)
->setSubjectPrefix($subject_prefix)
- ->setFrom($sender->getPHID())
+ ->setFrom($author->getPHID())
->setBody($body->render())
->saveAndSend();
}
diff --git a/src/applications/differential/mail/DifferentialRevisionMailReceiver.php b/src/applications/differential/mail/DifferentialRevisionMailReceiver.php
--- a/src/applications/differential/mail/DifferentialRevisionMailReceiver.php
+++ b/src/applications/differential/mail/DifferentialRevisionMailReceiver.php
@@ -13,7 +13,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'D');
+ $id = (int)substr($pattern, 1);
return id(new DifferentialRevisionQuery())
->setViewer($viewer)
diff --git a/src/applications/files/mail/FileCreateMailReceiver.php b/src/applications/files/mail/FileCreateMailReceiver.php
--- a/src/applications/files/mail/FileCreateMailReceiver.php
+++ b/src/applications/files/mail/FileCreateMailReceiver.php
@@ -9,7 +9,8 @@
protected function processReceivedMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
+ PhutilEmailAddress $target) {
+ $author = $this->getAuthor();
$attachment_phids = $mail->getAttachments();
if (empty($attachment_phids)) {
@@ -21,6 +22,11 @@
$first_phid = head($attachment_phids);
$mail->setRelatedPHID($first_phid);
+ $sender = $this->getSender();
+ if (!$sender) {
+ return;
+ }
+
$attachment_count = count($attachment_phids);
if ($attachment_count > 1) {
$subject = pht('You successfully uploaded %d files.', $attachment_count);
diff --git a/src/applications/files/mail/FileMailReceiver.php b/src/applications/files/mail/FileMailReceiver.php
--- a/src/applications/files/mail/FileMailReceiver.php
+++ b/src/applications/files/mail/FileMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'F');
+ $id = (int)substr($pattern, 1);
return id(new PhabricatorFileQuery())
->setViewer($viewer)
diff --git a/src/applications/legalpad/mail/LegalpadMailReceiver.php b/src/applications/legalpad/mail/LegalpadMailReceiver.php
--- a/src/applications/legalpad/mail/LegalpadMailReceiver.php
+++ b/src/applications/legalpad/mail/LegalpadMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'L');
+ $id = (int)substr($pattern, 1);
return id(new LegalpadDocumentQuery())
->setViewer($viewer)
diff --git a/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php b/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php
--- a/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php
+++ b/src/applications/maniphest/mail/ManiphestCreateMailReceiver.php
@@ -9,15 +9,20 @@
protected function processReceivedMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
+ PhutilEmailAddress $target) {
- $task = ManiphestTask::initializeNewTask($sender);
- $task->setOriginalEmailSource($mail->getHeader('From'));
+ $author = $this->getAuthor();
+ $task = ManiphestTask::initializeNewTask($author);
+
+ $from_address = $mail->newFromAddress();
+ if ($from_address) {
+ $task->setOriginalEmailSource((string)$from_address);
+ }
$handler = new ManiphestReplyHandler();
$handler->setMailReceiver($task);
- $handler->setActor($sender);
+ $handler->setActor($author);
$handler->setExcludeMailRecipientPHIDs(
$mail->loadAllRecipientPHIDs());
if ($this->getApplicationEmail()) {
diff --git a/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php b/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php
--- a/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php
+++ b/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'T');
+ $id = (int)substr($pattern, 1);
return id(new ManiphestTaskQuery())
->setViewer($viewer)
diff --git a/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php b/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php
--- a/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php
+++ b/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php
@@ -262,7 +262,9 @@
->setLimit(1)
->setValue($v_default)
->setCaption(pht(
- 'Used if the "From:" address does not map to a known account.')));
+ 'Used if the "From:" address does not map to a known account. '.
+ 'Setting this will allow anyone on the public internet to send '.
+ 'mail to this address.')));
if ($is_new) {
$title = pht('New Address');
@@ -369,9 +371,17 @@
$email_space = null;
}
+ $default_author_phid = $email->getDefaultAuthorPHID();
+ if (!$default_author_phid) {
+ $default_author = phutil_tag('em', array(), pht('None'));
+ } else {
+ $default_author = $viewer->renderHandle($default_author_phid);
+ }
+
$rows[] = array(
$email_space,
$email->getAddress(),
+ $default_author,
$button_edit,
$button_remove,
);
@@ -383,11 +393,13 @@
array(
pht('Space'),
pht('Email'),
+ pht('Default'),
pht('Edit'),
pht('Delete'),
));
$table->setColumnClasses(
array(
+ '',
'',
'wide',
'action',
@@ -398,6 +410,7 @@
array(
PhabricatorSpacesNamespaceQuery::getViewerSpacesExist($viewer),
true,
+ true,
$is_edit,
$is_edit,
));
diff --git a/src/applications/metamta/constants/MetaMTAReceivedMailStatus.php b/src/applications/metamta/constants/MetaMTAReceivedMailStatus.php
--- a/src/applications/metamta/constants/MetaMTAReceivedMailStatus.php
+++ b/src/applications/metamta/constants/MetaMTAReceivedMailStatus.php
@@ -6,7 +6,6 @@
const STATUS_DUPLICATE = 'err:duplicate';
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';
const STATUS_DISABLED_SENDER = 'err:disabled-sender';
const STATUS_NO_PUBLIC_MAIL = 'err:no-public-mail';
@@ -23,7 +22,6 @@
self::STATUS_DUPLICATE => pht('Duplicate Message'),
self::STATUS_FROM_PHABRICATOR => pht('Phabricator Mail'),
self::STATUS_NO_RECEIVERS => pht('No Receivers'),
- self::STATUS_ABUNDANT_RECEIVERS => pht('Multiple Receivers'),
self::STATUS_UNKNOWN_SENDER => pht('Unknown Sender'),
self::STATUS_DISABLED_SENDER => pht('Disabled Sender'),
self::STATUS_NO_PUBLIC_MAIL => pht('No Public Mail'),
diff --git a/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php
--- a/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php
+++ b/src/applications/metamta/management/PhabricatorMailManagementReceiveTestWorkflow.php
@@ -33,6 +33,7 @@
}
public function execute(PhutilArgumentParser $args) {
+ $viewer = $this->getViewer();
$console = PhutilConsole::getConsole();
$to = $args->getArg('to');
@@ -95,14 +96,23 @@
if (preg_match('/.+@.+/', $to)) {
$header_content['to'] = $to;
} else {
+
// We allow the user to use an object name instead of a real address
// as a convenience. To build the mail, we build a similar message and
// look for a receiver which will accept it.
+
+ // In the general case, mail may be processed by multiple receivers,
+ // but mail to objects only ever has one receiver today.
+
$pseudohash = PhabricatorObjectMailReceiver::computeMailHash('x', 'y');
+
+ $raw_target = $to.'+1+'.$pseudohash;
+ $target = new PhutilEmailAddress($raw_target.'@local.cli');
+
$pseudomail = id(new PhabricatorMetaMTAReceivedMail())
->setHeaders(
array(
- 'to' => $to.'+1+'.$pseudohash,
+ 'to' => $raw_target,
));
$receivers = id(new PhutilClassMapQuery())
@@ -112,7 +122,11 @@
$receiver = null;
foreach ($receivers as $possible_receiver) {
- if (!$possible_receiver->canAcceptMail($pseudomail)) {
+ $possible_receiver = id(clone $possible_receiver)
+ ->setViewer($viewer)
+ ->setSender($user);
+
+ if (!$possible_receiver->canAcceptMail($pseudomail, $target)) {
continue;
}
$receiver = $possible_receiver;
diff --git a/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php
--- a/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php
+++ b/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php
@@ -121,12 +121,11 @@
$actor->setEmailAddress($xuser->getAccountID());
- // NOTE: This effectively drops all outbound mail to unrecognized
- // addresses unless "phabricator.allow-email-users" is set. See T12237
- // for context.
- $allow_key = 'phabricator.allow-email-users';
- $allow_value = PhabricatorEnv::getEnvConfig($allow_key);
- $actor->setIsVerified((bool)$allow_value);
+ // Circa T7477, it appears that we never intentionally send email to
+ // external users (even when they email "bugs@" to create a task).
+ // Mark these users as unverified so mail to them is always dropped.
+ // See also T12237. This may not always be the right behavior.
+ $actor->setIsVerified(false);
}
}
diff --git a/src/applications/metamta/receiver/PhabricatorApplicationMailReceiver.php b/src/applications/metamta/receiver/PhabricatorApplicationMailReceiver.php
--- a/src/applications/metamta/receiver/PhabricatorApplicationMailReceiver.php
+++ b/src/applications/metamta/receiver/PhabricatorApplicationMailReceiver.php
@@ -3,32 +3,105 @@
abstract class PhabricatorApplicationMailReceiver
extends PhabricatorMailReceiver {
+ private $applicationEmail;
+ private $emailList;
+ private $author;
+
abstract protected function newApplication();
+ final protected function setApplicationEmail(
+ PhabricatorMetaMTAApplicationEmail $email) {
+ $this->applicationEmail = $email;
+ return $this;
+ }
+
+ final protected function getApplicationEmail() {
+ return $this->applicationEmail;
+ }
+
+ final protected function setAuthor(PhabricatorUser $author) {
+ $this->author = $author;
+ return $this;
+ }
+
+ final protected function getAuthor() {
+ return $this->author;
+ }
+
final public function isEnabled() {
return $this->newApplication()->isInstalled();
}
- final public function canAcceptMail(PhabricatorMetaMTAReceivedMail $mail) {
- $application = $this->newApplication();
+ final public function canAcceptMail(
+ PhabricatorMetaMTAReceivedMail $mail,
+ PhutilEmailAddress $target) {
+
$viewer = $this->getViewer();
+ $sender = $this->getSender();
+
+ foreach ($this->loadApplicationEmailList() as $application_email) {
+ $create_address = $application_email->newAddress();
+
+ if (!PhabricatorMailUtil::matchAddresses($create_address, $target)) {
+ continue;
+ }
- $application_emails = id(new PhabricatorMetaMTAApplicationEmailQuery())
- ->setViewer($viewer)
- ->withApplicationPHIDs(array($application->getPHID()))
- ->execute();
-
- foreach ($mail->newTargetAddresses() as $address) {
- foreach ($application_emails as $application_email) {
- $create_address = $application_email->newAddress();
- if (PhabricatorMailUtil::matchAddresses($create_address, $address)) {
- $this->setApplicationEmail($application_email);
- return true;
+ if ($sender) {
+ $author = $sender;
+ } else {
+ $author_phid = $application_email->getDefaultAuthorPHID();
+
+ // If this mail isn't from a recognized sender and the target address
+ // does not have a default author, we can't accept it, and it's an
+ // error because you tried to send it here.
+
+ // You either need to be sending from a real address or be sending to
+ // an address which accepts mail from the public internet.
+
+ if (!$author_phid) {
+ throw new PhabricatorMetaMTAReceivedMailProcessingException(
+ MetaMTAReceivedMailStatus::STATUS_UNKNOWN_SENDER,
+ pht(
+ 'You are sending from an unrecognized email address to '.
+ 'an address which does not support public email ("%s").',
+ (string)$target));
+ }
+
+ $author = id(new PhabricatorPeopleQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($author_phid))
+ ->executeOne();
+ if (!$author) {
+ throw new Exception(
+ pht(
+ 'Application email ("%s") has an invalid default author ("%s").',
+ (string)$create_address,
+ $author_phid));
}
}
+
+ $this
+ ->setApplicationEmail($application_email)
+ ->setAuthor($author);
+
+ return true;
}
return false;
}
+ private function loadApplicationEmailList() {
+ if ($this->emailList === null) {
+ $viewer = $this->getViewer();
+ $application = $this->newApplication();
+
+ $this->emailList = id(new PhabricatorMetaMTAApplicationEmailQuery())
+ ->setViewer($viewer)
+ ->withApplicationPHIDs(array($application->getPHID()))
+ ->execute();
+ }
+
+ return $this->emailList;
+ }
+
}
diff --git a/src/applications/metamta/receiver/PhabricatorMailReceiver.php b/src/applications/metamta/receiver/PhabricatorMailReceiver.php
--- a/src/applications/metamta/receiver/PhabricatorMailReceiver.php
+++ b/src/applications/metamta/receiver/PhabricatorMailReceiver.php
@@ -2,167 +2,40 @@
abstract class PhabricatorMailReceiver extends Phobject {
- private $applicationEmail;
+ private $viewer;
+ private $sender;
- public function setApplicationEmail(
- PhabricatorMetaMTAApplicationEmail $email) {
- $this->applicationEmail = $email;
+ final public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
return $this;
}
- public function getApplicationEmail() {
- return $this->applicationEmail;
+ final public function getViewer() {
+ return $this->viewer;
}
- abstract public function isEnabled();
- abstract public function canAcceptMail(PhabricatorMetaMTAReceivedMail $mail);
-
- abstract protected function processReceivedMail(
- PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender);
-
- final public function receiveMail(
- PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
- $this->processReceivedMail($mail, $sender);
+ final public function setSender(PhabricatorUser $sender) {
+ $this->sender = $sender;
+ return $this;
}
- public function getViewer() {
- return PhabricatorUser::getOmnipotentUser();
+ final public function getSender() {
+ return $this->sender;
}
- public function validateSender(
+ abstract public function isEnabled();
+ abstract public function canAcceptMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
-
- $failure_reason = null;
- if ($sender->getIsDisabled()) {
- $failure_reason = pht(
- 'Your account (%s) is disabled, so you can not interact with '.
- 'Phabricator over email.',
- $sender->getUsername());
- } else if ($sender->getIsStandardUser()) {
- if (!$sender->getIsApproved()) {
- $failure_reason = pht(
- 'Your account (%s) has not been approved yet. You can not interact '.
- 'with Phabricator over email until your account is approved.',
- $sender->getUsername());
- } else if (PhabricatorUserEmail::isEmailVerificationRequired() &&
- !$sender->getIsEmailVerified()) {
- $failure_reason = pht(
- 'You have not verified the email address for your account (%s). '.
- 'You must verify your email address before you can interact '.
- 'with Phabricator over email.',
- $sender->getUsername());
- }
- }
-
- if ($failure_reason) {
- throw new PhabricatorMetaMTAReceivedMailProcessingException(
- MetaMTAReceivedMailStatus::STATUS_DISABLED_SENDER,
- $failure_reason);
- }
- }
-
- /**
- * 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);
+ PhutilEmailAddress $target);
- $reasons = array();
-
- // Try to find a user with this email address.
- $user = PhabricatorUser::loadOneWithEmailAddress($from);
- if ($user) {
- return $user;
- } else {
- $reasons[] = pht(
- 'This email was sent from "%s", but that address is not recognized by '.
- 'Phabricator and does not correspond to any known user account.',
- $raw_from);
- }
-
- // 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) {
- $from_obj = new PhutilEmailAddress($from);
- $xuser = id(new PhabricatorExternalAccountQuery())
- ->setViewer($this->getViewer())
- ->withAccountTypes(array('email'))
- ->withAccountDomains(array($from_obj->getDomainName(), 'self'))
- ->withAccountIDs(array($from_obj->getAddress()))
- ->requireCapabilities(
- array(
- PhabricatorPolicyCapability::CAN_VIEW,
- PhabricatorPolicyCapability::CAN_EDIT,
- ))
- ->loadOneOrCreate();
- return $xuser->getPhabricatorUser();
- } else {
- // NOTE: Currently, we'll always drop this mail (since it's headed to
- // an unverified recipient). See T12237. These details are still useful
- // because they'll appear in the mail logs and Mail web UI.
-
- $reasons[] = pht(
- 'Phabricator is also not configured to allow unknown external users '.
- 'to send mail to the system using just an email address.');
- $reasons[] = pht(
- 'To interact with Phabricator, add this address ("%s") to your '.
- 'account.',
- $raw_from);
- }
-
- if ($this->getApplicationEmail()) {
- $application_email = $this->getApplicationEmail();
- $default_user_phid = $application_email->getConfigValue(
- PhabricatorMetaMTAApplicationEmail::CONFIG_DEFAULT_AUTHOR);
-
- if ($default_user_phid) {
- $user = id(new PhabricatorUser())->loadOneWhere(
- 'phid = %s',
- $default_user_phid);
- if ($user) {
- return $user;
- }
-
- $reasons[] = pht(
- 'Phabricator is misconfigured: the application email '.
- '"%s" is set to user "%s", but that user does not exist.',
- $application_email->getAddress(),
- $default_user_phid);
- }
- }
-
- $reasons = implode("\n\n", $reasons);
-
- throw new PhabricatorMetaMTAReceivedMailProcessingException(
- MetaMTAReceivedMailStatus::STATUS_UNKNOWN_SENDER,
- $reasons);
- }
+ abstract protected function processReceivedMail(
+ PhabricatorMetaMTAReceivedMail $mail,
+ PhutilEmailAddress $target);
- /**
- * Reduce an email address to its canonical form. For example, an address
- * 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));
+ final public function receiveMail(
+ PhabricatorMetaMTAReceivedMail $mail,
+ PhutilEmailAddress $target) {
+ $this->processReceivedMail($mail, $target);
}
}
diff --git a/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php b/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php
--- a/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php
+++ b/src/applications/metamta/receiver/PhabricatorObjectMailReceiver.php
@@ -29,52 +29,23 @@
final protected function processReceivedMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
-
- $object = $this->loadObjectFromMail($mail, $sender);
- $mail->setRelatedPHID($object->getPHID());
-
- $this->processReceivedObjectMail($mail, $object, $sender);
-
- return $this;
- }
-
- protected function processReceivedObjectMail(
- PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorLiskDAO $object,
- PhabricatorUser $sender) {
+ PhutilEmailAddress $target) {
- $handler = $this->getTransactionReplyHandler();
- if ($handler) {
- return $handler
- ->setMailReceiver($object)
- ->setActor($sender)
- ->setExcludeMailRecipientPHIDs($mail->loadAllRecipientPHIDs())
- ->processEmail($mail);
+ $parts = $this->matchObjectAddress($target);
+ if (!$parts) {
+ // We should only make it here if we matched already in "canAcceptMail()",
+ // so this is a surprise.
+ throw new Exception(
+ pht(
+ 'Failed to parse object address ("%s") during processing.',
+ (string)$target));
}
- throw new PhutilMethodNotImplementedException();
- }
-
- protected function getTransactionReplyHandler() {
- return null;
- }
-
- public function loadMailReceiverObject($pattern, PhabricatorUser $viewer) {
- return $this->loadObject($pattern, $viewer);
- }
-
- public function validateSender(
- PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
-
- parent::validateSender($mail, $sender);
-
- $parts = $this->matchObjectAddressInMail($mail);
$pattern = $parts['pattern'];
+ $sender = $this->getSender();
try {
- $object = $this->loadObjectFromMail($mail, $sender);
+ $object = $this->loadObject($pattern, $sender);
} catch (PhabricatorPolicyException $policy_exception) {
throw new PhabricatorMetaMTAReceivedMailProcessingException(
MetaMTAReceivedMailStatus::STATUS_POLICY_PROBLEM,
@@ -95,7 +66,6 @@
}
$sender_identifier = $parts['sender'];
-
if ($sender_identifier === 'public') {
if (!PhabricatorEnv::getEnvConfig('metamta.public-replies')) {
throw new PhabricatorMetaMTAReceivedMailProcessingException(
@@ -136,28 +106,50 @@
'is correct.',
$pattern));
}
+
+ $mail->setRelatedPHID($object->getPHID());
+ $this->processReceivedObjectMail($mail, $object, $sender);
+
+ return $this;
}
+ protected function processReceivedObjectMail(
+ PhabricatorMetaMTAReceivedMail $mail,
+ PhabricatorLiskDAO $object,
+ PhabricatorUser $sender) {
- final public function canAcceptMail(PhabricatorMetaMTAReceivedMail $mail) {
- if ($this->matchObjectAddressInMail($mail)) {
- return true;
+ $handler = $this->getTransactionReplyHandler();
+ if ($handler) {
+ return $handler
+ ->setMailReceiver($object)
+ ->setActor($sender)
+ ->setExcludeMailRecipientPHIDs($mail->loadAllRecipientPHIDs())
+ ->processEmail($mail);
}
- return false;
+ throw new PhutilMethodNotImplementedException();
}
- private function matchObjectAddressInMail(
- PhabricatorMetaMTAReceivedMail $mail) {
+ protected function getTransactionReplyHandler() {
+ return null;
+ }
- foreach ($mail->newTargetAddresses() as $address) {
- $parts = $this->matchObjectAddress($address);
- if ($parts) {
- return $parts;
- }
+ public function loadMailReceiverObject($pattern, PhabricatorUser $viewer) {
+ return $this->loadObject($pattern, $viewer);
+ }
+
+ final public function canAcceptMail(
+ PhabricatorMetaMTAReceivedMail $mail,
+ PhutilEmailAddress $target) {
+
+ // If we don't have a valid sender user account, we can never accept
+ // mail to any object.
+ $sender = $this->getSender();
+ if (!$sender) {
+ return false;
}
- return null;
+ return (bool)$this->matchObjectAddress($target);
}
private function matchObjectAddress(PhutilEmailAddress $address) {
@@ -188,16 +180,6 @@
return $regexp;
}
- private function loadObjectFromMail(
- PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
- $parts = $this->matchObjectAddressInMail($mail);
-
- return $this->loadObject(
- phutil_utf8_strtoupper($parts['pattern']),
- $sender);
- }
-
public static function computeMailHash($mail_key, $phid) {
$hash = PhabricatorHash::digestWithNamedKey(
$mail_key.$phid,
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmail.php b/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmail.php
--- a/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmail.php
@@ -67,6 +67,9 @@
return idx($this->configData, $key, $default);
}
+ public function getDefaultAuthorPHID() {
+ return $this->getConfigValue(self::CONFIG_DEFAULT_AUTHOR);
+ }
public function getInUseMessage() {
$applications = PhabricatorApplication::getAllApplications();
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
--- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
@@ -125,6 +125,7 @@
}
public function processReceivedMail() {
+ $viewer = $this->getViewer();
$sender = null;
try {
@@ -132,26 +133,94 @@
$this->dropMailAlreadyReceived();
$this->dropEmptyMail();
- $receiver = $this->loadReceiver();
- $sender = $receiver->loadSender($this);
- $receiver->validateSender($this, $sender);
-
- $this->setAuthorPHID($sender->getPHID());
-
- // Now that we've identified the sender, mark them as the author of
- // any attached files.
- $attachments = $this->getAttachments();
- if ($attachments) {
- $files = id(new PhabricatorFileQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withPHIDs($attachments)
- ->execute();
- foreach ($files as $file) {
- $file->setAuthorPHID($sender->getPHID())->save();
+ $sender = $this->loadSender();
+ if ($sender) {
+ $this->setAuthorPHID($sender->getPHID());
+
+ // If we've identified the sender, mark them as the author of any
+ // attached files. We do this before we validate them (below), since
+ // they still authored these files even if their account is not allowed
+ // to interact via email.
+
+ $attachments = $this->getAttachments();
+ if ($attachments) {
+ $files = id(new PhabricatorFileQuery())
+ ->setViewer($viewer)
+ ->withPHIDs($attachments)
+ ->execute();
+ foreach ($files as $file) {
+ $file->setAuthorPHID($sender->getPHID())->save();
+ }
+ }
+
+ $this->validateSender($sender);
+ }
+
+ $receivers = id(new PhutilClassMapQuery())
+ ->setAncestorClass('PhabricatorMailReceiver')
+ ->setFilterMethod('isEnabled')
+ ->execute();
+
+ $any_accepted = false;
+ $receiver_exception = null;
+
+ $targets = $this->newTargetAddresses();
+ foreach ($receivers as $receiver) {
+ $receiver = id(clone $receiver)
+ ->setViewer($viewer);
+
+ if ($sender) {
+ $receiver->setSender($sender);
+ }
+
+ foreach ($targets as $target) {
+ try {
+ if (!$receiver->canAcceptMail($this, $target)) {
+ continue;
+ }
+
+ $any_accepted = true;
+
+ $receiver->receiveMail($this, $target);
+ } catch (Exception $ex) {
+ // If receivers raise exceptions, we'll keep the first one in hope
+ // that it points at a root cause.
+ if (!$receiver_exception) {
+ $receiver_exception = $ex;
+ }
+ }
}
}
- $receiver->receiveMail($this, $sender);
+ if ($receiver_exception) {
+ throw $receiver_exception;
+ }
+
+ if (!$any_accepted) {
+ if (!$sender) {
+ // NOTE: Currently, we'll always drop this mail (since it's headed to
+ // an unverified recipient). See T12237. These details are still
+ // useful because they'll appear in the mail logs and Mail web UI.
+
+ throw new PhabricatorMetaMTAReceivedMailProcessingException(
+ MetaMTAReceivedMailStatus::STATUS_UNKNOWN_SENDER,
+ pht(
+ 'This email was sent from an unknown email address ("%s"). To '.
+ 'interact with Phabricator via email, add this address to your '.
+ 'account.',
+ (string)$this->newFromAddress()));
+ } else {
+ throw new PhabricatorMetaMTAReceivedMailProcessingException(
+ MetaMTAReceivedMailStatus::STATUS_NO_RECEIVERS,
+ pht(
+ 'Phabricator can not process this mail because no application '.
+ 'knows how to handle it. Check that the address you sent it to '.
+ 'is correct.'.
+ "\n\n".
+ '(No concrete, enabled subclass of PhabricatorMailReceiver can '.
+ 'accept this mail.)'));
+ }
+ }
} catch (PhabricatorMetaMTAReceivedMailProcessingException $ex) {
switch ($ex->getStatusCode()) {
case MetaMTAReceivedMailStatus::STATUS_DUPLICATE:
@@ -311,51 +380,6 @@
'text, and signatures are discarded and ignored.'));
}
- /**
- * Load a concrete instance of the @{class:PhabricatorMailReceiver} which
- * accepts this mail, if one exists.
- */
- private function loadReceiver() {
- $receivers = id(new PhutilClassMapQuery())
- ->setAncestorClass('PhabricatorMailReceiver')
- ->setFilterMethod('isEnabled')
- ->execute();
-
- $accept = array();
- foreach ($receivers as $key => $receiver) {
- if ($receiver->canAcceptMail($this)) {
- $accept[$key] = $receiver;
- }
- }
-
- if (!$accept) {
- throw new PhabricatorMetaMTAReceivedMailProcessingException(
- MetaMTAReceivedMailStatus::STATUS_NO_RECEIVERS,
- pht(
- 'Phabricator can not process this mail because no application '.
- 'knows how to handle it. Check that the address you sent it to is '.
- 'correct.'.
- "\n\n".
- '(No concrete, enabled subclass of PhabricatorMailReceiver can '.
- 'accept this mail.)'));
- }
-
- if (count($accept) > 1) {
- $names = implode(', ', array_keys($accept));
- throw new PhabricatorMetaMTAReceivedMailProcessingException(
- MetaMTAReceivedMailStatus::STATUS_ABUNDANT_RECEIVERS,
- pht(
- 'Phabricator is not able to process this mail because more than '.
- 'one application is willing to accept it, creating ambiguity. '.
- 'Mail needs to be accepted by exactly one receiving application.'.
- "\n\n".
- 'Accepting receivers: %s.',
- $names));
- }
-
- return head($accept);
- }
-
private function sendExceptionMail(
Exception $ex,
PhabricatorUser $viewer = null) {
@@ -434,4 +458,74 @@
));
}
+ public function newFromAddress() {
+ $raw_from = $this->getHeader('From');
+
+ if (strlen($raw_from)) {
+ return new PhutilEmailAddress($raw_from);
+ }
+
+ return null;
+ }
+
+ private function getViewer() {
+ return PhabricatorUser::getOmnipotentUser();
+ }
+
+ /**
+ * Identify 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.
+ */
+ private function loadSender() {
+ $viewer = $this->getViewer();
+
+ // Try to identify the user based on their "From" address.
+ $from_address = $this->newFromAddress();
+ if ($from_address) {
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer($viewer)
+ ->withEmails(array($from_address->getAddress()))
+ ->executeOne();
+ if ($user) {
+ return $user;
+ }
+ }
+
+ return null;
+ }
+
+ private function validateSender(PhabricatorUser $sender) {
+ $failure_reason = null;
+ if ($sender->getIsDisabled()) {
+ $failure_reason = pht(
+ 'Your account ("%s") is disabled, so you can not interact with '.
+ 'Phabricator over email.',
+ $sender->getUsername());
+ } else if ($sender->getIsStandardUser()) {
+ if (!$sender->getIsApproved()) {
+ $failure_reason = pht(
+ 'Your account ("%s") has not been approved yet. You can not '.
+ 'interact with Phabricator over email until your account is '.
+ 'approved.',
+ $sender->getUsername());
+ } else if (PhabricatorUserEmail::isEmailVerificationRequired() &&
+ !$sender->getIsEmailVerified()) {
+ $failure_reason = pht(
+ 'You have not verified the email address for your account ("%s"). '.
+ 'You must verify your email address before you can interact '.
+ 'with Phabricator over email.',
+ $sender->getUsername());
+ }
+ }
+
+ if ($failure_reason) {
+ throw new PhabricatorMetaMTAReceivedMailProcessingException(
+ MetaMTAReceivedMailStatus::STATUS_DISABLED_SENDER,
+ $failure_reason);
+ }
+ }
+
}
diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php
--- a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php
+++ b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php
@@ -48,11 +48,15 @@
}
public function testDropUnreceivableMail() {
+ $user = $this->generateNewTestUser()
+ ->save();
+
$mail = new PhabricatorMetaMTAReceivedMail();
$mail->setHeaders(
array(
'Message-ID' => 'test@example.com',
'To' => 'does+not+exist@example.com',
+ 'From' => $user->loadPrimaryEmail()->getAddress(),
));
$mail->setBodies(
array(
@@ -70,10 +74,6 @@
public function testDropUnknownSenderMail() {
$this->setManiphestCreateEmail();
- $env = PhabricatorEnv::beginScopedEnv();
- $env->overrideEnvConfig('phabricator.allow-email-users', false);
- $env->overrideEnvConfig('metamta.maniphest.default-public-author', null);
-
$mail = new PhabricatorMetaMTAReceivedMail();
$mail->setHeaders(
array(
diff --git a/src/applications/paste/mail/PasteCreateMailReceiver.php b/src/applications/paste/mail/PasteCreateMailReceiver.php
--- a/src/applications/paste/mail/PasteCreateMailReceiver.php
+++ b/src/applications/paste/mail/PasteCreateMailReceiver.php
@@ -9,7 +9,8 @@
protected function processReceivedMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
+ PhutilEmailAddress $target) {
+ $author = $this->getAuthor();
$title = $mail->getSubject();
if (!$title) {
@@ -26,18 +27,23 @@
->setTransactionType(PhabricatorPasteTitleTransaction::TRANSACTIONTYPE)
->setNewValue($title);
- $paste = PhabricatorPaste::initializeNewPaste($sender);
+ $paste = PhabricatorPaste::initializeNewPaste($author);
$content_source = $mail->newContentSource();
$editor = id(new PhabricatorPasteEditor())
- ->setActor($sender)
+ ->setActor($author)
->setContentSource($content_source)
->setContinueOnNoEffect(true);
$xactions = $editor->applyTransactions($paste, $xactions);
$mail->setRelatedPHID($paste->getPHID());
+ $sender = $this->getSender();
+ if (!$sender) {
+ return;
+ }
+
$subject_prefix =
PhabricatorEnv::getEnvConfig('metamta.paste.subject-prefix');
$subject = pht('You successfully created a paste.');
@@ -56,5 +62,4 @@
->saveAndSend();
}
-
}
diff --git a/src/applications/paste/mail/PasteMailReceiver.php b/src/applications/paste/mail/PasteMailReceiver.php
--- a/src/applications/paste/mail/PasteMailReceiver.php
+++ b/src/applications/paste/mail/PasteMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'P');
+ $id = (int)substr($pattern, 1);
return id(new PhabricatorPasteQuery())
->setViewer($viewer)
diff --git a/src/applications/phame/mail/PhamePostMailReceiver.php b/src/applications/phame/mail/PhamePostMailReceiver.php
--- a/src/applications/phame/mail/PhamePostMailReceiver.php
+++ b/src/applications/phame/mail/PhamePostMailReceiver.php
@@ -13,7 +13,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)substr($pattern, 4);
+ $id = (int)substr($pattern, 1);
return id(new PhamePostQuery())
->setViewer($viewer)
diff --git a/src/applications/pholio/mail/PholioMockMailReceiver.php b/src/applications/pholio/mail/PholioMockMailReceiver.php
--- a/src/applications/pholio/mail/PholioMockMailReceiver.php
+++ b/src/applications/pholio/mail/PholioMockMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'M');
+ $id = (int)substr($pattern, 1);
return id(new PholioMockQuery())
->setViewer($viewer)
diff --git a/src/applications/ponder/mail/PonderAnswerMailReceiver.php b/src/applications/ponder/mail/PonderAnswerMailReceiver.php
--- a/src/applications/ponder/mail/PonderAnswerMailReceiver.php
+++ b/src/applications/ponder/mail/PonderAnswerMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'ANSR');
+ $id = (int)substr($pattern, 4);
return id(new PonderAnswerQuery())
->setViewer($viewer)
diff --git a/src/applications/ponder/mail/PonderQuestionCreateMailReceiver.php b/src/applications/ponder/mail/PonderQuestionCreateMailReceiver.php
--- a/src/applications/ponder/mail/PonderQuestionCreateMailReceiver.php
+++ b/src/applications/ponder/mail/PonderQuestionCreateMailReceiver.php
@@ -9,7 +9,8 @@
protected function processReceivedMail(
PhabricatorMetaMTAReceivedMail $mail,
- PhabricatorUser $sender) {
+ PhutilEmailAddress $target) {
+ $author = $this->getAuthor();
$title = $mail->getSubject();
if (!strlen($title)) {
@@ -26,18 +27,17 @@
->setTransactionType(PonderQuestionTransaction::TYPE_CONTENT)
->setNewValue($mail->getCleanTextBody());
- $question = PonderQuestion::initializeNewQuestion($sender);
+ $question = PonderQuestion::initializeNewQuestion($author);
$content_source = $mail->newContentSource();
$editor = id(new PonderQuestionEditor())
- ->setActor($sender)
+ ->setActor($author)
->setContentSource($content_source)
->setContinueOnNoEffect(true);
$xactions = $editor->applyTransactions($question, $xactions);
$mail->setRelatedPHID($question->getPHID());
-
}
diff --git a/src/applications/ponder/mail/PonderQuestionMailReceiver.php b/src/applications/ponder/mail/PonderQuestionMailReceiver.php
--- a/src/applications/ponder/mail/PonderQuestionMailReceiver.php
+++ b/src/applications/ponder/mail/PonderQuestionMailReceiver.php
@@ -12,7 +12,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)trim($pattern, 'Q');
+ $id = (int)substr($pattern, 1);
return id(new PonderQuestionQuery())
->setViewer($viewer)
diff --git a/src/applications/slowvote/mail/PhabricatorSlowvoteMailReceiver.php b/src/applications/slowvote/mail/PhabricatorSlowvoteMailReceiver.php
--- a/src/applications/slowvote/mail/PhabricatorSlowvoteMailReceiver.php
+++ b/src/applications/slowvote/mail/PhabricatorSlowvoteMailReceiver.php
@@ -13,7 +13,7 @@
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)substr($pattern, 4);
+ $id = (int)substr($pattern, 1);
return id(new PhabricatorSlowvoteQuery())
->setViewer($viewer)

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 2, 4:39 AM (3 w, 2 d ago)
Storage Engine
amazon-s3
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
phabricator/secure/jt/yd/gp27um27och7cvjz
Default Alt Text
D19952.id47626.diff (45 KB)

Event Timeline

Phabricator · Built by Phacility