diff --git a/src/applications/auth/adapter/PhutilAuthAdapter.php b/src/applications/auth/adapter/PhutilAuthAdapter.php index 7d33b5a6f3..f8a980313c 100644 --- a/src/applications/auth/adapter/PhutilAuthAdapter.php +++ b/src/applications/auth/adapter/PhutilAuthAdapter.php @@ -34,6 +34,11 @@ abstract class PhutilAuthAdapter extends Phobject { return $identifiers; } + final protected function newAccountIdentifier($raw_identifier) { + return id(new PhabricatorExternalAccountIdentifier()) + ->setIdentifierRaw($raw_identifier); + } + /** * Get a unique identifier associated with the account. * diff --git a/src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php b/src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php index 725d2dfc49..2aa7aca168 100644 --- a/src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php +++ b/src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php @@ -56,9 +56,12 @@ final class PhabricatorAuthManagementLDAPWorkflow $console->writeOut("\n"); $console->writeOut("%s\n", pht('Connecting to LDAP...')); - $account_id = $adapter->getAccountID(); - if ($account_id) { - $console->writeOut("%s\n", pht('Found LDAP Account: %s', $account_id)); + $account_ids = $adapter->getAccountIdentifiers(); + if ($account_ids) { + $value_list = mpull($account_ids, 'getIdentifierRaw'); + $value_list = implode(', ', $value_list); + + $console->writeOut("%s\n", pht('Found LDAP Account: %s', $value_list)); } else { $console->writeOut("%s\n", pht('Unable to find LDAP account!')); } diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index eaf52552ef..6ac6846148 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -190,39 +190,51 @@ abstract class PhabricatorAuthProvider extends Phobject { return; } - protected function loadOrCreateAccount($account_id) { - if (!strlen($account_id)) { - throw new Exception(pht('Empty account ID!')); + protected function loadOrCreateAccount(array $identifiers) { + assert_instances_of($identifiers, 'PhabricatorExternalAccountIdentifier'); + + if (!$identifiers) { + throw new Exception( + pht( + 'Authentication provider (of class "%s") is attempting to '. + 'load or create an external account, but provided no account '. + 'identifiers.', + get_class($this))); + } + + if (count($identifiers) !== 1) { + throw new Exception( + pht( + 'Unexpected number of account identifiers returned (by class "%s").', + get_class($this))); + } + + $config = $this->getProviderConfig(); + $viewer = PhabricatorUser::getOmnipotentUser(); + + $raw_identifiers = mpull($identifiers, 'getIdentifierRaw'); + + $accounts = id(new PhabricatorExternalAccountQuery()) + ->setViewer($viewer) + ->withProviderConfigPHIDs(array($config->getPHID())) + ->withAccountIDs($raw_identifiers) + ->execute(); + if (!$accounts) { + $account = $this->newExternalAccount() + ->setAccountID(head($raw_identifiers)); + } else if (count($accounts) === 1) { + $account = head($accounts); + } else { + throw new Exception( + pht( + 'Authentication provider (of class "%s") is attempting to load '. + 'or create an external account, but provided a list of '. + 'account identifiers which map to more than one account: %s.', + get_class($this), + implode(', ', $raw_identifiers))); } $adapter = $this->getAdapter(); - $adapter_class = get_class($adapter); - - if (!strlen($adapter->getAdapterType())) { - throw new Exception( - pht( - "AuthAdapter (of class '%s') has an invalid implementation: ". - "no adapter type.", - $adapter_class)); - } - - if (!strlen($adapter->getAdapterDomain())) { - throw new Exception( - pht( - "AuthAdapter (of class '%s') has an invalid implementation: ". - "no adapter domain.", - $adapter_class)); - } - - $account = id(new PhabricatorExternalAccount())->loadOneWhere( - 'accountType = %s AND accountDomain = %s AND accountID = %s', - $adapter->getAdapterType(), - $adapter->getAdapterDomain(), - $account_id); - if (!$account) { - $account = $this->newExternalAccount() - ->setAccountID($account_id); - } $account->setUsername($adapter->getAccountName()); $account->setRealName($adapter->getAccountRealName()); @@ -240,6 +252,7 @@ abstract class PhabricatorAuthProvider extends Phobject { // file entry for it, but there's no convenient way to do this with // PhabricatorFile right now. The storage will get shared, so the impact // here is negligible. + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $image_file = PhabricatorFile::newFromFileDownload( $image_uri, diff --git a/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php b/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php index 4a4babcc12..1132f99857 100644 --- a/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php @@ -164,7 +164,7 @@ final class PhabricatorLDAPAuthProvider extends PhabricatorAuthProvider { // See T3351. DarkConsoleErrorLogPluginAPI::enableDiscardMode(); - $account_id = $adapter->getAccountID(); + $identifiers = $adapter->getAccountIdentifiers(); DarkConsoleErrorLogPluginAPI::disableDiscardMode(); } else { throw new Exception(pht('Username and password are required!')); @@ -180,7 +180,7 @@ final class PhabricatorLDAPAuthProvider extends PhabricatorAuthProvider { } } - return array($this->loadOrCreateAccount($account_id), $response); + return array($this->loadOrCreateAccount($identifiers), $response); } diff --git a/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php b/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php index 2fbc637460..2e603162c3 100644 --- a/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php +++ b/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php @@ -100,13 +100,13 @@ abstract class PhabricatorOAuth1AuthProvider // an access token. try { - $account_id = $adapter->getAccountID(); + $identifiers = $adapter->getAccountIdentifiers(); } catch (Exception $ex) { // TODO: Handle this in a more user-friendly way. throw $ex; } - if (!strlen($account_id)) { + if (!$identifiers) { $response = $controller->buildProviderErrorResponse( $this, pht( @@ -115,7 +115,7 @@ abstract class PhabricatorOAuth1AuthProvider return array($account, $response); } - return array($this->loadOrCreateAccount($account_id), $response); + return array($this->loadOrCreateAccount($identifiers), $response); } public function processEditForm( diff --git a/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php b/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php index a3300126af..7d56506339 100644 --- a/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php +++ b/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php @@ -80,13 +80,13 @@ abstract class PhabricatorOAuth2AuthProvider // an access token. try { - $account_id = $adapter->getAccountID(); + $identifiers = $adapter->getAccountIdentifiers(); } catch (Exception $ex) { // TODO: Handle this in a more user-friendly way. throw $ex; } - if (!strlen($account_id)) { + if (!$identifiers) { $response = $controller->buildProviderErrorResponse( $this, pht( @@ -95,7 +95,7 @@ abstract class PhabricatorOAuth2AuthProvider return array($account, $response); } - return array($this->loadOrCreateAccount($account_id), $response); + return array($this->loadOrCreateAccount($identifiers), $response); } public function processEditForm(