1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 20:10:55 +01:00
phorge-phorge/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php

158 lines
4.3 KiB
PHP
Raw Normal View History

Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
<?php
final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
private $phids = array();
private $viewer;
public function setViewer(PhabricatorUser $viewer) {
$this->viewer = $viewer;
return $this;
}
public function getViewer() {
return $this->viewer;
}
public function withPHIDs(array $phids) {
$this->phids = $phids;
return $this;
}
public function execute() {
$phids = array_fuse($this->phids);
$actors = array();
$type_map = array();
foreach ($phids as $phid) {
$type_map[phid_get_type($phid)][] = $phid;
$actors[$phid] = id(new PhabricatorMetaMTAActor())->setPHID($phid);
}
// TODO: Move this to PhabricatorPHIDType, or the objects, or some
// interface.
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
foreach ($type_map as $type => $phids) {
switch ($type) {
case PhabricatorPeopleUserPHIDType::TYPECONST:
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
$this->loadUserActors($actors, $phids);
break;
case PhabricatorPeopleExternalPHIDType::TYPECONST:
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
$this->loadExternalUserActors($actors, $phids);
break;
default:
$this->loadUnknownActors($actors, $phids);
break;
}
}
return $actors;
}
private function loadUserActors(array $actors, array $phids) {
assert_instances_of($actors, 'PhabricatorMetaMTAActor');
$emails = id(new PhabricatorUserEmail())->loadAllWhere(
'userPHID IN (%Ls) AND isPrimary = 1',
$phids);
$emails = mpull($emails, null, 'getUserPHID');
$users = id(new PhabricatorPeopleQuery())
->setViewer($this->getViewer())
->withPHIDs($phids)
->execute();
$users = mpull($users, null, 'getPHID');
foreach ($phids as $phid) {
$actor = $actors[$phid];
$user = idx($users, $phid);
if (!$user) {
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNLOADABLE);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
} else {
$actor->setName($this->getUserName($user));
if ($user->getIsDisabled()) {
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_DISABLED);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
}
if ($user->getIsSystemAgent()) {
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_BOT);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
}
Improve handling of email verification and "activated" accounts Summary: Small step forward which improves existing stuff or lays groudwork for future stuff: - Currently, to check for email verification, we have to single-query the email address on every page. Instead, denoramlize it into the user object. - Migrate all the existing users. - When the user verifies an email, mark them as `isEmailVerified` if the email is their primary email. - Just make the checks look at the `isEmailVerified` field. - Add a new check, `isUserActivated()`, to cover email-verified plus disabled. Currently, a non-verified-but-not-disabled user could theoretically use Conduit over SSH, if anyone deployed it. Tighten that up. - Add an `isApproved` flag, which is always true for now. In a future diff, I want to add a default-on admin approval queue for new accounts, to prevent configuration mistakes. The way it will work is: - When the queue is enabled, registering users are created with `isApproved = false`. - Admins are sent an email, "[Phabricator] New User Approval (alincoln)", telling them that a new user is waiting for approval. - They go to the web UI and approve the user. - Manually-created accounts are auto-approved. - The email will have instructions for disabling the queue. I think this queue will be helpful for new installs and give them peace of mind, and when you go to disable it we have a better opportunity to warn you about exactly what that means. Generally, I want to improve the default safety of registration, since if you just blindly coast through the path of least resistance right now your install ends up pretty open, and realistically few installs are on VPNs. Test Plan: - Ran migration, verified `isEmailVerified` populated correctly. - Created a new user, checked DB for verified (not verified). - Verified, checked DB (now verified). - Used Conduit, People, Diffusion. Reviewers: btrahan Reviewed By: btrahan CC: chad, aran Differential Revision: https://secure.phabricator.com/D7572
2013-11-12 23:37:04 +01:00
// NOTE: We do send email to unapproved users, and to unverified users,
// because it would otherwise be impossible to get them to verify their
// email addresses. Possibly we should white-list this kind of mail and
// deny all other types of mail.
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
}
$email = idx($emails, $phid);
if (!$email) {
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_NO_ADDRESS);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
} else {
$actor->setEmailAddress($email->getAddress());
}
}
}
private function loadExternalUserActors(array $actors, array $phids) {
assert_instances_of($actors, 'PhabricatorMetaMTAActor');
$xusers = id(new PhabricatorExternalAccountQuery())
->setViewer($this->getViewer())
->withPHIDs($phids)
->execute();
$xusers = mpull($xusers, null, 'getPHID');
foreach ($phids as $phid) {
$actor = $actors[$phid];
$xuser = idx($xusers, $phid);
if (!$xuser) {
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNLOADABLE);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
continue;
}
$actor->setName($xuser->getDisplayName());
if ($xuser->getAccountType() != 'email') {
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_EXTERNAL_TYPE);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
continue;
}
$actor->setEmailAddress($xuser->getAccountID());
}
}
private function loadUnknownActors(array $actors, array $phids) {
foreach ($phids as $phid) {
$actor = $actors[$phid];
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNMAILABLE);
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
}
}
/**
* Small helper function to make sure we format the username properly as
* specified by the `metamta.user-address-format` configuration value.
*/
private function getUserName(PhabricatorUser $user) {
$format = PhabricatorEnv::getEnvConfig('metamta.user-address-format');
switch ($format) {
case 'short':
$name = $user->getUserName();
break;
case 'real':
$name = strlen($user->getRealName()) ?
$user->getRealName() : $user->getUserName();
Show why recipients were excluded from mail Summary: Ref T3306. This interface has a hard time balancing security/policy issues and I'm not sure what the best way forward is. Some possibilities: # We just let you see everything from the web UI. - This makes debugging easier. - Anyone who can see this stuff can trivially take over any user's account with five seconds of work and no technical expertise (reset their password from the web UI, then go read the email and click the link). # We let you see everything, but only for messages you were a recipient of or author of. - This makes it much more difficult to debug issues with mailing lists. - But maybe we could just say mailing list recipients are "public", or define some other ruleset. - Generally this gets privacy and ease of use right. # We could move the whole thing to the CLI. - Makes the UI/UX way worse. # We could strike an awkward balance between concerns, as we do now. - We expose //who// sent and received messages, but not the content of the messages. This doesn't feel great. I'm inclined to probably go with (2) and figure something out for mailing lists? Anyway, irrespective of that this should generally make things more clear, and improves the code a lot if nothing else. Test Plan: {F49546} - Looked at a bunch of mail. - Sent mail from different apps. - Checked that recipients seem correct. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T3306 Differential Revision: https://secure.phabricator.com/D6413
2013-07-11 00:17:38 +02:00
break;
case 'full':
default:
$name = $user->getFullName();
break;
}
return $name;
}
}