From cd73fe78db364983c7bfc6080a89a09a2665b51e Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 12 Nov 2013 14:36:49 -0800 Subject: [PATCH] Roadblock users trying to register with external accounts that have invalid emails Summary: Ref T3472. Currently, if an install only allows "@mycompany.com" emails and you try to register with an "@personal.com" account, we let you pick an "@mycompany.com" address instead. This is secure: you still have to verify the email. However, it defies user expectation -- it's somewhat confusing that we let you register. Instead, provide a hard roadblock. (These accounts can still be linked, just not used for registration.) Test Plan: See screenshot. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3472 Differential Revision: https://secure.phabricator.com/D7571 --- .../PhabricatorAuthRegisterController.php | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php index fe4ebd7b95..a5530c4604 100644 --- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php +++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php @@ -59,16 +59,30 @@ final class PhabricatorAuthRegisterController $default_realname = $account->getRealName(); $default_email = $account->getEmail(); if ($default_email) { - // If the account source provided an email but it's not allowed by - // the configuration, just pretend we didn't get an email at all. + // If the account source provided an email, but it's not allowed by + // the configuration, roadblock the user. Previously, we let the user + // pick a valid email address instead, but this does not align well with + // user expectation and it's not clear the cases it enables are valuable. + // See discussion in T3472. if (!PhabricatorUserEmail::isAllowedAddress($default_email)) { - $default_email = null; + return $this->renderError( + array( + pht( + 'The account you are attempting to register with has an invalid '. + 'email address (%s). This Phabricator install only allows '. + 'registration with specific email addresses:', + $default_email), + phutil_tag('br'), + phutil_tag('br'), + PhabricatorUserEmail::describeAllowedAddresses(), + )); } // 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) { $same_email = id(new PhabricatorUserEmail())->loadOneWhere(