Commit 95098dc2 authored by Felipe Andrade's avatar Felipe Andrade Committed by Commit Bot

Handle accounts list from KerberosClient responses on KerberosCredentialsManager

Update KerberosCredentialsManager to use the list of remaining accounts from RemoveAccount()
and ClearAccounts() methods from KerberosClient. This way, the active principal can be
validated synchronously without calling the KerberosClient again.

Bug: 952251
Change-Id: I64a9016595a91f6241653b0c7e43f6fae7cb634a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1916900
Commit-Queue: Felipe Andrade <fsandrade@chromium.org>
Reviewed-by: default avatarLutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718090}
parent 4870dfc9
...@@ -500,7 +500,7 @@ void KerberosCredentialsManager::OnRemoveAccount( ...@@ -500,7 +500,7 @@ void KerberosCredentialsManager::OnRemoveAccount(
if (Succeeded(response.error())) { if (Succeeded(response.error())) {
// Reassign active principal if it got deleted. // Reassign active principal if it got deleted.
if (GetActivePrincipalName() == principal_name) if (GetActivePrincipalName() == principal_name)
ValidateActivePrincipal(); ValidateActivePrincipal(response.accounts());
// Express our condolence to the observers. // Express our condolence to the observers.
NotifyAccountsChanged(); NotifyAccountsChanged();
...@@ -532,7 +532,7 @@ void KerberosCredentialsManager::OnClearAccounts( ...@@ -532,7 +532,7 @@ void KerberosCredentialsManager::OnClearAccounts(
case kerberos::CLEAR_ONLY_MANAGED_ACCOUNTS: case kerberos::CLEAR_ONLY_MANAGED_ACCOUNTS:
case kerberos::CLEAR_ONLY_UNMANAGED_ACCOUNTS: case kerberos::CLEAR_ONLY_UNMANAGED_ACCOUNTS:
// Check if the active account was wiped and if so, replace it. // Check if the active account was wiped and if so, replace it.
ValidateActivePrincipal(); ValidateActivePrincipal(response.accounts());
break; break;
case kerberos::CLEAR_ONLY_UNMANAGED_REMEMBERED_PASSWORDS: case kerberos::CLEAR_ONLY_UNMANAGED_REMEMBERED_PASSWORDS:
...@@ -560,7 +560,7 @@ void KerberosCredentialsManager::OnListAccounts( ...@@ -560,7 +560,7 @@ void KerberosCredentialsManager::OnListAccounts(
const kerberos::ListAccountsResponse& response) { const kerberos::ListAccountsResponse& response) {
LogError("ListAccounts", response.error()); LogError("ListAccounts", response.error());
// Lazily validate principal here while we're at it. // Lazily validate principal here while we're at it.
DoValidateActivePrincipal(response); ValidateActivePrincipal(response.accounts());
std::move(callback).Run(response); std::move(callback).Run(response);
} }
...@@ -727,25 +727,18 @@ void KerberosCredentialsManager::ClearActivePrincipalName() { ...@@ -727,25 +727,18 @@ void KerberosCredentialsManager::ClearActivePrincipalName() {
kerberos_files_handler_.DeleteFiles(); kerberos_files_handler_.DeleteFiles();
} }
void KerberosCredentialsManager::ValidateActivePrincipal() { void KerberosCredentialsManager::ValidateActivePrincipal(
kerberos::ListAccountsRequest request; const RepeatedAccountField& accounts) {
KerberosClient::Get()->ListAccounts(
request,
base::BindOnce(&KerberosCredentialsManager::DoValidateActivePrincipal,
weak_factory_.GetWeakPtr()));
}
void KerberosCredentialsManager::DoValidateActivePrincipal(
const kerberos::ListAccountsResponse& response) {
const std::string& active_principal = GetActivePrincipalName(); const std::string& active_principal = GetActivePrincipalName();
bool found = false; bool found = false;
for (int n = 0; n < response.accounts_size() && !found; ++n)
found |= response.accounts(n).principal_name() == active_principal; for (const kerberos::Account& account : accounts)
found |= account.principal_name() == active_principal;
if (!found) { if (!found) {
VLOG(1) << "Active principal got removed. Restoring."; VLOG(1) << "Active principal got removed. Restoring.";
if (response.accounts_size() > 0) if (accounts.size() > 0)
SetActivePrincipalName(response.accounts(0).principal_name()); SetActivePrincipalName(accounts.Get(0).principal_name());
else else
ClearActivePrincipalName(); ClearActivePrincipalName();
} }
......
...@@ -149,6 +149,8 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer { ...@@ -149,6 +149,8 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
private: private:
friend class KerberosAddAccountRunner; friend class KerberosAddAccountRunner;
using RepeatedAccountField =
google::protobuf::RepeatedPtrField<kerberos::Account>;
// Callback on KerberosAddAccountRunner::Done. // Callback on KerberosAddAccountRunner::Done.
void OnAddAccountRunnerDone(KerberosAddAccountRunner* runner, void OnAddAccountRunnerDone(KerberosAddAccountRunner* runner,
...@@ -205,15 +207,11 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer { ...@@ -205,15 +207,11 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
void SetActivePrincipalName(const std::string& principal_name); void SetActivePrincipalName(const std::string& principal_name);
void ClearActivePrincipalName(); void ClearActivePrincipalName();
// Gets the current account list and calls DoValidateActivePrincipal(). // Checks whether the active principal is contained in the given |accounts|.
void ValidateActivePrincipal();
// Checks whether the active principal is contained in the given |response|.
// If not, resets it to the first principal or clears it if the list is empty. // If not, resets it to the first principal or clears it if the list is empty.
// It's not expected that this ever triggers, but it provides a fail safe if // It's expected to trigger if the active account is removed by
// the active principal should ever break for whatever reason. // |RemoveAccount()| or |ClearAccounts()|.
void DoValidateActivePrincipal( void ValidateActivePrincipal(const RepeatedAccountField& accounts);
const kerberos::ListAccountsResponse& response);
// Notification shown when the Kerberos ticket is about to expire. // Notification shown when the Kerberos ticket is about to expire.
void ShowTicketExpiryNotification(); void ShowTicketExpiryNotification();
......
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