Commit 903f5b28 authored by Andrey Zaytsev's avatar Andrey Zaytsev Committed by Commit Bot

Safety check: updated password check to separately handle the case of no passwords

Bug: 1063016, 1015841
Change-Id: I4ed8b40738ce57a8b0fcb7aea66db3e0cc9f600b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109701
Commit-Queue: Andrey Zaytsev <andzaytsev@google.com>
Reviewed-by: default avatarEsmael Elmoslimany <aee@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752407}
parent 2a63cb9d
...@@ -66,6 +66,7 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate { ...@@ -66,6 +66,7 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate {
void SetOptedInForAccountStorage(bool opted_in); void SetOptedInForAccountStorage(bool opted_in);
void AddCompromisedCredential(int id); void AddCompromisedCredential(int id);
void ClearSavedPasswordsList() { current_entries_.clear(); }
void ResetPlaintextPassword() { plaintext_password_.reset(); } void ResetPlaintextPassword() { plaintext_password_.reset(); }
bool ImportPasswordsTriggered() const { return import_passwords_triggered_; } bool ImportPasswordsTriggered() const { return import_passwords_triggered_; }
bool ExportPasswordsTriggered() const { return export_passwords_triggered_; } bool ExportPasswordsTriggered() const { return export_passwords_triggered_; }
......
...@@ -466,6 +466,14 @@ base::string16 SafetyCheckHandler::GetStringForParentRan( ...@@ -466,6 +466,14 @@ base::string16 SafetyCheckHandler::GetStringForParentRan(
} }
} }
void SafetyCheckHandler::DetermineIfNoPasswordsOrSafe(
const std::vector<extensions::api::passwords_private::PasswordUiEntry>&
passwords) {
OnPasswordsCheckResult(passwords.empty() ? PasswordsStatus::kNoPasswords
: PasswordsStatus::kSafe,
0);
}
void SafetyCheckHandler::OnStateChanged( void SafetyCheckHandler::OnStateChanged(
password_manager::BulkLeakCheckService::State state) { password_manager::BulkLeakCheckService::State state) {
using password_manager::BulkLeakCheckService; using password_manager::BulkLeakCheckService;
...@@ -475,7 +483,9 @@ void SafetyCheckHandler::OnStateChanged( ...@@ -475,7 +483,9 @@ void SafetyCheckHandler::OnStateChanged(
size_t num_compromised = size_t num_compromised =
passwords_delegate_->GetCompromisedCredentials().size(); passwords_delegate_->GetCompromisedCredentials().size();
if (num_compromised == 0) { if (num_compromised == 0) {
OnPasswordsCheckResult(PasswordsStatus::kSafe, 0); passwords_delegate_->GetSavedPasswordsList(
base::BindOnce(&SafetyCheckHandler::DetermineIfNoPasswordsOrSafe,
base::Unretained(this)));
} else { } else {
OnPasswordsCheckResult(PasswordsStatus::kCompromisedExist, OnPasswordsCheckResult(PasswordsStatus::kCompromisedExist,
num_compromised); num_compromised);
...@@ -501,8 +511,6 @@ void SafetyCheckHandler::OnStateChanged( ...@@ -501,8 +511,6 @@ void SafetyCheckHandler::OnStateChanged(
OnPasswordsCheckResult(PasswordsStatus::kError, 0); OnPasswordsCheckResult(PasswordsStatus::kError, 0);
break; break;
} }
// TODO(crbug.com/1015841): implement detecting the following states if it is
// possible: kNoPasswords, kQuotaLimit, and kTooManyPasswords.
// Stop observing the leak service in all terminal states. // Stop observing the leak service in all terminal states.
observed_leak_check_.Remove(leak_service_); observed_leak_check_.Remove(leak_service_);
......
...@@ -152,6 +152,15 @@ class SafetyCheckHandler ...@@ -152,6 +152,15 @@ class SafetyCheckHandler
ReenabledUser reenabled_user, ReenabledUser reenabled_user,
ReenabledAdmin reenabled_admin); ReenabledAdmin reenabled_admin);
// Since the password check API does not distinguish between the cases of
// having no compromised passwords and not having any passwords at all, it is
// necessary to use this method as a callback for
// |PasswordsPrivateDelegate::GetSavedPasswordsList| to distinguish the two
// states here.
void DetermineIfNoPasswordsOrSafe(
const std::vector<extensions::api::passwords_private::PasswordUiEntry>&
passwords);
// BulkLeakCheckService::Observer implementation. // BulkLeakCheckService::Observer implementation.
void OnStateChanged( void OnStateChanged(
password_manager::BulkLeakCheckService::State state) override; password_manager::BulkLeakCheckService::State state) override;
......
...@@ -652,15 +652,16 @@ TEST_F(SafetyCheckHandlerTest, CheckPasswords_RunningOneCompromised) { ...@@ -652,15 +652,16 @@ TEST_F(SafetyCheckHandlerTest, CheckPasswords_RunningOneCompromised) {
} }
TEST_F(SafetyCheckHandlerTest, CheckPasswords_NoPasswords) { TEST_F(SafetyCheckHandlerTest, CheckPasswords_NoPasswords) {
test_passwords_delegate_.ClearSavedPasswordsList();
test_passwords_delegate_.SetStartPasswordCheckState( test_passwords_delegate_.SetStartPasswordCheckState(
password_manager::BulkLeakCheckService::State::kIdle); password_manager::BulkLeakCheckService::State::kIdle);
safety_check_->PerformSafetyCheck(); safety_check_->PerformSafetyCheck();
const base::DictionaryValue* event = const base::DictionaryValue* event =
GetSafetyCheckStatusChangedWithDataIfExists( GetSafetyCheckStatusChangedWithDataIfExists(
kPasswords, kPasswords,
static_cast<int>(SafetyCheckHandler::PasswordsStatus::kSafe)); static_cast<int>(SafetyCheckHandler::PasswordsStatus::kNoPasswords));
EXPECT_TRUE(event); EXPECT_TRUE(event);
VerifyDisplayString(event, "No compromised passwords found"); VerifyDisplayString(event, "No saved passwords");
} }
TEST_F(SafetyCheckHandlerTest, CheckExtensions_NoExtensions) { TEST_F(SafetyCheckHandlerTest, CheckExtensions_NoExtensions) {
......
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