[iOS][Password Check] Password Edit

This change adds possibility to edit password on Password Details
Screen.

Bug: 1075494
Change-Id: I7647237b41c7cabcd93fc4a19bf92f5ea7c3b5ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2309692
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793164}

ios/chrome/app/strings/ios_strings.grd

@@ -1663,6 +1663,15 @@ While in incognito, sites can't use cookies to see your browsing activity across
       <message name="IDS_IOS_DELETE_COMPROMISED_PASSWORD_DESCRIPTION" desc="Message on top of the confirmation alert when the user tapped delete password button. [iOS only]" meaning="Explaining to the user that deleting password locally won't delete account on a website.">
         Deleting this password will not delete your account on <ph name="WEBSITE">$1<ex>twitter.com</ex></ph>. Change your password on <ph name="WEBSITE">$1<ex>twitter.com</ex></ph> to keep it safe from others.
       </message>
+      <message name="IDS_IOS_EDIT_PASSWORD_DESCRIPTION" desc="Message inside confirmation alert when the user is trying to edit password [iOS only]" meaning="Telling user to ensure provided password matches password on the website.">
+        Make sure the password you are saving matches your password for <ph name="WEBSITE">$1<ex>twitter.com</ex></ph>
+      </message>
+      <message name="IDS_IOS_CONFIRM_PASSWORD_EDIT" desc="Confirm button inside confirmation alert when the user is trying to edit password [iOS only]" meaning="Save edited password">
+        Save Password
+      </message>
+      <message name="IDS_IOS_CANCEL_PASSWORD_EDIT" desc="Cancel button inside confirmation alert when the user is trying to edit password [iOS only]" meaning="Cancels editing">
+        Cancel
+      </message>
       <message name="IDS_IOS_SEARCH_COPIED" desc="The message displayed when the search is copied via long press (contextual search) [Length: 10em] [iOS only]">
         Copied
       </message>
@@ -1729,6 +1738,9 @@ While in incognito, sites can't use cookies to see your browsing activity across
       <message name="IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_SHOW" desc="Message explaining that the reason why we are requiring the user to re-authenticate is to be able to show the password in plain text. This is shown by iOS in the Touch ID reauthentication system dialogue, right under the OS string 'Touch ID for $BROWSER_NAME'. [Length: 22em]">
         Show password
       </message>
+      <message name="IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_EDIT" desc="Message explaining that the reason why we are requiring the user to re-authenticate is to be able to edit the password. This is shown by iOS in the Touch ID reauthentication system dialogue, right under the OS string 'Touch ID for $BROWSER_NAME'. [Length: 22em]">
+        Edit password
+      </message>
       <message name="IDS_IOS_SETTINGS_PASSWORD_SHOW_BUTTON" desc="Button that the user can press in order to have the password displayed in plain text. [Length: 12em]">
         Show
       </message>

ios/chrome/app/strings/ios_strings_grd/IDS_IOS_CANCEL_PASSWORD_EDIT.png.sha1

+e2624575ee712989a58b450268980ff791356d09
\ No newline at end of file

ios/chrome/app/strings/ios_strings_grd/IDS_IOS_CONFIRM_PASSWORD_EDIT.png.sha1

+e2624575ee712989a58b450268980ff791356d09
\ No newline at end of file

ios/chrome/app/strings/ios_strings_grd/IDS_IOS_EDIT_PASSWORD_DESCRIPTION.png.sha1

+e2624575ee712989a58b450268980ff791356d09
\ No newline at end of file

ios/chrome/app/strings/ios_strings_grd/IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_EDIT.png.sha1

+d8a35bdd3a6a1407fb3d9229ac57247507ddeb00
\ No newline at end of file

ios/chrome/browser/passwords/ios_chrome_password_check_manager.h

@@ -69,6 +69,14 @@ class IOSChromePasswordCheckManager
   GetSavedPasswordsFor(
       const password_manager::CredentialWithPassword& credential) const;
 
+  // Edits password for |form|.
+  void EditPasswordForm(const autofill::PasswordForm& form,
+                        base::StringPiece password);
+
+  // Edits password form using |compromised_credentials_manager_|.
+  void EditCompromisedPasswordForm(const autofill::PasswordForm& form,
+                                   base::StringPiece password);
+
   // Deletes |form| and its duplicates.
   void DeletePasswordForm(const autofill::PasswordForm& form);
 

