Skip to content

Commit 679f778

Browse files
bobtrahanbobtrahan
committed
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
1 parent eb9645e commit 679f778

File tree

11 files changed

+242
-25
lines changed

11 files changed

+242
-25
lines changed

‎conf/default.conf.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@
146146
'phabricator.csrf-key',
147147
'facebook.application-secret',
148148
'github.application-secret',
149+
'google.application-secret',
150+
'phabricator.application-secret',
151+
'disqus.application-secret',
149152
'phabricator.mail-key',
150153
'security.hmac-key',
151154
),
@@ -512,6 +515,25 @@
512515
// The Google "Client Secret" to use for Google API access.
513516
'google.application-secret' => null,
514517

518+
// -- Disqus OAuth ---------------------------------------------------------- //
519+
520+
// Can users use Disqus credentials to login to Phabricator?
521+
'disqus.auth-enabled' => false,
522+
523+
// Can users use Disqus credentials to create new Phabricator accounts?
524+
'disqus.registration-enabled' => true,
525+
526+
// Are Disqus accounts permanently linked to Phabricator accounts, or can
527+
// the user unlink them?
528+
'disqus.auth-permanent' => false,
529+
530+
// The Disqus "Client ID" to use for Disqus API access.
531+
'disqus.application-id' => null,
532+
533+
// The Disqus "Client Secret" to use for Disqus API access.
534+
'disqus.application-secret' => null,
535+
536+
515537
// -- Phabricator OAuth ----------------------------------------------------- //
516538

517539
// Meta-town -- Phabricator is itself an OAuth Provider

‎src/__phutil_library_map__.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,7 @@
731731
'PhabricatorOAuthFailureView' => 'applications/auth/view/oauthfailure',
732732
'PhabricatorOAuthLoginController' => 'applications/auth/controller/oauth',
733733
'PhabricatorOAuthProvider' => 'applications/auth/oauth/provider/base',
734+
'PhabricatorOAuthProviderDisqus' => 'applications/auth/oauth/provider/disqus',
734735
'PhabricatorOAuthProviderException' => 'applications/auth/oauth/provider/exception',
735736
'PhabricatorOAuthProviderFacebook' => 'applications/auth/oauth/provider/facebook',
736737
'PhabricatorOAuthProviderGitHub' => 'applications/auth/oauth/provider/github',
@@ -1646,6 +1647,7 @@
16461647
'PhabricatorOAuthDiagnosticsController' => 'PhabricatorAuthController',
16471648
'PhabricatorOAuthFailureView' => 'AphrontView',
16481649
'PhabricatorOAuthLoginController' => 'PhabricatorAuthController',
1650+
'PhabricatorOAuthProviderDisqus' => 'PhabricatorOAuthProvider',
16491651
'PhabricatorOAuthProviderFacebook' => 'PhabricatorOAuthProvider',
16501652
'PhabricatorOAuthProviderGitHub' => 'PhabricatorOAuthProvider',
16511653
'PhabricatorOAuthProviderGoogle' => 'PhabricatorOAuthProvider',

‎src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,7 @@ public function processRequest() {
5959
}
6060

6161
$userinfo_uri = new PhutilURI($provider->getUserInfoURI());
62-
$userinfo_uri->setQueryParams(
63-
array(
64-
'access_token' => $this->accessToken,
65-
));
62+
$userinfo_uri->setQueryParam('access_token', $this->accessToken);
6663

