Skip to content

Commit

Permalink
Remove LSO token fetch from TokenService.
Browse files Browse the repository at this point in the history
BUG=269532
TBR=kinaba@chromium.org

Review URL: https://chromiumcodereview.appspot.com/22571004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218025 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rogerta@chromium.org committed Aug 16, 2013
1 parent 804462a commit dcaaf89
Show file tree
Hide file tree
Showing 13 changed files with 21 additions and 92 deletions.
3 changes: 1 addition & 2 deletions chrome/browser/google_apis/auth_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,7 @@ void AuthService::OnRefreshTokenAvailable(const std::string& account_id) {
OnHandleRefreshToken(true);
}

void AuthService::OnRefreshTokenRevoked(const std::string& account_id,
const GoogleServiceAuthError& error) {
void AuthService::OnRefreshTokenRevoked(const std::string& account_id) {
OnHandleRefreshToken(false);
}

Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/google_apis/auth_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ class AuthService : public AuthServiceInterface,

// Overridden from OAuth2TokenService::Observer:
virtual void OnRefreshTokenAvailable(const std::string& account_id) OVERRIDE;
virtual void OnRefreshTokenRevoked(
const std::string& account_id,
const GoogleServiceAuthError& error) OVERRIDE;
virtual void OnRefreshTokenRevoked(const std::string& account_id) OVERRIDE;

private:
// Called when the state of the refresh token changes.
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/signin/oauth2_token_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -577,10 +577,9 @@ void OAuth2TokenService::FireRefreshTokenAvailable(
}

void OAuth2TokenService::FireRefreshTokenRevoked(
const std::string& account_id,
const GoogleServiceAuthError& error) {
const std::string& account_id) {
FOR_EACH_OBSERVER(Observer, observer_list_,
OnRefreshTokenRevoked(account_id, error));
OnRefreshTokenRevoked(account_id));
}

