Commit 5e6bc8b7 authored by dizg's avatar dizg Committed by Commit Bot

[signin] Supports OAuth outage in Dice.

This is the client side implementation of go/gaiafe-dice-fallback.
The Gaia server will send parameter no_authorization_code in the Dice
header, when there is a OAuth outage.

When this happens and the feature kSupportOAuthOutageInDice  is enabled,
Chrome disable the reconcilor temporarily, so that the user can stay
signed in on the web.
After some time, the reconcilor is restarted, and at this point the user
is signed out, and will need to sign in again.

Also added unit tests.

Bug: 1115247
Change-Id: Ib71f55f26f50c185175a1abeec58edd4d7d72605
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362757Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Commit-Queue: Diana Zagidullina <dizg@google.com>
Cr-Commit-Position: refs/heads/master@{#800512}
parent 77c2c069
...@@ -33,6 +33,12 @@ ...@@ -33,6 +33,12 @@
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
const int kDiceTokenFetchTimeoutSeconds = 10; const int kDiceTokenFetchTimeoutSeconds = 10;
// Timeout for locking the account reconcilor when
// there was OAuth outage in Dice.
const int kLockAccountReconcilorTimeoutHours = 12;
const base::Feature kSupportOAuthOutageInDice{
"SupportOAuthOutageInDice", base::FEATURE_DISABLED_BY_DEFAULT};
namespace { namespace {
...@@ -226,9 +232,9 @@ void DiceResponseHandler::ProcessDiceHeader( ...@@ -226,9 +232,9 @@ void DiceResponseHandler::ProcessDiceHeader(
case signin::DiceAction::SIGNIN: { case signin::DiceAction::SIGNIN: {
const signin::DiceResponseParams::AccountInfo& info = const signin::DiceResponseParams::AccountInfo& info =
dice_params.signin_info->account_info; dice_params.signin_info->account_info;
ProcessDiceSigninHeader(info.gaia_id, info.email, ProcessDiceSigninHeader(
dice_params.signin_info->authorization_code, info.gaia_id, info.email, dice_params.signin_info->authorization_code,
std::move(delegate)); dice_params.signin_info->no_authorization_code, std::move(delegate));
return; return;
} }
case signin::DiceAction::ENABLE_SYNC: { case signin::DiceAction::ENABLE_SYNC: {
...@@ -252,11 +258,30 @@ size_t DiceResponseHandler::GetPendingDiceTokenFetchersCountForTesting() const { ...@@ -252,11 +258,30 @@ size_t DiceResponseHandler::GetPendingDiceTokenFetchersCountForTesting() const {
return token_fetchers_.size(); return token_fetchers_.size();
} }
void DiceResponseHandler::OnTimeoutUnlockReconcilor() {
lock_.reset();
}
void DiceResponseHandler::ProcessDiceSigninHeader( void DiceResponseHandler::ProcessDiceSigninHeader(
const std::string& gaia_id, const std::string& gaia_id,
const std::string& email, const std::string& email,
const std::string& authorization_code, const std::string& authorization_code,
bool no_authorization_code,
std::unique_ptr<ProcessDiceHeaderDelegate> delegate) { std::unique_ptr<ProcessDiceHeaderDelegate> delegate) {
if (no_authorization_code) {
if (base::FeatureList::IsEnabled(kSupportOAuthOutageInDice)) {
lock_ = std::make_unique<AccountReconcilor::Lock>(account_reconcilor_);
// If there is already another lock, the timer will be reset and
// we'll wait another full timeout.
timer_.Start(
FROM_HERE,
base::TimeDelta::FromHours(kLockAccountReconcilorTimeoutHours),
base::BindOnce(&DiceResponseHandler::OnTimeoutUnlockReconcilor,
base::Unretained(this)));
}
return;
}
DCHECK(!gaia_id.empty()); DCHECK(!gaia_id.empty());
DCHECK(!email.empty()); DCHECK(!email.empty());
DCHECK(!authorization_code.empty()); DCHECK(!authorization_code.empty());
......
...@@ -30,6 +30,9 @@ class IdentityManager; ...@@ -30,6 +30,9 @@ class IdentityManager;
// Exposed for testing. // Exposed for testing.
extern const int kDiceTokenFetchTimeoutSeconds; extern const int kDiceTokenFetchTimeoutSeconds;
// Exposed for testing.
extern const int kLockAccountReconcilorTimeoutHours;
extern const base::Feature kSupportOAuthOutageInDice;
// Delegate interface for processing a dice request. // Delegate interface for processing a dice request.
class ProcessDiceHeaderDelegate { class ProcessDiceHeaderDelegate {
...@@ -132,6 +135,7 @@ class DiceResponseHandler : public KeyedService { ...@@ -132,6 +135,7 @@ class DiceResponseHandler : public KeyedService {
const std::string& gaia_id, const std::string& gaia_id,
const std::string& email, const std::string& email,
const std::string& authorization_code, const std::string& authorization_code,
bool no_authorization_code,
std::unique_ptr<ProcessDiceHeaderDelegate> delegate); std::unique_ptr<ProcessDiceHeaderDelegate> delegate);
// Process the Dice enable sync action. // Process the Dice enable sync action.
...@@ -152,6 +156,8 @@ class DiceResponseHandler : public KeyedService { ...@@ -152,6 +156,8 @@ class DiceResponseHandler : public KeyedService {
bool is_under_advanced_protection); bool is_under_advanced_protection);
void OnTokenExchangeFailure(DiceTokenFetcher* token_fetcher, void OnTokenExchangeFailure(DiceTokenFetcher* token_fetcher,
const GoogleServiceAuthError& error); const GoogleServiceAuthError& error);
// Called to unlock the reconcilor after a SLO outage.
void OnTimeoutUnlockReconcilor();
SigninClient* signin_client_; SigninClient* signin_client_;
signin::IdentityManager* identity_manager_; signin::IdentityManager* identity_manager_;
...@@ -159,6 +165,10 @@ class DiceResponseHandler : public KeyedService { ...@@ -159,6 +165,10 @@ class DiceResponseHandler : public KeyedService {
AboutSigninInternals* about_signin_internals_; AboutSigninInternals* about_signin_internals_;
base::FilePath profile_path_; base::FilePath profile_path_;
std::vector<std::unique_ptr<DiceTokenFetcher>> token_fetchers_; std::vector<std::unique_ptr<DiceTokenFetcher>> token_fetchers_;
// Lock the account reconcilor for kLockAccountReconcilorTimeoutHours
// when there was OAuth outage in Dice.
std::unique_ptr<AccountReconcilor::Lock> lock_;
base::OneShotTimer timer_;
DISALLOW_COPY_AND_ASSIGN(DiceResponseHandler); DISALLOW_COPY_AND_ASSIGN(DiceResponseHandler);
}; };
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/notreached.h" #include "base/notreached.h"
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -262,6 +263,115 @@ TEST_F(DiceResponseHandlerTest, Signin) { ...@@ -262,6 +263,115 @@ TEST_F(DiceResponseHandlerTest, Signin) {
.is_under_advanced_protection); .is_under_advanced_protection);
} }
// Checks that the account reconcilor is blocked when where was OAuth
// outage in Dice, and unblocked after the timeout.
TEST_F(DiceResponseHandlerTest, SupportOAuthOutageInDice) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kSupportOAuthOutageInDice);
DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNIN);
dice_params.signin_info->authorization_code.clear();
dice_params.signin_info->no_authorization_code = true;
dice_response_handler_->ProcessDiceHeader(
dice_params, std::make_unique<TestProcessDiceHeaderDelegate>(this));
// Check that the reconcilor was blocked and not unblocked before timeout.
EXPECT_EQ(1, reconcilor_blocked_count_);
EXPECT_EQ(0, reconcilor_unblocked_count_);
task_environment_.FastForwardBy(
base::TimeDelta::FromHours(kLockAccountReconcilorTimeoutHours + 1));
// Check that the reconcilor was unblocked.
EXPECT_EQ(1, reconcilor_unblocked_count_);
EXPECT_EQ(1, reconcilor_blocked_count_);
}
// Check that after receiving two headers with no authorization code,
// timeout still restarts.
TEST_F(DiceResponseHandlerTest, CheckTimersDuringOutageinDice) {
ASSERT_GT(kLockAccountReconcilorTimeoutHours, 3);
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kSupportOAuthOutageInDice);
// Create params for the first header with no authorization code.
DiceResponseParams dice_params_1 = MakeDiceParams(DiceAction::SIGNIN);
dice_params_1.signin_info->authorization_code.clear();
dice_params_1.signin_info->no_authorization_code = true;
dice_response_handler_->ProcessDiceHeader(
dice_params_1, std::make_unique<TestProcessDiceHeaderDelegate>(this));
// Check that the reconcilor was blocked and not unblocked before timeout.
EXPECT_EQ(1, reconcilor_blocked_count_);
EXPECT_EQ(0, reconcilor_unblocked_count_);
// Wait half of the timeout.
task_environment_.FastForwardBy(
base::TimeDelta::FromHours(kLockAccountReconcilorTimeoutHours / 2));
// Create params for the second header with no authorization code.
DiceResponseParams dice_params_2 = MakeDiceParams(DiceAction::SIGNIN);
dice_params_2.signin_info->authorization_code.clear();
dice_params_2.signin_info->no_authorization_code = true;
dice_response_handler_->ProcessDiceHeader(
dice_params_2, std::make_unique<TestProcessDiceHeaderDelegate>(this));
task_environment_.FastForwardBy(base::TimeDelta::FromHours(
(kLockAccountReconcilorTimeoutHours + 1) / 2 + 1));
// Check that the reconcilor was not unblocked after the first timeout
// passed, timer should be restarted after getting the second header.
EXPECT_EQ(1, reconcilor_blocked_count_);
EXPECT_EQ(0, reconcilor_unblocked_count_);
task_environment_.FastForwardBy(
base::TimeDelta::FromHours((kLockAccountReconcilorTimeoutHours + 1) / 2));
// Check that the reconcilor was unblocked.
EXPECT_EQ(1, reconcilor_blocked_count_);
EXPECT_EQ(1, reconcilor_unblocked_count_);
}
// Check that signin works normally (the token is fetched and added to chrome)
// on valid headers after getting a no_authorization_code header.
TEST_F(DiceResponseHandlerTest, CheckSigninAfterOutageInDice) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kSupportOAuthOutageInDice);
// Create params for the header with no authorization code.
DiceResponseParams dice_params_1 = MakeDiceParams(DiceAction::SIGNIN);
dice_params_1.signin_info->authorization_code.clear();
dice_params_1.signin_info->no_authorization_code = true;
dice_response_handler_->ProcessDiceHeader(
dice_params_1, std::make_unique<TestProcessDiceHeaderDelegate>(this));
// Create params for the valid header with an authorization code.
DiceResponseParams dice_params_2 = MakeDiceParams(DiceAction::SIGNIN);
const auto& account_info_2 = dice_params_2.signin_info->account_info;
CoreAccountId account_id_2 = identity_manager()->PickAccountIdForAccount(
account_info_2.gaia_id, account_info_2.email);
EXPECT_FALSE(identity_manager()->HasAccountWithRefreshToken(account_id_2));
dice_response_handler_->ProcessDiceHeader(
dice_params_2, std::make_unique<TestProcessDiceHeaderDelegate>(this));
// Check that the reconcilor was blocked and not unblocked before timeout.
EXPECT_EQ(1, reconcilor_blocked_count_);
EXPECT_EQ(0, reconcilor_unblocked_count_);
// Check that a GaiaAuthFetcher has been created.
GaiaAuthConsumer* consumer = signin_client_.GetAndClearConsumer();
ASSERT_THAT(consumer, testing::NotNull());
// Simulate GaiaAuthFetcher success.
consumer->OnClientOAuthSuccess(GaiaAuthConsumer::ClientOAuthResult(
"refresh_token", "access_token", 10, false /* is_child_account */,
true /* is_advanced_protection*/));
// Check that the token has been inserted in the token service.
EXPECT_TRUE(identity_manager()->HasAccountWithRefreshToken(account_id_2));
EXPECT_TRUE(auth_error_email_.empty());
EXPECT_EQ(GoogleServiceAuthError::NONE, auth_error_.state());
// Check HandleTokenExchangeSuccess parameters.
EXPECT_EQ(token_exchange_account_id_, account_id_2);
EXPECT_TRUE(token_exchange_is_new_account_);
EXPECT_EQ(1, reconcilor_blocked_count_);
EXPECT_EQ(0, reconcilor_unblocked_count_);
// Check that the AccountInfo::is_under_advanced_protection is set.
EXPECT_TRUE(
identity_manager()
->FindExtendedAccountInfoForAccountWithRefreshTokenByAccountId(
account_id_2)
.value()
.is_under_advanced_protection);
task_environment_.FastForwardBy(
base::TimeDelta::FromHours(kLockAccountReconcilorTimeoutHours + 1));
// Check that the reconcilor was unblocked.
EXPECT_EQ(1, reconcilor_unblocked_count_);
EXPECT_EQ(1, reconcilor_blocked_count_);
}
// Checks that a SIGNIN action triggers a token exchange request when the // Checks that a SIGNIN action triggers a token exchange request when the
// account is in authentication error. // account is in authentication error.
TEST_F(DiceResponseHandlerTest, Reauth) { TEST_F(DiceResponseHandlerTest, Reauth) {
......
...@@ -26,6 +26,7 @@ const char kRequestSigninAll[] = "all_accounts"; ...@@ -26,6 +26,7 @@ const char kRequestSigninAll[] = "all_accounts";
const char kSigninActionAttrName[] = "action"; const char kSigninActionAttrName[] = "action";
const char kSigninAuthUserAttrName[] = "authuser"; const char kSigninAuthUserAttrName[] = "authuser";
const char kSigninAuthorizationCodeAttrName[] = "authorization_code"; const char kSigninAuthorizationCodeAttrName[] = "authorization_code";
const char kSigninNoAuthorizationCodeAttrName[] = "no_authorization_code";
const char kSigninEmailAttrName[] = "email"; const char kSigninEmailAttrName[] = "email";
const char kSigninIdAttrName[] = "id"; const char kSigninIdAttrName[] = "id";
...@@ -101,6 +102,13 @@ DiceResponseParams DiceHeaderHelper::BuildDiceSigninResponseParams( ...@@ -101,6 +102,13 @@ DiceResponseParams DiceHeaderHelper::BuildDiceSigninResponseParams(
params.signin_info->authorization_code = value; params.signin_info->authorization_code = value;
else else
DLOG(WARNING) << "Authorization code expected only with SIGNIN action"; DLOG(WARNING) << "Authorization code expected only with SIGNIN action";
} else if (key_name == kSigninNoAuthorizationCodeAttrName) {
if (params.signin_info) {
params.signin_info->no_authorization_code = true;
} else {
DLOG(WARNING)
<< "No authorization code header expected only with SIGNIN action";
}
} else { } else {
DLOG(WARNING) << "Unexpected Gaia header attribute '" << key_name << "'."; DLOG(WARNING) << "Unexpected Gaia header attribute '" << key_name << "'.";
} }
...@@ -112,9 +120,11 @@ DiceResponseParams DiceHeaderHelper::BuildDiceSigninResponseParams( ...@@ -112,9 +120,11 @@ DiceResponseParams DiceHeaderHelper::BuildDiceSigninResponseParams(
return DiceResponseParams(); return DiceResponseParams();
} }
if (params.signin_info && params.signin_info->authorization_code.empty()) { if (params.signin_info && params.signin_info->authorization_code.empty() &&
DLOG(WARNING) << "Missing authorization code in Dice SIGNIN header: " !params.signin_info->no_authorization_code) {
<< header_value; DLOG(WARNING)
<< "Missing authorization code and no authorization code headers"
<< "in Dice SIGNIN header: " << header_value;
return DiceResponseParams(); return DiceResponseParams();
} }
......
...@@ -114,6 +114,9 @@ struct DiceResponseParams { ...@@ -114,6 +114,9 @@ struct DiceResponseParams {
AccountInfo account_info; AccountInfo account_info;
// Authorization code to fetch a refresh token. // Authorization code to fetch a refresh token.
std::string authorization_code; std::string authorization_code;
// Whether Dice response contains the 'no_authorization_code' header value.
// If true then LSO was unavailable for provision of auth code.
bool no_authorization_code = false;
}; };
// Parameters for the SIGNOUT action. // Parameters for the SIGNOUT action.
......
...@@ -488,7 +488,23 @@ TEST_F(SigninHeaderHelperTest, TestBuildDiceResponseParams) { ...@@ -488,7 +488,23 @@ TEST_F(SigninHeaderHelperTest, TestBuildDiceResponseParams) {
} }
{ {
// Missing authorization code. // Signin response with no_authorization_code and missing
// authorization_code.
DiceResponseParams params = BuildDiceSigninResponseParams(
base::StringPrintf("action=SIGNIN,id=%s,email=%s,authuser=%i,no_"
"authorization_code=true",
kGaiaID, kEmail, kSessionIndex));
EXPECT_EQ(DiceAction::SIGNIN, params.user_intention);
ASSERT_TRUE(params.signin_info);
EXPECT_EQ(kGaiaID, params.signin_info->account_info.gaia_id);
EXPECT_EQ(kEmail, params.signin_info->account_info.email);
EXPECT_EQ(kSessionIndex, params.signin_info->account_info.session_index);
EXPECT_TRUE(params.signin_info->authorization_code.empty());
EXPECT_TRUE(params.signin_info->no_authorization_code);
}
{
// Missing authorization code and no_authorization_code.
DiceResponseParams params = BuildDiceSigninResponseParams( DiceResponseParams params = BuildDiceSigninResponseParams(
base::StringPrintf("action=SIGNIN,id=%s,email=%s,authuser=%i", kGaiaID, base::StringPrintf("action=SIGNIN,id=%s,email=%s,authuser=%i", kGaiaID,
kEmail, kSessionIndex)); kEmail, kSessionIndex));
......
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