[iOS][Credential-Provider] Remove credentials by identifier

This is a step in supporting Android passwords, which depend on the
affiliation matcher being available. However the matcher might not be
available if the user disables Sync, after saving Android passwords.
Deleting by the recordIdentifier allows deletion of Android passwords
without creating a valid Credential.

Bug: 1090776
Change-Id: Iec8010f3c1514e6b59a1052db590bcaaaf83d8e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410250Reviewed-by: David Jean <djean@chromium.org>
Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807005}

ios/chrome/browser/credential_provider/BUILD.gn

@@ -17,6 +17,8 @@ source_set("credential_provider") {
     "credential_provider_service_factory.mm",
     "credential_provider_support.cc",
     "credential_provider_support.h",
+    "credential_provider_util.h",
+    "credential_provider_util.mm",
   ]
   deps = [
     ":buildflags",

ios/chrome/browser/credential_provider/archivable_credential+password_form.mm

@@ -9,6 +9,7 @@
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h"
 #include "components/password_manager/core/browser/password_ui_utils.h"
+#import "ios/chrome/browser/credential_provider/credential_provider_util.h"
 #include "url/gurl.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -19,22 +20,11 @@ namespace {
 
 using base::SysUTF8ToNSString;
 using base::SysUTF16ToNSString;
-using base::UTF8ToUTF16;
 
 }  // namespace
 
 @implementation ArchivableCredential (PasswordForm)
 
