revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00
Code Issues Releases Wiki Activity

Tighten scope requests with Google OAuth

Browse source 
Summary: We currently make a ludicrously gigantic permission request to do Google auth (read/write access to the entire address book), since I couldn't figure out how to do a more narrowly tailored request when I implemented it. @csilvers pointed me at some much more sensible APIs; we can now just ask for user ID, name, and email address.

Test Plan: Created a new account via Google Oauth. Linked/unlinked an existing account. Verified diagnostics page still works correctly. Logged in with a pre-existing Google account created with the old API (to verify user IDs are the same through both methods).

Reviewers: btrahan, vrana, csilvers, Makinde

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2378
This commit is contained in:
epriestley 2012-05-03 13:25:06 -07:00
parent 5604a662df
commit 6a04328430
2 changed files with 14 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
src/applications/auth/oauth/provider/google/PhabricatorOAuthProviderGoogle.php
View file

@ -71,44 +71,31 @@ final class PhabricatorOAuthProviderGoogle extends PhabricatorOAuthProvider {
} }
public function getUserInfoURI() { public function getUserInfoURI() {
return 'https://www.google.com/m8/feeds/contacts/default/full'; return 'https://www.googleapis.com/oauth2/v1/userinfo';
} }
public function getMinimumScope() { public function getMinimumScope() {
// This is the Google contacts API, which is apparently the best way to get $scopes = array(
// the user ID / login / email since Google doesn't apparently have a 'https://www.googleapis.com/auth/userinfo.email',
// more generic "user.info" sort of call (or, if it does, I couldn't find 'https://www.googleapis.com/auth/userinfo.profile',
// it). This is sort of terrifying since it lets Phabricator read your whole );
// address book and possibly your physical address and such, so it would
// be really nice to find a way to restrict this scope to something less return implode(' ', $scopes);
// crazily permissive. But users will click anything and the dialog isn't
// very scary, so whatever.
return 'https://www.google.com/m8/feeds';
} }
public function setUserData($data) { public function setUserData($data) {
// SimpleXMLElement will throw if $data is unusably malformed, which to $data = json_decode($data, true);
// us is just a provider issue $this->validateUserData($data);
try {
$xml = new SimpleXMLElement($data);
} catch (Exception $e) {
throw new PhabricatorOAuthProviderException();
}
$id = (string)$xml->id;
$this->userData = array(
'id' => $id,
'email' => (string)$xml->author[0]->email,
'real' => (string)$xml->author[0]->name,
// Guess account name from email address, this is just a hint anyway. // Guess account name from email address, this is just a hint anyway.
'account' => head(explode('@', $id)), $data['account'] = head(explode('@', $data['email']));
);
$this->userData = $data;
return $this; return $this;
} }
public function retrieveUserID() { public function retrieveUserID() {
return $this->userData['id']; return $this->userData['email'];
} }
public function retrieveUserEmail() { public function retrieveUserEmail() {
@ -130,7 +117,7 @@ final class PhabricatorOAuthProviderGoogle extends PhabricatorOAuthProvider {
} }
public function retrieveUserRealName() { public function retrieveUserRealName() {
return $this->userData['real']; return $this->userData['name'];
} }
public function getExtraAuthParameters() { public function getExtraAuthParameters() {

1
src/applications/auth/oauth/provider/google/__init__.php
View file

@ -7,7 +7,6 @@
phutil_require_module('phabricator', 'applications/auth/oauth/provider/base'); phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
phutil_require_module('phabricator', 'applications/auth/oauth/provider/exception');
phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/env');
phutil_require_module('phutil', 'utils'); phutil_require_module('phutil', 'utils');