From 87207b2f4edb03ce258afaae9f7b84f0308e0adf Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 May 2012 10:29:33 -0700 Subject: [PATCH] Allow users to have multiple email addresses, and verify emails Summary: - Move email to a separate table. - Migrate existing email to new storage. - Allow users to add and remove email addresses. - Allow users to verify email addresses. - Allow users to change their primary email address. - Convert all the registration/reset/login code to understand these changes. - There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific. - This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up. Not included here (next steps): - Allow configuration to restrict email to certain domains. - Allow configuration to require validated email. Test Plan: This is a fairly extensive, difficult-to-test change. - From "Email Addresses" interface: - Added new email (verified email verifications sent). - Changed primary email (verified old/new notificactions sent). - Resent verification emails (verified they sent). - Removed email. - Tried to add already-owned email. - Created new users with "accountadmin". Edited existing users with "accountadmin". - Created new users with "add_user.php". - Created new users with web interface. - Clicked welcome email link, verified it verified email. - Reset password. - Linked/unlinked oauth accounts. - Logged in with oauth account. - Logged in with email. - Registered with Oauth account. - Tried to register with OAuth account with duplicate email. - Verified errors for email verification with bad tokens, etc. Reviewers: btrahan, vrana, jungejason Reviewed By: btrahan CC: aran Maniphest Tasks: T1184 Differential Revision: https://secure.phabricator.com/D2393 --- resources/sql/patches/emailtable.sql | 12 + resources/sql/patches/emailtableport.php | 49 +++ resources/sql/patches/emailtableremove.sql | 1 + scripts/user/account_admin.php | 61 ++-- scripts/user/add_user.php | 14 +- src/__phutil_library_map__.php | 4 + ...AphrontDefaultApplicationConfiguration.php | 3 + .../redirect/AphrontRedirectResponse.php | 2 +- .../email/PhabricatorEmailLoginController.php | 13 +- .../auth/controller/email/__init__.php | 1 + .../PhabricatorEmailTokenController.php | 43 ++- .../auth/controller/emailtoken/__init__.php | 1 + .../login/PhabricatorLoginController.php | 4 +- .../oauth/PhabricatorOAuthLoginController.php | 4 +- .../auth/controller/oauth/__init__.php | 1 + ...atorOAuthDefaultRegistrationController.php | 32 +- .../oauthregistration/default/__init__.php | 1 + .../user/base/ConduitAPI_user_Method.php | 1 - .../base/DifferentialFieldSpecification.php | 3 +- .../storage/mail/PhabricatorMetaMTAMail.php | 10 +- .../PhabricatorMetaMTAReceivedMail.php | 8 +- .../edit/PhabricatorPeopleEditController.php | 42 ++- .../people/controller/edit/__init__.php | 1 + ...PhabricatorEmailVerificationController.php | 81 +++++ .../controller/emailverification/__init__.php | 18 + .../PhabricatorUserSettingsController.php | 2 +- ...icatorUserEmailSettingsPanelController.php | 335 +++++++++++++++--- .../settings/panels/email/__init__.php | 13 +- ...torUserPasswordSettingsPanelController.php | 12 +- .../settings/panels/password/__init__.php | 1 + .../storage/email/PhabricatorUserEmail.php | 160 +++++++++ .../people/storage/email/__init__.php | 17 + .../people/storage/user/PhabricatorUser.php | 61 +++- .../people/storage/user/__init__.php | 1 + .../data/PhabricatorObjectHandleData.php | 9 +- .../phid/handle/data/__init__.php | 1 + ...torRepositoryCommitMessageDetailParser.php | 6 +- .../PhabricatorBuiltinPatchList.php | 12 + 38 files changed, 900 insertions(+), 140 deletions(-) create mode 100644 resources/sql/patches/emailtable.sql create mode 100644 resources/sql/patches/emailtableport.php create mode 100644 resources/sql/patches/emailtableremove.sql create mode 100644 src/applications/people/controller/emailverification/PhabricatorEmailVerificationController.php create mode 100644 src/applications/people/controller/emailverification/__init__.php create mode 100644 src/applications/people/storage/email/PhabricatorUserEmail.php create mode 100644 src/applications/people/storage/email/__init__.php diff --git a/resources/sql/patches/emailtable.sql b/resources/sql/patches/emailtable.sql new file mode 100644 index 0000000000..b6599375a6 --- /dev/null +++ b/resources/sql/patches/emailtable.sql @@ -0,0 +1,12 @@ +CREATE TABLE {$NAMESPACE}_user.user_email ( + `id` int unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY, + userPHID varchar(64) collate utf8_bin NOT NULL, + address varchar(128) collate utf8_general_ci NOT NULL, + isVerified bool not null default 0, + isPrimary bool not null default 0, + verificationCode varchar(64) collate utf8_bin, + dateCreated int unsigned not null, + dateModified int unsigned not null, + KEY (userPHID, isPrimary), + UNIQUE KEY (address) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/resources/sql/patches/emailtableport.php b/resources/sql/patches/emailtableport.php new file mode 100644 index 0000000000..9c4a59ee90 --- /dev/null +++ b/resources/sql/patches/emailtableport.php @@ -0,0 +1,49 @@ +establishConnection('r'); + +$emails = queryfx_all( + $conn, + 'SELECT phid, email FROM %T', + $table->getTableName()); +$emails = ipull($emails, 'email', 'phid'); + +$etable = new PhabricatorUserEmail(); +$econn = $etable->establishConnection('w'); + +foreach ($emails as $phid => $email) { + + // NOTE: Grandfather all existing email in as primary / verified. We generate + // verification codes because they are used for password resets, etc. + + echo "Migrating '{$phid}'...\n"; + queryfx( + $econn, + 'INSERT INTO %T (userPHID, address, verificationCode, isVerified, isPrimary) + VALUES (%s, %s, %s, 1, 1)', + $etable->getTableName(), + $phid, + $email, + PhabricatorFile::readRandomCharacters(24)); +} + +echo "Done.\n"; diff --git a/resources/sql/patches/emailtableremove.sql b/resources/sql/patches/emailtableremove.sql new file mode 100644 index 0000000000..be31125fec --- /dev/null +++ b/resources/sql/patches/emailtableremove.sql @@ -0,0 +1 @@ +ALTER TABLE {$NAMESPACE}_user.user DROP email; \ No newline at end of file diff --git a/scripts/user/account_admin.php b/scripts/user/account_admin.php index 220ed68c1c..2570f0a3ae 100755 --- a/scripts/user/account_admin.php +++ b/scripts/user/account_admin.php @@ -55,6 +55,8 @@ if (!$user) { } $user = new PhabricatorUser(); $user->setUsername($username); + + $is_new = true; } else { $original = clone $user; @@ -66,6 +68,8 @@ if (!$user) { echo "Cancelled.\n"; exit(1); } + + $is_new = false; } $user_realname = $user->getRealName(); @@ -79,30 +83,28 @@ $realname = nonempty( $user_realname); $user->setRealName($realname); -$user_email = $user->getEmail(); -if (strlen($user_email)) { - $email_prompt = ' ['.$user_email.']'; -} else { - $email_prompt = ''; -} +// When creating a new user we prompt for an email address; when editing an +// existing user we just skip this because it would be quite involved to provide +// a reasonable CLI interface for editing multiple addresses and managing email +// verification and primary addresses. -do { - $email = nonempty( - phutil_console_prompt("Enter user email address{$email_prompt}:"), - $user_email); - $duplicate = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $email); - if ($duplicate && $duplicate->getID() != $user->getID()) { - $duplicate_username = $duplicate->getUsername(); - echo "ERROR: There is already a user with that email address ". - "({$duplicate_username}). Each user must have a unique email ". - "address.\n"; - } else { - break; - } -} while (true); -$user->setEmail($email); +$new_email = null; +if ($is_new) { + do { + $email = phutil_console_prompt("Enter user email address:"); + $duplicate = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', + $email); + if ($duplicate) { + echo "ERROR: There is already a user with that email address. ". + "Each user must have a unique email address.\n"; + } else { + break; + } + } while (true); + + $new_email = $email; +} $changed_pass = false; // This disables local echo, so the user's password is not shown as they type @@ -126,7 +128,9 @@ $tpl = "%12s %-30s %-30s\n"; printf($tpl, null, 'OLD VALUE', 'NEW VALUE'); printf($tpl, 'Username', $original->getUsername(), $user->getUsername()); printf($tpl, 'Real Name', $original->getRealName(), $user->getRealName()); -printf($tpl, 'Email', $original->getEmail(), $user->getEmail()); +if ($new_email) { + printf($tpl, 'Email', '', $new_email); +} printf($tpl, 'Password', null, ($changed_pass !== false) ? 'Updated' @@ -153,4 +157,13 @@ if ($changed_pass !== false) { $user->save(); } +if ($new_email) { + id(new PhabricatorUserEmail()) + ->setUserPHID($user->getPHID()) + ->setAddress($new_email) + ->setIsVerified(1) + ->setIsPrimary(1) + ->save(); +} + echo "Saved changes.\n"; diff --git a/scripts/user/add_user.php b/scripts/user/add_user.php index af00b6b6b4..2ab010e29c 100755 --- a/scripts/user/add_user.php +++ b/scripts/user/add_user.php @@ -50,20 +50,26 @@ if ($existing_user) { "There is already a user with the username '{$username}'!"); } -$existing_user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', +$existing_email = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', $email); -if ($existing_user) { +if ($existing_email) { throw new Exception( "There is already a user with the email '{$email}'!"); } $user = new PhabricatorUser(); $user->setUsername($username); -$user->setEmail($email); $user->setRealname($realname); $user->save(); +$email_object = id(new PhabricatorUserEmail()) + ->setUserPHID($user->getPHID()) + ->setAddress($email) + ->setIsVerified(1) + ->setIsPrimary(1) + ->save(); + $user->sendWelcomeEmail($admin); echo "Created user '{$username}' (realname='{$realname}', email='{$email}').\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ceb2221168..33246c7a81 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -595,6 +595,7 @@ phutil_register_library_map(array( 'PhabricatorEdgeQuery' => 'infrastructure/edges/query/edge', 'PhabricatorEmailLoginController' => 'applications/auth/controller/email', 'PhabricatorEmailTokenController' => 'applications/auth/controller/emailtoken', + 'PhabricatorEmailVerificationController' => 'applications/people/controller/emailverification', 'PhabricatorEnv' => 'infrastructure/env', 'PhabricatorEnvTestCase' => 'infrastructure/env/__tests__', 'PhabricatorErrorExample' => 'applications/uiexample/examples/error', @@ -946,6 +947,7 @@ phutil_register_library_map(array( 'PhabricatorUserAccountSettingsPanelController' => 'applications/people/controller/settings/panels/account', 'PhabricatorUserConduitSettingsPanelController' => 'applications/people/controller/settings/panels/conduit', 'PhabricatorUserDAO' => 'applications/people/storage/base', + 'PhabricatorUserEmail' => 'applications/people/storage/email', 'PhabricatorUserEmailPreferenceSettingsPanelController' => 'applications/people/controller/settings/panels/emailpref', 'PhabricatorUserEmailSettingsPanelController' => 'applications/people/controller/settings/panels/email', 'PhabricatorUserLog' => 'applications/people/storage/log', @@ -1534,6 +1536,7 @@ phutil_register_library_map(array( 'PhabricatorEdgeQuery' => 'PhabricatorQuery', 'PhabricatorEmailLoginController' => 'PhabricatorAuthController', 'PhabricatorEmailTokenController' => 'PhabricatorAuthController', + 'PhabricatorEmailVerificationController' => 'PhabricatorPeopleController', 'PhabricatorEnvTestCase' => 'PhabricatorTestCase', 'PhabricatorErrorExample' => 'PhabricatorUIExample', 'PhabricatorEvent' => 'PhutilEvent', @@ -1819,6 +1822,7 @@ phutil_register_library_map(array( 'PhabricatorUserAccountSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserConduitSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserDAO' => 'PhabricatorLiskDAO', + 'PhabricatorUserEmail' => 'PhabricatorUserDAO', 'PhabricatorUserEmailPreferenceSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserEmailSettingsPanelController' => 'PhabricatorUserSettingsPanelController', 'PhabricatorUserLog' => 'PhabricatorUserDAO', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index ee6b5ca26b..417d62d265 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -433,6 +433,9 @@ class AphrontDefaultApplicationConfiguration 'testpaymentform/' => 'PhortuneStripeTestPaymentFormController', ), ), + + '/emailverify/(?P[^/]+)/' => + 'PhabricatorEmailVerificationController', ); } diff --git a/src/aphront/response/redirect/AphrontRedirectResponse.php b/src/aphront/response/redirect/AphrontRedirectResponse.php index d51631125c..f49681436b 100644 --- a/src/aphront/response/redirect/AphrontRedirectResponse.php +++ b/src/aphront/response/redirect/AphrontRedirectResponse.php @@ -31,7 +31,7 @@ class AphrontRedirectResponse extends AphrontResponse { } public function getURI() { - return $this->uri; + return (string)$this->uri; } public function getHeaders() { diff --git a/src/applications/auth/controller/email/PhabricatorEmailLoginController.php b/src/applications/auth/controller/email/PhabricatorEmailLoginController.php index 314e0b2597..af4c0aa212 100644 --- a/src/applications/auth/controller/email/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/email/PhabricatorEmailLoginController.php @@ -57,17 +57,24 @@ final class PhabricatorEmailLoginController // it expensive to fish for valid email addresses while giving the user // a better error if they goof their email. - $target_user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', + $target_email = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', $email); + $target_user = null; + if ($target_email) { + $target_user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $target_email->getUserPHID()); + } + if (!$target_user) { $errors[] = "There is no account associated with that email address."; $e_email = "Invalid"; } if (!$errors) { - $uri = $target_user->getEmailLoginURI(); + $uri = $target_user->getEmailLoginURI($target_email); if ($is_serious) { $body = <<token; $email = $request->getStr('email'); - $target_user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', + // NOTE: We need to bind verification to **addresses**, not **users**, + // because we verify addresses when they're used to login this way, and if + // we have a user-based verification you can: + // + // - Add some address you do not own; + // - request a password reset; + // - change the URI in the email to the address you don't own; + // - login via the email link; and + // - get a "verified" address you don't control. + + $target_email = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', $email); - if (!$target_user || !$target_user->validateEmailToken($token)) { + $target_user = null; + if ($target_email) { + $target_user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $target_email->getUserPHID()); + } + + if (!$target_email || + !$target_user || + !$target_user->validateEmailToken($target_email, $token)) { + $view = new AphrontRequestFailureView(); $view->setHeader('Unable to Login'); $view->appendChild( @@ -71,19 +91,32 @@ final class PhabricatorEmailTokenController '
'. 'Send Another Email'. '
'); + return $this->buildStandardPageResponse( $view, array( - 'title' => 'Email Sent', + 'title' => 'Login Failure', )); } + // Verify email so that clicking the link in the "Welcome" email is good + // enough, without requiring users to go through a second round of email + // verification. + + $target_email->setIsVerified(1); + $target_email->save(); + $session_key = $target_user->establishSession('web'); $request->setCookie('phusr', $target_user->getUsername()); $request->setCookie('phsid', $session_key); if (PhabricatorEnv::getEnvConfig('account.editable')) { - $next = '/settings/page/password/?token='.$token; + $next = (string)id(new PhutilURI('/settings/page/password/')) + ->setQueryParams( + array( + 'token' => $token, + 'email' => $email, + )); } else { $next = '/'; } diff --git a/src/applications/auth/controller/emailtoken/__init__.php b/src/applications/auth/controller/emailtoken/__init__.php index b8b5598007..3f25468b53 100644 --- a/src/applications/auth/controller/emailtoken/__init__.php +++ b/src/applications/auth/controller/emailtoken/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'aphront/response/400'); phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/auth/controller/base'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/page/failure'); diff --git a/src/applications/auth/controller/login/PhabricatorLoginController.php b/src/applications/auth/controller/login/PhabricatorLoginController.php index e62925b644..5e7df9ee59 100644 --- a/src/applications/auth/controller/login/PhabricatorLoginController.php +++ b/src/applications/auth/controller/login/PhabricatorLoginController.php @@ -113,9 +113,7 @@ final class PhabricatorLoginController $username_or_email); if (!$user) { - $user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $username_or_email); + $user = PhabricatorUser::loadOneWithEmailAddress($username_or_email); } if (!$errors) { diff --git a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php index 8c07c25bed..adb21f782b 100644 --- a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php +++ b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php @@ -176,8 +176,8 @@ final class PhabricatorOAuthLoginController $oauth_email = $provider->retrieveUserEmail(); if ($oauth_email) { - $known_email = id(new PhabricatorUser()) - ->loadOneWhere('email = %s', $oauth_email); + $known_email = id(new PhabricatorUserEmail()) + ->loadOneWhere('address = %s', $oauth_email); if ($known_email) { $dialog = new AphrontDialogView(); $dialog->setUser($current_user); diff --git a/src/applications/auth/controller/oauth/__init__.php b/src/applications/auth/controller/oauth/__init__.php index 356e746074..130183baef 100644 --- a/src/applications/auth/controller/oauth/__init__.php +++ b/src/applications/auth/controller/oauth/__init__.php @@ -13,6 +13,7 @@ phutil_require_module('phabricator', 'aphront/writeguard'); phutil_require_module('phabricator', 'applications/auth/controller/base'); phutil_require_module('phabricator', 'applications/auth/oauth/provider/base'); phutil_require_module('phabricator', 'applications/auth/view/oauthfailure'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'applications/people/storage/useroauthinfo'); phutil_require_module('phabricator', 'infrastructure/env'); diff --git a/src/applications/auth/controller/oauthregistration/default/PhabricatorOAuthDefaultRegistrationController.php b/src/applications/auth/controller/oauthregistration/default/PhabricatorOAuthDefaultRegistrationController.php index 63af461081..6d88f989d8 100644 --- a/src/applications/auth/controller/oauthregistration/default/PhabricatorOAuthDefaultRegistrationController.php +++ b/src/applications/auth/controller/oauthregistration/default/PhabricatorOAuthDefaultRegistrationController.php @@ -33,7 +33,8 @@ final class PhabricatorOAuthDefaultRegistrationController $user->setUsername($provider->retrieveUserAccountName()); $user->setRealName($provider->retrieveUserRealName()); - $user->setEmail($provider->retrieveUserEmail()); + + $new_email = $provider->retrieveUserEmail(); if ($request->isFormPost()) { @@ -49,9 +50,9 @@ final class PhabricatorOAuthDefaultRegistrationController $e_username = null; } - if ($user->getEmail() === null) { - $user->setEmail($request->getStr('email')); - if (!strlen($user->getEmail())) { + if (!$new_email) { + $new_email = trim($request->getStr('email')); + if (!$new_email) { $e_email = 'Required'; $errors[] = 'Email is required.'; } else { @@ -84,12 +85,29 @@ final class PhabricatorOAuthDefaultRegistrationController try { $user->save(); + // NOTE: We don't verify OAuth email addresses by default because + // OAuth providers might associate email addresses with accounts that + // haven't actually verified they own them. We could selectively + // auto-verify some providers that we trust here, but the stakes for + // verifying an email address are high because having a corporate + // address at a company is sometimes the key to the castle. + + $new_email = id(new PhabricatorUserEmail()) + ->setUserPHID($user->getPHID()) + ->setAddress($new_email) + ->setIsPrimary(1) + ->setIsVerified(0) + ->save(); + $oauth_info->setUserID($user->getID()); $oauth_info->save(); $session_key = $user->establishSession('web'); $request->setCookie('phusr', $user->getUsername()); $request->setCookie('phsid', $session_key); + + $new_email->sendVerificationEmail($user); + return id(new AphrontRedirectResponse())->setURI('/'); } catch (AphrontQueryDuplicateKeyException $exception) { @@ -97,9 +115,9 @@ final class PhabricatorOAuthDefaultRegistrationController 'userName = %s', $user->getUserName()); - $same_email = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $user->getEmail()); + $same_email = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', + $new_email); if ($same_username) { $e_username = 'Duplicate'; diff --git a/src/applications/auth/controller/oauthregistration/default/__init__.php b/src/applications/auth/controller/oauthregistration/default/__init__.php index 441cdbc3d4..5062b237bb 100644 --- a/src/applications/auth/controller/oauthregistration/default/__init__.php +++ b/src/applications/auth/controller/oauthregistration/default/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/auth/controller/oauthregistration/base'); phutil_require_module('phabricator', 'applications/files/storage/file'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/submit'); diff --git a/src/applications/conduit/method/user/base/ConduitAPI_user_Method.php b/src/applications/conduit/method/user/base/ConduitAPI_user_Method.php index c74029b021..b126358713 100644 --- a/src/applications/conduit/method/user/base/ConduitAPI_user_Method.php +++ b/src/applications/conduit/method/user/base/ConduitAPI_user_Method.php @@ -26,7 +26,6 @@ abstract class ConduitAPI_user_Method extends ConduitAPIMethod { 'phid' => $user->getPHID(), 'userName' => $user->getUserName(), 'realName' => $user->getRealName(), - 'email' => $user->getEmail(), 'image' => $user->loadProfileImageURI(), 'uri' => PhabricatorEnv::getURI('/p/'.$user->getUsername().'/'), ); diff --git a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php index 9d47c9c57d..642d2b9bcb 100644 --- a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php @@ -672,8 +672,7 @@ abstract class DifferentialFieldSpecification { $object_map = array(); $users = id(new PhabricatorUser())->loadAllWhere( - '(username IN (%Ls)) OR (email IN (%Ls))', - $value, + '(username IN (%Ls))', $value); $user_map = mpull($users, 'getPHID', 'getUsername'); diff --git a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php index 8af6df2fde..5c67c3cf3f 100644 --- a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php @@ -108,6 +108,11 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return $this; } + public function addRawTos(array $raw_email) { + $this->setParam('raw-to', $raw_email); + return $this; + } + public function addCCs(array $phids) { $phids = array_unique($phids); $this->setParam('cc', $phids); @@ -367,9 +372,12 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $handles, $exclude); if ($emails) { - $add_to = $emails; + $add_to = array_merge($add_to, $emails); } break; + case 'raw-to': + $add_to = array_merge($add_to, $value); + break; case 'cc': $emails = $this->getDeliverableEmailsFromHandles( $value, diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index 916269c369..107c91b953 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -274,9 +274,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { $from = idx($this->headers, 'from'); $from = $this->getRawEmailAddress($from); - $user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $from); + $user = PhabricatorUser::loadOneWithEmailAddress($from); // If Phabricator is configured to allow "Reply-To" authentication, try // the "Reply-To" address if we failed to match the "From" address. @@ -287,9 +285,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { $reply_to = idx($this->headers, 'reply-to'); $reply_to = $this->getRawEmailAddress($reply_to); if ($reply_to) { - $user = id(new PhabricatorUser())->loadOneWhere( - 'email = %s', - $reply_to); + $user = PhabricatorUser::loadOneWithEmailAddress($reply_to); } } diff --git a/src/applications/people/controller/edit/PhabricatorPeopleEditController.php b/src/applications/people/controller/edit/PhabricatorPeopleEditController.php index 50de1f7b81..02829864dd 100644 --- a/src/applications/people/controller/edit/PhabricatorPeopleEditController.php +++ b/src/applications/people/controller/edit/PhabricatorPeopleEditController.php @@ -123,13 +123,20 @@ final class PhabricatorPeopleEditController $welcome_checked = true; + $new_email = null; + $request = $this->getRequest(); if ($request->isFormPost()) { $welcome_checked = $request->getInt('welcome'); if (!$user->getID()) { $user->setUsername($request->getStr('username')); - $user->setEmail($request->getStr('email')); + + $new_email = $request->getStr('email'); + if (!strlen($new_email)) { + $errors[] = 'Email is required.'; + $e_email = 'Required'; + } if ($request->getStr('role') == 'agent') { $user->setIsSystemAgent(true); @@ -154,13 +161,6 @@ final class PhabricatorPeopleEditController $e_realname = null; } - if (!strlen($user->getEmail())) { - $errors[] = 'Email is required.'; - $e_email = 'Required'; - } else { - $e_email = null; - } - if (!$errors) { try { $is_new = !$user->getID(); @@ -168,6 +168,14 @@ final class PhabricatorPeopleEditController $user->save(); if ($is_new) { + + $email = id(new PhabricatorUserEmail()) + ->setUserPHID($user->getPHID()) + ->setAddress($new_email) + ->setIsPrimary(1) + ->setIsVerified(0) + ->save(); + $log = PhabricatorUserLog::newLog( $admin, $user, @@ -187,8 +195,8 @@ final class PhabricatorPeopleEditController $same_username = id(new PhabricatorUser()) ->loadOneWhere('username = %s', $user->getUsername()); - $same_email = id(new PhabricatorUser()) - ->loadOneWhere('email = %s', $user->getEmail()); + $same_email = id(new PhabricatorUserEmail()) + ->loadOneWhere('address = %s', $new_email); if ($same_username) { $e_username = 'Duplicate'; @@ -236,15 +244,19 @@ final class PhabricatorPeopleEditController ->setLabel('Real Name') ->setName('realname') ->setValue($user->getRealName()) - ->setError($e_realname)) - ->appendChild( + ->setError($e_realname)); + + if (!$user->getID()) { + $form->appendChild( id(new AphrontFormTextControl()) ->setLabel('Email') ->setName('email') ->setDisabled($is_immutable) - ->setValue($user->getEmail()) - ->setError($e_email)) - ->appendChild($this->getRoleInstructions()); + ->setValue($new_email) + ->setError($e_email)); + } + + $form->appendChild($this->getRoleInstructions()); if (!$user->getID()) { $form diff --git a/src/applications/people/controller/edit/__init__.php b/src/applications/people/controller/edit/__init__.php index 3b769e25e1..9ee6aaffb0 100644 --- a/src/applications/people/controller/edit/__init__.php +++ b/src/applications/people/controller/edit/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/people/controller/base'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/log'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'infrastructure/env'); diff --git a/src/applications/people/controller/emailverification/PhabricatorEmailVerificationController.php b/src/applications/people/controller/emailverification/PhabricatorEmailVerificationController.php new file mode 100644 index 0000000000..21902373ee --- /dev/null +++ b/src/applications/people/controller/emailverification/PhabricatorEmailVerificationController.php @@ -0,0 +1,81 @@ +code = $data['code']; + } + + public function processRequest() { + $request = $this->getRequest(); + $user = $request->getUser(); + + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'userPHID = %s AND verificationCode = %s', + $user->getPHID(), + $this->code); + + $settings_link = phutil_render_tag( + 'a', + array( + 'href' => '/settings/page/email/', + ), + 'Return to Email Settings'); + $settings_link = '

