mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-20 13:52:40 +01:00
Application Emails - make various user email editing paths respect application emails
Summary: Ref T3404. The only mildly sketchy bit is these codepaths all load the application email directly, by-passing privacy. I think this is necessary because not getting to see an application doesn't mean you should be able to break the application by registering a colliding email address. Test Plan: Tried to add a registered application email to a user account via the web ui and got a pretty error. Ran unit tests. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T3404 Differential Revision: https://secure.phabricator.com/D11565
This commit is contained in:
parent
1077e7a80c
commit
7d309a8e46
5 changed files with 71 additions and 1 deletions
|
@ -62,6 +62,18 @@ final class PhabricatorAuthRegisterController
|
|||
if (!PhabricatorUserEmail::isValidAddress($default_email)) {
|
||||
$default_email = null;
|
||||
}
|
||||
if ($default_email !== null) {
|
||||
// We should bypass policy here becase e.g. limiting an application use
|
||||
// to a subset of users should not allow the others to overwrite
|
||||
// configured application emails
|
||||
$application_email = id(new PhabricatorMetaMTAApplicationEmailQuery())
|
||||
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||
->withAddresses(array($default_email))
|
||||
->executeOne();
|
||||
if ($application_email) {
|
||||
$default_email = null;
|
||||
}
|
||||
}
|
||||
|
||||
if ($default_email !== null) {
|
||||
// If the account source provided an email, but it's not allowed by
|
||||
|
@ -86,7 +98,6 @@ final class PhabricatorAuthRegisterController
|
|||
// If the account source provided an email, but another account already
|
||||
// has that email, just pretend we didn't get an email.
|
||||
|
||||
// TODO: See T3340.
|
||||
// TODO: See T3472.
|
||||
|
||||
if ($default_email !== null) {
|
||||
|
|
|
@ -61,6 +61,27 @@ final class PhabricatorMetaMTAApplicationEmail
|
|||
return idx($this->configData, $key, $default);
|
||||
}
|
||||
|
||||
|
||||
public function getInUseMessage() {
|
||||
$applications = PhabricatorApplication::getAllApplications();
|
||||
$applications = mpull($applications, null, 'getPHID');
|
||||
$application = idx(
|
||||
$applications,
|
||||
$this->getApplicationPHID());
|
||||
if ($application) {
|
||||
$message = pht(
|
||||
'The address %s is configured to be used by the %s Application.',
|
||||
$this->getAddress(),
|
||||
$application->getName());
|
||||
} else {
|
||||
$message = pht(
|
||||
'The address %s is configured to be used by an application.',
|
||||
$this->getAddress());
|
||||
}
|
||||
|
||||
return $message;
|
||||
}
|
||||
|
||||
/* -( PhabricatorPolicyInterface )----------------------------------------- */
|
||||
|
||||
|
||||
|
|
|
@ -578,6 +578,14 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
|
|||
if (!PhabricatorUserEmail::isAllowedAddress($email->getAddress())) {
|
||||
throw new Exception(PhabricatorUserEmail::describeAllowedAddresses());
|
||||
}
|
||||
|
||||
$application_email = id(new PhabricatorMetaMTAApplicationEmailQuery())
|
||||
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||
->withAddresses(array($email->getAddress()))
|
||||
->executeOne();
|
||||
if ($application_email) {
|
||||
throw new Exception($application_email->getInUseMessage());
|
||||
}
|
||||
}
|
||||
|
||||
private function revokePasswordResetLinks(PhabricatorUser $user) {
|
||||
|
|
|
@ -53,6 +53,26 @@ final class PhabricatorUserEditorTestCase extends PhabricatorTestCase {
|
|||
$this->assertTrue($caught instanceof Exception);
|
||||
}
|
||||
|
||||
public function testRegistrationEmailApplicationEmailCollide() {
|
||||
$app_email = 'bugs@whitehouse.gov';
|
||||
$app_email_object =
|
||||
PhabricatorMetaMTAApplicationEmail::initializeNewAppEmail(
|
||||
$this->generateNewTestUser());
|
||||
$app_email_object->setAddress($app_email);
|
||||
$app_email_object->setApplicationPHID('test');
|
||||
$app_email_object->save();
|
||||
|
||||
$caught = null;
|
||||
try {
|
||||
$this->registerUser(
|
||||
'PhabricatorUserEditorTestCaseDomain',
|
||||
$app_email);
|
||||
} catch (Exception $ex) {
|
||||
$caught = $ex;
|
||||
}
|
||||
$this->assertTrue($caught instanceof Exception);
|
||||
}
|
||||
|
||||
private function registerUser($username, $email) {
|
||||
$user = id(new PhabricatorUser())
|
||||
->setUsername($username)
|
||||
|
|
|
@ -185,6 +185,16 @@ final class PhabricatorEmailAddressesSettingsPanel
|
|||
$e_email = pht('Disallowed');
|
||||
$errors[] = PhabricatorUserEmail::describeAllowedAddresses();
|
||||
}
|
||||
if ($e_email === true) {
|
||||
$application_email = id(new PhabricatorMetaMTAApplicationEmailQuery())
|
||||
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||
->withAddresses(array($email))
|
||||
->executeOne();
|
||||
if ($application_email) {
|
||||
$e_email = pht('In Use');
|
||||
$errors[] = $application_email->getInUseMessage();
|
||||
}
|
||||
}
|
||||
|
||||
if (!$errors) {
|
||||
$object = id(new PhabricatorUserEmail())
|
||||
|
|
Loading…
Reference in a new issue