@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Make external account identifier APIs return multiple identifiers

Summary:
Depends on D21012. Ref T13493. Currently, auth adapters return a single identifier for each external account.

Allow them to return more than one identifier, to better handle cases where an API changes from providing a lower-quality identifier to a higher-quality identifier.

On its own, this change doesn't change any user-facing behavior.

Test Plan: Linked and unlinked external accounts.

Maniphest Tasks: T13493

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

+54 -33
+5
src/applications/auth/adapter/PhutilAuthAdapter.php
··· 34 34 return $identifiers; 35 35 } 36 36 37 + final protected function newAccountIdentifier($raw_identifier) { 38 + return id(new PhabricatorExternalAccountIdentifier()) 39 + ->setIdentifierRaw($raw_identifier); 40 + } 41 + 37 42 /** 38 43 * Get a unique identifier associated with the account. 39 44 *
+6 -3
src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php
··· 56 56 $console->writeOut("\n"); 57 57 $console->writeOut("%s\n", pht('Connecting to LDAP...')); 58 58 59 - $account_id = $adapter->getAccountID(); 60 - if ($account_id) { 61 - $console->writeOut("%s\n", pht('Found LDAP Account: %s', $account_id)); 59 + $account_ids = $adapter->getAccountIdentifiers(); 60 + if ($account_ids) { 61 + $value_list = mpull($account_ids, 'getIdentifierRaw'); 62 + $value_list = implode(', ', $value_list); 63 + 64 + $console->writeOut("%s\n", pht('Found LDAP Account: %s', $value_list)); 62 65 } else { 63 66 $console->writeOut("%s\n", pht('Unable to find LDAP account!')); 64 67 }
+35 -22
src/applications/auth/provider/PhabricatorAuthProvider.php
··· 190 190 return; 191 191 } 192 192 193 - protected function loadOrCreateAccount($account_id) { 194 - if (!strlen($account_id)) { 195 - throw new Exception(pht('Empty account ID!')); 196 - } 193 + protected function loadOrCreateAccount(array $identifiers) { 194 + assert_instances_of($identifiers, 'PhabricatorExternalAccountIdentifier'); 197 195 198 - $adapter = $this->getAdapter(); 199 - $adapter_class = get_class($adapter); 200 - 201 - if (!strlen($adapter->getAdapterType())) { 196 + if (!$identifiers) { 202 197 throw new Exception( 203 198 pht( 204 - "AuthAdapter (of class '%s') has an invalid implementation: ". 205 - "no adapter type.", 206 - $adapter_class)); 199 + 'Authentication provider (of class "%s") is attempting to '. 200 + 'load or create an external account, but provided no account '. 201 + 'identifiers.', 202 + get_class($this))); 207 203 } 208 204 209 - if (!strlen($adapter->getAdapterDomain())) { 205 + if (count($identifiers) !== 1) { 210 206 throw new Exception( 211 207 pht( 212 - "AuthAdapter (of class '%s') has an invalid implementation: ". 213 - "no adapter domain.", 214 - $adapter_class)); 208 + 'Unexpected number of account identifiers returned (by class "%s").', 209 + get_class($this))); 215 210 } 216 211 217 - $account = id(new PhabricatorExternalAccount())->loadOneWhere( 218 - 'accountType = %s AND accountDomain = %s AND accountID = %s', 219 - $adapter->getAdapterType(), 220 - $adapter->getAdapterDomain(), 221 - $account_id); 222 - if (!$account) { 212 + $config = $this->getProviderConfig(); 213 + $viewer = PhabricatorUser::getOmnipotentUser(); 214 + 215 + $raw_identifiers = mpull($identifiers, 'getIdentifierRaw'); 216 + 217 + $accounts = id(new PhabricatorExternalAccountQuery()) 218 + ->setViewer($viewer) 219 + ->withProviderConfigPHIDs(array($config->getPHID())) 220 + ->withAccountIDs($raw_identifiers) 221 + ->execute(); 222 + if (!$accounts) { 223 223 $account = $this->newExternalAccount() 224 - ->setAccountID($account_id); 224 + ->setAccountID(head($raw_identifiers)); 225 + } else if (count($accounts) === 1) { 226 + $account = head($accounts); 227 + } else { 228 + throw new Exception( 229 + pht( 230 + 'Authentication provider (of class "%s") is attempting to load '. 231 + 'or create an external account, but provided a list of '. 232 + 'account identifiers which map to more than one account: %s.', 233 + get_class($this), 234 + implode(', ', $raw_identifiers))); 225 235 } 236 + 237 + $adapter = $this->getAdapter(); 226 238 227 239 $account->setUsername($adapter->getAccountName()); 228 240 $account->setRealName($adapter->getAccountRealName()); ··· 240 252 // file entry for it, but there's no convenient way to do this with 241 253 // PhabricatorFile right now. The storage will get shared, so the impact 242 254 // here is negligible. 255 + 243 256 $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); 244 257 $image_file = PhabricatorFile::newFromFileDownload( 245 258 $image_uri,
+2 -2
src/applications/auth/provider/PhabricatorLDAPAuthProvider.php
··· 164 164 // See T3351. 165 165 166 166 DarkConsoleErrorLogPluginAPI::enableDiscardMode(); 167 - $account_id = $adapter->getAccountID(); 167 + $identifiers = $adapter->getAccountIdentifiers(); 168 168 DarkConsoleErrorLogPluginAPI::disableDiscardMode(); 169 169 } else { 170 170 throw new Exception(pht('Username and password are required!')); ··· 180 180 } 181 181 } 182 182 183 - return array($this->loadOrCreateAccount($account_id), $response); 183 + return array($this->loadOrCreateAccount($identifiers), $response); 184 184 } 185 185 186 186
+3 -3
src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php
··· 100 100 // an access token. 101 101 102 102 try { 103 - $account_id = $adapter->getAccountID(); 103 + $identifiers = $adapter->getAccountIdentifiers(); 104 104 } catch (Exception $ex) { 105 105 // TODO: Handle this in a more user-friendly way. 106 106 throw $ex; 107 107 } 108 108 109 - if (!strlen($account_id)) { 109 + if (!$identifiers) { 110 110 $response = $controller->buildProviderErrorResponse( 111 111 $this, 112 112 pht( ··· 115 115 return array($account, $response); 116 116 } 117 117 118 - return array($this->loadOrCreateAccount($account_id), $response); 118 + return array($this->loadOrCreateAccount($identifiers), $response); 119 119 } 120 120 121 121 public function processEditForm(
+3 -3
src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php
··· 80 80 // an access token. 81 81 82 82 try { 83 - $account_id = $adapter->getAccountID(); 83 + $identifiers = $adapter->getAccountIdentifiers(); 84 84 } catch (Exception $ex) { 85 85 // TODO: Handle this in a more user-friendly way. 86 86 throw $ex; 87 87 } 88 88 89 - if (!strlen($account_id)) { 89 + if (!$identifiers) { 90 90 $response = $controller->buildProviderErrorResponse( 91 91 $this, 92 92 pht( ··· 95 95 return array($account, $response); 96 96 } 97 97 98 - return array($this->loadOrCreateAccount($account_id), $response); 98 + return array($this->loadOrCreateAccount($identifiers), $response); 99 99 } 100 100 101 101 public function processEditForm(