'.$settings_link.'

'; + + if (!$email) { + $content = id(new AphrontErrorView()) + ->setTitle('Unable To Verify') + ->appendChild( + '

The verification code is incorrect, the email address has '. + 'been removed, or the email address is owned by another user. Make '. + 'sure you followed the link in the email correctly.

'); + } else if ($email->getIsVerified()) { + $content = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle('Address Already Verified') + ->appendChild( + '

This email address has already been verified.

'. + $settings_link); + } else { + + $guard = AphrontWriteGuard::beginScopedUnguardedWrites(); + $email->setIsVerified(1); + $email->save(); + unset($guard); + + $content = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle('Address Verified') + ->appendChild( + '

This email address has now been verified. Thanks!

'. + $settings_link); + } + + return $this->buildStandardPageResponse( + $content, + array( + 'title' => 'Verify Email', + )); + } + +} diff --git a/src/applications/people/controller/emailverification/__init__.php b/src/applications/people/controller/emailverification/__init__.php new file mode 100644 index 0000000000..e846d8857a --- /dev/null +++ b/src/applications/people/controller/emailverification/__init__.php @@ -0,0 +1,18 @@ +addFilter('profile', 'Profile') ->addSpacer() ->addLabel('Email') - ->addFilter('email', 'Email Address') + ->addFilter('email', 'Email Addresses') ->addFilter('emailpref', 'Email Preferences') ->addSpacer() ->addLabel('Authentication'); diff --git a/src/applications/people/controller/settings/panels/email/PhabricatorUserEmailSettingsPanelController.php b/src/applications/people/controller/settings/panels/email/PhabricatorUserEmailSettingsPanelController.php index e5c1294818..d9defa72e3 100644 --- a/src/applications/people/controller/settings/panels/email/PhabricatorUserEmailSettingsPanelController.php +++ b/src/applications/people/controller/settings/panels/email/PhabricatorUserEmailSettingsPanelController.php @@ -25,72 +25,317 @@ final class PhabricatorUserEmailSettingsPanelController $user = $request->getUser(); $editable = $this->getAccountEditable(); - $e_email = true; - $errors = array(); - if ($request->isFormPost()) { - if (!$editable) { - return new Aphront400Response(); + $uri = $request->getRequestURI(); + $uri->setQueryParams(array()); + + if ($editable) { + $new = $request->getStr('new'); + if ($new) { + return $this->returnNewAddressResponse($uri, $new); } - $user->setEmail($request->getStr('email')); + $delete = $request->getInt('delete'); + if ($delete) { + return $this->returnDeleteAddressResponse($uri, $delete); + } + } - if (!strlen($user->getEmail())) { - $errors[] = 'You must enter an e-mail address.'; + $verify = $request->getInt('verify'); + if ($verify) { + return $this->returnVerifyAddressResponse($uri, $verify); + } + + $primary = $request->getInt('primary'); + if ($primary) { + return $this->returnPrimaryAddressResponse($uri, $primary); + } + + $emails = id(new PhabricatorUserEmail())->loadAllWhere( + 'userPHID = %s', + $user->getPHID()); + + $rowc = array(); + $rows = array(); + foreach ($emails as $email) { + + if ($email->getIsPrimary()) { + $action = phutil_render_tag( + 'a', + array( + 'class' => 'button small disabled', + ), + 'Primary'); + $remove = $action; + $rowc[] = 'highlighted'; + } else { + if ($email->getIsVerified()) { + $action = javelin_render_tag( + 'a', + array( + 'class' => 'button small grey', + 'href' => $uri->alter('primary', $email->getID()), + 'sigil' => 'workflow', + ), + 'Make Primary'); + } else { + $action = javelin_render_tag( + 'a', + array( + 'class' => 'button small grey', + 'href' => $uri->alter('verify', $email->getID()), + 'sigil' => 'workflow', + ), + 'Verify'); + } + $remove = javelin_render_tag( + 'a', + array( + 'class' => 'button small grey', + 'href' => $uri->alter('delete', $email->getID()), + 'sigil' => 'workflow' + ), + 'Remove'); + $rowc[] = null; + } + + $rows[] = array( + phutil_escape_html($email->getAddress()), + $action, + $remove, + ); + } + + $table = new AphrontTableView($rows); + $table->setHeaders( + array( + 'Email', + 'Status', + 'Remove', + )); + $table->setColumnClasses( + array( + 'wide', + 'action', + 'action', + )); + $table->setRowClasses($rowc); + $table->setColumnVisibility( + array( + true, + true, + $editable, + )); + + $view = new AphrontPanelView(); + if ($editable) { + $view->addButton( + javelin_render_tag( + 'a', + array( + 'href' => $uri->alter('new', 'true'), + 'class' => 'green button', + 'sigil' => 'workflow', + ), + 'Add New Address')); + } + $view->setHeader('Email Addresses'); + $view->appendChild($table); + + return $view; + } + + private function returnNewAddressResponse(PhutilURI $uri, $new) { + $request = $this->getRequest(); + $user = $request->getUser(); + + $e_email = true; + $email = trim($request->getStr('email')); + $errors = array(); + if ($request->isDialogFormPost()) { + + if ($new == 'verify') { + // The user clicked "Done" from the "an email has been sent" dialog. + return id(new AphrontReloadResponse())->setURI($uri); + } + + if (!strlen($email)) { $e_email = 'Required'; + $errors[] = 'Email is required.'; } if (!$errors) { - $user->save(); - return id(new AphrontRedirectResponse()) - ->setURI('/settings/page/email/?saved=true'); + $object = id(new PhabricatorUserEmail()) + ->setUserPHID($user->getPHID()) + ->setAddress($email) + ->setIsVerified(0) + ->setIsPrimary(0); + + try { + $object->save(); + + $object->sendVerificationEmail($user); + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->addHiddenInput('new', 'verify') + ->setTitle('Verification Email Sent') + ->appendChild( + '

A verification email has been sent. Click the link in the '. + 'email to verify your address.

') + ->setSubmitURI($uri) + ->addSubmitButton('Done'); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } catch (AphrontQueryDuplicateKeyException $ex) { + $email = 'Duplicate'; + $errors[] = 'Another user already has this email.'; + } } } - $notice = null; - if (!$errors) { - if ($request->getStr('saved')) { - $notice = new AphrontErrorView(); - $notice->setSeverity(AphrontErrorView::SEVERITY_NOTICE); - $notice->setTitle('Changes Saved'); - $notice->appendChild('

Your changes have been saved.

'); - } - } else { - $notice = new AphrontErrorView(); - $notice->setTitle('Form Errors'); - $notice->setErrors($errors); + if ($errors) { + $errors = id(new AphrontErrorView()) + ->setWidth(AphrontErrorView::WIDTH_DIALOG) + ->setErrors($errors); } - $form = new AphrontFormView(); - $form - ->setUser($user) + $form = id(new AphrontFormLayoutView()) ->appendChild( id(new AphrontFormTextControl()) ->setLabel('Email') ->setName('email') - ->setDisabled(!$editable) - ->setCaption( - 'Note: there is no email validation yet; double-check your '. - 'typing.') - ->setValue($user->getEmail()) + ->setValue($request->getStr('email')) ->setError($e_email)); - if ($editable) { - $form - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue('Save')); + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->addHiddenInput('new', 'true') + ->setTitle('New Address') + ->appendChild($errors) + ->appendChild($form) + ->addSubmitButton('Save') + ->addCancelButton($uri); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + + private function returnDeleteAddressResponse(PhutilURI $uri, $email_id) { + $request = $this->getRequest(); + $user = $request->getUser(); + + // NOTE: You can only delete your own email addresses, and you can not + // delete your primary address. + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'id = %d AND userPHID = %s AND isPrimary = 0', + $email_id, + $user->getPHID()); + + if (!$email) { + return new Aphront404Response(); } - $panel = new AphrontPanelView(); - $panel->setHeader('Email Settings'); - $panel->setWidth(AphrontPanelView::WIDTH_FORM); - $panel->appendChild($form); + if ($request->isFormPost()) { + $email->delete(); + return id(new AphrontRedirectResponse())->setURI($uri); + } - return id(new AphrontNullView()) + $address = $email->getAddress(); + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->addHiddenInput('delete', $email_id) + ->setTitle("Really delete address '{$address}'?") ->appendChild( - array( - $notice, - $panel, - )); + '

Are you sure you want to delete this address? You will no '. + 'longer be able to use it to login.

') + ->addSubmitButton('Delete') + ->addCancelButton($uri); + + return id(new AphrontDialogResponse())->setDialog($dialog); } + + private function returnVerifyAddressResponse(PhutilURI $uri, $email_id) { + $request = $this->getRequest(); + $user = $request->getUser(); + + // NOTE: You can only send more email for your unverified addresses. + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'id = %d AND userPHID = %s AND isVerified = 0', + $email_id, + $user->getPHID()); + + if (!$email) { + return new Aphront404Response(); + } + + if ($request->isFormPost()) { + $email->sendVerificationEmail($user); + return id(new AphrontRedirectResponse())->setURI($uri); + } + + $address = $email->getAddress(); + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->addHiddenInput('verify', $email_id) + ->setTitle("Send Another Verification Email?") + ->appendChild( + '

Send another copy of the verification email to '. + phutil_escape_html($address).'?

') + ->addSubmitButton('Send Email') + ->addCancelButton($uri); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + + private function returnPrimaryAddressResponse(PhutilURI $uri, $email_id) { + $request = $this->getRequest(); + $user = $request->getUser(); + + // NOTE: You can only make your own verified addresses primary. + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'id = %d AND userPHID = %s AND isVerified = 1 AND isPrimary = 0', + $email_id, + $user->getPHID()); + + if (!$email) { + return new Aphront404Response(); + } + + if ($request->isFormPost()) { + + // TODO: Transactions! + + $email->setIsPrimary(1); + + $old_primary = $user->loadPrimaryEmail(); + if ($old_primary) { + $old_primary->setIsPrimary(0); + $old_primary->save(); + } + $email->save(); + + if ($old_primary) { + $old_primary->sendOldPrimaryEmail($user, $email); + } + $email->sendNewPrimaryEmail($user); + + return id(new AphrontRedirectResponse())->setURI($uri); + } + + $address = $email->getAddress(); + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->addHiddenInput('primary', $email_id) + ->setTitle("Change primary email address?") + ->appendChild( + '

