From b355acf5ab87e62815072e5e16babd89345e522f Mon Sep 17 00:00:00 2001 From: Ali Juma Date: Wed, 10 Jun 2020 16:53:10 +0000 Subject: [PATCH] Reland: [iOS] Make password leak checks toggleable when Safe Browsing is not available This relands r776646 (previously reviewed at http://crrev.com/c/2237716), with test fixes: 1. On the settings pages, scroll up when needed (on small devices) to make the Safe Browsing and password leak check items visible. 2. In testTogglePasswordLeakCheckWhenSafeBrowsingNotAvailable, add coverage for the case where a user opts-out of Safe Browsing on another device. Description of previous CL: This CL fixes a regression from http://crrev.com/c/2174995 that prevents users who do not yet have Safe Browsing rolled out to them from toggling the password leak check setting. This CL ensures that the value of the Safe Browsing opt-out pref is always initialized in GoogleServicesSettingsMediator, regardless of whether Safe Browsing has been rolled out. With this change, the other logic changes in http://crrev.com/c/2174995 work as expected, whether or not Safe Browsing is available. Bug: 1090406, 1093306 Change-Id: Id511dc339be146bcc15d865d90feb5cf93369b51 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239300 Reviewed-by: Sergio Collazos Commit-Queue: Ali Juma Cr-Commit-Position: refs/heads/master@{#776981} --- .../ui/settings/google_services/BUILD.gn | 6 + .../google_services_settings_constants.h | 6 + .../google_services_settings_constants.mm | 6 + .../google_services_settings_egtest.mm | 142 ++++++++++++++++++ .../google_services_settings_mediator.mm | 14 +- 5 files changed, 167 insertions(+), 7 deletions(-) diff --git a/ios/chrome/browser/ui/settings/google_services/BUILD.gn b/ios/chrome/browser/ui/settings/google_services/BUILD.gn index a1e0c0d558c4ed..ea6b65e0497391 100644 --- a/ios/chrome/browser/ui/settings/google_services/BUILD.gn +++ b/ios/chrome/browser/ui/settings/google_services/BUILD.gn @@ -104,6 +104,9 @@ source_set("eg_tests") { ":constants", ":test_support", "//base/test:test_support", + "//components/password_manager/core/common", + "//components/safe_browsing/core:features", + "//components/safe_browsing/core/common:safe_browsing_prefs", "//ios/chrome/app/strings", "//ios/chrome/browser/tabs", "//ios/chrome/browser/ui:feature_flags", @@ -177,6 +180,9 @@ source_set("eg2_tests") { ":eg_test_support+eg2", "//base", "//base/test:test_support", + "//components/password_manager/core/common", + "//components/safe_browsing/core:features", + "//components/safe_browsing/core/common:safe_browsing_prefs", "//ios/chrome/app/strings", "//ios/chrome/browser/ui:feature_flags", "//ios/chrome/browser/ui/authentication:eg_test_support+eg2", diff --git a/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.h b/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.h index cecc8e2cb81d90..e1f1934b3b0793 100644 --- a/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.h +++ b/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.h @@ -16,4 +16,10 @@ extern NSString* const kManageSyncCellAccessibilityIdentifier; // Accessibility identifier for the account list cell. extern NSString* const kAccountListItemAccessibilityIdentifier; +// Accessibility identifier for the password leak check cell. +extern NSString* const kPasswordLeakCheckItemAccessibilityIdentifier; + +// Accessibility identifier for the Safe Browsing cell. +extern NSString* const kSafeBrowsingItemAccessibilityIdentifier; + #endif // IOS_CHROME_BROWSER_UI_SETTINGS_GOOGLE_SERVICES_GOOGLE_SERVICES_SETTINGS_CONSTANTS_H_ diff --git a/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.mm b/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.mm index 0a231b18c30734..dbb369e780ae2d 100644 --- a/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.mm +++ b/ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.mm @@ -16,3 +16,9 @@ NSString* const kAccountListItemAccessibilityIdentifier = @"AccountListItemAccessibilityIdentifier"; + +NSString* const kPasswordLeakCheckItemAccessibilityIdentifier = + @"PasswordLeakCheckItemAccessibilityIdentifier"; + +NSString* const kSafeBrowsingItemAccessibilityIdentifier = + @"SafeBrowsingItemAccessibilityIdentifier"; diff --git a/ios/chrome/browser/ui/settings/google_services/google_services_settings_egtest.mm b/ios/chrome/browser/ui/settings/google_services/google_services_settings_egtest.mm index 3db0488d92fa51..e5ec52dc175f17 100644 --- a/ios/chrome/browser/ui/settings/google_services/google_services_settings_egtest.mm +++ b/ios/chrome/browser/ui/settings/google_services/google_services_settings_egtest.mm @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "components/password_manager/core/common/password_manager_pref_names.h" +#include "components/safe_browsing/core/common/safe_browsing_prefs.h" +#include "components/safe_browsing/core/features.h" #import "ios/chrome/browser/ui/authentication/signin_earl_grey_ui.h" #import "ios/chrome/browser/ui/authentication/signin_earlgrey_utils.h" #import "ios/chrome/browser/ui/settings/google_services/google_services_settings_app_interface.h" @@ -9,10 +12,12 @@ #import "ios/chrome/browser/ui/settings/google_services/manage_sync_settings_constants.h" #include "ios/chrome/grit/ios_chromium_strings.h" #include "ios/chrome/grit/ios_strings.h" +#import "ios/chrome/test/earl_grey/chrome_actions.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h" #import "ios/chrome/test/earl_grey/chrome_matchers.h" #import "ios/chrome/test/earl_grey/chrome_test_case.h" +#import "ios/testing/earl_grey/app_launch_manager.h" #import "ios/testing/earl_grey/earl_grey_test.h" #import "ui/base/l10n/l10n_util.h" @@ -159,6 +164,143 @@ - (void)testOpenSSOAddAccount { [[GREYUIThreadExecutor sharedInstance] drainUntilIdle]; } +// Tests that password leak detection can be toggled when Safe Browsing isn't +// available. +- (void)testTogglePasswordLeakCheckWhenSafeBrowsingNotAvailable { + AppLaunchConfiguration config; + config.features_disabled.push_back( + safe_browsing::kSafeBrowsingAvailableOnIOS); + [[AppLaunchManager sharedManager] ensureAppLaunchedWithConfiguration:config]; + + // Ensure that Safe Browsing and password leak detection opt-outs start in + // their default (opted-in) state. + [ChromeEarlGrey setBoolValue:YES forUserPref:prefs::kSafeBrowsingEnabled]; + [ChromeEarlGrey + setBoolValue:YES + forUserPref:password_manager::prefs::kPasswordLeakDetectionEnabled]; + + // Sign in. + FakeChromeIdentity* fakeIdentity = [SigninEarlGreyUtils fakeIdentity1]; + [SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity]; + // Open "Google Services" settings. + [self openGoogleServicesSettings]; + + // Check that the password leak check toggle is enabled, and toggle it off. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kPasswordLeakCheckItemAccessibilityIdentifier, + /*is_toggled_on=*/YES, + /*enabled=*/YES)] + performAction:chrome_test_util::TurnSettingsSwitchOn(NO)]; + + // Check the underlying pref value. + GREYAssertFalse( + [ChromeEarlGrey userBooleanPref:password_manager::prefs:: + kPasswordLeakDetectionEnabled], + @"Failed to toggle-off password leak checks"); + + // Toggle it back on. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kPasswordLeakCheckItemAccessibilityIdentifier, + /*is_toggled_on=*/NO, + /*enabled=*/YES)] + performAction:chrome_test_util::TurnSettingsSwitchOn(YES)]; + + // Check the underlying pref value. + GREYAssertTrue( + [ChromeEarlGrey userBooleanPref:password_manager::prefs:: + kPasswordLeakDetectionEnabled], + @"Failed to toggle-on password leak checks"); + + // Close settings. + [[EarlGrey selectElementWithMatcher:SettingsDoneButton()] + performAction:grey_tap()]; + + // Simulate the user opting out of Safe Browsing on another device. + [ChromeEarlGrey setBoolValue:NO forUserPref:prefs::kSafeBrowsingEnabled]; + + // Verify that the password leak check toggle is now disabled. + [self openGoogleServicesSettings]; + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kPasswordLeakCheckItemAccessibilityIdentifier, + /*is_toggled_on=*/NO, + /*enabled=*/NO)] assertWithMatcher:grey_notNil()]; +} + +// Tests that when Safe Browsing is available, password leak detection can only +// be toggled if Safe Browsing is enabled. +- (void)testTogglePasswordLeakCheckWhenSafeBrowsingAvailable { + AppLaunchConfiguration config; + config.features_enabled.push_back(safe_browsing::kSafeBrowsingAvailableOnIOS); + [[AppLaunchManager sharedManager] ensureAppLaunchedWithConfiguration:config]; + + // Ensure that Safe Browsing and password leak detection opt-outs start in + // their default (opted-in) state. + [ChromeEarlGrey setBoolValue:YES forUserPref:prefs::kSafeBrowsingEnabled]; + [ChromeEarlGrey + setBoolValue:YES + forUserPref:password_manager::prefs::kPasswordLeakDetectionEnabled]; + + // Sign in. + FakeChromeIdentity* fakeIdentity = [SigninEarlGreyUtils fakeIdentity1]; + [SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity]; + // Open "Google Services" settings. + [self openGoogleServicesSettings]; + + // Check that Safe Browsing is enabled, and toggle it off. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kSafeBrowsingItemAccessibilityIdentifier, + /*is_toggled_on=*/YES, + /*enabled=*/YES)] + performAction:chrome_test_util::TurnSettingsSwitchOn(NO)]; + + // Check that the password leak check toggle is both toggled off and disabled. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kPasswordLeakCheckItemAccessibilityIdentifier, + /*is_toggled_on=*/NO, + /*enabled=*/NO)] assertWithMatcher:grey_notNil()]; + + // Toggle Safe Browsing on. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kSafeBrowsingItemAccessibilityIdentifier, + /*is_toggled_on=*/NO, + /*enabled=*/YES)] + performAction:chrome_test_util::TurnSettingsSwitchOn(YES)]; + + // Check that the password leak check toggle is enabled, and toggle it off. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kPasswordLeakCheckItemAccessibilityIdentifier, + /*is_toggled_on=*/YES, + /*enabled=*/YES)] + performAction:chrome_test_util::TurnSettingsSwitchOn(NO)]; + + // Check the underlying pref value. + GREYAssertFalse( + [ChromeEarlGrey userBooleanPref:password_manager::prefs:: + kPasswordLeakDetectionEnabled], + @"Failed to toggle-off password leak checks"); + + // Toggle password leak check detection back on. + [[self elementInteractionWithGreyMatcher: + chrome_test_util::SettingsSwitchCell( + kPasswordLeakCheckItemAccessibilityIdentifier, + /*is_toggled_on=*/NO, + /*enabled=*/YES)] + performAction:chrome_test_util::TurnSettingsSwitchOn(YES)]; + + // Check the underlying pref value. + GREYAssertTrue( + [ChromeEarlGrey userBooleanPref:password_manager::prefs:: + kPasswordLeakDetectionEnabled], + @"Failed to toggle-on password leak checks"); +} + #pragma mark - Helpers // Opens the Google services settings. diff --git a/ios/chrome/browser/ui/settings/google_services/google_services_settings_mediator.mm b/ios/chrome/browser/ui/settings/google_services/google_services_settings_mediator.mm index 3ab0b55cc7fd35..d9d03b3d1ae0ff 100644 --- a/ios/chrome/browser/ui/settings/google_services/google_services_settings_mediator.mm +++ b/ios/chrome/browser/ui/settings/google_services/google_services_settings_mediator.mm @@ -187,12 +187,10 @@ - (instancetype)initWithUserPrefService:(PrefService*)userPrefService initWithPrefService:userPrefService prefName:prefs::kSearchSuggestEnabled]; _autocompleteSearchPreference.observer = self; - if (base::FeatureList::IsEnabled(kSafeBrowsingAvailableOnIOS)) { - _safeBrowsingPreference = [[PrefBackedBoolean alloc] - initWithPrefService:userPrefService - prefName:prefs::kSafeBrowsingEnabled]; - _safeBrowsingPreference.observer = self; - } + _safeBrowsingPreference = [[PrefBackedBoolean alloc] + initWithPrefService:userPrefService + prefName:prefs::kSafeBrowsingEnabled]; + _safeBrowsingPreference.observer = self; _sendDataUsagePreference = [[PrefBackedBoolean alloc] initWithPrefService:localPrefService prefName:metrics::prefs::kMetricsReportingEnabled]; @@ -597,6 +595,8 @@ - (ItemArray)nonPersonalizedItems { detailStringID: IDS_IOS_GOOGLE_SERVICES_SETTINGS_SAFE_BROWSING_DETAIL dataType:0]; + safeBrowsingItem.accessibilityIdentifier = + kSafeBrowsingItemAccessibilityIdentifier; [items addObject:safeBrowsingItem]; } [items addObject:self.passwordLeakCheckItem]; @@ -631,7 +631,7 @@ - (SettingsSwitchItem*)passwordLeakCheckItem { l10n_util::GetNSString(IDS_IOS_LEAK_CHECK_SWITCH); passwordLeakCheckItem.on = [self passwordLeakCheckItemOnState]; passwordLeakCheckItem.accessibilityIdentifier = - @"passwordLeakCheckItem_switch"; + kPasswordLeakCheckItemAccessibilityIdentifier; passwordLeakCheckItem.enabled = self.isAuthenticated; _passwordLeakCheckItem = passwordLeakCheckItem; }