Skip to content

Commit

Permalink
Revert "Use new people.get api instead of oauth2/v1/userinfo."
Browse files Browse the repository at this point in the history 
This fixes a problem where we can't get userinfo for domains that have G+
APIs disabled (http://crbug.com/387707).

This reverts commit 431a075.

BUG=320354,387707
TBR=rogerta@chromium.org

Review URL: https://codereview.chromium.org/353073003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280081 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
atwilson@chromium.org committed Jun 26, 2014
1 parent 221538c commit 34aa940
Show file tree
Hide file tree
Showing 21 changed files with 214 additions and 297 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ void PolicyOAuth2TokenFetcher::StartFetchingAccessToken() {
std::vector<std::string> scopes;
scopes.push_back(GaiaConstants::kDeviceManagementServiceOAuth);
scopes.push_back(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
scopes.push_back(GaiaConstants::kGoogleUserInfoEmail);
scopes.push_back(GaiaConstants::kGoogleUserInfoProfile);
access_token_fetcher_.reset(
new OAuth2AccessTokenFetcherImpl(this,
system_context_getter_.get(),
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/policy/wildcard_login_checker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace {

// Presence of this key in the userinfo response indicates whether the user is
// on a hosted domain.
const char kHostedDomainKey[] = "domain";
const char kHostedDomainKey[] = "hd";

// UMA histogram names.
const char kUMADelayPolicyTokenFetch[] =
Expand Down
13 changes: 3 additions & 10 deletions chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "content/public/browser/notification_source.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "google_apis/gaia/gaia_constants.h"
#include "google_apis/gaia/gaia_oauth_client.h"
#include "google_apis/gaia/google_service_auth_error.h"
#include "net/http/http_status_code.h"
#include "net/url_request/test_url_fetcher_factory.h"
Expand Down Expand Up @@ -75,7 +74,7 @@ const char kValidTokenResponse[] =

const char kHostedDomainResponse[] =
"{"
" \"domain\": \"test.com\""
" \"hd\": \"test.com\""
"}";

class SigninManagerFake : public FakeSigninManager {
Expand Down Expand Up @@ -232,15 +231,10 @@ class UserPolicySigninServiceTest : public testing::Test {
return static_cast<FakeProfileOAuth2TokenService*>(service);
}

// Returns true if a request for policy information is active. A request
// is considered active if there is an active fetcher for an access token
// hosted domain information (i.e. the gaia oauth client) or some other
// fecther used in the code (id 0).
bool IsRequestActive() {
if (!GetTokenService()->GetPendingRequests().empty())
return true;
return url_factory_.GetFetcherByID(0) ||
url_factory_.GetFetcherByID(gaia::GaiaOAuthClient::kUrlFetcherId);
return url_factory_.GetFetcherByID(0);
}

void MakeOAuthTokenFetchSucceed() {
Expand All @@ -258,8 +252,7 @@ class UserPolicySigninServiceTest : public testing::Test {

void ReportHostedDomainStatus(bool is_hosted_domain) {
ASSERT_TRUE(IsRequestActive());
net::TestURLFetcher* fetcher =
url_factory_.GetFetcherByID(gaia::GaiaOAuthClient::kUrlFetcherId);
net::TestURLFetcher* fetcher = url_factory_.GetFetcherByID(0);
fetcher->set_response_code(net::HTTP_OK);
fetcher->SetResponseString(is_hosted_domain ? kHostedDomainResponse : "{}");
fetcher->delegate()->OnURLFetchComplete(fetcher);
Expand Down
120 changes: 69 additions & 51 deletions chrome/browser/profiles/profile_downloader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,24 @@ namespace {
const char kAuthorizationHeader[] =
"Authorization: Bearer %s";

// URL requesting user info.
const char kUserEntryURL[] =
"https://www.googleapis.com/oauth2/v1/userinfo?alt=json";

// OAuth scope for the user info API.
// For more info, see https://developers.google.com/accounts/docs/OAuth2LoginV1.
const char kAPIScope[] = "https://www.googleapis.com/auth/userinfo.profile";

// Path in JSON dictionary to user's photo thumbnail URL.
const char kPhotoThumbnailURLPath[] = "image.url";
const char kPhotoThumbnailURLPath[] = "picture";

// From the user info API, this field corresponds to the full name of the user.
const char kFullNamePath[] = "displayName";
const char kFullNamePath[] = "name";

const char kGivenNamePath[] = "name.givenName";
const char kGivenNamePath[] = "given_name";

// Path in JSON dictionary to user's preferred locale.
const char kLocalePath[] = "language";
const char kLocalePath[] = "locale";

// Path format for specifying thumbnail's size.
const char kThumbnailSizeFormat[] = "s%d-c";
Expand Down Expand Up @@ -125,7 +133,7 @@ bool GetImageURLWithSize(const GURL& old_url, int size, GURL* new_url) {
// Parses the entry response and gets the name and profile image URL.
// |data| should be the JSON formatted data return by the response.
// Returns false to indicate a parsing error.
bool ProfileDownloader::ParseProfileJSON(base::DictionaryValue* root_dictionary,
bool ProfileDownloader::ParseProfileJSON(const std::string& data,
base::string16* full_name,
base::string16* given_name,
std::string* url,
Expand All @@ -141,6 +149,23 @@ bool ProfileDownloader::ParseProfileJSON(base::DictionaryValue* root_dictionary,
*url = std::string();
*profile_locale = std::string();

int error_code = -1;
std::string error_message;
scoped_ptr<base::Value> root_value(base::JSONReader::ReadAndReturnError(
data, base::JSON_PARSE_RFC, &error_code, &error_message));
if (!root_value) {
LOG(ERROR) << "Error while parsing user entry response: "
<< error_message;
return false;
}
if (!root_value->IsType(base::Value::TYPE_DICTIONARY)) {
LOG(ERROR) << "JSON root is not a dictionary: "
<< root_value->GetType();
return false;
}
base::DictionaryValue* root_dictionary =
static_cast<base::DictionaryValue*>(root_value.get());

root_dictionary->GetString(kFullNamePath, full_name);
root_dictionary->GetString(kGivenNamePath, given_name);
root_dictionary->GetString(kLocalePath, profile_locale);
Expand Down Expand Up @@ -248,17 +273,24 @@ std::string ProfileDownloader::GetProfilePictureURL() const {
}

void ProfileDownloader::StartFetchingImage() {
DCHECK(!auth_token_.empty());
VLOG(1) << "Fetching user entry with token: " << auth_token_;
gaia_client_.reset(new gaia::GaiaOAuthClient(
delegate_->GetBrowserProfile()->GetRequestContext()));
gaia_client_->GetUserInfo(auth_token_, 0, this);
user_entry_fetcher_.reset(net::URLFetcher::Create(
GURL(kUserEntryURL), net::URLFetcher::GET, this));
user_entry_fetcher_->SetRequestContext(
delegate_->GetBrowserProfile()->GetRequestContext());
user_entry_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
net::LOAD_DO_NOT_SAVE_COOKIES);
if (!auth_token_.empty()) {
user_entry_fetcher_->SetExtraRequestHeaders(
base::StringPrintf(kAuthorizationHeader, auth_token_.c_str()));
}
user_entry_fetcher_->Start();
}

void ProfileDownloader::StartFetchingOAuth2AccessToken() {
Profile* profile = delegate_->GetBrowserProfile();
OAuth2TokenService::ScopeSet scopes;
scopes.insert(GaiaConstants::kGoogleUserInfoProfile);
scopes.insert(kAPIScope);
ProfileOAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
oauth2_access_token_request_ = token_service->StartRequest(
Expand All @@ -275,10 +307,27 @@ ProfileDownloader::~ProfileDownloader() {
service->RemoveObserver(this);
}

void ProfileDownloader::OnGetUserInfoResponse(
scoped_ptr<base::DictionaryValue> user_info) {
void ProfileDownloader::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
std::string data;
source->GetResponseAsString(&data);
bool network_error =
source->GetStatus().status() != net::URLRequestStatus::SUCCESS;
if (network_error || source->GetResponseCode() != 200) {
LOG(WARNING) << "Fetching profile data failed";
DVLOG(1) << " Status: " << source->GetStatus().status();
DVLOG(1) << " Error: " << source->GetStatus().error();
DVLOG(1) << " Response code: " << source->GetResponseCode();
DVLOG(1) << " Url: " << source->GetURL().spec();
delegate_->OnProfileDownloadFailure(this, network_error ?
ProfileDownloaderDelegate::NETWORK_ERROR :
ProfileDownloaderDelegate::SERVICE_ERROR);
return;
}

if (source == user_entry_fetcher_.get()) {
std::string image_url;
if (!ParseProfileJSON(user_info.get(),
if (!ParseProfileJSON(data,
&profile_full_name_,
&profile_given_name_,
&image_url,
Expand Down Expand Up @@ -318,45 +367,14 @@ void ProfileDownloader::OnGetUserInfoResponse(
base::StringPrintf(kAuthorizationHeader, auth_token_.c_str()));
}
profile_image_fetcher_->Start();
}

void ProfileDownloader::OnOAuthError() {
LOG(WARNING) << "OnOAuthError: Fetching profile data failed";
delegate_->OnProfileDownloadFailure(
this, ProfileDownloaderDelegate::SERVICE_ERROR);
}

void ProfileDownloader::OnNetworkError(int response_code) {
LOG(WARNING) << "OnNetworkError: Fetching profile data failed";
DVLOG(1) << " Response code: " << response_code;
delegate_->OnProfileDownloadFailure(
this, ProfileDownloaderDelegate::NETWORK_ERROR);
}

void ProfileDownloader::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
std::string data;
source->GetResponseAsString(&data);
bool network_error =
source->GetStatus().status() != net::URLRequestStatus::SUCCESS;
if (network_error || source->GetResponseCode() != 200) {
LOG(WARNING) << "Fetching profile data failed";
DVLOG(1) << " Status: " << source->GetStatus().status();
DVLOG(1) << " Error: " << source->GetStatus().error();
DVLOG(1) << " Response code: " << source->GetResponseCode();
DVLOG(1) << " Url: " << source->GetURL().spec();
delegate_->OnProfileDownloadFailure(this, network_error ?
ProfileDownloaderDelegate::NETWORK_ERROR :
ProfileDownloaderDelegate::SERVICE_ERROR);
return;
} else if (source == profile_image_fetcher_.get()) {
VLOG(1) << "Decoding the image...";
scoped_refptr<ImageDecoder> image_decoder = new ImageDecoder(
this, data, ImageDecoder::DEFAULT_CODEC);
scoped_refptr<base::MessageLoopProxy> task_runner =
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
image_decoder->Start(task_runner);
}

VLOG(1) << "Decoding the image...";
scoped_refptr<ImageDecoder> image_decoder = new ImageDecoder(
this, data, ImageDecoder::DEFAULT_CODEC);
scoped_refptr<base::MessageLoopProxy> task_runner =
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
image_decoder->Start(task_runner);
}

void ProfileDownloader::OnImageDecoded(const ImageDecoder* decoder,
Expand Down
14 changes: 3 additions & 11 deletions chrome/browser/profiles/profile_downloader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/memory/scoped_ptr.h"
#include "base/strings/string16.h"
#include "chrome/browser/image_decoder.h"
#include "google_apis/gaia/gaia_oauth_client.h"
#include "google_apis/gaia/oauth2_token_service.h"
#include "net/url_request/url_fetcher_delegate.h"
#include "third_party/skia/include/core/SkBitmap.h"
Expand All @@ -27,8 +26,7 @@ class URLFetcher;

// Downloads user profile information. The profile picture is decoded in a
// sandboxed process.
class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
public net::URLFetcherDelegate,
class ProfileDownloader : public net::URLFetcherDelegate,
public ImageDecoder::Delegate,
public OAuth2TokenService::Observer,
public OAuth2TokenService::Consumer {
Expand Down Expand Up @@ -82,12 +80,6 @@ class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, ParseData);
FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, DefaultURL);

// gaia::GaiaOAuthClient::Delegate implementation.
virtual void OnGetUserInfoResponse(
scoped_ptr<base::DictionaryValue> user_info) OVERRIDE;
virtual void OnOAuthError() OVERRIDE;
virtual void OnNetworkError(int response_code) OVERRIDE;

// Overriden from net::URLFetcherDelegate:
virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;

Expand All @@ -109,7 +101,7 @@ class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
// Parses the entry response and gets the name, profile image URL and locale.
// |data| should be the JSON formatted data return by the response.
// Returns false to indicate a parsing error.
static bool ParseProfileJSON(base::DictionaryValue* root_dictionary,
static bool ParseProfileJSON(const std::string& data,
base::string16* full_name,
base::string16* given_name,
std::string* url,
Expand All @@ -131,7 +123,7 @@ class ProfileDownloader : public gaia::GaiaOAuthClient::Delegate,
ProfileDownloaderDelegate* delegate_;
std::string account_id_;
std::string auth_token_;
scoped_ptr<gaia::GaiaOAuthClient> gaia_client_;
scoped_ptr<net::URLFetcher> user_entry_fetcher_;
scoped_ptr<net::URLFetcher> profile_image_fetcher_;
scoped_ptr<OAuth2TokenService::Request> oauth2_access_token_request_;
base::string16 profile_full_name_;
Expand Down
47 changes: 26 additions & 21 deletions chrome/browser/profiles/profile_downloader_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,38 @@

#include "chrome/browser/profiles/profile_downloader.h"

#include "base/json/json_reader.h"
#include "base/memory/scoped_ptr.h"
#include "base/strings/string_piece.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace {

void GetJSonData(const std::string& full_name,
const std::string& given_name,
const std::string& url,
const std::string& locale,
base::DictionaryValue* dict) {
if (!full_name.empty())
dict->SetString("displayName", full_name);

if (!given_name.empty())
dict->SetString("name.givenName", given_name);

if (!url.empty())
dict->SetString("image.url", url);
std::string GetJSonData(const std::string& full_name,
const std::string& given_name,
const std::string& url,
const std::string& locale) {
std::stringstream stream;
bool started = false;

stream << "{ ";
if (!full_name.empty()) {
stream << "\"name\": \"" << full_name << "\"";
started = true;
}
if (!given_name.empty()) {
stream << (started ? ", " : "") << "\"given_name\": \"" << given_name
<< "\"";
started = true;
}
if (!url.empty()) {
stream << (started ? ", " : "") << "\"picture\": \"" << url << "\"";
started = true;
}

if (!locale.empty())
dict->SetString("language", locale);
stream << (started ? ", " : "") << "\"locale\": \"" << locale << "\"";

stream << " }";
return stream.str();
}

} // namespace
Expand All @@ -51,10 +58,8 @@ class ProfileDownloaderTest : public testing::Test {
base::string16 parsed_given_name;
std::string parsed_url;
std::string parsed_locale;
scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
GetJSonData(full_name, given_name, url, locale, dict.get());
bool result = ProfileDownloader::ParseProfileJSON(
dict.get(),
GetJSonData(full_name, given_name, url, locale),
&parsed_full_name,
&parsed_given_name,
&parsed_url,
Expand Down
Loading

0 comments on commit 34aa940

Please sign in to comment.