From 8744cdb69981c754dc9ef4a056f5cb474b41ef5f Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 16 Jun 2013 09:55:55 -0700 Subject: [PATCH] Migrate PhabricatorUserLDAPInfo to PhabricatorExternalAccount Summary: Ref T1536. This is similar to D6172 but much simpler: we don't need to retain external interfaces here and can do a straight migration. Test Plan: TBA Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6173 --- resources/sql/patches/20130611.nukeldap.php | 41 +++++++++++++++++++ src/__phutil_library_map__.php | 2 - .../PhabricatorLDAPLoginController.php | 31 ++++++++------ .../PhabricatorLDAPRegistrationController.php | 2 +- .../PhabricatorLDAPUnlinkController.php | 12 +++--- .../PhabricatorPeopleLdapController.php | 10 +++-- .../people/editor/PhabricatorUserEditor.php | 7 ---- .../storage/PhabricatorUserLDAPInfo.php | 6 --- .../panel/PhabricatorSettingsPanelLDAP.php | 10 +++-- .../patch/PhabricatorBuiltinPatchList.php | 4 ++ 10 files changed, 84 insertions(+), 41 deletions(-) create mode 100644 resources/sql/patches/20130611.nukeldap.php delete mode 100644 src/applications/people/storage/PhabricatorUserLDAPInfo.php diff --git a/resources/sql/patches/20130611.nukeldap.php b/resources/sql/patches/20130611.nukeldap.php new file mode 100644 index 0000000000..ee97bb64ab --- /dev/null +++ b/resources/sql/patches/20130611.nukeldap.php @@ -0,0 +1,41 @@ +establishConnection('w'); + +$xaccount = new PhabricatorExternalAccount(); + +echo "Migrating LDAP to ExternalAccount...\n"; + +$rows = queryfx_all($conn_w, 'SELECT * FROM %T', $table_name); +foreach ($rows as $row) { + echo "Migrating row ID #".$row['id'].".\n"; + $user = id(new PhabricatorUser())->loadOneWhere( + 'id = %d', + $row['userID']); + if (!$user) { + echo "Bad user ID!\n"; + continue; + } + + + $xaccount = id(new PhabricatorExternalAccount()) + ->setUserPHID($user->getPHID()) + ->setAccountType('ldap') + ->setAccountDomain('self') + ->setAccountID($row['ldapUsername']) + ->setUsername($row['ldapUsername']) + ->setDateCreated($row['dateCreated']); + + try { + $xaccount->save(); + } catch (Exception $ex) { + phlog($ex); + } +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d81b7bec6c..b18cd32d16 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1554,7 +1554,6 @@ phutil_register_library_map(array( 'PhabricatorUserDAO' => 'applications/people/storage/PhabricatorUserDAO.php', 'PhabricatorUserEditor' => 'applications/people/editor/PhabricatorUserEditor.php', 'PhabricatorUserEmail' => 'applications/people/storage/PhabricatorUserEmail.php', - 'PhabricatorUserLDAPInfo' => 'applications/people/storage/PhabricatorUserLDAPInfo.php', 'PhabricatorUserLog' => 'applications/people/storage/PhabricatorUserLog.php', 'PhabricatorUserOAuthInfo' => 'applications/people/storage/PhabricatorUserOAuthInfo.php', 'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php', @@ -3403,7 +3402,6 @@ phutil_register_library_map(array( 'PhabricatorUserDAO' => 'PhabricatorLiskDAO', 'PhabricatorUserEditor' => 'PhabricatorEditor', 'PhabricatorUserEmail' => 'PhabricatorUserDAO', - 'PhabricatorUserLDAPInfo' => 'PhabricatorUserDAO', 'PhabricatorUserLog' => 'PhabricatorUserDAO', 'PhabricatorUserPreferences' => 'PhabricatorUserDAO', 'PhabricatorUserProfile' => 'PhabricatorUserDAO', diff --git a/src/applications/auth/controller/PhabricatorLDAPLoginController.php b/src/applications/auth/controller/PhabricatorLDAPLoginController.php index bbe64ccbd6..945e78dc61 100644 --- a/src/applications/auth/controller/PhabricatorLDAPLoginController.php +++ b/src/applications/auth/controller/PhabricatorLDAPLoginController.php @@ -34,11 +34,13 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController { if ($current_user->getPHID()) { if ($ldap_info->getID()) { - $existing_ldap = id(new PhabricatorUserLDAPInfo())->loadOneWhere( - 'userID = %d', - $current_user->getID()); + $existing_ldap = id(new PhabricatorExternalAccount())->loadOneWhere( + 'accountType = %s AND accountDomain = %s AND userPHID = %s', + 'ldap', + 'self', + $current_user->getPHID()); - if ($ldap_info->getUserID() != $current_user->getID() || + if ($ldap_info->getUserPHID() != $current_user->getPHID() || $existing_ldap) { $dialog = new AphrontDialogView(); $dialog->setUser($current_user); @@ -71,7 +73,7 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController { return id(new AphrontDialogResponse())->setDialog($dialog); } - $ldap_info->setUserID($current_user->getID()); + $ldap_info->setUserPHID($current_user->getPHID()); $this->saveLDAPInfo($ldap_info); @@ -82,8 +84,9 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController { if ($ldap_info->getID()) { $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $known_user = id(new PhabricatorUser())->load( - $ldap_info->getUserID()); + $known_user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $ldap_info->getUserPHID()); $session_key = $known_user->establishSession('web'); @@ -152,19 +155,23 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController { } private function retrieveLDAPInfo(PhabricatorLDAPProvider $provider) { - $ldap_info = id(new PhabricatorUserLDAPInfo())->loadOneWhere( - 'ldapUsername = %s', + $ldap_info = id(new PhabricatorExternalAccount())->loadOneWhere( + 'accountType = %s AND accountDomain = %s AND accountID = %s', + 'ldap', + 'self', $provider->retrieveUsername()); if (!$ldap_info) { - $ldap_info = new PhabricatorUserLDAPInfo(); - $ldap_info->setLDAPUsername($provider->retrieveUsername()); + $ldap_info = id(new PhabricatorExternalAccount()) + ->setAccountType('ldap') + ->setAccountDomain('self') + ->setAccountID($provider->retrieveUsername()); } return $ldap_info; } - private function saveLDAPInfo(PhabricatorUserLDAPInfo $info) { + private function saveLDAPInfo(PhabricatorExternalAccount $info) { // UNGUARDED WRITES: Logging-in users don't have their CSRF set up yet. $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $info->save(); diff --git a/src/applications/auth/controller/PhabricatorLDAPRegistrationController.php b/src/applications/auth/controller/PhabricatorLDAPRegistrationController.php index e52eea2134..2901b93d7a 100644 --- a/src/applications/auth/controller/PhabricatorLDAPRegistrationController.php +++ b/src/applications/auth/controller/PhabricatorLDAPRegistrationController.php @@ -116,7 +116,7 @@ extends PhabricatorAuthController { ->setActor($user) ->createNewUser($user, $email_obj); - $ldap_info->setUserID($user->getID()); + $ldap_info->setUserPHID($user->getPHID()); $ldap_info->save(); $session_key = $user->establishSession('web'); diff --git a/src/applications/auth/controller/PhabricatorLDAPUnlinkController.php b/src/applications/auth/controller/PhabricatorLDAPUnlinkController.php index c2c7aaf34b..16a5952d75 100644 --- a/src/applications/auth/controller/PhabricatorLDAPUnlinkController.php +++ b/src/applications/auth/controller/PhabricatorLDAPUnlinkController.php @@ -6,11 +6,13 @@ final class PhabricatorLDAPUnlinkController extends PhabricatorAuthController { $request = $this->getRequest(); $user = $request->getUser(); - $ldap_info = id(new PhabricatorUserLDAPInfo())->loadOneWhere( - 'userID = %d', - $user->getID()); + $ldap_account = id(new PhabricatorExternalAccount())->loadOneWhere( + 'userPHID = %s AND accountType = %s AND accountDomain = %s', + $user->getPHID(), + 'ldap', + 'self'); - if (!$ldap_info) { + if (!$ldap_account) { return new Aphront400Response(); } @@ -27,7 +29,7 @@ final class PhabricatorLDAPUnlinkController extends PhabricatorAuthController { return id(new AphrontDialogResponse())->setDialog($dialog); } - $ldap_info->delete(); + $ldap_account->delete(); return id(new AphrontRedirectResponse()) ->setURI('/settings/panel/ldap/'); diff --git a/src/applications/people/controller/PhabricatorPeopleLdapController.php b/src/applications/people/controller/PhabricatorPeopleLdapController.php index f5a2903f32..1a7307301f 100644 --- a/src/applications/people/controller/PhabricatorPeopleLdapController.php +++ b/src/applications/people/controller/PhabricatorPeopleLdapController.php @@ -96,10 +96,12 @@ final class PhabricatorPeopleLdapController ->setActor($admin) ->createNewUser($user, $email_obj); - $ldap_info = new PhabricatorUserLDAPInfo(); - $ldap_info->setLDAPUsername($username); - $ldap_info->setUserID($user->getID()); - $ldap_info->save(); + id(new PhabricatorExternalAccount()) + ->setUserPHID($user->getPHID()) + ->setAccountType('ldap') + ->setAccountDomain('self') + ->setAccountID($username) + ->save(); $header = pht('Successfully added %s', $username); $attribute = null; diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php index 29d081bed4..f8fb6811a6 100644 --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -303,13 +303,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor { } $user->openTransaction(); - $ldaps = id(new PhabricatorUserLDAPInfo())->loadAllWhere( - 'userID = %d', - $user->getID()); - foreach ($ldaps as $ldap) { - $ldap->delete(); - } - $externals = id(new PhabricatorExternalAccount())->loadAllWhere( 'userPHID = %s', $user->getPHID()); diff --git a/src/applications/people/storage/PhabricatorUserLDAPInfo.php b/src/applications/people/storage/PhabricatorUserLDAPInfo.php deleted file mode 100644 index 2173c48b9c..0000000000 --- a/src/applications/people/storage/PhabricatorUserLDAPInfo.php +++ /dev/null @@ -1,6 +0,0 @@ -getUser(); - $ldap_info = id(new PhabricatorUserLDAPInfo())->loadOneWhere( - 'userID = %d', - $user->getID()); + $ldap_account = id(new PhabricatorExternalAccount())->loadOneWhere( + 'userPHID = %s AND accountType = %s AND accountDomain = %s', + $user->getPHID(), + 'ldap', + 'self'); $forms = array(); - if (!$ldap_info) { + if (!$ldap_account) { $unlink = pht('Link LDAP Account'); $unlink_form = new AphrontFormView(); $unlink_form diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index d635d8ce47..58f713a77c 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1362,6 +1362,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'php', 'name' => $this->getPatchPath('20130611.migrateoauth.php'), ), + '20130611.nukeldap.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130611.nukeldap.php'), + ), ); } }