From 679f778235b40c4ddd4c8b86e9446b84f29a4dd5 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 8 May 2012 12:08:05 -0700 Subject: [PATCH] OAuth -- add support for Disqus Summary: also fix some bugs where we weren't properly capturing the expiry value or scope of access tokens. This code isn't the cleanest as some providers don't confirm what scope you've been granted. In that case, assume the access token is of the minimum scope Phabricator requires. This seems more useful to me as only Phabricator at the moment really easily / consistently lets the user increase / decrease the granted scope so its basically always the correct assumption at the time we make it. Test Plan: linked and unlinked Phabricator, Github, Disqus and Facebook accounts from Phabricator instaneces Reviewers: epriestley Reviewed By: epriestley CC: zeeg, aran, Koolvin Maniphest Tasks: T1110 Differential Revision: https://secure.phabricator.com/D2431 --- conf/default.conf.php | 22 +++ src/__phutil_library_map__.php | 2 + .../oauth/PhabricatorOAuthLoginController.php | 42 ++--- .../base/PhabricatorOAuthProvider.php | 20 +++ .../disqus/PhabricatorOAuthProviderDisqus.php | 144 ++++++++++++++++++ .../auth/oauth/provider/disqus/__init__.php | 15 ++ .../PhabricatorOAuthProviderFacebook.php | 4 + .../github/PhabricatorOAuthProviderGitHub.php | 5 + .../google/PhabricatorOAuthProviderGoogle.php | 4 + .../PhabricatorOAuthProviderPhabricator.php | 4 + ...icatorUserOAuthSettingsPanelController.php | 5 +- 11 files changed, 242 insertions(+), 25 deletions(-) create mode 100644 src/applications/auth/oauth/provider/disqus/PhabricatorOAuthProviderDisqus.php create mode 100644 src/applications/auth/oauth/provider/disqus/__init__.php diff --git a/conf/default.conf.php b/conf/default.conf.php index f89f122bf2..a54395cc56 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -146,6 +146,9 @@ return array( 'phabricator.csrf-key', 'facebook.application-secret', 'github.application-secret', + 'google.application-secret', + 'phabricator.application-secret', + 'disqus.application-secret', 'phabricator.mail-key', 'security.hmac-key', ), @@ -512,6 +515,25 @@ return array( // The Google "Client Secret" to use for Google API access. 'google.application-secret' => null, +// -- Disqus OAuth ---------------------------------------------------------- // + + // Can users use Disqus credentials to login to Phabricator? + 'disqus.auth-enabled' => false, + + // Can users use Disqus credentials to create new Phabricator accounts? + 'disqus.registration-enabled' => true, + + // Are Disqus accounts permanently linked to Phabricator accounts, or can + // the user unlink them? + 'disqus.auth-permanent' => false, + + // The Disqus "Client ID" to use for Disqus API access. + 'disqus.application-id' => null, + + // The Disqus "Client Secret" to use for Disqus API access. + 'disqus.application-secret' => null, + + // -- Phabricator OAuth ----------------------------------------------------- // // Meta-town -- Phabricator is itself an OAuth Provider diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d27cffbc88..3515566b4c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -731,6 +731,7 @@ phutil_register_library_map(array( 'PhabricatorOAuthFailureView' => 'applications/auth/view/oauthfailure', 'PhabricatorOAuthLoginController' => 'applications/auth/controller/oauth', 'PhabricatorOAuthProvider' => 'applications/auth/oauth/provider/base', + 'PhabricatorOAuthProviderDisqus' => 'applications/auth/oauth/provider/disqus', 'PhabricatorOAuthProviderException' => 'applications/auth/oauth/provider/exception', 'PhabricatorOAuthProviderFacebook' => 'applications/auth/oauth/provider/facebook', 'PhabricatorOAuthProviderGitHub' => 'applications/auth/oauth/provider/github', @@ -1646,6 +1647,7 @@ phutil_register_library_map(array( 'PhabricatorOAuthDiagnosticsController' => 'PhabricatorAuthController', 'PhabricatorOAuthFailureView' => 'AphrontView', 'PhabricatorOAuthLoginController' => 'PhabricatorAuthController', + 'PhabricatorOAuthProviderDisqus' => 'PhabricatorOAuthProvider', 'PhabricatorOAuthProviderFacebook' => 'PhabricatorOAuthProvider', 'PhabricatorOAuthProviderGitHub' => 'PhabricatorOAuthProvider', 'PhabricatorOAuthProviderGoogle' => 'PhabricatorOAuthProvider', diff --git a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php index adb21f782b..9bd82c1152 100644 --- a/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php +++ b/src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php @@ -59,10 +59,7 @@ final class PhabricatorOAuthLoginController } $userinfo_uri = new PhutilURI($provider->getUserInfoURI()); - $userinfo_uri->setQueryParams( - array( - 'access_token' => $this->accessToken, - )); + $userinfo_uri->setQueryParam('access_token', $this->accessToken); try { $user_data = @file_get_contents($userinfo_uri); @@ -129,9 +126,10 @@ final class PhabricatorOAuthLoginController hsprintf( '