ios/chrome/browser/passwords/ios_chrome_password_check_manager.mm

@@ -4,6 +4,7 @@
 
 #include "ios/chrome/browser/passwords/ios_chrome_password_check_manager.h"
 
+#include "base/strings/utf_string_conversions.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/prefs/pref_service.h"
@@ -150,11 +151,24 @@ IOSChromePasswordCheckManager::GetSavedPasswordsFor(
   return compromised_credentials_manager_.GetSavedPasswordsFor(credential);
 }
 
+void IOSChromePasswordCheckManager::EditPasswordForm(
+    const autofill::PasswordForm& form,
+    base::StringPiece password) {
+  saved_passwords_presenter_.EditPassword(form, base::UTF8ToUTF16(password));
+}
+
+void IOSChromePasswordCheckManager::EditCompromisedPasswordForm(
+    const autofill::PasswordForm& form,
+    base::StringPiece password) {
+  compromised_credentials_manager_.UpdateCompromisedCredentials(
+      password_manager::CredentialView(form), password);
+}
+
 void IOSChromePasswordCheckManager::DeletePasswordForm(
     const autofill::PasswordForm& form) {
   auto duplicates =
       GetDuplicatesOfForm(form, saved_passwords_presenter_.GetSavedPasswords());
-  for (const auto& duplicate : duplicates) {
+  for (auto& duplicate : duplicates) {
     password_store_->RemoveLogin(duplicate);
   }
 }

ios/chrome/browser/passwords/ios_chrome_password_check_manager_unittest.mm

@@ -46,6 +46,7 @@ constexpr char kExampleCom[] = "https://example.com";
 constexpr char kUsername1[] = "alice";
 
 constexpr char kPassword1[] = "s3cre3t";
+constexpr char kPassword2[] = "bett3r_S3cre3t";
 
 using autofill::PasswordForm;
 using password_manager::BulkLeakCheckServiceInterface;
@@ -357,3 +358,34 @@ TEST_F(IOSChromePasswordCheckManagerTest, DeleteDuplicatedPasswords) {
   RunUntilIdle();
   EXPECT_TRUE(store().stored_passwords().at(kExampleCom).empty());
 }
+
+// Tests password value is updated properly.
+TEST_F(IOSChromePasswordCheckManagerTest, EditPassword) {
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
+  RunUntilIdle();
+
+  manager().EditPasswordForm(store().stored_passwords().at(kExampleCom).at(0),
+                             kPassword2);
+  RunUntilIdle();
+
+  EXPECT_EQ(base::UTF8ToUTF16(kPassword2),
+            store().stored_passwords().at(kExampleCom).at(0).password_value);
+}
+
+// Tests compromised password value is updated properly.
+TEST_F(IOSChromePasswordCheckManagerTest, EditCompromisedPassword) {
+  PasswordForm form = MakeSavedPassword(kExampleCom, kUsername1);
+  store().AddLogin(form);
+  RunUntilIdle();
+
+  store().AddCompromisedCredentials(
+      MakeCompromised(kExampleCom, kUsername1, base::TimeDelta::FromMinutes(1),
+                      CompromiseType::kLeaked));
+  RunUntilIdle();
+
+  manager().EditCompromisedPasswordForm(form, kPassword2);
+  RunUntilIdle();
+
+  EXPECT_EQ(base::UTF8ToUTF16(kPassword2),
+            store().stored_passwords().at(kExampleCom).at(0).password_value);
+}

ios/chrome/browser/ui/settings/password/password_details/password_details_coordinator.mm

@@ -89,6 +89,7 @@
                                                passwordCheckManager:_manager];
   self.mediator.consumer = self.viewController;
   self.viewController.handler = self;
+  self.viewController.delegate = self.mediator;
   self.viewController.commandsDispatcher = self.dispatcher;
   self.viewController.reauthModule = self.reauthenticationModule;
 
@@ -113,8 +114,8 @@
       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.baseViewController
+  self.alertCoordinator =
+      [[AlertCoordinator alloc] initWithBaseViewController:self.viewController
                                                    browser:self.browser
                                                      title:title
                                                    message:message];
