Skip to content

Commit cd73fe7

Browse files
author
epriestley
committed
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
1 parent 30a51da commit cd73fe7

File tree

1 file changed

+17
-3
lines changed

1 file changed

+17
-3
lines changed

src/applications/auth/controller/PhabricatorAuthRegisterController.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,30 @@ public function processRequest() {
5959
$default_realname = $account->getRealName();
6060
$default_email = $account->getEmail();
6161
if ($default_email) {
62-
// If the account source provided an email but it's not allowed by
63-
// the configuration, just pretend we didn't get an email at all.
62+
// If the account source provided an email, but it's not allowed by
63+
// the configuration, roadblock the user. Previously, we let the user
64+
// pick a valid email address instead, but this does not align well with
65+
// user expectation and it's not clear the cases it enables are valuable.
66+
// See discussion in T3472.
6467
if (!PhabricatorUserEmail::isAllowedAddress($default_email)) {
65-
$default_email = null;
68+
return $this->renderError(
69+
array(
70+
pht(
71+
'The account you are attempting to register with has an invalid '.
72+
'email address (%s). This Phabricator install only allows '.
73+
'registration with specific email addresses:',
74+
$default_email),
75+
phutil_tag('br'),
76+
phutil_tag('br'),
77+
PhabricatorUserEmail::describeAllowedAddresses(),
78+
));
6679
}
6780

6881
// If the account source provided an email, but another account already
6982
// has that email, just pretend we didn't get an email.
7083

7184
// TODO: See T3340.
85+
// TODO: See T3472.
7286

7387
if ($default_email) {
7488
$same_email = id(new PhabricatorUserEmail())->loadOneWhere(

0 commit comments

Comments
 (0)