Link your %s account to your Phabricator account?

', $provider_name)); - $dialog->addHiddenInput('token', $provider->getAccessToken()); + $dialog->addHiddenInput('confirm_token', $provider->getAccessToken()); $dialog->addHiddenInput('expires', $oauth_info->getTokenExpires()); $dialog->addHiddenInput('state', $this->oauthState); + $dialog->addHiddenInput('scope', $oauth_info->getTokenScope()); $dialog->addSubmitButton('Link Accounts'); $dialog->addCancelButton('/settings/page/'.$provider_key.'/'); @@ -238,18 +236,18 @@ final class PhabricatorOAuthLoginController private function retrieveAccessToken(PhabricatorOAuthProvider $provider) { $request = $this->getRequest(); - $token = $request->getStr('token'); + $token = $request->getStr('confirm_token'); if ($token) { $this->tokenExpires = $request->getInt('expires'); - $this->accessToken = $token; - $this->oauthState = $request->getStr('state'); + $this->accessToken = $token; + $this->oauthState = $request->getStr('state'); return null; } - $client_id = $provider->getClientID(); - $client_secret = $provider->getClientSecret(); - $redirect_uri = $provider->getRedirectURI(); - $auth_uri = $provider->getTokenURI(); + $client_id = $provider->getClientID(); + $client_secret = $provider->getClientSecret(); + $redirect_uri = $provider->getRedirectURI(); + $auth_uri = $provider->getTokenURI(); $code = $request->getStr('code'); $query_data = array( @@ -294,12 +292,9 @@ final class PhabricatorOAuthLoginController return $this->buildErrorResponse(new PhabricatorOAuthFailureView()); } - if (idx($data, 'expires')) { - $this->tokenExpires = time() + $data['expires']; - } - - $this->accessToken = $token; - $this->oauthState = $request->getStr('state'); + $this->tokenExpires = $provider->getTokenExpiryFromArray($data); + $this->accessToken = $token; + $this->oauthState = $request->getStr('state'); return null; } @@ -311,16 +306,24 @@ final class PhabricatorOAuthLoginController $provider->getProviderKey(), $provider->retrieveUserID()); + $scope = $this->getRequest()->getStr('scope'); + if (!$oauth_info) { $oauth_info = new PhabricatorUserOAuthInfo(); $oauth_info->setOAuthProvider($provider->getProviderKey()); $oauth_info->setOAuthUID($provider->retrieveUserID()); + // some providers don't tell you what scope you got, so default + // to the minimum Phabricator requires rather than assuming no scope + if (!$scope) { + $scope = $provider->getMinimumScope(); + } } $oauth_info->setAccountURI($provider->retrieveUserAccountURI()); $oauth_info->setAccountName($provider->retrieveUserAccountName()); $oauth_info->setToken($provider->getAccessToken()); $oauth_info->setTokenStatus(PhabricatorUserOAuthInfo::TOKEN_STATUS_GOOD); + $oauth_info->setTokenScope($scope); // If we have out-of-date expiration info, just clear it out. Then replace // it with good info if the provider gave it to us. @@ -341,7 +344,4 @@ final class PhabricatorOAuthLoginController $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $info->save(); } - - - } diff --git a/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php b/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php index 5edfc6f76e..c9f87464aa 100644 --- a/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php +++ b/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php @@ -22,6 +22,7 @@ abstract class PhabricatorOAuthProvider { const PROVIDER_GITHUB = 'github'; const PROVIDER_GOOGLE = 'google'; const PROVIDER_PHABRICATOR = 'phabricator'; + const PROVIDER_DISQUS = 'disqus'; private $accessToken; @@ -55,6 +56,21 @@ abstract class PhabricatorOAuthProvider { abstract public function getTokenURI(); + /** + * Access tokens expire based on an implementation-specific key. + */ + abstract protected function getTokenExpiryKey(); + public function getTokenExpiryFromArray(array $data) { + $key = $this->getTokenExpiryKey(); + if ($key) { + $expiry_value = idx($data, $key, 0); + if ($expiry_value) { + return time() + $expiry_value; + } + } + return 0; + } + /** * If the provider needs extra stuff in the token request, return it here. * For example, Google needs a grant_type parameter. @@ -133,6 +149,9 @@ abstract class PhabricatorOAuthProvider { case self::PROVIDER_PHABRICATOR: $class = 'PhabricatorOAuthProviderPhabricator'; break; + case self::PROVIDER_DISQUS: + $class = 'PhabricatorOAuthProviderDisqus'; + break; default: throw new Exception('Unknown OAuth provider.'); } @@ -146,6 +165,7 @@ abstract class PhabricatorOAuthProvider { self::PROVIDER_GITHUB, self::PROVIDER_GOOGLE, self::PROVIDER_PHABRICATOR, + self::PROVIDER_DISQUS, ); $providers = array(); foreach ($all as $provider) { diff --git a/src/applications/auth/oauth/provider/disqus/PhabricatorOAuthProviderDisqus.php b/src/applications/auth/oauth/provider/disqus/PhabricatorOAuthProviderDisqus.php new file mode 100644 index 0000000000..ad07d3cad0 --- /dev/null +++ b/src/applications/auth/oauth/provider/disqus/PhabricatorOAuthProviderDisqus.php @@ -0,0 +1,144 @@ + 'code', + ); + } + + public function getExtraTokenParameters() { + return array( + 'grant_type' => 'authorization_code', + ); + } + + public function decodeTokenResponse($response) { + return json_decode($response, true); + } + + public function getTestURIs() { + return array( + 'http://disqus.com', + $this->getUserInfoURI(), + ); + } + + public function getUserInfoURI() { + return 'https://disqus.com/api/3.0/users/details.json?'. + 'api_key='.$this->getClientID(); + } + + public function getMinimumScope() { + return 'read'; + } + + public function setUserData($data) { + $data = idx(json_decode($data, true), 'response'); + $this->validateUserData($data); + $this->userData = $data; + return $this; + } + + public function retrieveUserID() { + return $this->userData['id']; + } + + public function retrieveUserEmail() { + return idx($this->userData, 'email'); + } + + public function retrieveUserAccountName() { + return $this->userData['username']; + } + + public function retrieveUserProfileImage() { + $avatar = idx($this->userData, 'avatar'); + if ($avatar) { + $uri = idx($avatar, 'permalink'); + if ($uri) { + return @file_get_contents($uri); + } + } + return null; + } + + public function retrieveUserAccountURI() { + return idx($this->userData, 'profileUrl'); + } + + public function retrieveUserRealName() { + return idx($this->userData, 'name'); + } + + public function shouldDiagnoseAppLogin() { + return true; + } +} diff --git a/src/applications/auth/oauth/provider/disqus/__init__.php b/src/applications/auth/oauth/provider/disqus/__init__.php new file mode 100644 index 0000000000..a57e19803a --- /dev/null +++ b/src/applications/auth/oauth/provider/disqus/__init__.php @@ -0,0 +1,15 @@ +getURI('/oauthserver/token/'); } + protected function getTokenExpiryKey() { + return 'expires_in'; + } + public function getUserInfoURI() { return $this->getURI('/api/user.whoami'); } diff --git a/src/applications/people/controller/settings/panels/oauth/PhabricatorUserOAuthSettingsPanelController.php b/src/applications/people/controller/settings/panels/oauth/PhabricatorUserOAuthSettingsPanelController.php index 37502edbfc..2c00bc441b 100644 --- a/src/applications/people/controller/settings/panels/oauth/PhabricatorUserOAuthSettingsPanelController.php +++ b/src/applications/people/controller/settings/panels/oauth/PhabricatorUserOAuthSettingsPanelController.php @@ -205,10 +205,7 @@ final class PhabricatorUserOAuthSettingsPanelController $userinfo_uri = new PhutilURI($provider->getUserInfoURI()); $token = $oauth_info->getToken(); try { - $userinfo_uri->setQueryParams( - array( - 'access_token' => $token, - )); + $userinfo_uri->setQueryParam('access_token', $token); $user_data = @file_get_contents($userinfo_uri); $provider->setUserData($user_data); $provider->setAccessToken($token);