Commit 0e2fa9d7 authored by James Cook's avatar James Cook Committed by Commit Bot

Migrate SigninErrorNotifier to unconsented primary account

SplitSettingsSync will allow Chrome OS users to opt-out of browser
sync.  However, IdentityAccessor::GetPrimaryAccountInfo() assumes
the user has consented to browser sync. We still need to detect cases
where the refresh token is invalid, even if the user has not consented
to browser sync.

Switch to using the "unconsented" primary account. On Chrome OS this
account always exists for the logged-in user account, whether or not
the user consented to browser sync.

See go/cros-sync-mock and go/cros-primary-account for details.

Bug: 1042400, 760610
Test: updated existing components_unittests
Test: Bug 760610 still does not repro with SplitSettingsSync enabled
Change-Id: I030e45eca6bcf82381bf1be9a4e6f829b7a4a3a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2040531
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739314}
parent 55ebad2e
......@@ -13,6 +13,7 @@
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_error_controller_factory.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/sync/driver/sync_service.h"
#include "components/user_manager/user_manager.h"
......@@ -70,7 +71,8 @@ void AuthSyncObserver::OnErrorChanged() {
signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(profile_);
HandleAuthError(identity_manager->GetErrorStateOfRefreshTokenForAccount(
identity_manager->GetPrimaryAccountId()));
identity_manager->GetPrimaryAccountId(
signin::ConsentLevel::kNotRequired)));
}
void AuthSyncObserver::HandleAuthError(
......
......@@ -38,6 +38,7 @@
#include "chrome/grit/theme_resources.h"
#include "chromeos/components/account_manager/account_manager_factory.h"
#include "components/account_id/account_id.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/user_manager/user_manager.h"
#include "components/vector_icons/vector_icons.h"
......@@ -143,8 +144,10 @@ void SigninErrorNotifier::OnErrorChanged() {
}
const CoreAccountId error_account_id = error_controller_->error_account_id();
if (error_account_id ==
identity_manager_->GetPrimaryAccountInfo().account_id) {
const CoreAccountId primary_account_id =
identity_manager_->GetPrimaryAccountId(
signin::ConsentLevel::kNotRequired);
if (error_account_id == primary_account_id) {
HandleDeviceAccountError();
} else {
HandleSecondaryAccountError(error_account_id);
......
......@@ -132,6 +132,25 @@ TEST_F(SigninErrorNotifierTest, ErrorResetForPrimaryAccount) {
display_service_->GetNotification(kPrimaryAccountErrorNotificationId));
}
TEST_F(SigninErrorNotifierTest, ErrorShownForUnconsentedPrimaryAccount) {
EXPECT_FALSE(
display_service_->GetNotification(kPrimaryAccountErrorNotificationId));
CoreAccountId account_id =
identity_test_env()
->MakeUnconsentedPrimaryAccountAvailable(kTestEmail)
.account_id;
SetAuthError(
account_id,
GoogleServiceAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
EXPECT_TRUE(
display_service_->GetNotification(kPrimaryAccountErrorNotificationId));
SetAuthError(account_id, GoogleServiceAuthError::AuthErrorNone());
EXPECT_FALSE(
display_service_->GetNotification(kPrimaryAccountErrorNotificationId));
}
TEST_F(SigninErrorNotifierTest, ErrorResetForSecondaryAccount) {
EXPECT_FALSE(
display_service_->GetNotification(kSecondaryAccountErrorNotificationId));
......
......@@ -5,10 +5,8 @@
#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_SIGNIN_ERROR_CONTROLLER_H_
#define COMPONENTS_SIGNIN_CORE_BROWSER_SIGNIN_ERROR_CONTROLLER_H_
#include <set>
#include <string>
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/scoped_observer.h"
......@@ -28,8 +26,8 @@ class SigninErrorController : public KeyedService,
// are in error state, only one of the errors is reported.
ANY_ACCOUNT,
// Only errors on the primary account are reported. Other accounts are
// ignored.
// Only errors on the primary account are reported. The primary account
// must have sync consent. Other accounts are ignored.
PRIMARY_ACCOUNT
};
......
......@@ -110,6 +110,31 @@ TEST(SigninErrorControllerTest, AccountTransitionAnyAccount) {
ASSERT_FALSE(error_controller.HasError());
}
// Verifies errors are reported in mode ANY_ACCOUNT even if the primary account
// has not consented to the browser sync feature.
TEST(SigninErrorControllerTest, UnconsentedPrimaryAccount) {
base::test::TaskEnvironment task_environment;
signin::IdentityTestEnvironment identity_test_env;
CoreAccountId test_account_id =
identity_test_env.MakeUnconsentedPrimaryAccountAvailable(kTestEmail)
.account_id;
SigninErrorController error_controller(
SigninErrorController::AccountMode::ANY_ACCOUNT,
identity_test_env.identity_manager());
ASSERT_FALSE(error_controller.HasError());
identity_test_env.UpdatePersistentErrorOfRefreshTokenForAccount(
test_account_id,
GoogleServiceAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
EXPECT_TRUE(error_controller.HasError());
EXPECT_EQ(test_account_id, error_controller.error_account_id());
identity_test_env.UpdatePersistentErrorOfRefreshTokenForAccount(
test_account_id, GoogleServiceAuthError::AuthErrorNone());
EXPECT_FALSE(error_controller.HasError());
}
// This test exercises behavior on signin/signout, which is not relevant on
// ChromeOS.
#if !defined(OS_CHROMEOS)
......
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