Write ExternalAccountIdentifiers when interacting with external authentication providers

Summary:
Depends on D21015. When we sync an external account and get a list of account identifiers, write them to the database.

Nothing reads them yet and we still write "accountId", this just prepares us for reads.

Test Plan: Linked, refreshed, unlinked, and re-linked an external account. Peeked at the database and saw a sensible-looking row.

Differential Revision: https://secure.phabricator.com/D21016

src/applications/auth/controller/PhabricatorAuthUnlinkController.php

@@ -67,7 +67,7 @@ final class PhabricatorAuthUnlinkController
       ->setWorkflowKey($workflow_key)
       ->requireHighSecurityToken($viewer, $request, $done_uri);
 
-    $account->delete();
+    $account->unlinkAccount();
 
     id(new PhabricatorAuthSessionEngine())->terminateLoginSessions(
       $viewer,

src/applications/auth/provider/PhabricatorAuthProvider.php

@@ -216,6 +216,7 @@ abstract class PhabricatorAuthProvider extends Phobject {
       ->setViewer($viewer)
       ->withProviderConfigPHIDs(array($config->getPHID()))
       ->withAccountIDs($raw_identifiers)
+      ->needAccountIdentifiers(true)
       ->execute();
     if (!$accounts) {
       $account = $this->newExternalAccount()
@@ -232,6 +233,17 @@ abstract class PhabricatorAuthProvider extends Phobject {
           implode(', ', $raw_identifiers)));
     }
 
+    // See T13493. Add all the identifiers to the account. In the case where
+    // an account initially has a lower-quality identifier (like an email
+    // address) and later adds a higher-quality identifier (like a GUID), this
+    // allows us to automatically upgrade toward the higher-quality identifier
+    // and survive API changes which remove the lower-quality identifier more
+    // gracefully.
+
+    foreach ($identifiers as $identifier) {
+      $account->appendIdentifier($identifier);
+    }
+
     return $this->didUpdateAccount($account);
   }
 
@@ -351,7 +363,8 @@ abstract class PhabricatorAuthProvider extends Phobject {
     return id(new PhabricatorExternalAccount())
       ->setAccountType($adapter->getAdapterType())
       ->setAccountDomain($adapter->getAdapterDomain())
-      ->setProviderConfigPHID($config->getPHID());
+      ->setProviderConfigPHID($config->getPHID())
+      ->attachAccountIdentifiers(array());
   }
 
   public function getLoginOrder() {

src/applications/auth/query/PhabricatorExternalAccountQuery.php

@@ -22,6 +22,7 @@ final class PhabricatorExternalAccountQuery
   private $needImages;
   private $accountSecrets;
   private $providerConfigPHIDs;
+  private $needAccountIdentifiers;
 
   public function withUserPHIDs(array $user_phids) {
     $this->userPHIDs = $user_phids;
@@ -63,6 +64,11 @@ final class PhabricatorExternalAccountQuery
     return $this;
   }
 
+  public function needAccountIdentifiers($need) {
+    $this->needAccountIdentifiers = $need;
+    return $this;
+  }
+
   public function withProviderConfigPHIDs(array $phids) {
     $this->providerConfigPHIDs = $phids;
     return $this;
@@ -132,6 +138,23 @@ final class PhabricatorExternalAccountQuery
       }
     }
 
+    if ($this->needAccountIdentifiers) {
+      $account_phids = mpull($accounts, 'getPHID');
+
+      $identifiers = id(new PhabricatorExternalAccountIdentifierQuery())
+        ->setViewer($viewer)
+        ->setParentQuery($this)
+        ->withExternalAccountPHIDs($account_phids)
+        ->execute();
+
+      $identifiers = mgroup($identifiers, 'getExternalAccountPHID');
+      foreach ($accounts as $account) {
+        $account_phid = $account->getPHID();
+        $account_identifiers = idx($identifiers, $account_phid, array());
+        $account->attachAccountIdentifiers($account_identifiers);
+      }
+    }
+
     return $accounts;
   }
 