void OAuth2TokenService::FireRefreshTokensLoaded() {
Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/signin/oauth2_token_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ class OAuth2TokenService {
virtual void OnRefreshTokenAvailable(const std::string& account_id) {}
// Called whenever the login-scoped refresh token becomes unavailable for
// account |account_id|.
virtual void OnRefreshTokenRevoked(const std::string& account_id,
const GoogleServiceAuthError& error) {}
virtual void OnRefreshTokenRevoked(const std::string& account_id) {}
// Called after all refresh tokens are loaded during OAuth2TokenService
// startup.
virtual void OnRefreshTokensLoaded() {}
Expand Down Expand Up @@ -205,8 +204,7 @@ class OAuth2TokenService {

// Called by subclasses to notify observers.
void FireRefreshTokenAvailable(const std::string& account_id);
void FireRefreshTokenRevoked(const std::string& account_id,
const GoogleServiceAuthError& error);
void FireRefreshTokenRevoked(const std::string& account_id);
void FireRefreshTokensLoaded();
void FireRefreshTokensCleared();

Expand Down
28 changes: 2 additions & 26 deletions chrome/browser/signin/profile_oauth2_token_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@ void ProfileOAuth2TokenService::Initialize(Profile* profile) {
registrar_.Add(this,
chrome::NOTIFICATION_TOKEN_AVAILABLE,
token_service_source);
registrar_.Add(this,
chrome::NOTIFICATION_TOKEN_REQUEST_FAILED,
token_service_source);
registrar_.Add(this,
chrome::NOTIFICATION_TOKEN_LOADING_FINISHED,
token_service_source);
Expand Down Expand Up @@ -147,25 +144,6 @@ void ProfileOAuth2TokenService::Observe(
}
break;
}
case chrome::NOTIFICATION_TOKEN_REQUEST_FAILED: {
TokenService::TokenRequestFailedDetails* tok_details =
content::Details<TokenService::TokenRequestFailedDetails>(details)
.ptr();
if (tok_details->service() == GaiaConstants::kLSOService ||
tok_details->service() ==
GaiaConstants::kGaiaOAuth2LoginRefreshToken) {
// TODO(fgorski): Canceling all requests will not be correct in a
// multi-login environment. We should cacnel only the requests related
// to the failed refresh token.
// Failed refresh token is not available at this point, but since
// there are no other refresh tokens, we cancel all active requests.
CancelAllRequests();
ClearCache();
UpdateAuthError(tok_details->error());
FireRefreshTokenRevoked(GetAccountId(profile_), tok_details->error());
}
break;
}
case chrome::NOTIFICATION_TOKENS_CLEARED: {
CancelAllRequests();
ClearCache();
Expand Down Expand Up @@ -254,8 +232,7 @@ void ProfileOAuth2TokenService::RevokeCredentials(
TokenWebData::FromBrowserContext(profile_);
if (token_web_data.get())
token_web_data->RemoveTokenForService(ApplyAccountIdPrefix(account_id));
FireRefreshTokenRevoked(account_id,
GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED));
FireRefreshTokenRevoked(account_id);

// TODO(fgorski): Notify diagnostic observers.
}
Expand All @@ -265,12 +242,11 @@ void ProfileOAuth2TokenService::RevokeAllCredentials() {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

CancelAllRequests();
GoogleServiceAuthError error(GoogleServiceAuthError::REQUEST_CANCELED);
for (std::map<std::string, std::string>::const_iterator iter =
refresh_tokens_.begin();
iter != refresh_tokens_.end();
++iter) {
FireRefreshTokenRevoked(iter->first, error);
FireRefreshTokenRevoked(iter->first);
}
refresh_tokens_.clear();

Expand Down
30 changes: 7 additions & 23 deletions chrome/browser/signin/profile_oauth2_token_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

using content::BrowserThread;

// Defining constant here to handle backward compatiblity tests, but this
// constant is no longer used in current versions of chrome.
static const char kLSOService[] = "lso";

class ProfileOAuth2TokenServiceTest : public TokenServiceTestHarness,
public OAuth2TokenService::Observer {
public:
Expand Down Expand Up @@ -44,9 +48,7 @@ class ProfileOAuth2TokenServiceTest : public TokenServiceTestHarness,
virtual void OnRefreshTokenAvailable(const std::string& account_id) OVERRIDE {
++token_available_count_;
}
virtual void OnRefreshTokenRevoked(
const std::string& account_id,
const GoogleServiceAuthError& error) OVERRIDE {
virtual void OnRefreshTokenRevoked(const std::string& account_id) OVERRIDE {
++token_revoked_count_;
}
virtual void OnRefreshTokensLoaded() OVERRIDE {
Expand Down Expand Up @@ -132,8 +134,7 @@ TEST_F(ProfileOAuth2TokenServiceTest, PersistenceDBUpgrade) {
// Populate DB with legacy tokens.
service()->OnIssueAuthTokenSuccess(GaiaConstants::kSyncService,
"syncServiceToken");
service()->OnIssueAuthTokenSuccess(GaiaConstants::kLSOService,
"lsoToken");
service()->OnIssueAuthTokenSuccess(kLSOService, "lsoToken");
service()->OnIssueAuthTokenSuccess(
GaiaConstants::kGaiaOAuth2LoginRefreshToken,
main_refresh_token);
Expand All @@ -158,8 +159,7 @@ TEST_F(ProfileOAuth2TokenServiceTest, PersistenceDBUpgrade) {
GaiaConstants::kGaiaOAuth2LoginRefreshToken,
"secondOldRefreshToken");
// Add some other legacy token. (Expected to get discarded).
service()->OnIssueAuthTokenSuccess(GaiaConstants::kLSOService,
"lsoToken");
service()->OnIssueAuthTokenSuccess(kLSOService, "lsoToken");
// Also add a token using PO2TS.UpdateCredentials and make sure upgrade does
// not wipe it.
std::string other_account_id("other_account_id");
Expand Down Expand Up @@ -275,22 +275,6 @@ TEST_F(ProfileOAuth2TokenServiceTest, PersistanceNotifications) {
ResetObserverCounts();
}

// Until the TokenService class is removed, problems fetching the LSO token
// should translate to problems fetching the oauth2 refresh token.
TEST_F(ProfileOAuth2TokenServiceTest, LsoNotification) {
EXPECT_EQ(0, oauth2_service_->cache_size_for_testing());

// Get a valid token.
service()->IssueAuthTokenForTest(GaiaConstants::kGaiaOAuth2LoginRefreshToken,
"refreshToken");
ExpectOneTokenAvailableNotification();

service()->OnIssueAuthTokenFailure(
GaiaConstants::kLSOService,
GoogleServiceAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
ExpectOneTokenRevokedNotification();
}

// Until the TokenService class is removed, finish token loading in TokenService
// should translate to finish token loading in ProfileOAuth2TokenService.
TEST_F(ProfileOAuth2TokenServiceTest, TokensLoaded) {
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/signin/signin_internals_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const char kObfuscatedGaiaIdFetcherToken[] = "ObfuscatedGaiaIdFetcher";
const char kOAuth2MintTokenFlowToken[] = "OAuth2MintTokenFlow";
const char* kTokenPrefsArray[] = {
GaiaConstants::kSyncService,
GaiaConstants::kLSOService,
GaiaConstants::kGaiaOAuth2LoginRefreshToken,
kOperationsBaseToken,
kUserPolicySigninServiceToken,
Expand Down
20 changes: 1 addition & 19 deletions chrome/browser/signin/token_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ namespace {

// List of services that are capable of ClientLogin-based authentication.
const char* kServices[] = {
GaiaConstants::kSyncService,
GaiaConstants::kLSOService
GaiaConstants::kSyncService
};

#define FOR_DIAGNOSTICS_OBSERVERS(func) \
Expand Down Expand Up @@ -110,23 +109,6 @@ void TokenService::AddAuthTokenManually(const std::string& service,
token_map_[service] = auth_token;
FireTokenAvailableNotification(service, auth_token);
SaveAuthTokenToDB(service, auth_token);

// We don't ever want to fetch OAuth2 tokens from LSO service token in case
// when ChromeOS is in forced OAuth2 use mode. OAuth2 token should only
// arrive into token service exclusively through UpdateCredentialsWithOAuth2.
#if !defined(OS_CHROMEOS)
// If we got ClientLogin token for "lso" service, and we don't already have
// OAuth2 tokens, start fetching OAuth2 login scoped token pair.
if (service == GaiaConstants::kLSOService && !HasOAuthLoginToken()) {
int index = GetServiceIndex(service);
CHECK_GE(index, 0);
// iOS fetches the service tokens outside of the TokenService.
if (!fetchers_[index].get()) {
fetchers_[index].reset(new GaiaAuthFetcher(this, source_, getter_.get()));
}
fetchers_[index]->StartLsoForOAuthLoginTokenExchange(auth_token);
}
#endif
}


Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/signin/token_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ class TokenService : public GaiaAuthConsumer,
// number of entries in this array must match the number of entries in the
// kServices array declared in the cc file. If not, a compile time error
// will occur.
scoped_ptr<GaiaAuthFetcher> fetchers_[2];
scoped_ptr<GaiaAuthFetcher> fetchers_[1];

// Map from service to token.
std::map<std::string, std::string> token_map_;
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/sync/profile_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -697,16 +697,16 @@ void ProfileSyncService::OnRefreshTokenAvailable(
}

void ProfileSyncService::OnRefreshTokenRevoked(
const std::string& account_id,
const GoogleServiceAuthError& error) {
const std::string& account_id) {
if (!IsOAuthRefreshTokenAvailable()) {
// The additional check around IsOAuthRefreshTokenAvailable() above
// prevents us sounding the alarm if we actually have a valid token but
// a refresh attempt by TokenService failed for any variety of reasons
// (e.g. flaky network). It's possible the token we do have is also
// invalid, but in that case we should already have (or can expect) an
// auth error sent from the sync backend.
UpdateAuthErrorState(error);
UpdateAuthErrorState(
GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED));
}
}

Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/sync/profile_sync_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -610,9 +610,7 @@ class ProfileSyncService : public ProfileSyncServiceBase,

// OAuth2TokenService::Observer implementation.
virtual void OnRefreshTokenAvailable(const std::string& account_id) OVERRIDE;
virtual void OnRefreshTokenRevoked(
const std::string& account_id,
const GoogleServiceAuthError& error) OVERRIDE;
virtual void OnRefreshTokenRevoked(const std::string& account_id) OVERRIDE;
virtual void OnRefreshTokensLoaded() OVERRIDE;
virtual void OnRefreshTokensCleared() OVERRIDE;

Expand Down
3 changes: 0 additions & 3 deletions google_apis/gaia/gaia_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ const char kChromeSyncManagedOAuth2Scope[] =
const char kGoogleTalkOAuth2Scope[] =
"https://www.googleapis.com/auth/googletalk";

// Service for LSO endpoint of Google that exposes OAuth APIs.
const char kLSOService[] = "lso";

// Used to mint uber auth tokens when needed.
const char kGaiaSid[] = "sid";
const char kGaiaLsid[] = "lsid";
Expand Down
1 change: 0 additions & 1 deletion google_apis/gaia/gaia_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ extern const char kAnyApiOAuth2Scope[];
extern const char kChromeSyncOAuth2Scope[];
extern const char kChromeSyncManagedOAuth2Scope[];
extern const char kGoogleTalkOAuth2Scope[];
extern const char kLSOService[];

// Used with uber auth tokens when needed.
extern const char kGaiaSid[];
Expand Down

0 comments on commit dcaaf89

Please sign in to comment.