Commit f3534cc8 authored by James Cook's avatar James Cook Committed by Commit Bot

chromeos: Add switch to allow login without setting primary account

Today, all Chrome OS users must consent to sync during login. For
the SplitSettingsSync project we want to allow them to use their
device without consenting to browser sync. However, Chrome OS
always sets the IdentityManager primary account during login, which
implies sync consent. We would like to switch to using the
"unconsented" primary account, which exists whether or not the
user has consented to sync.

However, there are a large number of places in the Chrome OS code
that use GetPrimaryAccount() to get the "main" user's account ID,
email or tokens. Add a command line switch that skips setting the
primary account during login and instead directly sets the
"unconsented" primary account. This allows me to manually test
to identify areas that need to change.

Convert some login and ARC ToS code to use the unconsented
primary account. This is a no-op if the switch isn't set, because
without the switch the primary account and unconsented primary
account are the same.

Bug: 1042400
Test: existing automated tests, also sign-in and ARC++ work on
      device without the switch set

Change-Id: Idfeece6ac79b97a102111fb165062614a81f5a9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2001018
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734549}
parent 69554cb3
......@@ -654,7 +654,8 @@ void UserSessionManager::RestoreAuthenticationSession(Profile* user_profile) {
auto* identity_manager = IdentityManagerFactory::GetForProfile(user_profile);
const bool account_id_valid =
identity_manager && !identity_manager->GetPrimaryAccountId().empty();
identity_manager &&
!identity_manager->GetUnconsentedPrimaryAccountId().empty();
if (!account_id_valid)
LOG(ERROR) << "No account is associated with sign-in manager on restore.";
UMA_HISTOGRAM_BOOLEAN("UserSessionManager.RestoreOnCrash.AccountIdValid",
......@@ -967,7 +968,7 @@ void UserSessionManager::OnSessionRestoreStateChanged(
user_status =
(identity_manager &&
identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
identity_manager->GetPrimaryAccountInfo().account_id))
identity_manager->GetUnconsentedPrimaryAccountInfo().account_id))
? user_manager::User::OAUTH2_TOKEN_STATUS_INVALID
: user_manager::User::OAUTH2_TOKEN_STATUS_VALID;
break;
......@@ -1303,7 +1304,9 @@ void UserSessionManager::InitProfilePreferences(
// Set initial prefs if the user is new, or if the user was already present on
// the device and the profile was re-created. This can happen e.g. in ext4
// migration in wipe mode.
if (user_manager->IsCurrentUserNew() || profile->IsNewProfile()) {
const bool is_new_profile =
user_manager->IsCurrentUserNew() || profile->IsNewProfile();
if (is_new_profile) {
SetFirstLoginPrefs(profile, user_context.GetPublicSessionLocale(),
user_context.GetPublicSessionInputMethod());
......@@ -1420,10 +1423,24 @@ void UserSessionManager::InitProfilePreferences(
->FindExtendedAccountInfoForAccountWithRefreshTokenByGaiaId(
gaia_id);
DCHECK(account_info.has_value());
if (user_manager->IsCurrentUserNew() || profile->IsNewProfile()) {
// TODO(jamescook): Replace switch with IsSplitSettingsSyncEnabled().
if (switches::UseUnconsentedPrimaryAccount()) {
if (is_new_profile) {
if (!identity_manager->HasPrimaryAccount()) {
// Set the account without recording browser sync consent.
identity_manager->GetPrimaryAccountMutator()
->SetUnconsentedPrimaryAccount(account_info->account_id);
}
}
CHECK(identity_manager->HasUnconsentedPrimaryAccount());
CHECK_EQ(identity_manager->GetUnconsentedPrimaryAccountInfo().gaia,
gaia_id);
} else {
// Set a primary account here because the profile might have been
// created with switches::UseUnconsentedPrimaryAccount set. Then the
// profile might only have an unconsented primary account.
identity_manager->GetPrimaryAccountMutator()->SetPrimaryAccount(
account_info->account_id);
} else {
CHECK(identity_manager->HasPrimaryAccount());
CHECK_EQ(identity_manager->GetPrimaryAccountInfo().gaia, gaia_id);
}
......@@ -1438,7 +1455,8 @@ void UserSessionManager::InitProfilePreferences(
gaia_id, user_context.GetAccountId().GetUserEmail());
}
CoreAccountId account_id = identity_manager->GetPrimaryAccountId();
CoreAccountId account_id =
identity_manager->GetUnconsentedPrimaryAccountId();
VLOG(1) << "Seed IdentityManager with the authenticated account info, "
<< "success=" << !account_id.empty();
......
......@@ -81,14 +81,15 @@ void OAuth2LoginManager::ContinueSessionRestore() {
void OAuth2LoginManager::RestoreSessionFromSavedTokens() {
signin::IdentityManager* identity_manager = GetIdentityManager();
if (identity_manager->HasPrimaryAccountWithRefreshToken()) {
if (identity_manager->HasAccountWithRefreshToken(
GetUnconsentedPrimaryAccountId())) {
VLOG(1) << "OAuth2 refresh token is already loaded.";
VerifySessionCookies();
} else {
VLOG(1) << "Waiting for OAuth2 refresh token being loaded from database.";
const CoreAccountInfo account_info =
identity_manager->GetPrimaryAccountInfo();
identity_manager->GetUnconsentedPrimaryAccountInfo();
// Flag user with unknown token status in case there are no saved tokens
// and OnRefreshTokenAvailable is not called. Flagging it here would
// cause user to go through Gaia in next login to obtain a new refresh
......@@ -129,7 +130,7 @@ void OAuth2LoginManager::OnRefreshTokenUpdatedForAccount(
return;
}
// Only restore session cookies for the primary account in the profile.
if (GetPrimaryAccountId() == account_info.account_id) {
if (GetUnconsentedPrimaryAccountId() == account_info.account_id) {
// The refresh token has changed, so stop any ongoing actions that were
// based on the old refresh token.
Stop();
......@@ -148,9 +149,10 @@ signin::IdentityManager* OAuth2LoginManager::GetIdentityManager() {
return IdentityManagerFactory::GetForProfile(user_profile_);
}
CoreAccountId OAuth2LoginManager::GetPrimaryAccountId() {
CoreAccountId OAuth2LoginManager::GetUnconsentedPrimaryAccountId() {
// Use the primary ID whether or not the user has consented to browser sync.
const CoreAccountId primary_account_id =
GetIdentityManager()->GetPrimaryAccountId();
GetIdentityManager()->GetUnconsentedPrimaryAccountId();
LOG_IF(ERROR, primary_account_id.empty()) << "Primary account id is empty.";
return primary_account_id;
}
......@@ -160,9 +162,9 @@ void OAuth2LoginManager::StoreOAuth2Token() {
signin::IdentityManager* identity_manager = GetIdentityManager();
// The primary account must be already set at this point.
DCHECK(identity_manager->HasPrimaryAccount());
DCHECK(identity_manager->HasUnconsentedPrimaryAccount());
const CoreAccountInfo primary_account_info =
identity_manager->GetPrimaryAccountInfo();
identity_manager->GetUnconsentedPrimaryAccountInfo();
// We already have the refresh token at this
// point, and will not get any additional callbacks from Account Manager or
......@@ -175,9 +177,9 @@ void OAuth2LoginManager::StoreOAuth2Token() {
void OAuth2LoginManager::VerifySessionCookies() {
DCHECK(!login_verifier_.get());
login_verifier_.reset(new OAuth2LoginVerifier(this, GetIdentityManager(),
GetPrimaryAccountId(),
oauthlogin_access_token_));
login_verifier_ = std::make_unique<OAuth2LoginVerifier>(
this, GetIdentityManager(), GetUnconsentedPrimaryAccountId(),
oauthlogin_access_token_);
if (restore_strategy_ == RESTORE_FROM_SAVED_OAUTH2_REFRESH_TOKEN) {
login_verifier_->VerifyUserCookies();
......@@ -215,7 +217,7 @@ void OAuth2LoginManager::OnListAccountsSuccess(
const std::vector<gaia::ListedAccount>& accounts) {
MergeVerificationOutcome outcome = POST_MERGE_SUCCESS;
// Let's analyze which accounts we see logged in here:
CoreAccountId user_account_id = GetPrimaryAccountId();
CoreAccountId user_account_id = GetUnconsentedPrimaryAccountId();
if (!accounts.empty()) {
bool found = false;
bool first = true;
......
......@@ -161,8 +161,8 @@ class OAuth2LoginManager : public KeyedService,
// Retrieves IdentityManager for |user_profile_|.
signin::IdentityManager* GetIdentityManager();
// Retrieves the primary account for |user_profile_|.
CoreAccountId GetPrimaryAccountId();
// Retrieves the primary account ID for |user_profile_|.
CoreAccountId GetUnconsentedPrimaryAccountId();
// Records |refresh_token_| to token service. The associated account id is
// assumed to be the primary account id of the user profile. If the primary
......
......@@ -328,8 +328,10 @@ void ArcTermsOfServiceScreenHandler::RecordConsents(
consent_auditor::ConsentAuditor* consent_auditor =
ConsentAuditorFactory::GetForProfile(profile);
auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
DCHECK(identity_manager->HasPrimaryAccount());
const CoreAccountId account_id = identity_manager->GetPrimaryAccountId();
// The account may or may not have consented to browser sync.
DCHECK(identity_manager->HasUnconsentedPrimaryAccount());
const CoreAccountId account_id =
identity_manager->GetUnconsentedPrimaryAccountId();
ArcPlayTermsOfServiceConsent play_consent;
play_consent.set_status(tos_accepted ? UserConsentTypes::GIVEN
......
......@@ -504,6 +504,13 @@ const char kTetherHostScansIgnoreWiredConnections[] =
// Shows all Bluetooth devices in UI (System Tray/Settings Page.)
const char kUnfilteredBluetoothDevices[] = "unfiltered-bluetooth-devices";
// Skips the call to IdentityManager SetPrimaryAccount() on login, using
// SetUnconsentedPrimaryAccount() instead. This marks the primary account as
// *not* consented to browser sync, allowing consent to be set later. Used for
// manual testing, not intended for production. See also
// chromeos::features::kSplitSettingsSync.
const char kUseUnconsentedPrimaryAccount[] = "use-unconsented-primary-account";
// Used to tell the policy infrastructure to not let profile initialization
// complete until policy is manually set by a test. This is used to provide
// backward compatibility with a few tests that incorrectly use the
......@@ -601,5 +608,10 @@ bool IsUnfilteredBluetoothDevicesEnabled() {
kUnfilteredBluetoothDevices);
}
bool UseUnconsentedPrimaryAccount() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(
kUseUnconsentedPrimaryAccount);
}
} // namespace switches
} // namespace chromeos
......@@ -200,6 +200,8 @@ extern const char kWaitForInitialPolicyFetchForTest[];
COMPONENT_EXPORT(CHROMEOS_CONSTANTS) extern const char kWakeOnWifiPacket[];
COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
extern const char kUnfilteredBluetoothDevices[];
COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
extern const char kUseUnconsentedPrimaryAccount[];
////////////////////////////////////////////////////////////////////////////////
......@@ -257,6 +259,9 @@ COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsArcCpuRestrictionDisabled();
// Returns true if all Bluetooth devices in UI (System Tray/Settings Page.)
COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsUnfilteredBluetoothDevicesEnabled();
// See kUseUnconsentedPrimaryAccount.
COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool UseUnconsentedPrimaryAccount();
} // namespace switches
} // namespace chromeos
......
......@@ -275,7 +275,7 @@ void AccountTrackerService::SetAccountImage(const CoreAccountId& account_id,
void AccountTrackerService::SetIsChildAccount(const CoreAccountId& account_id,
bool is_child_account) {
DCHECK(base::Contains(accounts_, account_id));
DCHECK(base::Contains(accounts_, account_id)) << account_id.ToString();
AccountInfo& account_info = accounts_[account_id];
if (account_info.is_child_account == is_child_account)
return;
......@@ -288,7 +288,7 @@ void AccountTrackerService::SetIsChildAccount(const CoreAccountId& account_id,
void AccountTrackerService::SetIsAdvancedProtectionAccount(
const CoreAccountId& account_id,
bool is_under_advanced_protection) {
DCHECK(base::Contains(accounts_, account_id));
DCHECK(base::Contains(accounts_, account_id)) << account_id.ToString();
AccountInfo& account_info = accounts_[account_id];
if (account_info.is_under_advanced_protection == is_under_advanced_protection)
return;
......
......@@ -52,6 +52,15 @@ bool PrimaryAccountMutatorImpl::SetPrimaryAccount(
}
#if defined(OS_CHROMEOS)
void PrimaryAccountMutatorImpl::SetUnconsentedPrimaryAccount(
const CoreAccountId& account_id) {
// On Chrome OS the UPA can only be set once and never removed or changed.
DCHECK(!account_id.empty());
DCHECK(!primary_account_manager_->HasUnconsentedPrimaryAccount());
AccountInfo account_info = account_tracker_->GetAccountInfo(account_id);
primary_account_manager_->SetUnconsentedPrimaryAccountInfo(account_info);
}
bool PrimaryAccountMutatorImpl::DeprecatedSetPrimaryAccountAndUpdateAccountInfo(
const std::string& gaia_id,
const std::string& email) {
......
......@@ -27,6 +27,7 @@ class PrimaryAccountMutatorImpl : public PrimaryAccountMutator {
// PrimaryAccountMutator implementation.
bool SetPrimaryAccount(const CoreAccountId& account_id) override;
#if defined(OS_CHROMEOS)
void SetUnconsentedPrimaryAccount(const CoreAccountId& account_id) override;
bool DeprecatedSetPrimaryAccountAndUpdateAccountInfo(
const std::string& gaia_id,
const std::string& email) override;
......
......@@ -500,8 +500,13 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const {
if (HasPrimaryAccount())
return GetPrimaryAccountInfo();
#if defined(OS_CHROMEOS) || defined(OS_IOS) || defined(OS_ANDROID)
// On ChromeOS and on mobile platforms, we support only the primary account as
#if defined(OS_CHROMEOS)
// Chrome OS directly sets either the primary account or the unconsented
// primary account during login. The user is not allowed to sign out, so
// keep the value set at login (don't reset).
return base::nullopt;
#elif defined(OS_IOS) || defined(OS_ANDROID)
// On iOS and Android platforms, we support only the primary account as
// the unconsented primary account. By this early return, we avoid an extra
// request to GAIA that lists cookie accounts.
return CoreAccountInfo();
......
......@@ -162,19 +162,22 @@ class IdentityManager : public KeyedService,
// Provides access to the core information of the user's primary account.
// Returns an empty struct if no such info is available, either because there
// is no primary account yet or because the user signed out.
// is no primary account yet or because the user signed out. A non-empty
// struct implies that the user has blessed this account for sync (see
// ./README.md).
CoreAccountInfo GetPrimaryAccountInfo() const;
// Provides access to the account ID of the user's primary account. Simple
// convenience wrapper over GetPrimaryAccountInfo().account_id.
CoreAccountId GetPrimaryAccountId() const;
// Returns whether the user's primary account is available.
// Returns whether the user's primary account is available. True implies that
// the user has blessed this account for sync (see ./README.md).
bool HasPrimaryAccount() const;
// Provides access to the core information of the user's unconsented primary
// account (see ./README.md). Returns an empty info, if there is no such
// account.
// account. The user may or may not have blessed the account for sync.
CoreAccountInfo GetUnconsentedPrimaryAccountInfo() const;
// Provides access to the account ID of the user's unconsented primary
......
......@@ -60,6 +60,13 @@ class PrimaryAccountMutator {
virtual bool SetPrimaryAccount(const CoreAccountId& account_id) = 0;
#if defined(OS_CHROMEOS)
// Sets the account with |account_id| as the unconsented primary account
// (i.e. without implying browser sync consent). Requires that the account
// is known by the IdentityManager. See README.md for details on the meaning
// of "unconsented".
virtual void SetUnconsentedPrimaryAccount(
const CoreAccountId& account_id) = 0;
// Updates the info of the account corresponding to (|gaia_id|, |email|),
// marks it as the primary account, and returns whether the operation
// succeeded or not. Currently, this method is guaranteed to succeed.
......
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