Commit f6324711 authored by David Roger's avatar David Roger Committed by Commit Bot

[Dice] Force ListAccount server call on authentication errors

This CL adds an API to OAuth2TokenServiceDelegate::Observer to
listen to changes in auth errors.

AccountReconcilor uses this API to trigger a ListAccount call
when a token becomes invalid.
This is necessary because in that case, the account can be invalid
in the cookie without the cookie changing. Chrome needs to make a
call to ListAccount to have an accurate view of the Gaia accounts in
the cookie.

Bug: 823714
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ie22226adfe6c425acaffe875a314c4a6b8de5e88
Reviewed-on: https://chromium-review.googlesource.com/986268
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549493}
parent 9f40c76c
...@@ -263,10 +263,8 @@ MutableProfileOAuth2TokenServiceDelegate::AccountStatus::~AccountStatus() { ...@@ -263,10 +263,8 @@ MutableProfileOAuth2TokenServiceDelegate::AccountStatus::~AccountStatus() {
void MutableProfileOAuth2TokenServiceDelegate::AccountStatus::SetLastAuthError( void MutableProfileOAuth2TokenServiceDelegate::AccountStatus::SetLastAuthError(
const GoogleServiceAuthError& error) { const GoogleServiceAuthError& error) {
if (error.state() != last_auth_error_.state()) { last_auth_error_ = error;
last_auth_error_ = error; signin_error_controller_->AuthStatusChanged();
signin_error_controller_->AuthStatusChanged();
}
} }
std::string std::string
...@@ -344,11 +342,11 @@ MutableProfileOAuth2TokenServiceDelegate::CreateAccessTokenFetcher( ...@@ -344,11 +342,11 @@ MutableProfileOAuth2TokenServiceDelegate::CreateAccessTokenFetcher(
return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token); return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token);
} }
bool MutableProfileOAuth2TokenServiceDelegate::RefreshTokenHasError( GoogleServiceAuthError MutableProfileOAuth2TokenServiceDelegate::GetAuthError(
const std::string& account_id) const { const std::string& account_id) const {
auto it = refresh_tokens_.find(account_id); auto it = refresh_tokens_.find(account_id);
return it == refresh_tokens_.end() ? false return (it == refresh_tokens_.end()) ? GoogleServiceAuthError::AuthErrorNone()
: IsError(it->second->GetAuthStatus()); : it->second->GetAuthStatus();
} }
void MutableProfileOAuth2TokenServiceDelegate::UpdateAuthError( void MutableProfileOAuth2TokenServiceDelegate::UpdateAuthError(
...@@ -375,7 +373,12 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateAuthError( ...@@ -375,7 +373,12 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateAuthError(
NOTREACHED(); NOTREACHED();
return; return;
} }
refresh_tokens_[account_id]->SetLastAuthError(error);
AccountStatus* status = refresh_tokens_[account_id].get();
if (error != status->GetAuthStatus()) {
status->SetLastAuthError(error);
FireAuthErrorChanged(account_id, error);
}
} }
bool MutableProfileOAuth2TokenServiceDelegate::RefreshTokenIsAvailable( bool MutableProfileOAuth2TokenServiceDelegate::RefreshTokenIsAvailable(
...@@ -497,7 +500,10 @@ void MutableProfileOAuth2TokenServiceDelegate::OnWebDataServiceRequestDone( ...@@ -497,7 +500,10 @@ void MutableProfileOAuth2TokenServiceDelegate::OnWebDataServiceRequestDone(
load_credentials_state_ = load_credentials_state_ =
LOAD_CREDENTIALS_FINISHED_WITH_NO_TOKEN_FOR_PRIMARY_ACCOUNT; LOAD_CREDENTIALS_FINISHED_WITH_NO_TOKEN_FOR_PRIMARY_ACCOUNT;
} }
AddAccountStatus(loading_primary_account_id_, std::string()); AddAccountStatus(loading_primary_account_id_, std::string(),
GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_MISSING));
} }
// If we don't have a refresh token for a known account, signal an error. // If we don't have a refresh token for a known account, signal an error.
...@@ -678,6 +684,13 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory( ...@@ -678,6 +684,13 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory(
DCHECK(!account_id.empty()); DCHECK(!account_id.empty());
DCHECK(!refresh_token.empty()); DCHECK(!refresh_token.empty());
GoogleServiceAuthError error =
(refresh_token == kInvalidRefreshToken)
? GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_CLIENT)
: GoogleServiceAuthError::AuthErrorNone();
bool refresh_token_present = refresh_tokens_.count(account_id) > 0; bool refresh_token_present = refresh_tokens_.count(account_id) > 0;
// If token present, and different from the new one, cancel its requests, // If token present, and different from the new one, cancel its requests,
// and clear the entries in cache related to that account. // and clear the entries in cache related to that account.
...@@ -687,19 +700,12 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory( ...@@ -687,19 +700,12 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory(
<< "account_id=" << account_id; << "account_id=" << account_id;
RevokeCredentialsOnServer(refresh_tokens_[account_id]->refresh_token()); RevokeCredentialsOnServer(refresh_tokens_[account_id]->refresh_token());
refresh_tokens_[account_id]->set_refresh_token(refresh_token); refresh_tokens_[account_id]->set_refresh_token(refresh_token);
UpdateAuthError(account_id, error);
} else { } else {
VLOG(1) << "MutablePO2TS::UpdateCredentials; Refresh Token was absent. " VLOG(1) << "MutablePO2TS::UpdateCredentials; Refresh Token was absent. "
<< "account_id=" << account_id; << "account_id=" << account_id;
AddAccountStatus(account_id, refresh_token); AddAccountStatus(account_id, refresh_token, error);
} }
GoogleServiceAuthError error =
(refresh_token == kInvalidRefreshToken)
? GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_CLIENT)
: GoogleServiceAuthError::AuthErrorNone();
UpdateAuthError(account_id, error);
} }
void MutableProfileOAuth2TokenServiceDelegate::PersistCredentials( void MutableProfileOAuth2TokenServiceDelegate::PersistCredentials(
...@@ -806,10 +812,13 @@ const net::BackoffEntry* ...@@ -806,10 +812,13 @@ const net::BackoffEntry*
void MutableProfileOAuth2TokenServiceDelegate::AddAccountStatus( void MutableProfileOAuth2TokenServiceDelegate::AddAccountStatus(
const std::string& account_id, const std::string& account_id,
const std::string& refresh_token) { const std::string& refresh_token,
const GoogleServiceAuthError& error) {
DCHECK_EQ(0u, refresh_tokens_.count(account_id)); DCHECK_EQ(0u, refresh_tokens_.count(account_id));
AccountStatus* status = AccountStatus* status =
new AccountStatus(signin_error_controller_, account_id, refresh_token); new AccountStatus(signin_error_controller_, account_id, refresh_token);
refresh_tokens_[account_id].reset(status); refresh_tokens_[account_id].reset(status);
status->Initialize(); status->Initialize();
status->SetLastAuthError(error);
FireAuthErrorChanged(account_id, status->GetAuthStatus());
} }
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "components/signin/core/browser/signin_error_controller.h" #include "components/signin/core/browser/signin_error_controller.h"
#include "components/webdata/common/web_data_service_base.h" #include "components/webdata/common/web_data_service_base.h"
#include "components/webdata/common/web_data_service_consumer.h" #include "components/webdata/common/web_data_service_consumer.h"
#include "google_apis/gaia/oauth2_token_service_delegate.h"
#include "net/base/backoff_entry.h" #include "net/base/backoff_entry.h"
#include "net/base/network_change_notifier.h" #include "net/base/network_change_notifier.h"
...@@ -54,7 +55,8 @@ class MutableProfileOAuth2TokenServiceDelegate ...@@ -54,7 +55,8 @@ class MutableProfileOAuth2TokenServiceDelegate
const GoogleServiceAuthError& error) override; const GoogleServiceAuthError& error) override;
bool RefreshTokenIsAvailable(const std::string& account_id) const override; bool RefreshTokenIsAvailable(const std::string& account_id) const override;
bool RefreshTokenHasError(const std::string& account_id) const override; GoogleServiceAuthError GetAuthError(
const std::string& account_id) const override;
std::vector<std::string> GetAccounts() override; std::vector<std::string> GetAccounts() override;
net::URLRequestContextGetter* GetRequestContext() const override; net::URLRequestContextGetter* GetRequestContext() const override;
...@@ -172,7 +174,8 @@ class MutableProfileOAuth2TokenServiceDelegate ...@@ -172,7 +174,8 @@ class MutableProfileOAuth2TokenServiceDelegate
// Creates a new AccountStatus and adds it to the AccountStatusMap. // Creates a new AccountStatus and adds it to the AccountStatusMap.
// The account must not be already in the map. // The account must not be already in the map.
void AddAccountStatus(const std::string& account_id, void AddAccountStatus(const std::string& account_id,
const std::string& refresh_token); const std::string& refresh_token,
const GoogleServiceAuthError& error);
// Maps the |account_id| of accounts known to ProfileOAuth2TokenService // Maps the |account_id| of accounts known to ProfileOAuth2TokenService
// to information about the account. // to information about the account.
......
...@@ -212,11 +212,11 @@ bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable( ...@@ -212,11 +212,11 @@ bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable(
return refresh_token_is_available == JNI_TRUE; return refresh_token_is_available == JNI_TRUE;
} }
bool OAuth2TokenServiceDelegateAndroid::RefreshTokenHasError( GoogleServiceAuthError OAuth2TokenServiceDelegateAndroid::GetAuthError(
const std::string& account_id) const { const std::string& account_id) const {
auto it = errors_.find(account_id); auto it = errors_.find(account_id);
// TODO(rogerta): should we distinguish between transient and persistent? return (it == errors_.end()) ? GoogleServiceAuthError::AuthErrorNone()
return it == errors_.end() ? false : IsError(it->second.error); : it->second.error;
} }
void OAuth2TokenServiceDelegateAndroid::UpdateAuthError( void OAuth2TokenServiceDelegateAndroid::UpdateAuthError(
...@@ -225,12 +225,14 @@ void OAuth2TokenServiceDelegateAndroid::UpdateAuthError( ...@@ -225,12 +225,14 @@ void OAuth2TokenServiceDelegateAndroid::UpdateAuthError(
DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAuthError" DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::UpdateAuthError"
<< " account=" << account_id << " account=" << account_id
<< " error=" << error.ToString(); << " error=" << error.ToString();
if (error.state() == GoogleServiceAuthError::NONE) {
if (error.IsTransientError())
return;
if (error.state() == GoogleServiceAuthError::NONE)
errors_.erase(account_id); errors_.erase(account_id);
} else { else
// TODO(rogerta): should we distinguish between transient and persistent?
errors_[account_id] = ErrorInfo(error); errors_[account_id] = ErrorInfo(error);
}
} }
std::vector<std::string> OAuth2TokenServiceDelegateAndroid::GetAccounts() { std::vector<std::string> OAuth2TokenServiceDelegateAndroid::GetAccounts() {
......
...@@ -48,7 +48,8 @@ class OAuth2TokenServiceDelegateAndroid : public OAuth2TokenServiceDelegate { ...@@ -48,7 +48,8 @@ class OAuth2TokenServiceDelegateAndroid : public OAuth2TokenServiceDelegate {
// OAuth2TokenServiceDelegate overrides: // OAuth2TokenServiceDelegate overrides:
bool RefreshTokenIsAvailable(const std::string& account_id) const override; bool RefreshTokenIsAvailable(const std::string& account_id) const override;
bool RefreshTokenHasError(const std::string& account_id) const override; GoogleServiceAuthError GetAuthError(
const std::string& account_id) const override;
void UpdateAuthError(const std::string& account_id, void UpdateAuthError(const std::string& account_id,
const GoogleServiceAuthError& error) override; const GoogleServiceAuthError& error) override;
std::vector<std::string> GetAccounts() override; std::vector<std::string> GetAccounts() override;
......
...@@ -262,6 +262,20 @@ void AccountReconcilor::OnRefreshTokensLoaded() { ...@@ -262,6 +262,20 @@ void AccountReconcilor::OnRefreshTokensLoaded() {
StartReconcile(); StartReconcile();
} }
void AccountReconcilor::OnAuthErrorChanged(
const std::string& account_id,
const GoogleServiceAuthError& error) {
// Gaia cookies may be invalidated server-side and the client does not get any
// notification when this happens.
// Gaia cookies derived from refresh tokens are always invalidated server-side
// when the tokens are revoked. Trigger a ListAccounts to Gaia when this
// happens to make sure that the cookies accounts are up-to-date.
// This should cover well the Mirror and Desktop Identity Consistency cases as
// the cookies are always bound to the refresh tokens in these cases.
if (error != GoogleServiceAuthError::AuthErrorNone())
cookie_manager_service_->TriggerListAccounts(kSource);
}
void AccountReconcilor::PerformMergeAction(const std::string& account_id) { void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
reconcile_is_noop_ = false; reconcile_is_noop_ = false;
if (!delegate_->IsAccountConsistencyEnforced()) { if (!delegate_->IsAccountConsistencyEnforced()) {
......
...@@ -149,6 +149,7 @@ class AccountReconcilor : public KeyedService, ...@@ -149,6 +149,7 @@ class AccountReconcilor : public KeyedService,
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileNoopWithDots); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileNoopWithDots);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileNoopMultiple); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileNoopMultiple);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileAddToCookie); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileAddToCookie);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, AuthErrorTriggersListAccount);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
SignoutAfterErrorDoesNotRecordUma); SignoutAfterErrorDoesNotRecordUma);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
...@@ -232,6 +233,8 @@ class AccountReconcilor : public KeyedService, ...@@ -232,6 +233,8 @@ class AccountReconcilor : public KeyedService,
// Overriden from OAuth2TokenService::Observer. // Overriden from OAuth2TokenService::Observer.
void OnEndBatchChanges() override; void OnEndBatchChanges() override;
void OnRefreshTokensLoaded() override; void OnRefreshTokensLoaded() override;
void OnAuthErrorChanged(const std::string& account_id,
const GoogleServiceAuthError& error) override;
// Lock related methods. // Lock related methods.
void IncrementLockCount(); void IncrementLockCount();
......
...@@ -33,7 +33,6 @@ ...@@ -33,7 +33,6 @@
#include "components/signin/core/browser/test_signin_client.h" #include "components/signin/core/browser/test_signin_client.h"
#include "components/sync_preferences/pref_service_syncable.h" #include "components/sync_preferences/pref_service_syncable.h"
#include "components/sync_preferences/testing_pref_service_syncable.h" #include "components/sync_preferences/testing_pref_service_syncable.h"
#include "google_apis/gaia/fake_oauth2_token_service_delegate.h"
#include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/gaia_constants.h"
#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/gaia_urls.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
...@@ -228,10 +227,6 @@ class AccountReconcilorTest : public ::testing::Test { ...@@ -228,10 +227,6 @@ class AccountReconcilorTest : public ::testing::Test {
FakeSigninManagerForTesting* signin_manager() { return &signin_manager_; } FakeSigninManagerForTesting* signin_manager() { return &signin_manager_; }
FakeProfileOAuth2TokenService* token_service() { return &token_service_; } FakeProfileOAuth2TokenService* token_service() { return &token_service_; }
FakeOAuth2TokenServiceDelegate* token_service_delegate() {
return static_cast<FakeOAuth2TokenServiceDelegate*>(
token_service_.GetDelegate());
}
DiceTestSigninClient* test_signin_client() { return &test_signin_client_; } DiceTestSigninClient* test_signin_client() { return &test_signin_client_; }
AccountTrackerService* account_tracker() { return &account_tracker_; } AccountTrackerService* account_tracker() { return &account_tracker_; }
FakeGaiaCookieManagerService* cookie_manager_service() { FakeGaiaCookieManagerService* cookie_manager_service() {
...@@ -660,9 +655,8 @@ class AccountReconcilorTestDice ...@@ -660,9 +655,8 @@ class AccountReconcilorTestDice
std::string account_id = std::string account_id =
PickAccountIdForAccount(token.gaia_id, token.email); PickAccountIdForAccount(token.gaia_id, token.email);
EXPECT_TRUE(token_service()->RefreshTokenIsAvailable(account_id)); EXPECT_TRUE(token_service()->RefreshTokenIsAvailable(account_id));
EXPECT_EQ( EXPECT_EQ(token.has_error,
token.has_error, token_service()->RefreshTokenHasError(account_id));
token_service()->GetDelegate()->RefreshTokenHasError(account_id));
if (token.is_authenticated) { if (token.is_authenticated) {
EXPECT_EQ(account_id, signin_manager()->GetAuthenticatedAccountId()); EXPECT_EQ(account_id, signin_manager()->GetAuthenticatedAccountId());
authenticated_account_found = true; authenticated_account_found = true;
...@@ -732,7 +726,7 @@ TEST_P(AccountReconcilorTestDice, TableRowTest) { ...@@ -732,7 +726,7 @@ TEST_P(AccountReconcilorTestDice, TableRowTest) {
else else
token_service()->UpdateCredentials(account_id, "refresh_token"); token_service()->UpdateCredentials(account_id, "refresh_token");
if (token.has_error) { if (token.has_error) {
token_service_delegate()->UpdateAuthError( token_service()->UpdateAuthErrorForTesting(
account_id, GoogleServiceAuthError( account_id, GoogleServiceAuthError(
GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS)); GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
} }
...@@ -1495,6 +1489,60 @@ TEST_F(AccountReconcilorTest, StartReconcileAddToCookie) { ...@@ -1495,6 +1489,60 @@ TEST_F(AccountReconcilorTest, StartReconcileAddToCookie) {
testing::ContainerEq(expected_counts)); testing::ContainerEq(expected_counts));
} }
TEST_F(AccountReconcilorTest, AuthErrorTriggersListAccount) {
class TestGaiaCookieObserver : public GaiaCookieManagerService::Observer {
public:
void OnGaiaAccountsInCookieUpdated(
const std::vector<gaia::ListedAccount>& accounts,
const std::vector<gaia::ListedAccount>& signed_out_accounts,
const GoogleServiceAuthError& error) override {
cookies_updated_ = true;
}
bool cookies_updated_ = false;
};
#if BUILDFLAG(ENABLE_DICE_SUPPORT)
signin::AccountConsistencyMethod account_consistency =
signin::AccountConsistencyMethod::kDice;
SetAccountConsistency(account_consistency);
#else
signin::AccountConsistencyMethod account_consistency =
signin::AccountConsistencyMethod::kMirror;
SetAccountConsistency(account_consistency);
#endif
// Add one account to Chrome and instantiate the reconcilor.
const std::string account_id =
ConnectProfileToAccount("12345", "user@gmail.com");
token_service()->UpdateCredentials(account_id, "refresh_token");
TestGaiaCookieObserver observer;
cookie_manager_service()->AddObserver(&observer);
AccountReconcilor* reconcilor = GetMockReconcilor();
base::RunLoop().RunUntilIdle();
ASSERT_FALSE(reconcilor->is_reconcile_started_);
cookie_manager_service()->SetListAccountsResponseOneAccount("user@gmail.com",
"12345");
if (account_consistency == signin::AccountConsistencyMethod::kDice) {
EXPECT_CALL(*GetMockReconcilor(), PerformLogoutAllAccountsAction())
.Times(1);
}
// Set an authentication error.
ASSERT_FALSE(observer.cookies_updated_);
token_service()->UpdateAuthErrorForTesting(
account_id, GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_SERVER));
base::RunLoop().RunUntilIdle();
// Check that a call to ListAccount was triggered.
EXPECT_TRUE(observer.cookies_updated_);
testing::Mock::VerifyAndClearExpectations(GetMockReconcilor());
cookie_manager_service()->RemoveObserver(&observer);
}
#if !defined(OS_CHROMEOS) #if !defined(OS_CHROMEOS)
// This test does not run on ChromeOS because it calls // This test does not run on ChromeOS because it calls
// FakeSigninManagerForTesting::SignOut() which doesn't exist for ChromeOS. // FakeSigninManagerForTesting::SignOut() which doesn't exist for ChromeOS.
...@@ -1846,7 +1894,7 @@ TEST_F(AccountReconcilorTest, NoLoopWithBadPrimary) { ...@@ -1846,7 +1894,7 @@ TEST_F(AccountReconcilorTest, NoLoopWithBadPrimary) {
// Now that we've tried once, the token service knows that the primary // Now that we've tried once, the token service knows that the primary
// account has an auth error. // account has an auth error.
token_service_delegate()->UpdateAuthError(account_id1, error); token_service()->UpdateAuthErrorForTesting(account_id1, error);
// A second attempt to reconcile should be a noop. // A second attempt to reconcile should be a noop.
reconcilor->StartReconcile(); reconcilor->StartReconcile();
...@@ -1865,7 +1913,7 @@ TEST_F(AccountReconcilorTest, WontMergeAccountsWithError) { ...@@ -1865,7 +1913,7 @@ TEST_F(AccountReconcilorTest, WontMergeAccountsWithError) {
token_service()->UpdateCredentials(account_id2, "refresh_token"); token_service()->UpdateCredentials(account_id2, "refresh_token");
// Mark the secondary account in auth error state. // Mark the secondary account in auth error state.
token_service_delegate()->UpdateAuthError( token_service()->UpdateAuthErrorForTesting(
account_id2, account_id2,
GoogleServiceAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS)); GoogleServiceAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
......
...@@ -80,6 +80,12 @@ void FakeProfileOAuth2TokenService::IssueTokenForAllPendingRequests( ...@@ -80,6 +80,12 @@ void FakeProfileOAuth2TokenService::IssueTokenForAllPendingRequests(
expiration); expiration);
} }
void FakeProfileOAuth2TokenService::UpdateAuthErrorForTesting(
const std::string& account_id,
const GoogleServiceAuthError& error) {
ProfileOAuth2TokenService::UpdateAuthError(account_id, error);
}
void FakeProfileOAuth2TokenService::CompleteRequests( void FakeProfileOAuth2TokenService::CompleteRequests(
const std::string& account_id, const std::string& account_id,
bool all_scopes, bool all_scopes,
......
...@@ -81,6 +81,10 @@ class FakeProfileOAuth2TokenService : public ProfileOAuth2TokenService { ...@@ -81,6 +81,10 @@ class FakeProfileOAuth2TokenService : public ProfileOAuth2TokenService {
auto_post_fetch_response_on_message_loop_ = auto_post_response; auto_post_fetch_response_on_message_loop_ = auto_post_response;
} }
// Calls ProfileOAuth2TokenService::UpdateAuthError(). Exposed for testing.
void UpdateAuthErrorForTesting(const std::string& account_id,
const GoogleServiceAuthError& error);
protected: protected:
// OAuth2TokenService overrides. // OAuth2TokenService overrides.
void FetchOAuth2Token(RequestImpl* request, void FetchOAuth2Token(RequestImpl* request,
......
...@@ -35,7 +35,8 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate { ...@@ -35,7 +35,8 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
void Shutdown() override; void Shutdown() override;
bool RefreshTokenIsAvailable(const std::string& account_id) const override; bool RefreshTokenIsAvailable(const std::string& account_id) const override;
bool RefreshTokenHasError(const std::string& account_id) const override; GoogleServiceAuthError GetAuthError(
const std::string& account_id) const override;
void UpdateAuthError(const std::string& account_id, void UpdateAuthError(const std::string& account_id,
const GoogleServiceAuthError& error) override; const GoogleServiceAuthError& error) override;
......
...@@ -164,10 +164,8 @@ ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::~AccountStatus() { ...@@ -164,10 +164,8 @@ ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::~AccountStatus() {
void ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::SetLastAuthError( void ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::SetLastAuthError(
const GoogleServiceAuthError& error) { const GoogleServiceAuthError& error) {
if (error.state() != last_auth_error_.state()) {
last_auth_error_ = error; last_auth_error_ = error;
signin_error_controller_->AuthStatusChanged(); signin_error_controller_->AuthStatusChanged();
}
} }
std::string ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::GetAccountId() std::string ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::GetAccountId()
...@@ -325,12 +323,11 @@ bool ProfileOAuth2TokenServiceIOSDelegate::RefreshTokenIsAvailable( ...@@ -325,12 +323,11 @@ bool ProfileOAuth2TokenServiceIOSDelegate::RefreshTokenIsAvailable(
return accounts_.count(account_id) > 0; return accounts_.count(account_id) > 0;
} }
bool ProfileOAuth2TokenServiceIOSDelegate::RefreshTokenHasError( GoogleServiceAuthError ProfileOAuth2TokenServiceIOSDelegate::GetAuthError(
const std::string& account_id) const { const std::string& account_id) const {
DCHECK(thread_checker_.CalledOnValidThread());
auto it = accounts_.find(account_id); auto it = accounts_.find(account_id);
// TODO(rogerta): should we distinguish between transient and persistent? return (it == accounts_.end()) ? GoogleServiceAuthError::AuthErrorNone()
return it == accounts_.end() ? false : IsError(it->second->GetAuthStatus()); : it->second->GetAuthStatus();
} }
void ProfileOAuth2TokenServiceIOSDelegate::UpdateAuthError( void ProfileOAuth2TokenServiceIOSDelegate::UpdateAuthError(
...@@ -341,16 +338,19 @@ void ProfileOAuth2TokenServiceIOSDelegate::UpdateAuthError( ...@@ -341,16 +338,19 @@ void ProfileOAuth2TokenServiceIOSDelegate::UpdateAuthError(
// Do not report connection errors as these are not actually auth errors. // Do not report connection errors as these are not actually auth errors.
// We also want to avoid masking a "real" auth error just because we // We also want to avoid masking a "real" auth error just because we
// subsequently get a transient network error. // subsequently get a transient network error.
if (error.state() == GoogleServiceAuthError::CONNECTION_FAILED || if (error.IsTransientError())
error.state() == GoogleServiceAuthError::SERVICE_UNAVAILABLE) {
return; return;
}
if (accounts_.count(account_id) == 0) { if (accounts_.count(account_id) == 0) {
// Nothing to update as the account has already been removed. // Nothing to update as the account has already been removed.
return; return;
} }
accounts_[account_id]->SetLastAuthError(error);
AccountStatus* status = accounts_[account_id].get();
if (error.state() != status->GetAuthStatus().state()) {
status->SetLastAuthError(error);
FireAuthErrorChanged(account_id, error);
}
} }
// Clear the authentication error state and notify all observers that a new // Clear the authentication error state and notify all observers that a new
...@@ -375,6 +375,7 @@ void ProfileOAuth2TokenServiceIOSDelegate::AddOrUpdateAccount( ...@@ -375,6 +375,7 @@ void ProfileOAuth2TokenServiceIOSDelegate::AddOrUpdateAccount(
if (!account_present) { if (!account_present) {
accounts_[account_id].reset( accounts_[account_id].reset(
new AccountStatus(signin_error_controller_, account_id)); new AccountStatus(signin_error_controller_, account_id));
FireAuthErrorChanged(account_id, accounts_[account_id]->GetAuthStatus());
} }
UpdateAuthError(account_id, GoogleServiceAuthError::AuthErrorNone()); UpdateAuthError(account_id, GoogleServiceAuthError::AuthErrorNone());
......
...@@ -43,6 +43,8 @@ class ProfileOAuth2TokenServiceIOSDelegateTest ...@@ -43,6 +43,8 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
tokens_loaded_count_(0), tokens_loaded_count_(0),
access_token_success_(0), access_token_success_(0),
access_token_failure_(0), access_token_failure_(0),
error_changed_count_(0),
auth_error_changed_count_(0),
last_access_token_error_(GoogleServiceAuthError::NONE) {} last_access_token_error_(GoogleServiceAuthError::NONE) {}
void SetUp() override { void SetUp() override {
...@@ -93,7 +95,12 @@ class ProfileOAuth2TokenServiceIOSDelegateTest ...@@ -93,7 +95,12 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
++token_revoked_count_; ++token_revoked_count_;
} }
void OnRefreshTokensLoaded() override { ++tokens_loaded_count_; } void OnRefreshTokensLoaded() override { ++tokens_loaded_count_; }
void OnAuthErrorChanged(const std::string& account_id,
const GoogleServiceAuthError& error) override {
++auth_error_changed_count_;
}
// SigninErrorController::Observer implementation.
void OnErrorChanged() override { ++error_changed_count_; } void OnErrorChanged() override { ++error_changed_count_; }
void ResetObserverCounts() { void ResetObserverCounts() {
...@@ -103,6 +110,7 @@ class ProfileOAuth2TokenServiceIOSDelegateTest ...@@ -103,6 +110,7 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
token_available_count_ = 0; token_available_count_ = 0;
access_token_failure_ = 0; access_token_failure_ = 0;
error_changed_count_ = 0; error_changed_count_ = 0;
auth_error_changed_count_ = 0;
} }
std::string GetAccountId(const ProviderAccount& provider_account) { std::string GetAccountId(const ProviderAccount& provider_account) {
...@@ -126,6 +134,7 @@ class ProfileOAuth2TokenServiceIOSDelegateTest ...@@ -126,6 +134,7 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
int access_token_success_; int access_token_success_;
int access_token_failure_; int access_token_failure_;
int error_changed_count_; int error_changed_count_;
int auth_error_changed_count_;
GoogleServiceAuthError last_access_token_error_; GoogleServiceAuthError last_access_token_error_;
}; };
...@@ -298,10 +307,32 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, ...@@ -298,10 +307,32 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest,
ResetObserverCounts(); ResetObserverCounts();
GoogleServiceAuthError error(GoogleServiceAuthError::SERVICE_ERROR); GoogleServiceAuthError error(GoogleServiceAuthError::SERVICE_ERROR);
oauth2_delegate_->UpdateAuthError(GetAccountId(account1), error); oauth2_delegate_->UpdateAuthError(GetAccountId(account1), error);
EXPECT_EQ(error, oauth2_delegate_->GetAuthError("gaia_1"));
EXPECT_EQ(1, auth_error_changed_count_);
EXPECT_EQ(1, error_changed_count_); EXPECT_EQ(1, error_changed_count_);
oauth2_delegate_->RevokeAllCredentials(); oauth2_delegate_->RevokeAllCredentials();
ResetObserverCounts(); ResetObserverCounts();
oauth2_delegate_->UpdateAuthError(GetAccountId(account1), error); oauth2_delegate_->UpdateAuthError(GetAccountId(account1), error);
EXPECT_EQ(0, auth_error_changed_count_);
EXPECT_EQ(0, error_changed_count_); EXPECT_EQ(0, error_changed_count_);
} }
TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, GetAuthError) {
// Accounts have no error by default.
ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x");
oauth2_delegate_->ReloadCredentials(GetAccountId(account1));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
oauth2_delegate_->GetAuthError("gaia_1"));
// Update the error.
GoogleServiceAuthError error =
GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_SERVER);
oauth2_delegate_->UpdateAuthError("gaia_1", error);
EXPECT_EQ(error, oauth2_delegate_->GetAuthError("gaia_1"));
// Unknown account has no error.
EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
oauth2_delegate_->GetAuthError("gaia_2"));
}
...@@ -34,11 +34,11 @@ bool FakeOAuth2TokenServiceDelegate::RefreshTokenIsAvailable( ...@@ -34,11 +34,11 @@ bool FakeOAuth2TokenServiceDelegate::RefreshTokenIsAvailable(
return !GetRefreshToken(account_id).empty(); return !GetRefreshToken(account_id).empty();
} }
bool FakeOAuth2TokenServiceDelegate::RefreshTokenHasError( GoogleServiceAuthError FakeOAuth2TokenServiceDelegate::GetAuthError(
const std::string& account_id) const { const std::string& account_id) const {
auto it = refresh_tokens_.find(account_id); auto it = refresh_tokens_.find(account_id);
// TODO(rogerta): should we distinguish between transient and persistent? return (it == refresh_tokens_.end()) ? GoogleServiceAuthError::AuthErrorNone()
return it == refresh_tokens_.end() ? false : IsError(it->second->error); : it->second->error;
} }
std::string FakeOAuth2TokenServiceDelegate::GetRefreshToken( std::string FakeOAuth2TokenServiceDelegate::GetRefreshToken(
...@@ -104,7 +104,11 @@ FakeOAuth2TokenServiceDelegate::GetRequestContext() const { ...@@ -104,7 +104,11 @@ FakeOAuth2TokenServiceDelegate::GetRequestContext() const {
void FakeOAuth2TokenServiceDelegate::UpdateAuthError( void FakeOAuth2TokenServiceDelegate::UpdateAuthError(
const std::string& account_id, const std::string& account_id,
const GoogleServiceAuthError& error) { const GoogleServiceAuthError& error) {
if (GetAuthError(account_id) == error)
return;
auto it = refresh_tokens_.find(account_id); auto it = refresh_tokens_.find(account_id);
DCHECK(it != refresh_tokens_.end()); DCHECK(it != refresh_tokens_.end());
it->second->error = error; it->second->error = error;
FireAuthErrorChanged(account_id, error);
} }
...@@ -23,7 +23,8 @@ class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate { ...@@ -23,7 +23,8 @@ class FakeOAuth2TokenServiceDelegate : public OAuth2TokenServiceDelegate {
// Overriden to make sure it works on Android. // Overriden to make sure it works on Android.
bool RefreshTokenIsAvailable(const std::string& account_id) const override; bool RefreshTokenIsAvailable(const std::string& account_id) const override;
bool RefreshTokenHasError(const std::string& account_id) const override; GoogleServiceAuthError GetAuthError(
const std::string& account_id) const override;
void UpdateAuthError(const std::string& account_id, void UpdateAuthError(const std::string& account_id,
const GoogleServiceAuthError& error) override; const GoogleServiceAuthError& error) override;
......
...@@ -574,7 +574,14 @@ bool OAuth2TokenService::RefreshTokenIsAvailable( ...@@ -574,7 +574,14 @@ bool OAuth2TokenService::RefreshTokenIsAvailable(
bool OAuth2TokenService::RefreshTokenHasError( bool OAuth2TokenService::RefreshTokenHasError(
const std::string& account_id) const { const std::string& account_id) const {
return delegate_->RefreshTokenHasError(account_id); return GetAuthError(account_id) != GoogleServiceAuthError::AuthErrorNone();
}
GoogleServiceAuthError OAuth2TokenService::GetAuthError(
const std::string& account_id) const {
GoogleServiceAuthError error = delegate_->GetAuthError(account_id);
DCHECK(!error.IsTransientError());
return error;
} }
void OAuth2TokenService::RevokeAllCredentials() { void OAuth2TokenService::RevokeAllCredentials() {
......
...@@ -110,6 +110,11 @@ class OAuth2TokenService { ...@@ -110,6 +110,11 @@ class OAuth2TokenService {
virtual void OnStartBatchChanges() {} virtual void OnStartBatchChanges() {}
// Sent after a batch of refresh token changes is done. // Sent after a batch of refresh token changes is done.
virtual void OnEndBatchChanges() {} virtual void OnEndBatchChanges() {}
// Called when the authentication error state for |account_id| has changed.
// Note: It is always called after |OnRefreshTokenAvailable| when refresh
// token is updated. It is not called when the refresh token is revoked.
virtual void OnAuthErrorChanged(const std::string& account_id,
const GoogleServiceAuthError& auth_error) {}
protected: protected:
virtual ~Observer() {} virtual ~Observer() {}
...@@ -186,10 +191,14 @@ class OAuth2TokenService { ...@@ -186,10 +191,14 @@ class OAuth2TokenService {
// |StartRequest| will result in a Consumer::OnGetTokenFailure callback. // |StartRequest| will result in a Consumer::OnGetTokenFailure callback.
bool RefreshTokenIsAvailable(const std::string& account_id) const; bool RefreshTokenIsAvailable(const std::string& account_id) const;
// Returns true if a refresh token exists for |account_id| and it is in an // Returns true if a refresh token exists for |account_id| and it is in a
// error state. // persistent error state.
bool RefreshTokenHasError(const std::string& account_id) const; bool RefreshTokenHasError(const std::string& account_id) const;
// Returns the auth error associated with |account_id|. Only persistent errors
// will be returned.
GoogleServiceAuthError GetAuthError(const std::string& account_id) const;
// This method cancels all token requests, revoke all refresh tokens and // This method cancels all token requests, revoke all refresh tokens and
// cached access tokens. // cached access tokens.
void RevokeAllCredentials(); void RevokeAllCredentials();
......
...@@ -51,11 +51,6 @@ void OAuth2TokenServiceDelegate::RemoveObserver( ...@@ -51,11 +51,6 @@ void OAuth2TokenServiceDelegate::RemoveObserver(
observer_list_.RemoveObserver(observer); observer_list_.RemoveObserver(observer);
} }
// static
bool OAuth2TokenServiceDelegate::IsError(const GoogleServiceAuthError& error) {
return error.IsPersistentError();
}
void OAuth2TokenServiceDelegate::StartBatchChanges() { void OAuth2TokenServiceDelegate::StartBatchChanges() {
++batch_change_depth_; ++batch_change_depth_;
if (batch_change_depth_ == 1) { if (batch_change_depth_ == 1) {
...@@ -90,14 +85,21 @@ void OAuth2TokenServiceDelegate::FireRefreshTokensLoaded() { ...@@ -90,14 +85,21 @@ void OAuth2TokenServiceDelegate::FireRefreshTokensLoaded() {
observer.OnRefreshTokensLoaded(); observer.OnRefreshTokensLoaded();
} }
void OAuth2TokenServiceDelegate::FireAuthErrorChanged(
const std::string& account_id,
const GoogleServiceAuthError& error) {
for (auto& observer : observer_list_)
observer.OnAuthErrorChanged(account_id, error);
}
net::URLRequestContextGetter* OAuth2TokenServiceDelegate::GetRequestContext() net::URLRequestContextGetter* OAuth2TokenServiceDelegate::GetRequestContext()
const { const {
return nullptr; return nullptr;
} }
bool OAuth2TokenServiceDelegate::RefreshTokenHasError( GoogleServiceAuthError OAuth2TokenServiceDelegate::GetAuthError(
const std::string& account_id) const { const std::string& account_id) const {
return false; return GoogleServiceAuthError::AuthErrorNone();
} }
std::vector<std::string> OAuth2TokenServiceDelegate::GetAccounts() { std::vector<std::string> OAuth2TokenServiceDelegate::GetAccounts() {
......
...@@ -40,7 +40,8 @@ class OAuth2TokenServiceDelegate { ...@@ -40,7 +40,8 @@ class OAuth2TokenServiceDelegate {
OAuth2AccessTokenConsumer* consumer) = 0; OAuth2AccessTokenConsumer* consumer) = 0;
virtual bool RefreshTokenIsAvailable(const std::string& account_id) const = 0; virtual bool RefreshTokenIsAvailable(const std::string& account_id) const = 0;
virtual bool RefreshTokenHasError(const std::string& account_id) const; virtual GoogleServiceAuthError GetAuthError(
const std::string& account_id) const;
virtual void UpdateAuthError(const std::string& account_id, virtual void UpdateAuthError(const std::string& account_id,
const GoogleServiceAuthError& error) {} const GoogleServiceAuthError& error) {}
...@@ -75,10 +76,13 @@ class OAuth2TokenServiceDelegate { ...@@ -75,10 +76,13 @@ class OAuth2TokenServiceDelegate {
virtual LoadCredentialsState GetLoadCredentialsState() const; virtual LoadCredentialsState GetLoadCredentialsState() const;
protected: protected:
// Called by subclasses to notify observers. // Called by subclasses to notify observers. Some are virtual to allow Android
// to broadcast the notifications to Java code.
virtual void FireRefreshTokenAvailable(const std::string& account_id); virtual void FireRefreshTokenAvailable(const std::string& account_id);
virtual void FireRefreshTokenRevoked(const std::string& account_id); virtual void FireRefreshTokenRevoked(const std::string& account_id);
virtual void FireRefreshTokensLoaded(); virtual void FireRefreshTokensLoaded();
void FireAuthErrorChanged(const std::string& account_id,
const GoogleServiceAuthError& error);
// Helper class to scope batch changes. // Helper class to scope batch changes.
class ScopedBatchChange { class ScopedBatchChange {
...@@ -91,12 +95,6 @@ class OAuth2TokenServiceDelegate { ...@@ -91,12 +95,6 @@ class OAuth2TokenServiceDelegate {
DISALLOW_COPY_AND_ASSIGN(ScopedBatchChange); DISALLOW_COPY_AND_ASSIGN(ScopedBatchChange);
}; };
// This function is called by derived classes to help implement
// RefreshTokenHasError(). It centralizes the code for determining if
// |error| is worthy of being reported as an error for purposes of
// RefreshTokenHasError().
static bool IsError(const GoogleServiceAuthError& error);
private: private:
// List of observers to notify when refresh token availability changes. // List of observers to notify when refresh token availability changes.
// Makes sure list is empty on destruction. // Makes sure list is empty on destruction.
......
...@@ -397,10 +397,7 @@ NSDictionary* AuthenticationService::GetCachedMDMInfo( ...@@ -397,10 +397,7 @@ NSDictionary* AuthenticationService::GetCachedMDMInfo(
return nil; return nil;
} }
ProfileOAuth2TokenServiceIOSDelegate* token_service_delegate = if (!token_service_->RefreshTokenHasError(it->first)) {
static_cast<ProfileOAuth2TokenServiceIOSDelegate*>(
token_service_->GetDelegate());
if (!token_service_delegate->RefreshTokenHasError(it->first)) {
// Account has no error, invalidate the cache. // Account has no error, invalidate the cache.
cached_mdm_infos_.erase(it); cached_mdm_infos_.erase(it);
return nil; return nil;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment