Page MenuHomePhabricator

D21013.id50062.diff
No OneTemporary

D21013.id50062.diff

diff --git a/src/applications/auth/adapter/PhutilAuthAdapter.php b/src/applications/auth/adapter/PhutilAuthAdapter.php
--- a/src/applications/auth/adapter/PhutilAuthAdapter.php
+++ b/src/applications/auth/adapter/PhutilAuthAdapter.php
@@ -34,6 +34,11 @@
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
--- a/src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php
+++ b/src/applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php
@@ -56,9 +56,12 @@
$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
--- a/src/applications/auth/provider/PhabricatorAuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorAuthProvider.php
@@ -190,40 +190,52 @@
return;
}
- protected function loadOrCreateAccount($account_id) {
- if (!strlen($account_id)) {
- throw new Exception(pht('Empty account ID!'));
- }
-
- $adapter = $this->getAdapter();
- $adapter_class = get_class($adapter);
+ protected function loadOrCreateAccount(array $identifiers) {
+ assert_instances_of($identifiers, 'PhabricatorExternalAccountIdentifier');
- if (!strlen($adapter->getAdapterType())) {
+ if (!$identifiers) {
throw new Exception(
pht(
- "AuthAdapter (of class '%s') has an invalid implementation: ".
- "no adapter type.",
- $adapter_class));
+ 'Authentication provider (of class "%s") is attempting to '.
+ 'load or create an external account, but provided no account '.
+ 'identifiers.',
+ get_class($this)));
}
- if (!strlen($adapter->getAdapterDomain())) {
+ if (count($identifiers) !== 1) {
throw new Exception(
pht(
- "AuthAdapter (of class '%s') has an invalid implementation: ".
- "no adapter domain.",
- $adapter_class));
+ 'Unexpected number of account identifiers returned (by class "%s").',
+ get_class($this)));
}
- $account = id(new PhabricatorExternalAccount())->loadOneWhere(
- 'accountType = %s AND accountDomain = %s AND accountID = %s',
- $adapter->getAdapterType(),
- $adapter->getAdapterDomain(),
- $account_id);
- if (!$account) {
+ $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($account_id);
+ ->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();
+
$account->setUsername($adapter->getAccountName());
$account->setRealName($adapter->getAccountRealName());
$account->setEmail($adapter->getAccountEmail());
@@ -240,6 +252,7 @@
// 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
--- a/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php
@@ -164,7 +164,7 @@
// 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 @@
}
}
- 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
--- a/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php
@@ -100,13 +100,13 @@
// 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 @@
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
--- a/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php
@@ -80,13 +80,13 @@
// 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 @@
return array($account, $response);
}
- return array($this->loadOrCreateAccount($account_id), $response);
+ return array($this->loadOrCreateAccount($identifiers), $response);
}
public function processEditForm(

File Metadata

Mime Type
text/plain
Expires
Tue, Oct 15, 2:09 PM (2 d, 16 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6713100
Default Alt Text
D21013.id50062.diff (7 KB)

Event Timeline