[PwdCheckAndroid] Display reason for double-compromised credentials

This CL ensures that credentials who are compromised in two ways display
both reasons in the list.

Translation screenshot for
https://storage.cloud.google.com/chromium-translation-screenshots/cf6c7eaf298250e9e14f7c244dacb7e11f07dd47

Bug: 1117489, 1092444
Change-Id: I214e5e2211d1732cb5fa745a582cc64a37890293
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362616
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799676}

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckBridge.java

@@ -62,7 +62,7 @@ class PasswordCheckBridge {
         assert password != null;
         mPasswordCheckObserver.onCompromisedCredentialFound(new CompromisedCredential(signonRealm,
                 origin, username, displayOrigin, displayUsername, password, passwordChangeUrl,
-                associatedApp, false, hasScript));
+                associatedApp, true, false, hasScript));
     }
 
     @CalledByNative
@@ -84,9 +84,10 @@ class PasswordCheckBridge {
     private static void insertCredential(CompromisedCredential[] credentials, int index,
             String signonRealm, GURL origin, String username, String displayOrigin,
             String displayUsername, String password, String passwordChangeUrl, String associatedApp,
-            boolean phished, boolean hasScript) {
+            boolean leaked, boolean phished, boolean hasScript) {
         credentials[index] = new CompromisedCredential(signonRealm, origin, username, displayOrigin,
-                displayUsername, password, passwordChangeUrl, associatedApp, phished, hasScript);
+                displayUsername, password, passwordChangeUrl, associatedApp, leaked, phished,
+                hasScript);
     }
 
     /**

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckViewBinder.java

@@ -29,6 +29,7 @@ import android.widget.LinearLayout;
 import android.widget.TextView;
 
 import androidx.annotation.ColorRes;
+import androidx.annotation.StringRes;
 import androidx.annotation.VisibleForTesting;
 import androidx.appcompat.content.res.AppCompatResources;
 import androidx.core.graphics.drawable.DrawableCompat;
@@ -134,9 +135,7 @@ class PasswordCheckViewBinder {
             username.setText(credential.getDisplayUsername());
 
             TextView reason = view.findViewById(R.id.compromised_reason);
-            reason.setText(credential.isPhished()
-                            ? R.string.password_check_credential_row_reason_phished
-                            : R.string.password_check_credential_row_reason_leaked);
+            reason.setText(getReasonForCredential(credential));
 
             ListMenuButton more = view.findViewById(R.id.credential_menu_button);
             more.setDelegate(() -> {
@@ -172,6 +171,12 @@ class PasswordCheckViewBinder {
         }
     }
 
+    private static @StringRes int getReasonForCredential(CompromisedCredential credential) {
+        if (!credential.isPhished()) return R.string.password_check_credential_row_reason_leaked;
+        if (!credential.isLeaked()) return R.string.password_check_credential_row_reason_phished;
+        return R.string.password_check_credential_row_reason_leaked_and_phished;
+    }
+
     /**
      * Called whenever a property in the given model changes. It updates the given view
      * accordingly.

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings.grd

@@ -187,6 +187,9 @@
       <message name="IDS_PASSWORD_CHECK_CREDENTIAL_ROW_REASON_PHISHED" desc="Small description explaining that a credential is compromised because it was entered on a deceptive site.">
         Entered on a deceptive site
       </message>
+      <message name="IDS_PASSWORD_CHECK_CREDENTIAL_ROW_REASON_LEAKED_AND_PHISHED" desc="Small description explaining that a credential is compromised because it was entered on a deceptive site and in addition, was part of a data breach.">
+        Entered on a deceptive site and found in data breach
+      </message>
       <message name="IDS_PASSWORD_CHECK_CREDENTIAL_ROW_SCRIPT_BUTTON_EXPLANATION" desc="Small description explaining that an automated password change can be done with a script.">
         Let Google Assistant help you change your password
       </message>

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings_grd/IDS_PASSWORD_CHECK_CREDENTIAL_ROW_REASON_LEAKED_AND_PHISHED.png.sha1

+cf6c7eaf298250e9e14f7c244dacb7e11f07dd47
\ No newline at end of file

chrome/browser/password_check/android/java/src/org/chromium/chrome/browser/password_check/CompromisedCredential.java

@@ -30,14 +30,15 @@ public class CompromisedCredential implements Parcelable {
                     final String password = in.readString();
                     final String passwordChangeUrl = in.readString();
                     final String associatedApp = in.readString();
-                    boolean[] boolArguments = new boolean[2];
+                    boolean[] boolArguments = new boolean[3];
                     in.readBooleanArray(boolArguments);
-                    final boolean phished = boolArguments[0];
-                    final boolean hasScript = boolArguments[1];
+                    final boolean leaked = boolArguments[0];
+                    final boolean phished = boolArguments[1];
+                    final boolean hasScript = boolArguments[2];
 
                     return new CompromisedCredential(signonRealm, origin, username, displayOrigin,
-                            displayUsername, password, passwordChangeUrl, associatedApp, phished,
-                            hasScript);
+                            displayUsername, password, passwordChangeUrl, associatedApp, leaked,
+                            phished, hasScript);
                 }
 
                 @Override
@@ -54,6 +55,7 @@ public class CompromisedCredential implements Parcelable {
     private final String mPassword;
     private final String mPasswordChangeUrl;
     private final String mAssociatedApp;
+    private final boolean mLeaked;
     private final boolean mPhished;
     private final boolean mHasScript;
 
@@ -72,7 +74,7 @@ public class CompromisedCredential implements Parcelable {
      */
     public CompromisedCredential(String signonRealm, GURL origin, String username,
             String displayOrigin, String displayUsername, String password, String passwordChangeUrl,
-            String associatedApp, boolean phished, boolean hasScript) {
+            String associatedApp, boolean leaked, boolean phished, boolean hasScript) {
         assert origin != null : "Credential origin is null! Pass an empty one instead.";
         assert signonRealm != null;
         assert passwordChangeUrl != null : "Change URL may be empty but not null!";
@@ -80,6 +82,7 @@ public class CompromisedCredential implements Parcelable {
         assert !passwordChangeUrl.isEmpty()
                 || !associatedApp.isEmpty()
             : "Change URL and app name may not be empty at the same time!";
+        assert leaked || phished : "A compromised credential must be leaked or phished!";
         mSignonRealm = signonRealm;
         mOrigin = origin;
         mUsername = username;
@@ -88,6 +91,7 @@ public class CompromisedCredential implements Parcelable {
         mPassword = password;
         mPasswordChangeUrl = passwordChangeUrl;
         mAssociatedApp = associatedApp;
+        mLeaked = leaked;
         mPhished = phished;
         mHasScript = hasScript;
     }
@@ -114,16 +118,18 @@ public class CompromisedCredential implements Parcelable {
     public String getDisplayOrigin() {
         return mDisplayOrigin;
     }
-    public boolean isPhished() {
-        return mPhished;
-    }
     public String getAssociatedApp() {
         return mAssociatedApp;
     }
-
     public String getPasswordChangeUrl() {
         return mPasswordChangeUrl;
     }
+    public boolean isLeaked() {
+        return mLeaked;
+    }
+    public boolean isPhished() {
+        return mPhished;
+    }
     public boolean hasScript() {
         return mHasScript;
     }
@@ -138,8 +144,8 @@ public class CompromisedCredential implements Parcelable {
                 && mDisplayUsername.equals(that.mDisplayUsername)
                 && mPassword.equals(that.mPassword)
                 && mPasswordChangeUrl.equals(that.mPasswordChangeUrl)
-                && mAssociatedApp.equals(that.mAssociatedApp) && mPhished == that.mPhished
-                && mHasScript == that.mHasScript;
+                && mAssociatedApp.equals(that.mAssociatedApp) && mLeaked == that.mLeaked
+                && mPhished == that.mPhished && mHasScript == that.mHasScript;
     }
 
     @Override
@@ -149,15 +155,15 @@ public class CompromisedCredential implements Parcelable {
                 + ", username='" + mUsername + '\'' + ", displayOrigin='" + mDisplayOrigin + '\''
                 + ", displayUsername='" + mDisplayUsername + '\'' + ", password='" + mPassword
                 + '\'' + ", passwordChangeUrl='" + mPasswordChangeUrl + '\'' + ", associatedApp='"
-                + mAssociatedApp + '\'' + ", phished=" + mPhished + ", hasScript=" + mHasScript
-                + '}';
+                + mAssociatedApp + '\'' + ", leaked=" + mLeaked + ", phished=" + mPhished
+                + ", hasScript=" + mHasScript + '}';
     }
 
     @Override
     public int hashCode() {
         return Objects.hash(mSignonRealm, mOrigin.getPossiblyInvalidSpec(), mUsername,
                 mDisplayOrigin, mDisplayUsername, mPassword, mPasswordChangeUrl, mAssociatedApp,
-                mPhished, mHasScript);
+                mLeaked, mPhished, mHasScript);
     }
 
     @Override
@@ -170,7 +176,7 @@ public class CompromisedCredential implements Parcelable {
         parcel.writeString(mPassword);
         parcel.writeString(mPasswordChangeUrl);
         parcel.writeString(mAssociatedApp);
-        parcel.writeBooleanArray(new boolean[] {mPhished, mHasScript});
+        parcel.writeBooleanArray(new boolean[] {mLeaked, mPhished, mHasScript});
     }
 
     @Override

chrome/browser/password_check/android/javatests/src/org/chromium/chrome/browser/password_check/PasswordCheckEditViewTest.java

@@ -68,7 +68,7 @@ public class PasswordCheckEditViewTest {
     private static final CompromisedCredential ANA =
             new CompromisedCredential("https://some-url.com/signin",
                     new GURL("https://some-url.com/"), "Ana", "some-url.com", "Ana", "password",
-                    "https://some-url.com/.well-known/change-password", "", false, false);
+                    "https://some-url.com/.well-known/change-password", "", true, false, false);
 
     private PasswordCheckEditFragmentView mPasswordCheckEditView;
 

chrome/browser/password_check/android/javatests/src/org/chromium/chrome/browser/password_check/PasswordCheckViewTest.java

@@ -95,18 +95,23 @@ public class PasswordCheckViewTest {
     private static final CompromisedCredential ANA =
             new CompromisedCredential("https://some-url.com/signin",
                     new GURL("https://some-url.com/"), "Ana", "some-url.com", "Ana", "password",
-                    "https://some-url.com/.well-known/change-password", "", false, false);
+                    "https://some-url.com/.well-known/change-password", "", true, false, false);
     private static final CompromisedCredential PHISHED =
             new CompromisedCredential("http://example.com/signin", new GURL("http://example.com/"),
                     "", "http://example.com", "(No username)", "DoSomething",
-                    "http://example.com/.well-known/change-password", "", true, false);
+                    "http://example.com/.well-known/change-password", "", false, true, false);
     private static final CompromisedCredential LEAKED =
             new CompromisedCredential("https://some-other-url.com/signin",
                     new GURL("https://some-other-url.com/"), "AZiegler", "some-other-url.com",
-                    "AZiegler", "N0M3rcy", "", "com.other.package", false, false);
-    private static final CompromisedCredential SCRIPTED = new CompromisedCredential(
-            "https://script.com/signin", new GURL("https://script.com/"), "Charlie", "script.com",
-            "Charlie", "secret", "https://script.com/.well-known/change-password", "", false, true);
+                    "AZiegler", "N0M3rcy", "", "com.other.package", true, false, false);
+    private static final CompromisedCredential LEAKED_AND_PHISHED =
+            new CompromisedCredential("https://super-important.com/signin",
+                    new GURL("https://super-important.com/"), "HSong", "super-important.com",
+                    "HSong", "N3rfTh1s", "", "com.important.super", true, true, false);
+    private static final CompromisedCredential SCRIPTED =
+            new CompromisedCredential("https://script.com/signin", new GURL("https://script.com/"),
+                    "Charlie", "script.com", "Charlie", "secret",
+                    "https://script.com/.well-known/change-password", "", true, false, true);
 
     private static final int LEAKS_COUNT = 2;
 