-// Returns the equivalent of a unique record identifier. Built from the unique
-// columns in the logins database.
-NSString* recordIdentifierForPasswordForm(const autofill::PasswordForm& form) {
-  // These are the UNIQUE keys in the login database.
-  return SysUTF16ToNSString(
-      UTF8ToUTF16(form.url.spec() + "|") + form.username_element +
-      UTF8ToUTF16("|") + form.username_value + UTF8ToUTF16("|") +
-      form.password_element + UTF8ToUTF16("|" + form.signon_realm));
-}
-
 - (instancetype)initWithPasswordForm:(const autofill::PasswordForm&)passwordForm
                              favicon:(NSString*)favicon
                 validationIdentifier:(NSString*)validationIdentifier {
@@ -49,7 +39,7 @@ NSString* recordIdentifierForPasswordForm(const autofill::PasswordForm& form) {
   return [self initWithFavicon:favicon
             keychainIdentifier:keychainIdentifier
                           rank:passwordForm.times_used
-              recordIdentifier:recordIdentifierForPasswordForm(passwordForm)
+              recordIdentifier:RecordIdentifierForPasswordForm(passwordForm)
              serviceIdentifier:SysUTF8ToNSString(passwordForm.url.spec())
                    serviceName:SysUTF8ToNSString(site_name)
                           user:SysUTF16ToNSString(passwordForm.username_value)

ios/chrome/browser/credential_provider/credential_provider_service.mm

@@ -17,6 +17,7 @@
 #include "components/password_manager/core/browser/password_store_change.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "ios/chrome/browser/credential_provider/archivable_credential+password_form.h"
+#import "ios/chrome/browser/credential_provider/credential_provider_util.h"
 #include "ios/chrome/common/app_group/app_group_constants.h"
 #import "ios/chrome/common/credential_provider/archivable_credential.h"
 #import "ios/chrome/common/credential_provider/archivable_credential_store.h"
@@ -237,7 +238,7 @@ void CredentialProviderService::OnLoginsChanged(
   for (const PasswordStoreChange& change : changes) {
     ArchivableCredential* credential =
         CredentialFromForm(change.form(), account_validation_id_);
-    if (!credential) {
+    if (change.form().blocked_by_user) {
       continue;
     }
     switch (change.type()) {
@@ -248,7 +249,11 @@ void CredentialProviderService::OnLoginsChanged(
         [archivable_credential_store_ updateCredential:credential];
         break;
       case PasswordStoreChange::REMOVE:
-        [archivable_credential_store_ removeCredential:credential];
+        // Using the record identifier from the form, as the credential might
+        // not be valid anymore.
+        [archivable_credential_store_
+            removeCredentialWithRecordIdentifier:
+                RecordIdentifierForPasswordForm(change.form())];
         break;
 
       default:

ios/chrome/browser/credential_provider/credential_provider_util.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_CREDENTIAL_PROVIDER_CREDENTIAL_PROVIDER_UTIL_H_
+#define IOS_CHROME_BROWSER_CREDENTIAL_PROVIDER_CREDENTIAL_PROVIDER_UTIL_H_
+
+#import <Foundation/Foundation.h>
+
+#include "components/autofill/core/common/password_form.h"
+
+// Returns the equivalent of a unique record identifier. Built from the unique
+// columns in the logins database.
+NSString* RecordIdentifierForPasswordForm(const autofill::PasswordForm& form);
+
+#endif  // IOS_CHROME_BROWSER_CREDENTIAL_PROVIDER_CREDENTIAL_PROVIDER_UTIL_H_

ios/chrome/browser/credential_provider/credential_provider_util.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/credential_provider/credential_provider_util.h"
+
+#include "base/strings/sys_string_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using base::SysUTF16ToNSString;
+using base::UTF8ToUTF16;
+
+NSString* RecordIdentifierForPasswordForm(const autofill::PasswordForm& form) {
+  // These are the UNIQUE keys in the login database.
+  return SysUTF16ToNSString(
+      UTF8ToUTF16(form.url.spec() + "|") + form.username_element +
+      UTF8ToUTF16("|") + form.username_value + UTF8ToUTF16("|") +
+      form.password_element + UTF8ToUTF16("|" + form.signon_realm));
+}

ios/chrome/common/credential_provider/archivable_credential_store.mm

@@ -116,17 +116,16 @@
 }
 
 - (void)updateCredential:(id<Credential>)credential {
-  [self removeCredential:credential];
+  [self removeCredentialWithRecordIdentifier:credential.recordIdentifier];
   [self addCredential:credential];
 }
 
-- (void)removeCredential:(id<Credential>)credential {
-  DCHECK(credential.recordIdentifier)
-      << "credential must have a record identifier";
+- (void)removeCredentialWithRecordIdentifier:(NSString*)recordIdentifier {
+  DCHECK(recordIdentifier.length) << "Invalid |recordIdentifier| was passed.";
   dispatch_barrier_async(self.workingQueue, ^{
-    DCHECK(self.memoryStorage[credential.recordIdentifier])
+    DCHECK(self.memoryStorage[recordIdentifier])
         << "Credential doesn't exist in the storage";
-    self.memoryStorage[credential.recordIdentifier] = nil;
+    self.memoryStorage[recordIdentifier] = nil;
   });
 }
 

ios/chrome/common/credential_provider/archivable_credential_store_unittest.mm

@@ -100,7 +100,8 @@ TEST_F(ArchivableCredentialStoreTest, remove) {
   [credentialStore addCredential:credential];
   EXPECT_EQ(1u, credentialStore.credentials.count);
 
-  [credentialStore removeCredential:credential];
+  [credentialStore
+      removeCredentialWithRecordIdentifier:credential.recordIdentifier];
   EXPECT_EQ(0u, credentialStore.credentials.count);
 }
 

ios/chrome/common/credential_provider/credential_store.h

@@ -34,7 +34,7 @@
 
 // Removes a credential from the memory storage. Use |-saveDataWithCompletion:|
 // to update the data on disk.
-- (void)removeCredential:(id<Credential>)credential;
+- (void)removeCredentialWithRecordIdentifier:(NSString*)recordIdentifier;
 
 // Returns the credential with matching |identifier| or nil if none.
 - (id<Credential>)credentialWithIdentifier:(NSString*)identifier;