src/applications/people/storage/PhabricatorExternalAccount.php

@@ -23,6 +23,7 @@ final class PhabricatorExternalAccount
 
   private $profileImageFile = self::ATTACHABLE;
   private $providerConfig = self::ATTACHABLE;
+  private $accountIdentifiers = self::ATTACHABLE;
 
   public function getProfileImageFile() {
     return $this->assertAttached($this->profileImageFile);
@@ -78,7 +79,48 @@ final class PhabricatorExternalAccount
     if (!$this->getAccountSecret()) {
       $this->setAccountSecret(Filesystem::readRandomCharacters(32));
     }
-    return parent::save();
+
+    $this->openTransaction();
+
+      $result = parent::save();
+
+      $account_phid = $this->getPHID();
+      $config_phid = $this->getProviderConfigPHID();
+
+      if ($this->accountIdentifiers !== self::ATTACHABLE) {
+        foreach ($this->getAccountIdentifiers() as $identifier) {
+          $identifier
+            ->setExternalAccountPHID($account_phid)
+            ->setProviderConfigPHID($config_phid)
+            ->save();
+        }
+      }
+
+    $this->saveTransaction();
+
+    return $result;
+  }
+
+  public function unlinkAccount() {
+
+    // When unlinking an account, we disassociate it from the user and
+    // remove all the identifying information. We retain the PHID, the
+    // object itself, and the "ExternalAccountIdentifier" objects in the
+    // external table.
+
+    // TODO: This unlinks (but does not destroy) any profile image.
+
+    return $this
+      ->setUserPHID(null)
+      ->setDisplayName(null)
+      ->setUsername(null)
+      ->setRealName(null)
+      ->setEmail(null)
+      ->setEmailVerified(0)
+      ->setProfileImagePHID(null)
+      ->setAccountURI(null)
+      ->setProperties(array())
+      ->save();
   }
 
   public function setProperty($key, $value) {
@@ -131,6 +173,44 @@ final class PhabricatorExternalAccount
     return $this->assertAttached($this->providerConfig);
   }
 
+  public function getAccountIdentifiers() {
+    $raw = $this->assertAttached($this->accountIdentifiers);
+    return array_values($raw);
+  }
+
+  public function attachAccountIdentifiers(array $identifiers) {
+    assert_instances_of($identifiers, 'PhabricatorExternalAccountIdentifier');
+    $this->accountIdentifiers = mpull($identifiers, null, 'getIdentifierRaw');
+    return $this;
+  }
+
+  public function appendIdentifier(
+    PhabricatorExternalAccountIdentifier $identifier) {
+
+    $this->assertAttached($this->accountIdentifiers);
+
+    $map = $this->accountIdentifiers;
+    $raw = $identifier->getIdentifierRaw();
+
+    $old = idx($map, $raw);
+    $new = $identifier;
+
+    if ($old === null) {
+      $result = $new;
+    } else {
+      // Here, we already know about an identifier and have rediscovered it.
+
+      // We could copy properties from the new version of the identifier here,
+      // or merge them in some other way (for example, update a "last seen
+      // from the provider" timestamp), but no such properties currently exist.
+      $result = $old;
+    }
+
+    $this->accountIdentifiers[$raw] = $result;
+
+    return $this;
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
@@ -182,6 +262,8 @@ final class PhabricatorExternalAccount
       $engine->destroyObject($identifier);
     }
 
+    // TODO: This may leave a profile image behind.
+
     $this->delete();
   }
 

src/applications/people/storage/PhabricatorExternalAccountIdentifier.php

@@ -36,7 +36,10 @@ final class PhabricatorExternalAccountIdentifier
 
   public function save() {
     $identifier_raw = $this->getIdentifierRaw();
-    $this->identiferHash = PhabricatorHash::digestForIndex($identifier_raw);
+
+    $identifier_hash = PhabricatorHash::digestForIndex($identifier_raw);
+    $this->setIdentifierHash($identifier_hash);
+
     return parent::save();
   }