@@ -398,8 +403,9 @@ public class PasswordCheckViewTest {
         runOnUiThreadBlocking(() -> {
             mModel.get(ITEMS).add(buildCredentialItem(PHISHED));
             mModel.get(ITEMS).add(buildCredentialItem(LEAKED));
+            mModel.get(ITEMS).add(buildCredentialItem(LEAKED_AND_PHISHED));
         });
-        waitForListViewToHaveLength(2);
+        waitForListViewToHaveLength(3);
 
         // The phished credential is rendered first:
         assertThat(getCredentialOriginAt(0).getText(), is(PHISHED.getDisplayOrigin()));
@@ -416,6 +422,14 @@ public class PasswordCheckViewTest {
                 is(getString(R.string.password_check_credential_row_reason_leaked)));
         assertThat(getCredentialChangeButtonAt(1).getVisibility(), is(View.VISIBLE));
         assertThat(getCredentialChangeHintAt(1).getVisibility(), is(View.GONE));
+
+        // The leaked and phished credential is rendered third:
+        assertThat(getCredentialOriginAt(2).getText(), is(LEAKED_AND_PHISHED.getDisplayOrigin()));
+        assertThat(getCredentialUserAt(2).getText(), is(LEAKED_AND_PHISHED.getDisplayUsername()));
+        assertThat(getCredentialReasonAt(2).getText(),
+                is(getString(R.string.password_check_credential_row_reason_leaked_and_phished)));
+        assertThat(getCredentialChangeButtonAt(2).getVisibility(), is(View.VISIBLE));
+        assertThat(getCredentialChangeHintAt(2).getVisibility(), is(View.GONE));
     }
 
     @Test

