Commit 28988f10 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[PwdCheckAndroid] Start App over CCT if available

If the password for an App is compromised, the password check now offers
a change button to launch the app (provided the app package is locally
available).
If the app is not installed, a hint informs the user that they can
change the password in the app.

For regular websites, a CCT will navigate to the index page (or
.well-known/change-password if that feature is active).

Screenshot in the bug.

Bug: 1115117, 1092444
Change-Id: I6debb91070de44af62ecd74ca97fb2e4744d6385
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2351782
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797603}
parent 72ea286a
......@@ -49,6 +49,15 @@
android:text="@string/password_check_credential_row_change_button_caption"
style="@style/FilledButton.Flat" />
<TextView
android:id="@+id/credential_change_hint"
android:text="@string/password_check_credential_row_change_button_hint"
android:layout_marginTop="@dimen/compromised_credential_row_button_margin_top"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:textAppearance="@style/TextAppearance.TextMedium.Secondary"
android:visibility="gone" />
</LinearLayout>
<org.chromium.components.browser_ui.widget.listmenu.ListMenuButton
......
......@@ -67,6 +67,15 @@
android:text="@string/password_check_credential_row_change_manually_button_caption"
style="@style/TextButton"/>
<TextView
android:id="@+id/credential_change_hint"
android:text="@string/password_check_credential_row_change_button_hint"
android:layout_marginTop="@dimen/compromised_credential_row_button_margin_top"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:textAppearance="@style/TextAppearance.TextMedium.Secondary"
android:visibility="gone" />
</LinearLayout>
<org.chromium.components.browser_ui.widget.listmenu.ListMenuButton
......
......@@ -54,13 +54,15 @@ class PasswordCheckBridge {
// TODO(crbug.com/1102025): Add call from native.
void onCompromisedCredentialFound(String signonRealm, GURL origin, String username,
String displayOrigin, String displayUsername, String password, boolean hasScript) {
String displayOrigin, String displayUsername, String password, String passwordChangeUrl,
String associatedApp, boolean hasScript) {
assert signonRealm != null;
assert displayOrigin != null;
assert username != null;
assert password != null;
mPasswordCheckObserver.onCompromisedCredentialFound(new CompromisedCredential(signonRealm,
origin, username, displayOrigin, displayUsername, password, false, hasScript));
origin, username, displayOrigin, displayUsername, password, passwordChangeUrl,
associatedApp, false, hasScript));
}
@CalledByNative
......@@ -81,9 +83,10 @@ class PasswordCheckBridge {
@CalledByNative
private static void insertCredential(CompromisedCredential[] credentials, int index,
String signonRealm, GURL origin, String username, String displayOrigin,
String displayUsername, String password, boolean phished, boolean hasScript) {
String displayUsername, String password, String passwordChangeUrl, String associatedApp,
boolean phished, boolean hasScript) {
credentials[index] = new CompromisedCredential(signonRealm, origin, username, displayOrigin,
displayUsername, password, phished, hasScript);
displayUsername, password, passwordChangeUrl, associatedApp, phished, hasScript);
}
/**
......
......@@ -22,12 +22,14 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
import java.util.Objects;
/**
* Creates the PasswordCheckComponentUi. This class is responsible for managing the UI for the check
* of the leaked password.
*/
class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObserver {
private static final String WELL_KNOWN_URL_PATH = "/.well-known/change-password";
class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObserver,
PasswordCheckComponentUi.ChangePasswordDelegate {
private static final String AUTOFILL_ASSISTANT_PACKAGE =
"org.chromium.chrome.browser.autofill_assistant.";
private static final String AUTOFILL_ASSISTANT_ENABLED_KEY =
......@@ -37,8 +39,7 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
private static final String INTENT = "PASSWORD_CHANGE";
private final PasswordCheckFragmentView mFragmentView;
private final PasswordCheckMediator mMediator = new PasswordCheckMediator(
this::launchCctWithChangePasswordUrl, this::launchCctWithScript);
private final PasswordCheckMediator mMediator = new PasswordCheckMediator(this);
private PropertyModel mModel;
/**
......@@ -133,25 +134,41 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
/**
* Launches a CCT that points to the change password form or home page of |origin|.
* @param origin Origin of the site to be opened in a CCT.
* @param credential A {@link CompromisedCredential} to be changed in an App or in a CCT.
*/
private void launchCctWithChangePasswordUrl(String origin) {
// TODO(crbug.com/1092444): Handle the case when an app should be opened. Consider to set
// |browser_fallback_url|, it is used in case of error while opening a CCT.
Intent intent = buildIntent(origin + WELL_KNOWN_URL_PATH);
IntentUtils.safeStartActivity(mFragmentView.getActivity(), intent);
@Override
public void launchAppOrCctWithChangePasswordUrl(CompromisedCredential credential) {
// TODO(crbug.com/1092444): Always launch the URL if possible and let Android handle the
// match to open it.
IntentUtils.safeStartActivity(mFragmentView.getActivity(),
credential.getAssociatedApp().isEmpty()
? buildIntent(credential.getPasswordChangeUrl())
: getPackageLaunchIntent(credential.getAssociatedApp()));
}
@Override
public boolean canManuallyChangeCredential(CompromisedCredential credential) {
return !credential.getPasswordChangeUrl().isEmpty()
|| getPackageLaunchIntent(credential.getAssociatedApp()) != null;
}
/**
* Launches a CCT that starts a password change script for a {@link CompromisedCredential}.
* @param credential A {@link CompromisedCredential} to be changed with a script.
*/
private void launchCctWithScript(CompromisedCredential credential) {
@Override
public void launchCctWithScript(CompromisedCredential credential) {
Intent intent = buildIntent(credential.getOrigin().getSpec());
populateAutofillAssistantExtras(intent, credential.getUsername());
IntentUtils.safeStartActivity(mFragmentView.getActivity(), intent);
}
private Intent getPackageLaunchIntent(String packageName) {
return Objects.requireNonNull(mFragmentView.getActivity())
.getPackageManager()
.getLaunchIntentForPackage(packageName);
}
/**
* Builds an intent to launch a CCT.
* @param initialUrl Initial URL to launch a CCT.
......
......@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.password_check;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.HAS_MANUAL_CHANGE_BUTTON;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
......@@ -21,7 +22,6 @@ import android.util.Pair;
import androidx.appcompat.app.AlertDialog;
import org.chromium.base.Consumer;
import org.chromium.ui.modelutil.ListModel;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.modelutil.PropertyModel;
......@@ -32,15 +32,12 @@ import org.chromium.ui.modelutil.PropertyModel;
*/
class PasswordCheckMediator
implements PasswordCheckCoordinator.CredentialEventHandler, PasswordCheck.Observer {
private final PasswordCheckComponentUi.ChangePasswordDelegate mChangePasswordDelegate;
private PropertyModel mModel;
private PasswordCheckComponentUi.Delegate mDelegate;
private final Consumer<String> mLaunchCctWithChangePasswordUrl;
private final Consumer<CompromisedCredential> mLaunchCctWithScript;
PasswordCheckMediator(Consumer<String> launchCctWithChangePasswordUrl,
Consumer<CompromisedCredential> launchCctWithScript) {
this.mLaunchCctWithChangePasswordUrl = launchCctWithChangePasswordUrl;
this.mLaunchCctWithScript = launchCctWithScript;
PasswordCheckMediator(PasswordCheckCoordinator.ChangePasswordDelegate changePasswordDelegate) {
mChangePasswordDelegate = changePasswordDelegate;
}
void initialize(PropertyModel model, PasswordCheckComponentUi.Delegate delegate,
......@@ -76,15 +73,7 @@ class PasswordCheckMediator
if (items.size() > 1) items.removeRange(1, items.size() - 1);
for (CompromisedCredential credential : credentials) {
items.add(new ListItem(credential.hasScript()
? PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT
: PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
new PropertyModel
.Builder(PasswordCheckProperties.CompromisedCredentialProperties
.ALL_KEYS)
.with(COMPROMISED_CREDENTIAL, credential)
.with(CREDENTIAL_HANDLER, this)
.build()));
items.add(createEntryForCredential(credential));
}
}
......@@ -141,14 +130,7 @@ class PasswordCheckMediator
assert leakedCredential != null;
ListModel<ListItem> items = mModel.get(ITEMS);
assert items.size() >= 1 : "Needs to initialize list with header before adding items!";
items.add(new ListItem(leakedCredential.hasScript()
? PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT
: PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
new PropertyModel
.Builder(PasswordCheckProperties.CompromisedCredentialProperties.ALL_KEYS)
.with(COMPROMISED_CREDENTIAL, leakedCredential)
.with(CREDENTIAL_HANDLER, this)
.build()));
items.add(createEntryForCredential(leakedCredential));
}
@Override
......@@ -178,13 +160,13 @@ class PasswordCheckMediator
@Override
public void onChangePasswordButtonClick(CompromisedCredential credential) {
mLaunchCctWithChangePasswordUrl.accept(credential.getSignonRealm());
mChangePasswordDelegate.launchAppOrCctWithChangePasswordUrl(credential);
}
@Override
public void onChangePasswordWithScriptButtonClick(CompromisedCredential credential) {
assert credential.hasScript();
mLaunchCctWithScript.accept(credential);
mChangePasswordDelegate.launchCctWithScript(credential);
}
private void runCheck() {
......@@ -196,4 +178,17 @@ class PasswordCheckMediator
assert passwordCheck != null : "Password Check UI component needs native counterpart!";
return passwordCheck;
}
private ListItem createEntryForCredential(CompromisedCredential credential) {
return new ListItem(credential.hasScript()
? PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT
: PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
new PropertyModel
.Builder(PasswordCheckProperties.CompromisedCredentialProperties.ALL_KEYS)
.with(COMPROMISED_CREDENTIAL, credential)
.with(HAS_MANUAL_CHANGE_BUTTON,
mChangePasswordDelegate.canManuallyChangeCredential(credential))
.with(CREDENTIAL_HANDLER, this)
.build());
}
}
......@@ -44,8 +44,11 @@ class PasswordCheckProperties {
static final PropertyModel.ReadableObjectPropertyKey<
PasswordCheckCoordinator.CredentialEventHandler> CREDENTIAL_HANDLER =
new PropertyModel.ReadableObjectPropertyKey<>("credential_handler");
static final PropertyModel.ReadableBooleanPropertyKey HAS_MANUAL_CHANGE_BUTTON =
new PropertyModel.ReadableBooleanPropertyKey("has_change_button");
static final PropertyKey[] ALL_KEYS = {COMPROMISED_CREDENTIAL, CREDENTIAL_HANDLER};
static final PropertyKey[] ALL_KEYS = {
COMPROMISED_CREDENTIAL, CREDENTIAL_HANDLER, HAS_MANUAL_CHANGE_BUTTON};
private CompromisedCredentialProperties() {}
}
......
......@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.password_check;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.HAS_MANUAL_CHANGE_BUTTON;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
......@@ -153,6 +154,13 @@ class PasswordCheckViewBinder {
} else if (propertyKey == CREDENTIAL_HANDLER) {
assert model.get(CREDENTIAL_HANDLER) != null;
// Is read-only and must therefore be bound initially, so no action required.
} else if (propertyKey == HAS_MANUAL_CHANGE_BUTTON) {
ButtonCompat button = view.findViewById(R.id.credential_change_button);
button.setVisibility(model.get(HAS_MANUAL_CHANGE_BUTTON) ? View.VISIBLE : View.GONE);
TextView changeHint = view.findViewById(R.id.credential_change_hint);
changeHint.setVisibility(model.get(HAS_MANUAL_CHANGE_BUTTON) || credential.hasScript()
? View.GONE
: View.VISIBLE);
} else {
assert false : "Unhandled update to property:" + propertyKey;
}
......
......@@ -175,6 +175,9 @@
<message name="IDS_PASSWORD_CHECK_CREDENTIAL_ROW_CHANGE_BUTTON_CAPTION" desc="Caption for the primary button in the list of compromised credentials.">
Change password
</message>
<message name="IDS_PASSWORD_CHECK_CREDENTIAL_ROW_CHANGE_BUTTON_HINT" desc="Small hint text explaining that the user needs to manually change the password in an external app.">
Change the password in the app
</message>
<message name="IDS_PASSWORD_CHECK_CREDENTIAL_ROW_CHANGE_MANUALLY_BUTTON_CAPTION" desc="Caption for the secondary button in the list of compromised credentials.">
Change manually
</message>
......
......@@ -20,6 +20,8 @@ public class CompromisedCredential {
private final String mDisplayOrigin;
private final String mDisplayUsername;
private final String mPassword;
private final String mPasswordChangeUrl;
private final String mAssociatedApp;
private final boolean mPhished;
private final boolean mHasScript;
......@@ -31,20 +33,29 @@ public class CompromisedCredential {
* missing scheme).
* @param displayUsername The username displayed to the user (substituted if empty).
* @param password The compromised password.
* @param passwordChangeUrl A URL that links to the password change form of the affected site.
* @param associatedApp The associated app if the password originates from it.
* @param phished True iff the credential was entered on an unsafe site.
* @param hasScript True iff the credential can be automatically fixed.
*/
public CompromisedCredential(String signonRealm, GURL origin, String username,
String displayOrigin, String displayUsername, String password, boolean phished,
boolean hasScript) {
String displayOrigin, String displayUsername, String password, String passwordChangeUrl,
String associatedApp, 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!";
assert associatedApp != null : "App package name may be empty but not null!";
assert !passwordChangeUrl.isEmpty()
|| !associatedApp.isEmpty()
: "Change URL and app name may not be empty at the same time!";
mSignonRealm = signonRealm;
mOrigin = origin;
mUsername = username;
mDisplayOrigin = displayOrigin;
mDisplayUsername = displayUsername;
mPassword = password;
mPasswordChangeUrl = passwordChangeUrl;
mAssociatedApp = associatedApp;
mPhished = phished;
mHasScript = hasScript;
}
......@@ -74,6 +85,13 @@ public class CompromisedCredential {
public boolean isPhished() {
return mPhished;
}
public String getAssociatedApp() {
return mAssociatedApp;
}
public String getPasswordChangeUrl() {
return mPasswordChangeUrl;
}
public boolean hasScript() {
return mHasScript;
}
......@@ -86,7 +104,9 @@ public class CompromisedCredential {
return mSignonRealm.equals(that.mSignonRealm) && mOrigin.equals(that.mOrigin)
&& mUsername.equals(that.mUsername) && mDisplayOrigin.equals(that.mDisplayOrigin)
&& mDisplayUsername.equals(that.mDisplayUsername)
&& mPassword.equals(that.mPassword) && mPhished == that.mPhished
&& mPassword.equals(that.mPassword)
&& mPasswordChangeUrl.equals(that.mPasswordChangeUrl)
&& mAssociatedApp.equals(that.mAssociatedApp) && mPhished == that.mPhished
&& mHasScript == that.mHasScript;
}
......@@ -96,12 +116,14 @@ public class CompromisedCredential {
+ "signonRealm='" + mSignonRealm + ", origin='" + mOrigin + '\'' + '\''
+ ", username='" + mUsername + '\'' + ", displayOrigin='" + mDisplayOrigin + '\''
+ ", displayUsername='" + mDisplayUsername + '\'' + ", password='" + mPassword
+ '\'' + ", phished=" + mPhished + ", hasScript=" + mHasScript + '}';
+ '\'' + ", passwordChangeUrl='" + mPasswordChangeUrl + '\'' + ", associatedApp='"
+ mAssociatedApp + '\'' + ", phished=" + mPhished + ", hasScript=" + mHasScript
+ '}';
}
@Override
public int hashCode() {
return Objects.hash(mSignonRealm, mOrigin, mUsername, mDisplayOrigin, mDisplayUsername,
mPassword, mPhished, mHasScript);
mPassword, mPasswordChangeUrl, mAssociatedApp, mPhished, mHasScript);
}
}
......@@ -21,6 +21,31 @@ interface PasswordCheckComponentUi {
void removeCredential(CompromisedCredential credential);
}
/**
* Implementers of this delegate are expected to launch apps or Chrome Custom tabs that enable
* the user to change a compromised password.
*/
interface ChangePasswordDelegate {
/**
* @param credential A {@link CompromisedCredential}.
* @return True iff there is a valid URL to navigate to or an app that can be opened.
*/
boolean canManuallyChangeCredential(CompromisedCredential credential);
/**
* Launches an app (if available) or a CCT with the site the given credential was used on.
* @param credential A {@link CompromisedCredential}.
*/
void launchAppOrCctWithChangePasswordUrl(CompromisedCredential credential);
/**
* Launches a CCT with the site the given credential was used on and invokes the script that
* fixes the compromised credential automatically.
* @param credential A {@link CompromisedCredential}.
*/
void launchCctWithScript(CompromisedCredential credential);
}
/**
* Handle the request of the user to show the help page for the Check Passwords view.
* @param item A {@link MenuItem}.
......
......@@ -21,6 +21,7 @@ import static org.mockito.Mockito.verify;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.HAS_MANUAL_CHANGE_BUTTON;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
......@@ -91,18 +92,21 @@ import java.util.concurrent.atomic.AtomicInteger;
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
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", false, false);
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);
private static final CompromisedCredential PHISHED =
new CompromisedCredential("http://example.com/signin", new GURL("http://example.com/"),
"", "http://example.com", "(No username)", "DoSomething", 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", false, false);
private static final CompromisedCredential SCRIPTED =
new CompromisedCredential("https://script.com/signin", new GURL("https://script.com/"),
"Charlie", "script.com", "Charlie", "secret", false, true);
"", "http://example.com", "(No username)", "DoSomething",
"http://example.com/.well-known/change-password", "", 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);
private static final int LEAKS_COUNT = 2;
......@@ -390,7 +394,7 @@ public class PasswordCheckViewTest {
@Test
@MediumTest
public void testCrendentialDisplaysNameOriginAndReason() {
public void testCredentialDisplaysNameOriginAndReason() {
runOnUiThreadBlocking(() -> {
mModel.get(ITEMS).add(buildCredentialItem(PHISHED));
mModel.get(ITEMS).add(buildCredentialItem(LEAKED));
......@@ -402,17 +406,44 @@ public class PasswordCheckViewTest {
assertThat(getCredentialUserAt(0).getText(), is(PHISHED.getDisplayUsername()));
assertThat(getCredentialReasonAt(0).getText(),
is(getString(R.string.password_check_credential_row_reason_phished)));
assertThat(getCredentialChangeButtonAt(0).getVisibility(), is(View.VISIBLE));
assertThat(getCredentialChangeHintAt(0).getVisibility(), is(View.GONE));
// The leaked credential is rendered second:
assertThat(getCredentialOriginAt(1).getText(), is(LEAKED.getDisplayOrigin()));
assertThat(getCredentialUserAt(1).getText(), is(LEAKED.getDisplayUsername()));
assertThat(getCredentialReasonAt(1).getText(),
is(getString(R.string.password_check_credential_row_reason_leaked)));
assertThat(getCredentialChangeButtonAt(1).getVisibility(), is(View.VISIBLE));
assertThat(getCredentialChangeHintAt(1).getVisibility(), is(View.GONE));
}
@Test
@MediumTest
public void testCrendentialDisplaysChangeButtonWithScript() {
public void testHidesCredentialChangeButtonWithoutValidEntryPoint() {
runOnUiThreadBlocking(
()
-> mModel.get(ITEMS).add(new MVCListAdapter.ListItem(
PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
new PropertyModel
.Builder(PasswordCheckProperties
.CompromisedCredentialProperties.ALL_KEYS)
.with(COMPROMISED_CREDENTIAL, ANA)
.with(HAS_MANUAL_CHANGE_BUTTON, false)
.with(CREDENTIAL_HANDLER, mMockHandler)
.build())));
waitForListViewToHaveLength(1);
// The credential has no change button:
assertThat(getCredentialOriginAt(0).getText(), is(ANA.getDisplayOrigin()));
assertThat(getCredentialUserAt(0).getText(), is(ANA.getDisplayUsername()));
assertThat(getCredentialChangeButtonAt(0).getVisibility(), is(View.GONE));
assertThat(getCredentialChangeHintAt(0).getVisibility(), is(View.VISIBLE));
}
@Test
@MediumTest
public void testCredentialDisplaysChangeButtonWithScript() {
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildCredentialItem(SCRIPTED)); });
pollUiThread(() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(1)));
......@@ -439,6 +470,7 @@ public class PasswordCheckViewTest {
assertThat(getCredentialChangeButtonAt(0).getText(),
is(getString(
R.string.password_check_credential_row_change_manually_button_caption)));
assertThat(getCredentialChangeHintAt(0).getVisibility(), is(View.GONE));
}
@Test
......@@ -567,6 +599,7 @@ public class PasswordCheckViewTest {
new PropertyModel
.Builder(PasswordCheckProperties.CompromisedCredentialProperties.ALL_KEYS)
.with(COMPROMISED_CREDENTIAL, credential)
.with(HAS_MANUAL_CHANGE_BUTTON, true)
.with(CREDENTIAL_HANDLER, mMockHandler)
.build());
}
......@@ -656,6 +689,11 @@ public class PasswordCheckViewTest {
R.id.credential_change_button);
}
private TextView getCredentialChangeHintAt(int index) {
return getPasswordCheckViewList().getChildAt(index).findViewById(
R.id.credential_change_hint);
}
private ButtonCompat getCredentialChangeButtonWithScriptAt(int index) {
return getPasswordCheckViewList().getChildAt(index).findViewById(
R.id.credential_change_button_with_script);
......
......@@ -18,6 +18,7 @@ import static org.mockito.Mockito.when;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.HAS_MANUAL_CHANGE_BUTTON;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
......@@ -43,7 +44,6 @@ import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.Consumer;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.password_check.PasswordCheckProperties.ItemType;
......@@ -63,10 +63,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", false, false);
private static final CompromisedCredential BOB =
new CompromisedCredential("http://www.b.ch/signin", mock(GURL.class), "",
"http://www.b.ch", "(No username)", "DoneSth", false, true);
"Ana", "password", "", "xyz.a.some.package", 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);
@Rule
public TestRule mFeaturesProcessorRule = new Features.JUnitProcessor();
......@@ -76,9 +76,7 @@ public class PasswordCheckControllerTest {
@Mock
private PasswordCheckComponentUi.Delegate mDelegate;
@Mock
private Consumer<String> mLaunchCctWithChangePasswordUrlConsumer;
@Mock
private Consumer<CompromisedCredential> mLaunchCctWithScriptConsumer;
private PasswordCheckComponentUi.ChangePasswordDelegate mChangePasswordDelegate;
@Mock
private PasswordCheck mPasswordCheck;
......@@ -90,8 +88,7 @@ public class PasswordCheckControllerTest {
public void setUp() {
MockitoAnnotations.initMocks(this);
mModel = PasswordCheckProperties.createDefaultModel();
mMediator = new PasswordCheckMediator(
mLaunchCctWithChangePasswordUrlConsumer, mLaunchCctWithScriptConsumer);
mMediator = new PasswordCheckMediator(mChangePasswordDelegate);
PasswordCheckFactory.setPasswordCheckForTesting(mPasswordCheck);
mMediator.initialize(mModel, mDelegate, PasswordCheckReferrer.PASSWORD_SETTINGS);
}
......@@ -165,6 +162,7 @@ public class PasswordCheckControllerTest {
public void testCreatesEntryForExistingCredentials() {
when(mPasswordCheck.getCompromisedCredentials())
.thenReturn(new CompromisedCredential[] {ANA});
when(mChangePasswordDelegate.canManuallyChangeCredential(eq(ANA))).thenReturn(true);
mMediator.onPasswordCheckStatusChanged(IDLE);
mMediator.onCompromisedCredentialsFetchCompleted();
......@@ -172,12 +170,29 @@ public class PasswordCheckControllerTest {
assertThat(mModel.get(ITEMS).get(1).type, is(ItemType.COMPROMISED_CREDENTIAL));
assertThat(mModel.get(ITEMS).get(1).model.get(COMPROMISED_CREDENTIAL), equalTo(ANA));
assertThat(mModel.get(ITEMS).get(1).model.get(CREDENTIAL_HANDLER), is(mMediator));
assertThat(mModel.get(ITEMS).get(1).model.get(HAS_MANUAL_CHANGE_BUTTON), is(true));
}
@Test
public void testHidesChangeButtonIfManualChangeIsNotPossible() {
when(mPasswordCheck.getCompromisedCredentials())
.thenReturn(new CompromisedCredential[] {BOB});
when(mChangePasswordDelegate.canManuallyChangeCredential(eq(BOB))).thenReturn(false);
mMediator.onPasswordCheckStatusChanged(IDLE);
mMediator.onCompromisedCredentialsFetchCompleted();
assertThat(mModel.get(ITEMS).get(1).type, is(ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT));
assertThat(mModel.get(ITEMS).get(1).model.get(COMPROMISED_CREDENTIAL), equalTo(BOB));
assertThat(mModel.get(ITEMS).get(1).model.get(CREDENTIAL_HANDLER), is(mMediator));
assertThat(mModel.get(ITEMS).get(1).model.get(HAS_MANUAL_CHANGE_BUTTON), is(false));
}
@Test
public void testAppendsEntryForNewlyFoundCredentials() {
when(mPasswordCheck.getCompromisedCredentials())
.thenReturn(new CompromisedCredential[] {ANA});
when(mChangePasswordDelegate.canManuallyChangeCredential(eq(BOB))).thenReturn(true);
mMediator.onPasswordCheckStatusChanged(IDLE);
mMediator.onCompromisedCredentialsFetchCompleted();
assertThat(mModel.get(ITEMS).size(), is(2)); // Header + existing credentials.
......@@ -187,6 +202,7 @@ public class PasswordCheckControllerTest {
assertThat(mModel.get(ITEMS).get(2).type, is(ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT));
assertThat(mModel.get(ITEMS).get(2).model.get(COMPROMISED_CREDENTIAL), equalTo(BOB));
assertThat(mModel.get(ITEMS).get(2).model.get(CREDENTIAL_HANDLER), is(mMediator));
assertThat(mModel.get(ITEMS).get(2).model.get(HAS_MANUAL_CHANGE_BUTTON), is(true));
}
@Test
......@@ -225,13 +241,13 @@ public class PasswordCheckControllerTest {
@Test
public void testOnChangePasswordButtonClick() {
mMediator.onChangePasswordButtonClick(ANA);
verify(mLaunchCctWithChangePasswordUrlConsumer).accept(eq(ANA.getSignonRealm()));
verify(mChangePasswordDelegate).launchAppOrCctWithChangePasswordUrl(eq(ANA));
}
@Test
public void testOnChangePasswordWithScriptButtonClick() {
mMediator.onChangePasswordWithScriptButtonClick(BOB);
verify(mLaunchCctWithScriptConsumer).accept(eq(BOB));
verify(mChangePasswordDelegate).launchCctWithScript(eq(BOB));
}
private void assertIdleHeader(MVCListAdapter.ListItem header) {
......
......@@ -76,6 +76,9 @@ void PasswordCheckBridge::GetCompromisedCredentials(
base::android::ConvertUTF16ToJavaString(env,
credential.display_username),
base::android::ConvertUTF16ToJavaString(env, credential.password),
base::android::ConvertUTF8ToJavaString(env,
credential.change_password_url),
base::android::ConvertUTF8ToJavaString(env, credential.package_name),
(credential.compromise_type ==
password_manager::CompromiseTypeFlags::kCredentialPhished),
credential.has_script);
......
......@@ -4,6 +4,7 @@
#include "chrome/browser/password_check/android/password_check_manager.h"
#include "base/feature_list.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/password_check/android/password_check_bridge.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
......@@ -13,6 +14,7 @@
#include "components/password_manager/core/browser/password_manager_client.h"
#include "components/password_manager/core/browser/password_manager_util.h"
#include "components/password_manager/core/browser/ui/compromised_credentials_manager.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/strings/grit/components_strings.h"
#include "components/sync/driver/profile_sync_service.h"
#include "components/url_formatter/url_formatter.h"
......@@ -20,12 +22,22 @@
namespace {
constexpr char kWellKnownUrlPath[] = ".well-known/change-password";
base::string16 GetDisplayUsername(const base::string16& username) {
return username.empty()
? l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_EMPTY_LOGIN)
: username;
}
std::string CreateChangeUrl(const GURL& url) {
if (base::FeatureList::IsEnabled(
password_manager::features::kWellKnownChangePassword)) {
return url.GetOrigin().spec() + kWellKnownUrlPath;
}
return url.GetOrigin().spec();
}
} // namespace
using autofill::PasswordForm;
......@@ -183,7 +195,7 @@ CompromisedCredentialForUI PasswordCheckManager::MakeUICredential(
url_formatter::kFormatUrlOmitTrivialSubdomains |
url_formatter::kFormatUrlTrimAfterHost,
net::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
ui_credential.change_password_url = ui_credential.url.GetOrigin().spec();
ui_credential.change_password_url = CreateChangeUrl(ui_credential.url);
}
return ui_credential;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment