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

Author: epriestley

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;

resources/sql/patches/emailtableport.php

@@ -0,0 +1,49 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+echo "Migrating user emails...\n";
+
+$table  = new PhabricatorUser();
+$conn   = $table->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";

resources/sql/patches/emailtableremove.sql

@@ -0,0 +1 @@
+ALTER TABLE {$NAMESPACE}_user.user DROP email;

scripts/user/account_admin.php

@@ -55,6 +55,8 @@
   }
   $user = new PhabricatorUser();
   $user->setUsername($username);
+
+  $is_new = true;
 } else {
   $original = clone $user;
 
@@ -66,6 +68,8 @@
     echo "Cancelled.\n";
     exit(1);
   }
+
+  $is_new = false;
 }
 
 $user_realname = $user->getRealName();
@@ -79,31 +83,29 @@
   $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.
+
+$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;
 }
 
-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);
-
 $changed_pass = false;
 // This disables local echo, so the user's password is not shown as they type
 // it.
@@ -126,7 +128,9 @@
 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 @@
   $user->save();
 }
 
+if ($new_email) {
+  id(new PhabricatorUserEmail())
+    ->setUserPHID($user->getPHID())
+    ->setAddress($new_email)
+    ->setIsVerified(1)
+    ->setIsPrimary(1)
+    ->save();
+}
+
 echo "Saved changes.\n";

scripts/user/add_user.php

@@ -50,20 +50,26 @@
     "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";

src/__phutil_library_map__.php

@@ -595,6 +595,7 @@
     '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 @@
     '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 @@
     'PhabricatorEdgeQuery' => 'PhabricatorQuery',
     'PhabricatorEmailLoginController' => 'PhabricatorAuthController',
     'PhabricatorEmailTokenController' => 'PhabricatorAuthController',
+    'PhabricatorEmailVerificationController' => 'PhabricatorPeopleController',
     'PhabricatorEnvTestCase' => 'PhabricatorTestCase',
     'PhabricatorErrorExample' => 'PhabricatorUIExample',
     'PhabricatorEvent' => 'PhutilEvent',
@@ -1819,6 +1822,7 @@
     'PhabricatorUserAccountSettingsPanelController' => 'PhabricatorUserSettingsPanelController',
     'PhabricatorUserConduitSettingsPanelController' => 'PhabricatorUserSettingsPanelController',
     'PhabricatorUserDAO' => 'PhabricatorLiskDAO',
+    'PhabricatorUserEmail' => 'PhabricatorUserDAO',
     'PhabricatorUserEmailPreferenceSettingsPanelController' => 'PhabricatorUserSettingsPanelController',
     'PhabricatorUserEmailSettingsPanelController' => 'PhabricatorUserSettingsPanelController',
     'PhabricatorUserLog' => 'PhabricatorUserDAO',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -433,6 +433,9 @@ public function getURIMap() {
           'testpaymentform/' => 'PhortuneStripeTestPaymentFormController',
         ),
       ),
+
+      '/emailverify/(?P<code>[^/]+)/' =>
+        'PhabricatorEmailVerificationController',
     );
   }
 

src/aphront/response/redirect/AphrontRedirectResponse.php

@@ -31,7 +31,7 @@ public function setURI($uri) {
   }
 
   public function getURI() {
-    return $this->uri;
+    return (string)$this->uri;
   }
 
   public function getHeaders() {

src/applications/auth/controller/email/PhabricatorEmailLoginController.php

@@ -57,17 +57,24 @@ public function processRequest() {
         // 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 = <<<EOBODY
 You can use this link to reset your Phabricator password:

src/applications/auth/controller/email/__init__.php

@@ -9,6 +9,7 @@
 phutil_require_module('phabricator', 'aphront/response/400');
 phutil_require_module('phabricator', 'applications/auth/controller/base');
 phutil_require_module('phabricator', 'applications/metamta/storage/mail');
+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/form/base');