Commit 5fdcc009 authored by Viktor Semeniuk's avatar Viktor Semeniuk Committed by Commit Bot

[iOS][Password Check] Added possibility to cancel password check

This change adds possibility to cancel Password Check when it's running.

Bug: 1075494
Change-Id: If266b44db8b52cae16346d37782cac5cd1a4dbf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274881
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784363}
parent aaee5ca5
...@@ -1623,6 +1623,9 @@ While in incognito, sites can't use cookies to see your browsing activity across ...@@ -1623,6 +1623,9 @@ While in incognito, sites can't use cookies to see your browsing activity across
<message name="IDS_IOS_CHECK_PASSWORDS_NOW_BUTTON" desc="Label of a button which starts Password Check [Length: 22em] [iOS only]" meaning="By clicking this button user will start checking all passwords for security issues."> <message name="IDS_IOS_CHECK_PASSWORDS_NOW_BUTTON" desc="Label of a button which starts Password Check [Length: 22em] [iOS only]" meaning="By clicking this button user will start checking all passwords for security issues.">
Check Now Check Now
</message> </message>
<message name="IDS_IOS_CANCEL_PASSWORD_CHECK_BUTTON" desc="Label of a button which stops Password Check [Length: 22em] [iOS only]" meaning="By clicking this button user will stop checking all passwords for security issues.">
Cancel
</message>
<message name="IDS_IOS_PASSWORD_ISSUES_DESCRIPTION" desc="Description for the screen with Compromised passwords [iOS only]" meaning="This label explains what is compromised password and why is it compromised. It suggest user fixing the issues as soon as possible."> <message name="IDS_IOS_PASSWORD_ISSUES_DESCRIPTION" desc="Description for the screen with Compromised passwords [iOS only]" meaning="This label explains what is compromised password and why is it compromised. It suggest user fixing the issues as soon as possible.">
The following accounts use passwords which were exposed in a third-party data breach or entered on a deceptive website. Change these passwords immediately to keep your accounts safe. The following accounts use passwords which were exposed in a third-party data breach or entered on a deceptive website. Change these passwords immediately to keep your accounts safe.
</message> </message>
......
65c0bde9dad6a3ea032dee0d0e8e0049d1f1a6f2
\ No newline at end of file
...@@ -23,13 +23,14 @@ class IOSChromePasswordCheckManagerProxy; ...@@ -23,13 +23,14 @@ class IOSChromePasswordCheckManagerProxy;
// Enum which represents possible states of Password Check on UI. // Enum which represents possible states of Password Check on UI.
// It's created based on BulkLeakCheckService::State. // It's created based on BulkLeakCheckService::State.
enum class PasswordCheckState { enum class PasswordCheckState {
kCanceled,
kIdle, kIdle,
kRunning,
kSignedOut,
kOffline,
kNoPasswords, kNoPasswords,
kQuotaLimit, kOffline,
kOther, kOther,
kQuotaLimit,
kRunning,
kSignedOut,
}; };
// This class handles the bulk password check feature. // This class handles the bulk password check feature.
...@@ -51,6 +52,9 @@ class IOSChromePasswordCheckManager ...@@ -51,6 +52,9 @@ class IOSChromePasswordCheckManager
// Requests to start a check for compromised passwords. // Requests to start a check for compromised passwords.
void StartPasswordCheck(); void StartPasswordCheck();
// Stops checking for compromised passwords.
void StopPasswordCheck();
// Returns the current state of the password check. // Returns the current state of the password check.
PasswordCheckState GetPasswordCheckState() const; PasswordCheckState GetPasswordCheckState() const;
......
...@@ -56,6 +56,7 @@ PasswordCheckState ConvertBulkCheckState(State state) { ...@@ -56,6 +56,7 @@ PasswordCheckState ConvertBulkCheckState(State state) {
case State::kQuotaLimit: case State::kQuotaLimit:
return PasswordCheckState::kQuotaLimit; return PasswordCheckState::kQuotaLimit;
case State::kCanceled: case State::kCanceled:
return PasswordCheckState::kCanceled;
case State::kTokenRequestFailure: case State::kTokenRequestFailure:
case State::kHashingFailure: case State::kHashingFailure:
case State::kServiceError: case State::kServiceError:
...@@ -101,6 +102,11 @@ void IOSChromePasswordCheckManager::StartPasswordCheck() { ...@@ -101,6 +102,11 @@ void IOSChromePasswordCheckManager::StartPasswordCheck() {
is_check_running_ = true; is_check_running_ = true;
} }
void IOSChromePasswordCheckManager::StopPasswordCheck() {
bulk_leak_check_service_adapter_.StopBulkLeakCheck();
is_check_running_ = false;
}
PasswordCheckState IOSChromePasswordCheckManager::GetPasswordCheckState() PasswordCheckState IOSChromePasswordCheckManager::GetPasswordCheckState()
const { const {
if (saved_passwords_presenter_.GetSavedPasswords().empty()) { if (saved_passwords_presenter_.GetSavedPasswords().empty()) {
......
...@@ -77,6 +77,7 @@ ...@@ -77,6 +77,7 @@
case PasswordCheckState::kSignedOut: case PasswordCheckState::kSignedOut:
case PasswordCheckState::kOffline: case PasswordCheckState::kOffline:
case PasswordCheckState::kQuotaLimit: case PasswordCheckState::kQuotaLimit:
case PasswordCheckState::kCanceled:
case PasswordCheckState::kOther: { case PasswordCheckState::kOther: {
if (!_manager->GetCompromisedCredentials().empty()) { if (!_manager->GetCompromisedCredentials().empty()) {
return PasswordCheckStateUnSafe; return PasswordCheckStateUnSafe;
......
...@@ -875,9 +875,19 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf( ...@@ -875,9 +875,19 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf(
_checkForProblemsItem.textColor = [UIColor colorNamed:kBlueColor]; _checkForProblemsItem.textColor = [UIColor colorNamed:kBlueColor];
_checkForProblemsItem.accessibilityTraits &= _checkForProblemsItem.accessibilityTraits &=
~UIAccessibilityTraitNotEnabled; ~UIAccessibilityTraitNotEnabled;
_checkForProblemsItem.text =
l10n_util::GetNSString(IDS_IOS_CHECK_PASSWORDS_NOW_BUTTON);
break; break;
case PasswordCheckStateRunning: case PasswordCheckStateRunning:
_checkForProblemsItem.textColor = [UIColor colorNamed:kBlueColor];
_checkForProblemsItem.accessibilityTraits &=
~UIAccessibilityTraitNotEnabled;
_checkForProblemsItem.text =
l10n_util::GetNSString(IDS_IOS_CANCEL_PASSWORD_CHECK_BUTTON);
break;
case PasswordCheckStateDisabled: case PasswordCheckStateDisabled:
_checkForProblemsItem.text =
l10n_util::GetNSString(IDS_IOS_CHECK_PASSWORDS_NOW_BUTTON);
_checkForProblemsItem.textColor = UIColor.cr_secondaryLabelColor; _checkForProblemsItem.textColor = UIColor.cr_secondaryLabelColor;
_checkForProblemsItem.accessibilityTraits |= _checkForProblemsItem.accessibilityTraits |=
UIAccessibilityTraitNotEnabled; UIAccessibilityTraitNotEnabled;
...@@ -1163,7 +1173,13 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf( ...@@ -1163,7 +1173,13 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf(
} }
break; break;
case ItemTypeCheckForProblemsButton: case ItemTypeCheckForProblemsButton:
_passwordCheck->StartPasswordCheck(); if (_passwordCheck->GetPasswordCheckState() ==
PasswordCheckState::kRunning) {
_passwordCheck->StopPasswordCheck();
} else if (_passwordCheck->GetPasswordCheckState() !=
PasswordCheckState::kNoPasswords) {
_passwordCheck->StartPasswordCheck();
}
break; break;
default: default:
NOTREACHED(); NOTREACHED();
......
...@@ -13,8 +13,8 @@ ...@@ -13,8 +13,8 @@
#include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form.h"
#include "components/keyed_service/core/service_access_type.h" #include "components/keyed_service/core/service_access_type.h"
#include "components/password_manager/core/browser/mock_bulk_leak_check_service.h" #include "components/password_manager/core/browser/mock_bulk_leak_check_service.h"
#include "components/password_manager/core/browser/mock_password_store.h"
#include "components/password_manager/core/browser/password_manager_test_utils.h" #include "components/password_manager/core/browser/password_manager_test_utils.h"
#include "components/password_manager/core/browser/test_password_store.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/passwords/ios_chrome_bulk_leak_check_service_factory.h" #include "ios/chrome/browser/passwords/ios_chrome_bulk_leak_check_service_factory.h"
...@@ -42,8 +42,9 @@ ...@@ -42,8 +42,9 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
using password_manager::MockPasswordStore; using password_manager::TestPasswordStore;
using password_manager::MockBulkLeakCheckService; using password_manager::MockBulkLeakCheckService;
using ::testing::Return;
// Declaration to conformance to SavePasswordsConsumerDelegate and keep tests in // Declaration to conformance to SavePasswordsConsumerDelegate and keep tests in
// this file working. // this file working.
...@@ -91,7 +92,7 @@ class PasswordsTableViewControllerTest ...@@ -91,7 +92,7 @@ class PasswordsTableViewControllerTest
chrome_browser_state_.get(), chrome_browser_state_.get(),
base::BindRepeating( base::BindRepeating(
&password_manager::BuildPasswordStore<web::BrowserState, &password_manager::BuildPasswordStore<web::BrowserState,
MockPasswordStore>)); TestPasswordStore>));
IOSChromeBulkLeakCheckServiceFactory::GetInstance() IOSChromeBulkLeakCheckServiceFactory::GetInstance()
->SetTestingFactoryAndUse( ->SetTestingFactoryAndUse(
...@@ -120,8 +121,8 @@ class PasswordsTableViewControllerTest ...@@ -120,8 +121,8 @@ class PasswordsTableViewControllerTest
int SectionsOffset() { return GetParam().password_check_enabled ? 1 : 0; } int SectionsOffset() { return GetParam().password_check_enabled ? 1 : 0; }
MockPasswordStore& GetMockStore() { TestPasswordStore& GetTestStore() {
return *static_cast<MockPasswordStore*>( return *static_cast<TestPasswordStore*>(
IOSChromePasswordStoreFactory::GetForBrowserState( IOSChromePasswordStoreFactory::GetForBrowserState(
chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS) chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS)
.get()); .get());
...@@ -148,6 +149,7 @@ class PasswordsTableViewControllerTest ...@@ -148,6 +149,7 @@ class PasswordsTableViewControllerTest
void AddPasswordForm(std::unique_ptr<autofill::PasswordForm> form) { void AddPasswordForm(std::unique_ptr<autofill::PasswordForm> form) {
PasswordsTableViewController* passwords_controller = PasswordsTableViewController* passwords_controller =
static_cast<PasswordsTableViewController*>(controller()); static_cast<PasswordsTableViewController*>(controller());
GetTestStore().AddLogin(*form);
std::vector<std::unique_ptr<autofill::PasswordForm>> passwords; std::vector<std::unique_ptr<autofill::PasswordForm>> passwords;
passwords.push_back(std::move(form)); passwords.push_back(std::move(form));
[passwords_controller onGetPasswordStoreResults:std::move(passwords)]; [passwords_controller onGetPasswordStoreResults:std::move(passwords)];
...@@ -230,6 +232,8 @@ class PasswordsTableViewControllerTest ...@@ -230,6 +232,8 @@ class PasswordsTableViewControllerTest
base::test::ios::kWaitForUIElementTimeout, condition); base::test::ios::kWaitForUIElementTimeout, condition);
} }
void RunUntilIdle() { task_environment_.RunUntilIdle(); }
web::WebTaskEnvironment task_environment_; web::WebTaskEnvironment task_environment_;
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_; std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
...@@ -468,7 +472,6 @@ TEST_P(PasswordsTableViewControllerTest, PropagateDeletionToStore) { ...@@ -468,7 +472,6 @@ TEST_P(PasswordsTableViewControllerTest, PropagateDeletionToStore) {
AddPasswordForm(std::make_unique<autofill::PasswordForm>(form)); AddPasswordForm(std::make_unique<autofill::PasswordForm>(form));
EXPECT_CALL(GetMockStore(), RemoveLogin(form));
[passwords_controller passwordDetailsTableViewController:nil [passwords_controller passwordDetailsTableViewController:nil
deletePassword:form]; deletePassword:form];
} }
...@@ -544,6 +547,8 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStateDefault) { ...@@ -544,6 +547,8 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStateDefault) {
ChangePasswordCheckState(PasswordCheckStateDefault); ChangePasswordCheckState(PasswordCheckStateDefault);
CheckTextCellTextWithId(IDS_IOS_CHECK_PASSWORDS_NOW_BUTTON,
GetSectionIndex(PasswordCheck), 1);
CheckDetailItemTextWithIds(IDS_IOS_CHECK_PASSWORDS, CheckDetailItemTextWithIds(IDS_IOS_CHECK_PASSWORDS,
IDS_IOS_CHECK_PASSWORDS_DESCRIPTION, IDS_IOS_CHECK_PASSWORDS_DESCRIPTION,
GetSectionIndex(PasswordCheck), 0); GetSectionIndex(PasswordCheck), 0);
...@@ -561,6 +566,8 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStateRunning) { ...@@ -561,6 +566,8 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStateRunning) {
ChangePasswordCheckState(PasswordCheckStateRunning); ChangePasswordCheckState(PasswordCheckStateRunning);
CheckTextCellTextWithId(IDS_IOS_CANCEL_PASSWORD_CHECK_BUTTON,
GetSectionIndex(PasswordCheck), 1);
CheckDetailItemTextWithIds(IDS_IOS_CHECK_PASSWORDS, CheckDetailItemTextWithIds(IDS_IOS_CHECK_PASSWORDS,
IDS_IOS_CHECK_PASSWORDS_DESCRIPTION, IDS_IOS_CHECK_PASSWORDS_DESCRIPTION,
GetSectionIndex(PasswordCheck), 0); GetSectionIndex(PasswordCheck), 0);
...@@ -571,10 +578,31 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStateRunning) { ...@@ -571,10 +578,31 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStateRunning) {
EXPECT_FALSE(checkPassword.image); EXPECT_FALSE(checkPassword.image);
} }
// Test verifies clicking start triggers correct function in service. // Test verifies tapping start with no saved passwords has no effect.
TEST_P(PasswordsTableViewControllerTest, PasswordCheckStart) { TEST_P(PasswordsTableViewControllerTest, DisabledPasswordCheck) {
if (!GetParam().password_check_enabled)
return;
PasswordsTableViewController* passwords_controller =
static_cast<PasswordsTableViewController*>(controller());
EXPECT_CALL(GetMockPasswordCheckService(), CheckUsernamePasswordPairs)
.Times(0);
EXPECT_CALL(GetMockPasswordCheckService(), Cancel).Times(0);
[passwords_controller tableView:passwords_controller.tableView
didSelectRowAtIndexPath:[NSIndexPath
indexPathForItem:1
inSection:GetSectionIndex(
PasswordCheck)]];
}
// Test verifies tapping start triggers correct function in service.
TEST_P(PasswordsTableViewControllerTest, StartPasswordCheck) {
if (!GetParam().password_check_enabled) if (!GetParam().password_check_enabled)
return; return;
AddSavedForm1();
RunUntilIdle();
PasswordsTableViewController* passwords_controller = PasswordsTableViewController* passwords_controller =
static_cast<PasswordsTableViewController*>(controller()); static_cast<PasswordsTableViewController*>(controller());
...@@ -588,10 +616,32 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStart) { ...@@ -588,10 +616,32 @@ TEST_P(PasswordsTableViewControllerTest, PasswordCheckStart) {
PasswordCheck)]]; PasswordCheck)]];
} }
// Test verifies tapping cancel triggers correct function in service.
TEST_P(PasswordsTableViewControllerTest, StopPasswordCheck) {
if (!GetParam().password_check_enabled)
return;
AddSavedForm1();
RunUntilIdle();
PasswordsTableViewController* passwords_controller =
static_cast<PasswordsTableViewController*>(controller());
ON_CALL(GetMockPasswordCheckService(), GetState())
.WillByDefault(Return(
password_manager::BulkLeakCheckServiceInterface::State::kRunning));
EXPECT_CALL(GetMockPasswordCheckService(), Cancel);
[passwords_controller tableView:passwords_controller.tableView
didSelectRowAtIndexPath:[NSIndexPath
indexPathForItem:1
inSection:GetSectionIndex(
PasswordCheck)]];
}
const std::vector<PasswordCheckFeatureStatus> kPasswordCheckFeatureStatusCases{ const std::vector<PasswordCheckFeatureStatus> kPasswordCheckFeatureStatusCases{
// Passwords export disabled // Password check disabled
{FALSE}, {FALSE},
// Passwords export enabled // Password check enabled
{TRUE}}; {TRUE}};
INSTANTIATE_TEST_SUITE_P(PasswordCheckDisabledAndEnabled, INSTANTIATE_TEST_SUITE_P(PasswordCheckDisabledAndEnabled,
......
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