[iOS] Move reauth before opening password details with notes

Bug: 1414897 Change-Id: I9a8b0d62b7113aaf8b82dddb46a37414322b8bbe Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4315960 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Tommy Martino <tmartino@chromium.org> Commit-Queue: Rafał Godlewski <rgod@google.com> Cr-Commit-Position: refs/heads/main@{#1115628}
chromium · Mar 10, 2023 · 5fb3698 · 5fb3698
1 parent dbd2922
commit 5fb3698
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 26 deletions.
diff --git a/ios/chrome/browser/ui/settings/password/BUILD.gn b/ios/chrome/browser/ui/settings/password/BUILD.gn
@@ -22,6 +22,7 @@ source_set("password") {
     "//components/google/core/common",
     "//components/password_manager/core/common:features",
     "//components/signin/public/identity_manager/objc",
+    "//components/strings",
     "//components/sync",
     "//ios/chrome/app/strings",
     "//ios/chrome/browser/application_context",

diff --git a/ios/chrome/browser/ui/settings/password/legacy_password_settings_egtest.mm b/ios/chrome/browser/ui/settings/password/legacy_password_settings_egtest.mm
@@ -1329,7 +1329,8 @@ - (void)testExportFlow {
 
   OpenPasswordSettings();
 
-  [PasswordSettingsAppInterface setUpMockReauthenticationModuleForExport];
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
   [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
                                     ReauthenticationResult::kSuccess];
 
@@ -1917,7 +1918,8 @@ - (void)DISABLED_testAddNewDuplicatedPasswordCredential {
   SaveExamplePasswordForm();
 
   OpenPasswordSettings();
-  [PasswordSettingsAppInterface setUpMockReauthenticationModuleForExport];
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
   [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
                                     ReauthenticationResult::kSuccess];
 
@@ -2032,7 +2034,8 @@ - (void)testDuplicatedCredentialWithNoUsername {
 // when adding a new credential.
 - (void)testTLDMissingMessage {
   OpenPasswordSettings();
-  [PasswordSettingsAppInterface setUpMockReauthenticationModuleForExport];
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
   [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
                                     ReauthenticationResult::kSuccess];
 

diff --git a/...e/browser/ui/settings/password/password_details/password_details_table_view_controller.mm b/...e/browser/ui/settings/password/password_details/password_details_table_view_controller.mm
@@ -762,6 +762,8 @@ - (UIImage*)compromisedIcon {
 
 // Shows reauthentication dialog if needed. If the reauthentication is
 // successful reveals the password.
+// TODO(crbug.com/1414897): Add 5 min timeout and remove reauth in password
+// details page with notes enabled.
 - (void)attemptToShowPasswordFor:(ReauthenticationReason)reason {
   // If password was already shown (before editing or copying) or the flag to
   // override auth is YES, we don't need to request reauth again.

diff --git a/ios/chrome/browser/ui/settings/password/password_manager_egtest.mm b/ios/chrome/browser/ui/settings/password/password_manager_egtest.mm
@@ -1355,6 +1355,10 @@ - (void)testLayoutWithNotesEnabled {
   SaveExamplePasswordFormWithNote();
 
   OpenPasswordManager();
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
+  [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
+                                    ReauthenticationResult::kSuccess];
 
   [[self interactionForSinglePasswordEntryWithDomain:@"example.com"
                                             username:@"concrete username"]
@@ -1395,6 +1399,10 @@ - (void)testLayoutWithLongNotes {
   SaveExamplePasswordFormWithNote();
 
   OpenPasswordManager();
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
+  [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
+                                    ReauthenticationResult::kSuccess];
 
   [[self interactionForSinglePasswordEntryWithDomain:@"example.com"
                                             username:@"concrete username"]
@@ -2523,7 +2531,8 @@ - (void)DISABLED_testAddNewDuplicatedPasswordCredential {
   SaveExamplePasswordForm();
 
   OpenPasswordManager();
-  [PasswordSettingsAppInterface setUpMockReauthenticationModuleForExport];
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
   [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
                                     ReauthenticationResult::kSuccess];
 
@@ -2708,7 +2717,8 @@ - (void)testDuplicatedCredentialWithNoUsername {
 // when adding a new credential.
 - (void)testTLDMissingMessage {
   OpenPasswordManager();
-  [PasswordSettingsAppInterface setUpMockReauthenticationModuleForExport];
+  [PasswordSettingsAppInterface
+      setUpMockReauthenticationModuleForPasswordManager];
   [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
                                     ReauthenticationResult::kSuccess];
 

diff --git a/ios/chrome/browser/ui/settings/password/password_manager_view_controller.mm b/ios/chrome/browser/ui/settings/password/password_manager_view_controller.mm
@@ -28,6 +28,7 @@
 #import "components/password_manager/core/common/password_manager_pref_names.h"
 #import "components/prefs/pref_service.h"
 #import "components/strings/grit/components_strings.h"
+#import "components/sync/base/features.h"
 #import "components/sync/driver/sync_service.h"
 #import "components/sync/driver/sync_service_utils.h"
 #import "components/sync/driver/sync_user_settings.h"
@@ -146,6 +147,10 @@ bool IsPasswordCheckupEnabled() {
       password_manager::features::kIOSPasswordCheckup);
 }
 
+bool IsPasswordNotesWithBackupEnabled() {
+  return base::FeatureList::IsEnabled(syncer::kPasswordNotesWithBackup);
+}
+
 // Helper method to determine whether the Password Check cell is tappable or
 // not.
 bool IsPasswordCheckTappable(PasswordCheckUIState passwordCheckState) {
@@ -2421,6 +2426,21 @@ - (NSIndexPath*)checkButtonIndexPath {
                               sectionIdentifier:SectionIdentifierPasswordCheck];
 }
 
+- (void)showDetailedViewPageForItem:(TableViewItem*)item {
+  if (IsPasswordGroupingEnabled()) {
+    [self.handler
+        showDetailedViewForAffiliatedGroup:base::mac::ObjCCastStrict<
+                                               AffiliatedGroupTableViewItem>(
+                                               item)
+                                               .affiliatedGroup];
+  } else {
+    [self.handler
+        showDetailedViewForCredential:base::mac::ObjCCastStrict<
+                                          CredentialTableViewItem>(item)
+                                          .credential];
+  }
+}
+
 #pragma mark - UITableViewDelegate
 
 - (void)tableView:(UITableView*)tableView
@@ -2448,17 +2468,30 @@ - (void)tableView:(UITableView*)tableView
       DCHECK_EQ(SectionIdentifierSavedPasswords,
                 [model sectionIdentifierForSectionIndex:indexPath.section]);
       TableViewItem* item = [model itemAtIndexPath:indexPath];
-      if (IsPasswordGroupingEnabled()) {
-        [self.handler
-            showDetailedViewForAffiliatedGroup:
-                base::mac::ObjCCastStrict<AffiliatedGroupTableViewItem>(item)
-                    .affiliatedGroup];
+
+      if (!IsPasswordNotesWithBackupEnabled()) {
+        [self showDetailedViewPageForItem:item];
+      } else if ([self.reauthenticationModule canAttemptReauth]) {
+        void (^showPasswordDetailsHandler)(ReauthenticationResult) =
+            ^(ReauthenticationResult result) {
+              if (result == ReauthenticationResult::kFailure) {
+                return;
+              }
+
+              [self showDetailedViewPageForItem:item];
+            };
+
+        [self.reauthenticationModule
+            attemptReauthWithLocalizedReason:
+                l10n_util::GetNSString(
+                    IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_SHOW)
+                        canReusePreviousAuth:YES
+                                     handler:showPasswordDetailsHandler];
       } else {
-        [self.handler
-            showDetailedViewForCredential:base::mac::ObjCCastStrict<
-                                              CredentialTableViewItem>(item)
-                                              .credential];
+        DCHECK(self.handler);
+        [self.handler showSetupPasscodeDialog];
       }
+
       break;
     }
     case ItemTypeBlocked: {

diff --git a/ios/chrome/browser/ui/settings/password/password_settings_app_interface.h b/ios/chrome/browser/ui/settings/password/password_settings_app_interface.h
@@ -18,7 +18,7 @@
 // view password) and its options for next test.
 + (void)setUpMockReauthenticationModule;
 + (void)setUpMockReauthenticationModuleForAddPassword;
-+ (void)setUpMockReauthenticationModuleForExport;
++ (void)setUpMockReauthenticationModuleForPasswordManager;
 + (void)mockReauthenticationModuleExpectedResult:
     (ReauthenticationResult)expectedResult;
 + (void)mockReauthenticationModuleCanAttempt:(BOOL)canAttempt;

diff --git a/ios/chrome/browser/ui/settings/password/password_settings_app_interface.mm b/ios/chrome/browser/ui/settings/password/password_settings_app_interface.mm
@@ -31,9 +31,10 @@
 #endif
 
 using chrome_test_util::SetUpAndReturnMockReauthenticationModule;
-using chrome_test_util::SetUpAndReturnMockReauthenticationModuleForExport;
 using chrome_test_util::
     SetUpAndReturnMockReauthenticationModuleForExportFromSettings;
+using chrome_test_util::
+    SetUpAndReturnMockReauthenticationModuleForPasswordManager;
 using password_manager::PasswordForm;
 
 namespace {
@@ -164,9 +165,9 @@ + (void)setUpMockReauthenticationModuleForAddPassword {
   _mockReauthenticationModule = SetUpAndReturnMockReauthenticationModule(true);
 }
 
-+ (void)setUpMockReauthenticationModuleForExport {
++ (void)setUpMockReauthenticationModuleForPasswordManager {
   _mockReauthenticationModule =
-      SetUpAndReturnMockReauthenticationModuleForExport();
+      SetUpAndReturnMockReauthenticationModuleForPasswordManager();
 }
 
 + (void)mockReauthenticationModuleExpectedResult:

diff --git a/ios/chrome/browser/ui/settings/password/passwords_coordinator.mm b/ios/chrome/browser/ui/settings/password/passwords_coordinator.mm
@@ -9,17 +9,20 @@
 #import "components/password_manager/core/browser/password_manager_metrics_util.h"
 #import "components/password_manager/core/browser/ui/credential_ui_entry.h"
 #import "components/password_manager/core/common/password_manager_features.h"
+#import "components/strings/grit/components_strings.h"
 #import "ios/chrome/browser/favicon/favicon_loader.h"
 #import "ios/chrome/browser/favicon/ios_chrome_favicon_loader_factory.h"
 #import "ios/chrome/browser/main/browser.h"
 #import "ios/chrome/browser/passwords/ios_chrome_password_check_manager.h"
 #import "ios/chrome/browser/passwords/ios_chrome_password_check_manager_factory.h"
 #import "ios/chrome/browser/shared/public/commands/application_commands.h"
 #import "ios/chrome/browser/shared/public/commands/command_dispatcher.h"
+#import "ios/chrome/browser/shared/public/commands/open_new_tab_command.h"
 #import "ios/chrome/browser/signin/identity_manager_factory.h"
 #import "ios/chrome/browser/sync/sync_service_factory.h"
 #import "ios/chrome/browser/sync/sync_setup_service_factory.h"
 #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h"
+#import "ios/chrome/browser/ui/alert_coordinator/alert_coordinator.h"
 #import "ios/chrome/browser/ui/settings/password/password_checkup/password_checkup_coordinator.h"
 #import "ios/chrome/browser/ui/settings/password/password_checkup/password_checkup_utils.h"
 #import "ios/chrome/browser/ui/settings/password/password_details/add_password_coordinator.h"
@@ -98,6 +101,9 @@ @interface PasswordsCoordinator () <
 @property(nonatomic, strong)
     PasswordSettingsCoordinator* passwordSettingsCoordinator;
 
+// Modal alert for interactions with passwords list.
+@property(nonatomic, strong) AlertCoordinator* alertCoordinator;
+
 @end
 
 @implementation PasswordsCoordinator
@@ -291,6 +297,36 @@ - (void)showPasswordDeleteDialogWithOrigins:(NSArray<NSString*>*)origins
   [self.actionSheetCoordinator start];
 }
 
+- (void)showSetupPasscodeDialog {
+  NSString* title =
+      l10n_util::GetNSString(IDS_IOS_SETTINGS_SET_UP_SCREENLOCK_TITLE);
+  NSString* message =
+      l10n_util::GetNSString(IDS_IOS_SETTINGS_SET_UP_SCREENLOCK_CONTENT);
+  self.alertCoordinator =
+      [[AlertCoordinator alloc] initWithBaseViewController:self.viewController
+                                                   browser:self.browser
+                                                     title:title
+                                                   message:message];
+
+  __weak __typeof(self) weakSelf = self;
+  OpenNewTabCommand* command =
+      [OpenNewTabCommand commandWithURLFromChrome:GURL(kPasscodeArticleURL)];
+
+  [self.alertCoordinator addItemWithTitle:l10n_util::GetNSString(IDS_OK)
+                                   action:nil
+                                    style:UIAlertActionStyleCancel];
+
+  [self.alertCoordinator
+      addItemWithTitle:l10n_util::GetNSString(
+                           IDS_IOS_SETTINGS_SET_UP_SCREENLOCK_LEARN_HOW)
+                action:^{
+                  [weakSelf.dispatcher closeSettingsUIAndOpenURL:command];
+                }
+                 style:UIAlertActionStyleDefault];
+
+  [self.alertCoordinator start];
+}
+
 #pragma mark - PasswordManagerViewControllerPresentationDelegate
 
 - (void)PasswordManagerViewControllerDismissed {

diff --git a/ios/chrome/browser/ui/settings/password/passwords_settings_commands.h b/ios/chrome/browser/ui/settings/password/passwords_settings_commands.h
@@ -38,6 +38,10 @@ struct CredentialUIEntry;
 - (void)showPasswordDeleteDialogWithOrigins:(NSArray<NSString*>*)origins
                                  completion:(void (^)(void))completion;
 
+// Shows a dialog offering the user to set a passcode in order to see the
+// password details.
+- (void)showSetupPasscodeDialog;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_SETTINGS_PASSWORD_PASSWORDS_SETTINGS_COMMANDS_H_
diff --git a/ios/chrome/test/app/password_test_util.h b/ios/chrome/test/app/password_test_util.h
@@ -36,10 +36,11 @@ namespace chrome_test_util {
 MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule(
     bool is_add_new_password = false);
 
-// Replace the reauthentication module in Password Manager's
-// PasswordExporter with a fake one to avoid being
-// blocked with a reauth prompt, and return the fake reauthentication module.
-MockReauthenticationModule* SetUpAndReturnMockReauthenticationModuleForExport();
+// Replaces the reauthentication module in Password Manager's password list with
+// a fake one to avoid being blocked with a reauth prompt and returns the fake
+// reauthentication module.
+MockReauthenticationModule*
+SetUpAndReturnMockReauthenticationModuleForPasswordManager();
 
 // Replace the reauthentication module in Password Settings'
 // PasswordExporter with a fake one to avoid being

diff --git a/ios/chrome/test/app/password_test_util.mm b/ios/chrome/test/app/password_test_util.mm
@@ -70,11 +70,11 @@ - (void)attemptReauthWithLocalizedReason:(NSString*)localizedReason
   return mock_reauthentication_module;
 }
 
-// Replace the reauthentication module in Password Manager's
-// PasswordExporter with a fake one to avoid being
-// blocked with a reauth prompt, and return the fake reauthentication module.
+// Replaces the reauthentication module in Password Manager's passwords list
+// with a fake one to avoid being blocked with a reauth prompt and returns the
+// fake reauthentication module.
 MockReauthenticationModule*
-SetUpAndReturnMockReauthenticationModuleForExport() {
+SetUpAndReturnMockReauthenticationModuleForPasswordManager() {
   MockReauthenticationModule* mock_reauthentication_module =
       [[MockReauthenticationModule alloc] init];
   // TODO(crbug.com/754642): Stop using TopPresentedViewController();