6764
try {
6865
$user_data = @file_get_contents($userinfo_uri);
@@ -129,9 +126,10 @@ public function processRequest() {
129126
hsprintf(
130127
'<p>Link your %s account to your Phabricator account?</p>',
131128
$provider_name));
132-
$dialog->addHiddenInput('token', $provider->getAccessToken());
129+
$dialog->addHiddenInput('confirm_token', $provider->getAccessToken());
133130
$dialog->addHiddenInput('expires', $oauth_info->getTokenExpires());
134131
$dialog->addHiddenInput('state', $this->oauthState);
132+
$dialog->addHiddenInput('scope', $oauth_info->getTokenScope());
135133
$dialog->addSubmitButton('Link Accounts');
136134
$dialog->addCancelButton('/settings/page/'.$provider_key.'/');
137135

@@ -238,18 +236,18 @@ private function buildErrorResponse(PhabricatorOAuthFailureView $view) {
238236
private function retrieveAccessToken(PhabricatorOAuthProvider $provider) {
239237
$request = $this->getRequest();
240238

241-
$token = $request->getStr('token');
239+
$token = $request->getStr('confirm_token');
242240
if ($token) {
243241
$this->tokenExpires = $request->getInt('expires');
244-
$this->accessToken = $token;
245-
$this->oauthState = $request->getStr('state');
242+
$this->accessToken = $token;
243+
$this->oauthState = $request->getStr('state');
246244
return null;
247245
}
248246

249-
$client_id = $provider->getClientID();
250-
$client_secret = $provider->getClientSecret();
251-
$redirect_uri = $provider->getRedirectURI();
252-
$auth_uri = $provider->getTokenURI();
247+
$client_id = $provider->getClientID();
248+
$client_secret = $provider->getClientSecret();
249+
$redirect_uri = $provider->getRedirectURI();
250+
$auth_uri = $provider->getTokenURI();
253251

254252
$code = $request->getStr('code');
255253
$query_data = array(
@@ -294,12 +292,9 @@ private function retrieveAccessToken(PhabricatorOAuthProvider $provider) {
294292
return $this->buildErrorResponse(new PhabricatorOAuthFailureView());
295293
}
296294

297-
if (idx($data, 'expires')) {
298-
$this->tokenExpires = time() + $data['expires'];
299-
}
300-
301-
$this->accessToken = $token;
302-
$this->oauthState = $request->getStr('state');
295+
$this->tokenExpires = $provider->getTokenExpiryFromArray($data);
296+
$this->accessToken = $token;
297+
$this->oauthState = $request->getStr('state');
303298

304299
return null;
305300
}
@@ -311,16 +306,24 @@ private function retrieveOAuthInfo(PhabricatorOAuthProvider $provider) {
311306
$provider->getProviderKey(),
312307
$provider->retrieveUserID());
313308

309+
$scope = $this->getRequest()->getStr('scope');
310+
314311
if (!$oauth_info) {
315312
$oauth_info = new PhabricatorUserOAuthInfo();
316313
$oauth_info->setOAuthProvider($provider->getProviderKey());
317314
$oauth_info->setOAuthUID($provider->retrieveUserID());
315+
// some providers don't tell you what scope you got, so default
316+
// to the minimum Phabricator requires rather than assuming no scope
317+
if (!$scope) {
318+
$scope = $provider->getMinimumScope();
319+
}
318320
}
319321

320322
$oauth_info->setAccountURI($provider->retrieveUserAccountURI());
321323
$oauth_info->setAccountName($provider->retrieveUserAccountName());
322324
$oauth_info->setToken($provider->getAccessToken());
323325
$oauth_info->setTokenStatus(PhabricatorUserOAuthInfo::TOKEN_STATUS_GOOD);
326+
$oauth_info->setTokenScope($scope);
324327

325328
// If we have out-of-date expiration info, just clear it out. Then replace
326329
// it with good info if the provider gave it to us.
@@ -341,7 +344,4 @@ private function saveOAuthInfo(PhabricatorUserOAuthInfo $info) {
341344
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
342345
$info->save();
343346
}
344-
345-
346-
347347
}

‎src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ abstract class PhabricatorOAuthProvider {
2222
const PROVIDER_GITHUB = 'github';
2323
const PROVIDER_GOOGLE = 'google';
2424
const PROVIDER_PHABRICATOR = 'phabricator';
25+
const PROVIDER_DISQUS = 'disqus';
2526

2627
private $accessToken;
2728

@@ -55,6 +56,21 @@ public function shouldDiagnoseAppLogin() {
5556

5657
abstract public function getTokenURI();
5758

59+
/**
60+
* Access tokens expire based on an implementation-specific key.
61+
*/
62+
abstract protected function getTokenExpiryKey();
63+
public function getTokenExpiryFromArray(array $data) {
64+
$key = $this->getTokenExpiryKey();
65+
if ($key) {
66+
$expiry_value = idx($data, $key, 0);
67+
if ($expiry_value) {
68+
return time() + $expiry_value;
69+
}
70+
}
71+
return 0;
72+
}
73+
5874
/**
5975
* If the provider needs extra stuff in the token request, return it here.
6076
* For example, Google needs a grant_type parameter.
@@ -133,6 +149,9 @@ public static function newProvider($which) {
133149
case self::PROVIDER_PHABRICATOR:
134150
$class = 'PhabricatorOAuthProviderPhabricator';
135151
break;
152+
case self::PROVIDER_DISQUS:
153+
$class = 'PhabricatorOAuthProviderDisqus';
154+
break;
136155
default:
137156
throw new Exception('Unknown OAuth provider.');
138157
}
@@ -146,6 +165,7 @@ public static function getAllProviders() {
146165
self::PROVIDER_GITHUB,
147166
self::PROVIDER_GOOGLE,
148167
self::PROVIDER_PHABRICATOR,
168+
self::PROVIDER_DISQUS,
149169
);
150170
$providers = array();
151171
foreach ($all as $provider) {

‎src/applications/auth/oauth/provider/disqus/PhabricatorOAuthProviderDisqus.php‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
<?php
2+
3+
/*
4+
* Copyright 2012 Facebook, Inc.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
final class PhabricatorOAuthProviderDisqus extends PhabricatorOAuthProvider {
20+
21+
private $userData;
22+
23+
public function getProviderKey() {
24+
return self::PROVIDER_DISQUS;
25+
}
26+
27+
public function getProviderName() {
28+
return 'Disqus';
29+
}
30+
31+
public function isProviderEnabled() {
32+
return PhabricatorEnv::getEnvConfig('disqus.auth-enabled');
33+
}
34+
35+
public function isProviderLinkPermanent() {
36+
return PhabricatorEnv::getEnvConfig('disqus.auth-permanent');
37+
}
38+
39+
public function isProviderRegistrationEnabled() {
40+
return PhabricatorEnv::getEnvConfig('disqus.registration-enabled');
41+
}
42+
43+
public function getClientID() {
44+
return PhabricatorEnv::getEnvConfig('disqus.application-id');
45+
}
46+
47+
public function renderGetClientIDHelp() {
48+
return null;
49+
}
50+
51+
public function getClientSecret() {
52+
return PhabricatorEnv::getEnvConfig('disqus.application-secret');
53+
}
54+
55+
public function renderGetClientSecretHelp() {
56+
return null;
57+
}
58+
59+
public function getAuthURI() {
60+
return 'https://disqus.com/api/oauth/2.0/authorize/';
61+
}
62+
63+
public function getTokenURI() {
64+
return 'https://disqus.com/api/oauth/2.0/access_token/';
65+
}
66+
67+
protected function getTokenExpiryKey() {
68+
return 'expires_in';
69+
}
70+
71+
public function getExtraAuthParameters() {
72+
return array(
73+
'response_type' => 'code',
74+
);
75+
}
76+
77+
public function getExtraTokenParameters() {
78+
return array(
79+
'grant_type' => 'authorization_code',
80+
);
81+
}
82+
83+
public function decodeTokenResponse($response) {
84+
return json_decode($response, true);
85+
}
86+
87+
public function getTestURIs() {
88+
return array(
89+
'http://disqus.com',
90+
$this->getUserInfoURI(),
91+
);
92+
}
93+
94+
public function getUserInfoURI() {
95+
return 'https://disqus.com/api/3.0/users/details.json?'.
96+
'api_key='.$this->getClientID();
97+
}
98+
99+
public function getMinimumScope() {
100+
return 'read';
101+
}
102+
103+
public function setUserData($data) {
104+
$data = idx(json_decode($data, true), 'response');
105+
$this->validateUserData($data);
106+
$this->userData = $data;
107+
return $this;
108+
}
109+
110+
public function retrieveUserID() {
111+
return $this->userData['id'];
112+
}
113+
114+
public function retrieveUserEmail() {
115+
return idx($this->userData, 'email');
116+
}
117+
118+
public function retrieveUserAccountName() {
119+
return $this->userData['username'];
120+
}
121+
122+
public function retrieveUserProfileImage() {
123+
$avatar = idx($this->userData, 'avatar');
124+
if ($avatar) {
125+
$uri = idx($avatar, 'permalink');
126+
if ($uri) {
127+
return @file_get_contents($uri);
128+
}
129+
}
130+
return null;
131+
}
132+
133+
public function retrieveUserAccountURI() {
134+
return idx($this->userData, 'profileUrl');
135+
}
136+
137+
public function retrieveUserRealName() {
138+
return idx($this->userData, 'name');
139+
}
140+
141+
public function shouldDiagnoseAppLogin() {
142+
return true;
143+
}
144+
}

‎src/applications/auth/oauth/provider/disqus/__init__.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
/**
3+
* This file is automatically generated. Lint this module to rebuild it.
4+
* @generated
5+
*/
6+
7+
8+
9+
phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
10+
phutil_require_module('phabricator', 'infrastructure/env');
11+
12+
phutil_require_module('phutil', 'utils');
13+
14+
15+
phutil_require_source('PhabricatorOAuthProviderDisqus.php');

‎src/applications/auth/oauth/provider/facebook/PhabricatorOAuthProviderFacebook.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ public function getTokenURI() {
7373
return 'https://graph.facebook.com/oauth/access_token';
7474
}
7575

76+
protected function getTokenExpiryKey() {
77+
return 'expires';
78+
}
79+
7680
public function getUserInfoURI() {
7781
return 'https://graph.facebook.com/me';
7882
}

‎src/applications/auth/oauth/provider/github/PhabricatorOAuthProviderGitHub.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ public function getTokenURI() {
6464
return 'https://github.com/login/oauth/access_token';
6565
}
6666

67+
protected function getTokenExpiryKey() {
68+
// github access tokens do not have time-based expiry
69+
return null;
70+
}
71+
6772
public function getTestURIs() {
6873
return array(
6974
'http://github.com',

‎src/applications/auth/oauth/provider/google/PhabricatorOAuthProviderGoogle.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ public function getTokenURI() {
7070
return 'https://accounts.google.com/o/oauth2/token';
7171
}
7272

73+
protected function getTokenExpiryKey() {
74+
return 'expires_in';
75+
}
76+
7377
public function getUserInfoURI() {
7478
return 'https://www.googleapis.com/oauth2/v1/userinfo';
7579
}

0 commit comments

Comments
 (0)