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

[signin] In Mice mode, do not allow unknown accounts in the Gaia cookie

With Mirror and Dice, unknown accounts in the Gaia cookie are not
actively removed, to prevent unnecessary calls to /LogOut.
In Mice mode, we want the accounts in the cookie to exactly match the
accounts in the system, and thus only accounts known to Android are
allowed in the cookie.

Bug: 944826
Change-Id: I6c80c6207679b7828a5716571e6b7bbdde12a1bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538405Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644710}
parent 84c88085
...@@ -119,30 +119,6 @@ bool RevokeAllSecondaryTokens( ...@@ -119,30 +119,6 @@ bool RevokeAllSecondaryTokens(
return token_revoked; return token_revoked;
} }
// Returns true if current array of existing accounts in cookie is different
// from the desired one. If this returns false, the multilogin call would be a
// no-op.
bool AccountsNeedUpdate(
const signin::MultiloginParameters& parameters,
const std::vector<gaia::ListedAccount>& existing_accounts) {
if (parameters.mode ==
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER &&
!existing_accounts.empty() && !parameters.accounts_to_send.empty() &&
existing_accounts[0].id != parameters.accounts_to_send[0]) {
// In UPDATE mode update is needed if first accounts don't match.
return true;
}
// Maybe some accounts in cookies are not valid and need refreshing.
std::set<std::string> accounts_to_send_set(
parameters.accounts_to_send.begin(), parameters.accounts_to_send.end());
std::set<std::string> existing_accounts_set;
for (const gaia::ListedAccount& account : existing_accounts) {
if (account.valid)
existing_accounts_set.insert(account.id);
}
return (existing_accounts_set != accounts_to_send_set);
}
// Pick the account will become first after this reconcile is finished. // Pick the account will become first after this reconcile is finished.
std::string PickFirstGaiaAccount( std::string PickFirstGaiaAccount(
const signin::MultiloginParameters& parameters, const signin::MultiloginParameters& parameters,
...@@ -156,6 +132,20 @@ std::string PickFirstGaiaAccount( ...@@ -156,6 +132,20 @@ std::string PickFirstGaiaAccount(
: parameters.accounts_to_send[0]; : parameters.accounts_to_send[0];
} }
// Returns true if gaia_accounts contains an invalid account that is unknown to
// the identity manager.
bool HasUnknownInvalidAccountInCookie(
identity::IdentityManager* identity_manager,
const std::vector<gaia::ListedAccount>& gaia_accounts) {
for (const gaia::ListedAccount& account : gaia_accounts) {
if (!account.valid &&
!identity_manager->HasAccountWithRefreshToken(account.gaia_id)) {
return true;
}
}
return false;
}
} // namespace } // namespace
AccountReconcilor::Lock::Lock(AccountReconcilor* reconcilor) AccountReconcilor::Lock::Lock(AccountReconcilor* reconcilor)
...@@ -498,7 +488,7 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint( ...@@ -498,7 +488,7 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint(
primary_has_error); primary_has_error);
DCHECK(is_reconcile_started_); DCHECK(is_reconcile_started_);
if (AccountsNeedUpdate(parameters_for_multilogin, gaia_accounts)) { if (CookieNeedsUpdate(parameters_for_multilogin, gaia_accounts)) {
if (parameters_for_multilogin.mode == if (parameters_for_multilogin.mode ==
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER && gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER &&
parameters_for_multilogin.accounts_to_send.empty()) { parameters_for_multilogin.accounts_to_send.empty()) {
...@@ -532,7 +522,7 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint( ...@@ -532,7 +522,7 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint(
// is consistent). If it is not the case, second reconcile is expected to be // is consistent). If it is not the case, second reconcile is expected to be
// triggered after changes are made. For that one the state is supposed to // triggered after changes are made. For that one the state is supposed to
// be already consistent. // be already consistent.
DCHECK(!AccountsNeedUpdate(parameters_for_multilogin, gaia_accounts)); DCHECK(!CookieNeedsUpdate(parameters_for_multilogin, gaia_accounts));
std::string first_gaia_account_after_reconcile = std::string first_gaia_account_after_reconcile =
PickFirstGaiaAccount(parameters_for_multilogin, gaia_accounts); PickFirstGaiaAccount(parameters_for_multilogin, gaia_accounts);
delegate_->OnReconcileFinished(first_gaia_account_after_reconcile, delegate_->OnReconcileFinished(first_gaia_account_after_reconcile,
...@@ -684,6 +674,8 @@ void AccountReconcilor::FinishReconcile( ...@@ -684,6 +674,8 @@ void AccountReconcilor::FinishReconcile(
std::vector<gaia::ListedAccount>&& gaia_accounts) { std::vector<gaia::ListedAccount>&& gaia_accounts) {
VLOG(1) << "AccountReconcilor::FinishReconcile"; VLOG(1) << "AccountReconcilor::FinishReconcile";
DCHECK(add_to_cookie_.empty()); DCHECK(add_to_cookie_.empty());
DCHECK(delegate_->IsUnknownInvalidAccountInCookieAllowed())
<< "Only supported in UPDATE mode";
size_t number_gaia_accounts = gaia_accounts.size(); size_t number_gaia_accounts = gaia_accounts.size();
// If there are any accounts in the gaia cookie but not in chrome, then // If there are any accounts in the gaia cookie but not in chrome, then
...@@ -970,3 +962,36 @@ bool AccountReconcilor::IsMultiloginEndpointEnabled() const { ...@@ -970,3 +962,36 @@ bool AccountReconcilor::IsMultiloginEndpointEnabled() const {
#endif #endif
return base::FeatureList::IsEnabled(kUseMultiloginEndpoint); return base::FeatureList::IsEnabled(kUseMultiloginEndpoint);
} }
bool AccountReconcilor::CookieNeedsUpdate(
const signin::MultiloginParameters& parameters,
const std::vector<gaia::ListedAccount>& existing_accounts) {
bool should_remove_unknown_account =
!delegate_->IsUnknownInvalidAccountInCookieAllowed() &&
HasUnknownInvalidAccountInCookie(identity_manager_, existing_accounts);
if (should_remove_unknown_account) {
// Removing unknown accounts in the cookie is only supported for UPDATE
// mode.
DCHECK_EQ(parameters.mode,
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER);
return true;
}
if (parameters.mode ==
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER &&
!existing_accounts.empty() && !parameters.accounts_to_send.empty() &&
existing_accounts[0].id != parameters.accounts_to_send[0]) {
// In UPDATE mode update is needed if first accounts don't match.
return true;
}
// Maybe some accounts in cookies are not valid and need refreshing.
std::set<std::string> accounts_to_send_set(
parameters.accounts_to_send.begin(), parameters.accounts_to_send.end());
std::set<std::string> existing_accounts_set;
for (const gaia::ListedAccount& account : existing_accounts) {
if (account.valid)
existing_accounts_set.insert(account.id);
}
return (existing_accounts_set != accounts_to_send_set);
}
...@@ -142,6 +142,7 @@ class AccountReconcilor : public KeyedService, ...@@ -142,6 +142,7 @@ class AccountReconcilor : public KeyedService,
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestTable, TableRowTest); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestTable, TableRowTest);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestDiceMultilogin, TableRowTest); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestDiceMultilogin, TableRowTest);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestMirrorMultilogin, TableRowTest); FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestMirrorMultilogin, TableRowTest);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTestMiceMultilogin, TableRowTest);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorDiceEndpointParamTest, FRIEND_TEST_ALL_PREFIXES(AccountReconcilorDiceEndpointParamTest,
DiceTokenServiceRegistration); DiceTokenServiceRegistration);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorDiceEndpointParamTest, FRIEND_TEST_ALL_PREFIXES(AccountReconcilorDiceEndpointParamTest,
...@@ -306,6 +307,13 @@ class AccountReconcilor : public KeyedService, ...@@ -306,6 +307,13 @@ class AccountReconcilor : public KeyedService,
// Returns true is multilogin endpoint can be enabled. // Returns true is multilogin endpoint can be enabled.
bool IsMultiloginEndpointEnabled() const; bool IsMultiloginEndpointEnabled() const;
// Returns true if current array of existing accounts in cookie is different
// from the desired one. If this returns false, the multilogin call would be a
// no-op.
bool CookieNeedsUpdate(
const signin::MultiloginParameters& parameters,
const std::vector<gaia::ListedAccount>& existing_accounts);
std::unique_ptr<signin::AccountReconcilorDelegate> delegate_; std::unique_ptr<signin::AccountReconcilorDelegate> delegate_;
// The IdentityManager associated with this reconcilor. // The IdentityManager associated with this reconcilor.
......
...@@ -181,4 +181,8 @@ base::TimeDelta AccountReconcilorDelegate::GetReconcileTimeout() const { ...@@ -181,4 +181,8 @@ base::TimeDelta AccountReconcilorDelegate::GetReconcileTimeout() const {
void AccountReconcilorDelegate::OnReconcileError( void AccountReconcilorDelegate::OnReconcileError(
const GoogleServiceAuthError& error) {} const GoogleServiceAuthError& error) {}
bool AccountReconcilorDelegate::IsUnknownInvalidAccountInCookieAllowed() const {
return true;
}
} // namespace signin } // namespace signin
...@@ -103,6 +103,12 @@ class AccountReconcilorDelegate { ...@@ -103,6 +103,12 @@ class AccountReconcilorDelegate {
// |OnReconcileError| is called before |OnReconcileFinished|. // |OnReconcileError| is called before |OnReconcileFinished|.
virtual void OnReconcileError(const GoogleServiceAuthError& error); virtual void OnReconcileError(const GoogleServiceAuthError& error);
// If this returns false, the reconcilor ensures that all accounts unknown to
// Chrome are always removed from the cookies (even if their session is
// expired). Returning false is only supported in with multilogin UPDATE mode.
// Defaults to true.
virtual bool IsUnknownInvalidAccountInCookieAllowed() const;
void set_reconcilor(AccountReconcilor* reconcilor) { void set_reconcilor(AccountReconcilor* reconcilor) {
reconcilor_ = reconcilor; reconcilor_ = reconcilor;
} }
......
...@@ -70,4 +70,9 @@ gaia::MultiloginMode MiceAccountReconcilorDelegate::CalculateModeForReconcile( ...@@ -70,4 +70,9 @@ gaia::MultiloginMode MiceAccountReconcilorDelegate::CalculateModeForReconcile(
return gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER; return gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER;
} }
bool MiceAccountReconcilorDelegate::IsUnknownInvalidAccountInCookieAllowed()
const {
return false;
}
} // namespace signin } // namespace signin
...@@ -40,6 +40,7 @@ class MiceAccountReconcilorDelegate : public AccountReconcilorDelegate { ...@@ -40,6 +40,7 @@ class MiceAccountReconcilorDelegate : public AccountReconcilorDelegate {
const std::string primary_account, const std::string primary_account,
bool first_execution, bool first_execution,
bool primary_has_error) const override; bool primary_has_error) const override;
bool IsUnknownInvalidAccountInCookieAllowed() const override;
DISALLOW_COPY_AND_ASSIGN(MiceAccountReconcilorDelegate); DISALLOW_COPY_AND_ASSIGN(MiceAccountReconcilorDelegate);
}; };
......
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