@@ -143,7 +144,7 @@
       l10n_util::GetNSStringF(IDS_IOS_DELETE_COMPROMISED_PASSWORD_DESCRIPTION,
                               base::SysNSStringToUTF16(origin));
   self.actionSheetCoordinator = [[ActionSheetCoordinator alloc]
-      initWithBaseViewController:self.baseViewController
+      initWithBaseViewController:self.viewController
                          browser:self.browser
                            title:nil
                          message:message
@@ -168,4 +169,31 @@
   [self.actionSheetCoordinator start];
 }
 
+- (void)showPasswordEditDialogWithOrigin:(NSString*)origin {
+  NSString* message = l10n_util::GetNSStringF(IDS_IOS_EDIT_PASSWORD_DESCRIPTION,
+                                              base::SysNSStringToUTF16(origin));
+  self.actionSheetCoordinator = [[ActionSheetCoordinator alloc]
+      initWithBaseViewController:self.viewController
+                         browser:self.browser
+                           title:nil
+                         message:message
+                   barButtonItem:nil];
+
+  __weak __typeof(self) weakSelf = self;
+
+  [self.actionSheetCoordinator
+      addItemWithTitle:l10n_util::GetNSString(IDS_IOS_CONFIRM_PASSWORD_EDIT)
+                action:^{
+                  [weakSelf.viewController passwordEditingConfirmed];
+                }
+                 style:UIAlertActionStyleDefault];
+
+  [self.actionSheetCoordinator
+      addItemWithTitle:l10n_util::GetNSString(IDS_IOS_CANCEL_PASSWORD_EDIT)
+                action:nil
+                 style:UIAlertActionStyleCancel];
+
+  [self.actionSheetCoordinator start];
+}
+
 @end

ios/chrome/browser/ui/settings/password/password_details/password_details_handler.h

@@ -19,6 +19,9 @@
 // version. It is displayed inside dialog.
 - (void)showPasswordDeleteDialogWithOrigin:(NSString*)origin;
 
+// Called when the user wants to save edited password.
+- (void)showPasswordEditDialogWithOrigin:(NSString*)origin;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_SETTINGS_PASSWORD_PASSWORD_DETAILS_PASSWORD_DETAILS_HANDLER_H_

ios/chrome/browser/ui/settings/password/password_details/password_details_mediator.mm

@@ -4,6 +4,7 @@
 
 #import "ios/chrome/browser/ui/settings/password/password_details/password_details_mediator.h"
 
+#include "base/strings/sys_string_conversions.h"
 #include "components/autofill/core/common/password_form.h"
 #include "ios/chrome/browser/passwords/password_check_observer_bridge.h"
 #import "ios/chrome/browser/ui/settings/password/password_details/password_details.h"
@@ -58,7 +59,16 @@ using CompromisedCredentialsView =
 - (void)passwordDetailsViewController:
             (PasswordDetailsViewController*)viewController
                didEditPasswordDetails:(PasswordDetails*)password {
-  // TODO:(crbug.com/1075494) - Edit password accordingly.
+  if ([password.password length] != 0) {
+    password.compromised
+        ? _manager->EditCompromisedPasswordForm(
+              _password, base::SysNSStringToUTF8(password.password))
+        : _manager->EditPasswordForm(
+              _password, base::SysNSStringToUTF8(password.password));
+    _password.password_value = base::SysNSStringToUTF16(password.password);
+  } else {
+    [self fetchPasswordWith:_manager->GetCompromisedCredentials()];
+  }
 }
 
 #pragma mark - PasswordCheckObserver

ios/chrome/browser/ui/settings/password/password_details/password_details_view_controller.h

@@ -30,6 +30,9 @@
 // with password.
 @property(nonatomic, weak) id<ReauthenticationProtocol> reauthModule;
 
+// Called by coordinator when the user confirmed password editing from alert.
+- (void)passwordEditingConfirmed;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_SETTINGS_PASSWORD_PASSWORD_DETAILS_PASSWORD_DETAILS_VIEW_CONTROLLER_H_

ios/chrome/browser/ui/settings/password/password_details/password_details_view_controller.mm

@@ -106,14 +106,16 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
     return;
   }
 
-  [super editButtonPressed];
-
-  if (!self.tableView.editing) {
-    // TODO:(crbug.com/1075494) - Update |_password| accordingly.
-    [self.delegate passwordDetailsViewController:self
-                          didEditPasswordDetails:self.password];
+  if (self.tableView.editing) {
+    // If password value was changed show confirmation dialog before saving
+    // password. Editing mode will be exited only if user confirm saving.
+    if (self.password.password != self.passwordTextItem.textFieldValue) {
+      [self.handler showPasswordEditDialogWithOrigin:self.password.origin];
+      return;
+    }
   }
 
+  [super editButtonPressed];
   [self reloadData];
 }
 
