Commit f0c4946e authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Remove TOO_MANY_PASSWORDS case from Password Check

This change removes the TOO_MANY_PASSWORDS error case from the password
check and deletes relevant logic. The strings will be removed in a
follow-up CL to simplify merging.

Bug: 1061927
Change-Id: I73574316020de364cc76487191983cea94a54a66
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2105319Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751300}
parent 31f17b0a
......@@ -208,8 +208,6 @@ std::string FormatElapsedTime(base::Time time) {
} // namespace
constexpr size_t PasswordCheckDelegate::kTooManyPasswords;
PasswordCheckDelegate::PasswordCheckDelegate(Profile* profile)
: profile_(profile),
password_store_(PasswordStoreFactory::GetForProfile(
......@@ -440,20 +438,13 @@ PasswordCheckDelegate::GetPasswordCheckStatus() const {
return result;
}
if (saved_passwords.size() >= kTooManyPasswords) {
result.state =
api::passwords_private::PASSWORD_CHECK_STATE_TOO_MANY_PASSWORDS;
return result;
}
result.state = ConvertPasswordCheckState(state);
return result;
}
void PasswordCheckDelegate::OnSavedPasswordsChanged(SavedPasswordsView) {
// A change in the saved passwords might result in leaving or entering the
// NO_PASSWORDS or TOO_MANY_PASSWORDS state, thus we need to trigger a
// notification.
// NO_PASSWORDS state, thus we need to trigger a notification.
NotifyPasswordCheckStatusChanged();
}
......
......@@ -38,8 +38,6 @@ class PasswordCheckDelegate
public password_manager::CompromisedCredentialsProvider::Observer,
public password_manager::BulkLeakCheckService::Observer {
public:
static constexpr size_t kTooManyPasswords = 1000;
using CredentialPasswordsMap =
std::map<password_manager::CredentialWithPassword,
std::vector<autofill::PasswordForm>,
......
......@@ -694,19 +694,6 @@ TEST_F(PasswordCheckDelegateTest, GetPasswordCheckStatusNoPasswords) {
delegate().GetPasswordCheckStatus().state);
}
// Verifies that the case where the user has too many saved passwords is
// reported correctly.
TEST_F(PasswordCheckDelegateTest, GetPasswordCheckStatusTooManyPasswords) {
for (size_t i = 0; i < PasswordCheckDelegate::kTooManyPasswords; ++i) {
store().AddLogin(MakeSavedPassword(
kExampleCom, base::StrCat({kUsername1, base::NumberToString(i)})));
}
RunUntilIdle();
EXPECT_EQ(api::passwords_private::PASSWORD_CHECK_STATE_TOO_MANY_PASSWORDS,
delegate().GetPasswordCheckStatus().state);
}
// Verifies that the case where the check is idle is reported correctly.
TEST_F(PasswordCheckDelegateTest, GetPasswordCheckStatusIdle) {
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
......
......@@ -168,11 +168,9 @@ Polymer({
case CheckState.OFFLINE:
case CheckState.SIGNED_OUT:
case CheckState.OTHER_ERROR:
case CheckState.TOO_MANY_PASSWORDS:
this.passwordManager_.startBulkPasswordCheck();
return;
case CheckState.NO_PASSWORDS:
case CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT:
case CheckState.QUOTA_LIMIT:
}
assertNotReached(
......@@ -292,10 +290,6 @@ Polymer({
return this.i18n('checkPasswordsErrorSignedOut');
case CheckState.NO_PASSWORDS:
return this.i18n('checkPasswordsErrorNoPasswords');
case CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT:
return this.i18n('checkPasswordsErrorInterruptedTooManyPasswords');
case CheckState.TOO_MANY_PASSWORDS:
return this.i18n('checkPasswordsErrorTooManyPasswords');
case CheckState.QUOTA_LIMIT:
return this.i18n('checkPasswordsErrorQuota');
case CheckState.OTHER_ERROR:
......@@ -337,7 +331,6 @@ Polymer({
return this.waitsForFirstCheck_() ? this.i18n('checkPasswords') :
this.i18n('checkPasswordsAgain');
case CheckState.CANCELED:
case CheckState.TOO_MANY_PASSWORDS:
return this.i18n('checkPasswordsAgain');
case CheckState.RUNNING:
return this.i18n('checkPasswordsStop');
......@@ -347,7 +340,6 @@ Polymer({
case CheckState.OTHER_ERROR:
return this.i18n('checkPasswordsAgainAfterError');
case CheckState.QUOTA_LIMIT:
case CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT:
return ''; // Undefined behavior. Don't show any misleading text.
}
assertNotReached('Can\'t find a button text for state: ' + status.state);
......@@ -377,10 +369,7 @@ Polymer({
case CheckState.SIGNED_OUT:
case CheckState.OTHER_ERROR:
return false;
case CheckState.TOO_MANY_PASSWORDS:
return !this.suppressesCheckupLink_();
case CheckState.NO_PASSWORDS:
case CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT:
case CheckState.QUOTA_LIMIT:
return true;
}
......@@ -437,8 +426,6 @@ Polymer({
case CheckState.OFFLINE:
case CheckState.SIGNED_OUT:
case CheckState.NO_PASSWORDS:
case CheckState.TOO_MANY_PASSWORDS:
case CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT:
case CheckState.QUOTA_LIMIT:
case CheckState.OTHER_ERROR:
return true;
......@@ -466,10 +453,7 @@ Polymer({
case CheckState.NO_PASSWORDS:
case CheckState.QUOTA_LIMIT:
case CheckState.OTHER_ERROR:
case CheckState.TOO_MANY_PASSWORDS:
return false;
case CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT:
return true;
}
assertNotReached(
'Not specified whether to show passwords for state: ' +
......@@ -492,9 +476,7 @@ Polymer({
* @private
*/
suppressesCheckupLink_() {
if (this.status_.state != CheckState.TOO_MANY_PASSWORDS &&
this.status_.state != CheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT &&
this.status_.state != CheckState.QUOTA_LIMIT) {
if (this.status_.state != CheckState.QUOTA_LIMIT) {
return true; // Never show the retry link for other states.
}
if (!this.syncStatus_ || !this.syncStatus_.signedIn) {
......
......@@ -37,8 +37,7 @@ cr.define('settings', function() {
NO_PASSWORDS: 4,
SIGNED_OUT: 5,
QUOTA_LIMIT: 6,
TOO_MANY_PASSWORDS: 7,
ERROR: 8,
ERROR: 7,
};
/**
......
......@@ -430,7 +430,6 @@ Polymer({
case settings.SafetyCheckPasswordsStatus.NO_PASSWORDS:
case settings.SafetyCheckPasswordsStatus.SIGNED_OUT:
case settings.SafetyCheckPasswordsStatus.QUOTA_LIMIT:
case settings.SafetyCheckPasswordsStatus.TOO_MANY_PASSWORDS:
case settings.SafetyCheckPasswordsStatus.ERROR:
return 'cr:info';
default:
......
......@@ -363,9 +363,6 @@ base::string16 SafetyCheckHandler::GetStringForPasswords(PasswordsStatus status,
case PasswordsStatus::kQuotaLimit:
return l10n_util::GetStringUTF16(
IDS_SETTINGS_SAFETY_CHECK_PASSWORDS_QUOTA_LIMIT);
case PasswordsStatus::kTooManyPasswords:
return l10n_util::GetStringUTF16(
IDS_SETTINGS_SAFETY_CHECK_PASSWORDS_TOO_MANY_PASSWORDS);
case PasswordsStatus::kError:
return l10n_util::GetStringUTF16(
IDS_SETTINGS_SAFETY_CHECK_PASSWORDS_ERROR);
......
......@@ -57,7 +57,6 @@ class SafetyCheckHandler
kNoPasswords,
kSignedOut,
kQuotaLimit,
kTooManyPasswords,
kError,
};
enum class ExtensionsStatus {
......
......@@ -733,16 +733,12 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
{"checkPasswordsCanceled", IDS_SETTINGS_CHECK_PASSWORDS_CANCELED},
{"checkedPasswords", IDS_SETTINGS_CHECKED_PASSWORDS},
{"checkPasswordsDescription", IDS_SETTINGS_CHECK_PASSWORDS_DESCRIPTION},
{"checkPasswordsErrorInterruptedTooManyPasswords",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_INTERRUPTED_TOO_MANY_PASSWORDS},
{"checkPasswordsErrorOffline",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_OFFLINE},
{"checkPasswordsErrorSignedOut",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_SIGNED_OUT},
{"checkPasswordsErrorNoPasswords",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_NO_PASSWORDS},
{"checkPasswordsErrorTooManyPasswords",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_TOO_MANY_PASSWORDS},
{"checkPasswordsErrorQuota",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_QUOTA_LIMIT},
{"checkPasswordsErrorGeneric",
......
......@@ -48,13 +48,8 @@ namespace passwordsPrivate {
SIGNED_OUT,
// The user does not have any passwords saved.
NO_PASSWORDS,
// The user has too many passwords saved.
TOO_MANY_PASSWORDS,
// The user hit the quota limit.
QUOTA_LIMIT,
// The user has too many passwords saved and hit the quota limit. This is
// relevant for users that can't use the online Password Checkup.
TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT,
// Any other error state.
OTHER_ERROR
};
......
......@@ -244,129 +244,6 @@ cr.define('settings_passwords_check', function() {
});
});
// Test verifies that sync users see only the link to account checkup and no
// button to start the local leak check.
test('testOnlyCheckupLinkForTooManyPasswordsWhenSyncing', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.TOO_MANY_PASSWORDS);
const section = createCheckPasswordSection();
cr.webUIListenerCallback(
'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
sync_test_util.simulateSyncStatus({signedIn: true});
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
expectTrue(isElementVisible(section.$.linkToGoogleAccount));
expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
});
});
// Test verifies that too many passwords don't prevent the user from
// starting the check. The link to the password checkup is not displayed if
// the user has too many passwords but is not signed in.
test('testNoCheckupLinkForTooManyPasswordsWhenSignedOut', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS);
const section = createCheckPasswordSection();
cr.webUIListenerCallback(
'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
sync_test_util.simulateSyncStatus({signedIn: false});
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
expectFalse(isElementVisible(section.$.linkToGoogleAccount));
assertTrue(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgain'),
section.$.controlPasswordCheckButton.innerText);
});
});
// Test verifies that too many passwords don't prevent the user from
// starting the check. The link to the password checkup is not displayed if
// a sync-user encrypted their passwords with a custom passphrase.
test('testNoCheckupLinkForTooManyPasswordsForEncryption', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS);
const section = createCheckPasswordSection();
const syncPrefs = sync_test_util.getSyncAllPrefs();
syncPrefs.encryptAllData = true;
cr.webUIListenerCallback('sync-prefs-changed', syncPrefs);
sync_test_util.simulateSyncStatus({signedIn: true});
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
expectFalse(isElementVisible(section.$.linkToGoogleAccount));
assertTrue(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgain'),
section.$.controlPasswordCheckButton.innerText);
});
});
// Test verifies that 'Try again' is not visible if users have too many
// passwords and are out of quota. Instead, there should be a link to their
// the checkup in their Google Account.
test('testCheckupForTooManyPasswordsQuotaLimitWithSync', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT);
const section = createCheckPasswordSection();
cr.webUIListenerCallback(
'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
sync_test_util.simulateSyncStatus({signedIn: true});
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
expectTrue(isElementVisible(section.$.linkToGoogleAccount));
});
});
// There should be neither a Retry button nor a link to the password checkup
// if the user has too many passwords, is out of quota, and is signed out.
test('testNoCheckupForTooManyPasswordsQuotaLimitWhenSignedOut', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT);
const section = createCheckPasswordSection();
cr.webUIListenerCallback(
'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
sync_test_util.simulateSyncStatus({signedIn: false});
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
expectFalse(isElementVisible(section.$.linkToGoogleAccount));
});
});
// There should be neither a Retry button nor a link to the password checkup
// if a sync-user encrypted their passwords with a custom passphrase.
test('testNoCheckupForTooManyPasswordsQuotaLimitForEncryption', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT);
const section = createCheckPasswordSection();
const syncPrefs = sync_test_util.getSyncAllPrefs();
syncPrefs.encryptAllData = true;
cr.webUIListenerCallback('sync-prefs-changed', syncPrefs);
sync_test_util.simulateSyncStatus({signedIn: true});
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
expectFalse(isElementVisible(section.$.linkToGoogleAccount));
});
});
// Test verifies that 'Try again' visible and working when users encounter a
// generic error.
test('testShowRetryAfterGenericError', function() {
......@@ -814,41 +691,6 @@ cr.define('settings_passwords_check', function() {
});
});
// When too many passwords were saved to check them, only show an error.
test('testShowOnlyErrorWhenTooManyPasswords', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
const title = section.$.title;
assertTrue(isElementVisible(title));
expectEquals(section.i18n('checkPasswordsErrorTooManyPasswords'),
title.innerText);
expectFalse(isElementVisible(section.$.subtitle));
});
});
// When too many passwords were saved to check them, only show an error.
test('testShowyErrorAndSubtitleWhenTooManyPasswordsAndOutOfQuota', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
PasswordCheckState.TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
const title = section.$.title;
assertTrue(isElementVisible(title));
expectEquals(
section.i18n('checkPasswordsErrorInterruptedTooManyPasswords'),
title.innerText);
expectTrue(isElementVisible(section.$.subtitle));
});
});
// When users run out of quota, only show an error.
test('testShowOnlyErrorWhenQuotaIsHit', function() {
passwordManager.data.checkStatus =
......
......@@ -55,9 +55,7 @@ chrome.passwordsPrivate.PasswordCheckState = {
OFFLINE: 'OFFLINE',
SIGNED_OUT: 'SIGNED_OUT',
NO_PASSWORDS: 'NO_PASSWORDS',
TOO_MANY_PASSWORDS: 'TOO_MANY_PASSWORDS',
QUOTA_LIMIT: 'QUOTA_LIMIT',
TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT: 'TOO_MANY_PASSWORDS_AND_QUOTA_LIMIT',
OTHER_ERROR: 'OTHER_ERROR',
};
......
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