From bce4b7addfce832d14476302dddd8712f67b5817 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 14 Jun 2013 07:00:29 -0700 Subject: [PATCH] Internalize storage access for PhabricatorUserOAuthInfo Summary: Ref T1536. Move all access to the underlying storage to inside the class. My plan is: - Migrate the table to ExternalAccount. - Nuke the table. - Make this class read from and write to ExternalAccount instead. We can't get rid of OAuthInfo completely because Facebook still depends on it for now, via registration hooks. Test Plan: Logged in and registered with OAuth. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6171 --- .../PhabricatorOAuthLoginController.php | 27 ++--------------- .../PhabricatorOAuthUnlinkController.php | 5 ++-- .../PhabricatorPeopleProfileController.php | 4 +-- .../storage/PhabricatorUserOAuthInfo.php | 29 +++++++++++++++++++ .../panel/PhabricatorSettingsPanelOAuth.php | 7 ++--- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/applications/auth/controller/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/PhabricatorOAuthLoginController.php index 1f36e06df3..8604f8dfe6 100644 --- a/src/applications/auth/controller/PhabricatorOAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorOAuthLoginController.php @@ -89,9 +89,8 @@ final class PhabricatorOAuthLoginController } } - $existing_oauth = id(new PhabricatorUserOAuthInfo())->loadOneWhere( - 'userID = %d AND oauthProvider = %s', - $current_user->getID(), + $existing_oauth = PhabricatorUserOAuthInfo::loadOneByUserAndProviderKey( + $current_user, $provider_key); if ($existing_oauth) { @@ -182,25 +181,6 @@ final class PhabricatorOAuthLoginController 'Settings.', $provider_name))); - $user = id(new PhabricatorUser()) - ->loadOneWhere('phid = %s', $known_email->getUserPHID()); - $oauth_infos = id(new PhabricatorUserOAuthInfo()) - ->loadAllWhere('userID = %d', $user->getID()); - if ($oauth_infos) { - $providers = array(); - foreach ($oauth_infos as $info) { - $provider = $info->getOAuthProvider(); - $providers[] = PhabricatorOAuthProvider::newProvider($provider) - ->getProviderName(); - } - $dialog->appendChild(phutil_tag( - 'p', - array(), - pht( - 'The account is associated with: %s.', - implode(', ', $providers)))); - } - $dialog->addCancelButton('/login/'); return id(new AphrontDialogResponse())->setDialog($dialog); @@ -300,8 +280,7 @@ final class PhabricatorOAuthLoginController private function retrieveOAuthInfo(PhabricatorOAuthProvider $provider) { - $oauth_info = id(new PhabricatorUserOAuthInfo())->loadOneWhere( - 'oauthProvider = %s and oauthUID = %s', + $oauth_info = PhabricatorUserOAuthInfo::loadOneByProviderKeyAndAccountID( $provider->getProviderKey(), $provider->retrieveUserID()); diff --git a/src/applications/auth/controller/PhabricatorOAuthUnlinkController.php b/src/applications/auth/controller/PhabricatorOAuthUnlinkController.php index 97be2b7b8c..38ab3c12e9 100644 --- a/src/applications/auth/controller/PhabricatorOAuthUnlinkController.php +++ b/src/applications/auth/controller/PhabricatorOAuthUnlinkController.php @@ -21,9 +21,8 @@ final class PhabricatorOAuthUnlinkController extends PhabricatorAuthController { $provider_key = $provider->getProviderKey(); - $oauth_info = id(new PhabricatorUserOAuthInfo())->loadOneWhere( - 'userID = %d AND oauthProvider = %s', - $user->getID(), + $oauth_info = PhabricatorUserOAuthInfo::loadOneByUserAndProviderKey( + $user, $provider_key); if (!$oauth_info) { diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 4f68031126..d3c92507ef 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -64,9 +64,7 @@ final class PhabricatorPeopleProfileController // NOTE: applications install the various links through PhabricatorEvent // listeners - $oauths = id(new PhabricatorUserOAuthInfo())->loadAllWhere( - 'userID = %d', - $user->getID()); + $oauths = PhabricatorUserOAuthInfo::loadAllOAuthProvidersByUser($user); $oauths = mpull($oauths, null, 'getOAuthProvider'); $providers = PhabricatorOAuthProvider::getAllProviders(); diff --git a/src/applications/people/storage/PhabricatorUserOAuthInfo.php b/src/applications/people/storage/PhabricatorUserOAuthInfo.php index 6fcb332354..b1efb259d9 100644 --- a/src/applications/people/storage/PhabricatorUserOAuthInfo.php +++ b/src/applications/people/storage/PhabricatorUserOAuthInfo.php @@ -14,4 +14,33 @@ final class PhabricatorUserOAuthInfo extends PhabricatorUserDAO { protected $tokenScope = ''; protected $tokenStatus = 'unused'; + public static function loadOneByUserAndProviderKey( + PhabricatorUser $user, + $provider_key) { + + return id(new PhabricatorUserOAuthInfo())->loadOneWhere( + 'userID = %d AND oauthProvider = %s', + $user->getID(), + $provider_key); + } + + public static function loadAllOAuthProvidersByUser( + PhabricatorUser $user) { + + return id(new PhabricatorUserOAuthInfo())->loadAllWhere( + 'userID = %d', + $user->getID()); + } + + public static function loadOneByProviderKeyAndAccountID( + $provider_key, + $account_id) { + + return id(new PhabricatorUserOAuthInfo())->loadOneWhere( + 'oauthProvider = %s and oauthUID = %s', + $provider_key, + $account_id); + } + + } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php b/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php index 6800fec3ed..8143d21865 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php @@ -68,10 +68,9 @@ final class PhabricatorSettingsPanelOAuth $provider_name = $provider->getProviderName(); $provider_key = $provider->getProviderKey(); - $oauth_info = id(new PhabricatorUserOAuthInfo())->loadOneWhere( - 'userID = %d AND oauthProvider = %s', - $user->getID(), - $provider->getProviderKey()); + $oauth_info = PhabricatorUserOAuthInfo::loadOneByUserAndProviderKey( + $user, + $provider_key); $form = new AphrontFormView(); $form->setUser($user);