If you change your primary address, Phabricator will send all '. + 'email to '.phutil_escape_html($address).'.

') + ->addSubmitButton('Change Primary Address') + ->addCancelButton($uri); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + } diff --git a/src/applications/people/controller/settings/panels/email/__init__.php b/src/applications/people/controller/settings/panels/email/__init__.php index c991e43a42..86aed32284 100644 --- a/src/applications/people/controller/settings/panels/email/__init__.php +++ b/src/applications/people/controller/settings/panels/email/__init__.php @@ -6,16 +6,21 @@ -phutil_require_module('phabricator', 'aphront/response/400'); +phutil_require_module('phabricator', 'aphront/response/404'); +phutil_require_module('phabricator', 'aphront/response/dialog'); phutil_require_module('phabricator', 'aphront/response/redirect'); +phutil_require_module('phabricator', 'aphront/response/reload'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/base'); -phutil_require_module('phabricator', 'view/form/base'); -phutil_require_module('phabricator', 'view/form/control/submit'); +phutil_require_module('phabricator', 'applications/people/storage/email'); +phutil_require_module('phabricator', 'infrastructure/javelin/markup'); +phutil_require_module('phabricator', 'view/control/table'); +phutil_require_module('phabricator', 'view/dialog'); phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/error'); +phutil_require_module('phabricator', 'view/form/layout'); phutil_require_module('phabricator', 'view/layout/panel'); -phutil_require_module('phabricator', 'view/null'); +phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/people/controller/settings/panels/password/PhabricatorUserPasswordSettingsPanelController.php b/src/applications/people/controller/settings/panels/password/PhabricatorUserPasswordSettingsPanelController.php index 7ef23aec4f..9479aeb03f 100644 --- a/src/applications/people/controller/settings/panels/password/PhabricatorUserPasswordSettingsPanelController.php +++ b/src/applications/people/controller/settings/panels/password/PhabricatorUserPasswordSettingsPanelController.php @@ -40,10 +40,16 @@ final class PhabricatorUserPasswordSettingsPanelController // the workflow from a password reset email. $token = $request->getStr('token'); + + $valid_token = false; if ($token) { - $valid_token = $user->validateEmailToken($token); - } else { - $valid_token = false; + $email_address = $request->getStr('email'); + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', + $email_address); + if ($email) { + $valid_token = $user->validateEmailToken($email, $token); + } } $e_old = true; diff --git a/src/applications/people/controller/settings/panels/password/__init__.php b/src/applications/people/controller/settings/panels/password/__init__.php index 7a375a6fd8..d87675fd12 100644 --- a/src/applications/people/controller/settings/panels/password/__init__.php +++ b/src/applications/people/controller/settings/panels/password/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'aphront/response/400'); phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'aphront/writeguard'); phutil_require_module('phabricator', 'applications/people/controller/settings/panels/base'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/password'); diff --git a/src/applications/people/storage/email/PhabricatorUserEmail.php b/src/applications/people/storage/email/PhabricatorUserEmail.php new file mode 100644 index 0000000000..954409598b --- /dev/null +++ b/src/applications/people/storage/email/PhabricatorUserEmail.php @@ -0,0 +1,160 @@ +getVerificationCode().'/'; + } + + public function save() { + if (!$this->verificationCode) { + $this->setVerificationCode(Filesystem::readRandomCharacters(24)); + } + return parent::save(); + } + + +/* -( Email About Email )-------------------------------------------------- */ + + + /** + * Send a verification email from $user to this address. + * + * @param PhabricatorUser The user sending the verification. + * @return this + * @task email + */ + public function sendVerificationEmail(PhabricatorUser $user) { + $username = $user->getUsername(); + + $address = $this->getAddress(); + $link = PhabricatorEnv::getProductionURI($this->getVerificationURI()); + + + $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); + + $signature = null; + if (!$is_serious) { + $signature = <<addRawTos(array($address)) + ->setSubject('[Phabricator] Email Verification') + ->setBody($body) + ->setFrom($user->getPHID()) + ->setRelatedPHID($user->getPHID()) + ->saveAndSend(); + + return $this; + } + + + /** + * Send a notification email from $user to this address, informing the + * recipient that this is no longer their account's primary address. + * + * @param PhabricatorUser The user sending the notification. + * @param PhabricatorUserEmail New primary email address. + * @return this + * @task email + */ + public function sendOldPrimaryEmail( + PhabricatorUser $user, + PhabricatorUserEmail $new) { + $username = $user->getUsername(); + + $old_address = $this->getAddress(); + $new_address = $new->getAddress(); + + $body = <<addRawTos(array($old_address)) + ->setSubject('[Phabricator] Primary Address Changed') + ->setBody($body) + ->setFrom($user->getPHID()) + ->setRelatedPHID($user->getPHID()) + ->saveAndSend(); + } + + + /** + * Send a notification email from $user to this address, informing the + * recipient that this is now their account's new primary email address. + * + * @param PhabricatorUser The user sending the verification. + * @return this + * @task email + */ + public function sendNewPrimaryEmail(PhabricatorUser $user) { + $username = $user->getUsername(); + + $new_address = $this->getAddress(); + + $body = <<addRawTos(array($new_address)) + ->setSubject('[Phabricator] Primary Address Changed') + ->setBody($body) + ->setFrom($user->getPHID()) + ->setRelatedPHID($user->getPHID()) + ->saveAndSend(); + + return $this; + } + +} diff --git a/src/applications/people/storage/email/__init__.php b/src/applications/people/storage/email/__init__.php new file mode 100644 index 0000000000..77b722bb2d --- /dev/null +++ b/src/applications/people/storage/email/__init__.php @@ -0,0 +1,17 @@ +getPHID(), + $email->getVerificationCode(), + )); + return $this->generateToken( time() + ($offset * self::EMAIL_CYCLE_FREQUENCY), self::EMAIL_CYCLE_FREQUENCY, - PhabricatorEnv::getEnvConfig('phabricator.csrf-key').$this->getEmail(), + $key, self::EMAIL_TOKEN_LENGTH); } - public function validateEmailToken($token) { + public function validateEmailToken( + PhabricatorUserEmail $email, + $token) { for ($ii = -1; $ii <= 1; $ii++) { - $valid = $this->generateEmailToken($ii); + $valid = $this->generateEmailToken($email, $ii); if ($token == $valid) { return true; } @@ -378,11 +390,32 @@ final class PhabricatorUser extends PhabricatorUserDAO { return false; } - public function getEmailLoginURI() { - $token = $this->generateEmailToken(); + public function getEmailLoginURI(PhabricatorUserEmail $email = null) { + if (!$email) { + $email = $this->loadPrimaryEmail(); + if (!$email) { + throw new Exception("User has no primary email!"); + } + } + $token = $this->generateEmailToken($email); $uri = PhabricatorEnv::getProductionURI('/login/etoken/'.$token.'/'); $uri = new PhutilURI($uri); - return $uri->alter('email', $this->getEmail()); + return $uri->alter('email', $email->getAddress()); + } + + public function loadPrimaryEmailAddress() { + $email = $this->loadPrimaryEmail(); + if (!$email) { + throw new Exception("User has no primary email address!"); + } + return $email->getAddress(); + } + + public function loadPrimaryEmail() { + return id(new PhabricatorUserEmail())->loadOneWhere( + 'userPHID = %s AND isPrimary = %d', + $this->getPHID(), + 1); } public function loadPreferences() { @@ -534,4 +567,16 @@ EOBODY; return self::getDefaultProfileImageURI(); } + public static function loadOneWithEmailAddress($address) { + $email = id(new PhabricatorUserEmail())->loadOneWhere( + 'address = %s', + $address); + if (!$email) { + return null; + } + return id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $email->getUserPHID()); + } + } diff --git a/src/applications/people/storage/user/__init__.php b/src/applications/people/storage/user/__init__.php index 3201c0f2e1..251ba96e53 100644 --- a/src/applications/people/storage/user/__init__.php +++ b/src/applications/people/storage/user/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'aphront/writeguard'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/metamta/storage/mail'); phutil_require_module('phabricator', 'applications/people/storage/base'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/log'); phutil_require_module('phabricator', 'applications/people/storage/preferences'); phutil_require_module('phabricator', 'applications/phid/constants'); diff --git a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php index de1ca5f562..46701f974d 100644 --- a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php @@ -149,6 +149,13 @@ final class PhabricatorObjectHandleData { $images = mpull($images, 'getBestURI', 'getPHID'); } + // TODO: This probably should not be part of Handles anymore, only + // MetaMTA actually uses it. + $emails = id(new PhabricatorUserEmail())->loadAllWhere( + 'userPHID IN (%Ls) AND isPrimary = 1', + $phids); + $emails = mpull($emails, 'getAddress', 'getUserPHID'); + foreach ($phids as $phid) { $handle = new PhabricatorObjectHandle(); $handle->setPHID($phid); @@ -159,7 +166,7 @@ final class PhabricatorObjectHandleData { $user = $users[$phid]; $handle->setName($user->getUsername()); $handle->setURI('/p/'.$user->getUsername().'/'); - $handle->setEmail($user->getEmail()); + $handle->setEmail(idx($emails, $phid)); $handle->setFullName( $user->getUsername().' ('.$user->getRealName().')'); $handle->setAlternateID($user->getID()); diff --git a/src/applications/phid/handle/data/__init__.php b/src/applications/phid/handle/data/__init__.php index c2df9169cd..ed2961497f 100644 --- a/src/applications/phid/handle/data/__init__.php +++ b/src/applications/phid/handle/data/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('arcanist', 'differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); +phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/handle'); diff --git a/src/applications/repository/parser/base/PhabricatorRepositoryCommitMessageDetailParser.php b/src/applications/repository/parser/base/PhabricatorRepositoryCommitMessageDetailParser.php index 006d0f09f3..444efc55f8 100644 --- a/src/applications/repository/parser/base/PhabricatorRepositoryCommitMessageDetailParser.php +++ b/src/applications/repository/parser/base/PhabricatorRepositoryCommitMessageDetailParser.php @@ -1,7 +1,7 @@ loadOneWhere( - 'email = %s', - $email_address); + $by_email = PhabricatorUser::loadOneWithEmailAddress($email_address); if ($by_email) { return $by_email->getPHID(); } diff --git a/src/infrastructure/setup/sql/phabricator/PhabricatorBuiltinPatchList.php b/src/infrastructure/setup/sql/phabricator/PhabricatorBuiltinPatchList.php index 9941bb70a5..a599aabd2b 100644 --- a/src/infrastructure/setup/sql/phabricator/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/setup/sql/phabricator/PhabricatorBuiltinPatchList.php @@ -859,6 +859,18 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('userstatus.sql'), ), + 'emailtable.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('emailtable.sql'), + ), + 'emailtableport.sql' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('emailtableport.php'), + ), + 'emailtableremove.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('emailtableremove.sql'), + ), ); }