chrome/browser/password_check/android/junit/src/org/chromium/chrome/browser/password_check/PasswordCheckControllerTest.java

@@ -71,10 +71,10 @@ import org.chromium.url.GURL;
 public class PasswordCheckControllerTest {
     private static final CompromisedCredential ANA =
             new CompromisedCredential("https://m.a.xyz/signin", mock(GURL.class), "Ana", "m.a.xyz",
-                    "Ana", "password", "", "xyz.a.some.package", false, false);
+                    "Ana", "password", "", "xyz.a.some.package", true, false, false);
     private static final CompromisedCredential BOB = new CompromisedCredential(
             "http://www.b.ch/signin", mock(GURL.class), "", "http://www.b.ch", "(No username)",
-            "DoneSth", "http://www.b.ch/.well-known/change-password", "", false, true);
+            "DoneSth", "http://www.b.ch/.well-known/change-password", "", true, false, true);
 
     @Rule
     public TestRule mFeaturesProcessorRule = new Features.JUnitProcessor();

chrome/browser/password_check/android/password_check_bridge.cc

@@ -83,6 +83,8 @@ void PasswordCheckBridge::GetCompromisedCredentials(
         base::android::ConvertUTF8ToJavaString(env,
                                                credential.change_password_url),
         base::android::ConvertUTF8ToJavaString(env, credential.package_name),
+        (credential.compromise_type ==
+         password_manager::CompromiseTypeFlags::kCredentialLeaked),
         (credential.compromise_type ==
          password_manager::CompromiseTypeFlags::kCredentialPhished),
         credential.has_script);