@@ -400,12 +402,19 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
       return l10n_util::GetNSString(
           IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_COPY);
     case ReauthenticationReasonEdit:
-      // TODO:(crbug.com/1075494) - Add custom string for edit.
       return l10n_util::GetNSString(
-          IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_SHOW);
+          IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_EDIT);
   }
 }
 
+- (void)passwordEditingConfirmed {
+  self.password.password = self.passwordTextItem.textFieldValue;
+  [self.delegate passwordDetailsViewController:self
+                        didEditPasswordDetails:self.password];
+  [super editButtonPressed];
+  [self reloadData];
+}
+
 #pragma mark - Actions
 
 // Called when the user tapped on the show/hide button near password.

ios/chrome/browser/ui/settings/password/password_details/password_detials_view_controller_unittest.mm

@@ -38,6 +38,8 @@
 
 @property(nonatomic, assign) BOOL deletionCalled;
 
+@property(nonatomic, assign) BOOL editingCalled;
+
 @end
 
 @implementation FakePasswordDetailsHandler
@@ -53,12 +55,19 @@
   self.deletionCalled = YES;
 }
 
+- (void)showPasswordEditDialogWithOrigin:(NSString*)origin {
+  self.editingCalled = YES;
+}
+
 @end
 
 // Test class that conforms to PasswordDetailsViewControllerDelegate in order to
 // test the delegate methods are called correctly.
 @interface FakePasswordDetailsDelegate
     : NSObject <PasswordDetailsViewControllerDelegate>
+
+@property(nonatomic, strong) PasswordDetails* password;
+
 @end
 
 @implementation FakePasswordDetailsDelegate
@@ -66,6 +75,7 @@
 - (void)passwordDetailsViewController:
             (PasswordDetailsViewController*)viewController
                didEditPasswordDetails:(PasswordDetails*)password {
+  self.password = password;
 }
 
 @end
@@ -268,3 +278,47 @@ TEST_F(PasswordDetailsViewControllerTest, TestPasswordDelete) {
         forEvent:nil];
   EXPECT_TRUE(handler().deletionCalled);
 }
+
+// Tests password editing. User confirmed this action.
+TEST_F(PasswordDetailsViewControllerTest, TestEditPasswordConfirmed) {
+  SetPassword();
+
+  PasswordDetailsViewController* passwordDetails =
+      base::mac::ObjCCastStrict<PasswordDetailsViewController>(controller());
+  [passwordDetails editButtonPressed];
+  EXPECT_FALSE(handler().editingCalled);
+  EXPECT_FALSE(delegate().password);
+  EXPECT_TRUE(passwordDetails.tableView.editing);
+
+  TableViewTextEditItem* cell =
+      static_cast<TableViewTextEditItem*>(GetTableViewItem(0, 2));
+  cell.textFieldValue = @"new_password";
+
+  [passwordDetails editButtonPressed];
+  EXPECT_TRUE(handler().editingCalled);
+
+  [passwordDetails passwordEditingConfirmed];
+  EXPECT_TRUE(delegate().password);
+
+  EXPECT_NSEQ(@"new_password", delegate().password.password);
+  EXPECT_FALSE(passwordDetails.tableView.editing);
+}
+
+// Tests  password editing. User cancelled this action.
+TEST_F(PasswordDetailsViewControllerTest, TestEditPasswordCancel) {
+  SetPassword();
+
+  PasswordDetailsViewController* passwordDetails =
+      base::mac::ObjCCastStrict<PasswordDetailsViewController>(controller());
+  [passwordDetails editButtonPressed];
+  EXPECT_FALSE(delegate().password);
+  EXPECT_TRUE(passwordDetails.tableView.editing);
+
+  TableViewTextEditItem* cell =
+      static_cast<TableViewTextEditItem*>(GetTableViewItem(0, 2));
+  cell.textFieldValue = @"new_password";
+
+  [passwordDetails editButtonPressed];
+  EXPECT_FALSE(delegate().password);
+  EXPECT_TRUE(passwordDetails.tableView.editing);
+}