From aeb3e4473bb34e0bf4dbcb9a57c2ca83c1fa6551 Mon Sep 17 00:00:00 2001 From: Alexandru-Andrei Rotaru Date: Mon, 7 Aug 2017 12:21:31 +0100 Subject: [PATCH] Check password blacklist when setting credential If the password is valid by all other checks, see if it is present on the blacklist and disallow it if it is. Test: set a password blacklist, try and set a blacklisted password and see an explanation, set a non-blacklisted password successfully. Test: make ROBOTEST_FILTER=ChooseLockPasswordTest RunSettingsRoboTests Bug: 63578054 Fix: 65659151 Change-Id: Id155b824ad4b5839c23b6f5fd3fdfdcfc78c3df1 --- AndroidManifest.xml | 1 + res/values/strings.xml | 6 ++ .../settings/password/ChooseLockPassword.java | 45 ++++++++++----- .../password/ChooseLockPasswordTest.java | 55 +++++++++++++++---- 4 files changed, 83 insertions(+), 24 deletions(-) diff --git a/AndroidManifest.xml b/AndroidManifest.xml index 4beaf8fc5f7..68cac150a13 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -88,6 +88,7 @@ + Device admin doesn\'t allow using a recent PIN + + Common PINs are blocked by your IT admin. Try a different PIN. + This can\'t include an invalid character @@ -1388,6 +1391,9 @@ Device admin doesn\'t allow using a recent password + + Common passwords are blocked by your IT admin. Try a different password. + Ascending, descending, or repeated sequence of digits isn\'t allowed diff --git a/src/com/android/settings/password/ChooseLockPassword.java b/src/com/android/settings/password/ChooseLockPassword.java index e5fbe6c40f5..4e54f3ceb9a 100644 --- a/src/com/android/settings/password/ChooseLockPassword.java +++ b/src/com/android/settings/password/ChooseLockPassword.java @@ -228,19 +228,20 @@ public class ChooseLockPassword extends SettingsActivity { private static final int MIN_NON_LETTER_IN_PASSWORD = 5; // Error code returned from {@link #validatePassword(String)}. - private static final int NO_ERROR = 0; - private static final int CONTAIN_INVALID_CHARACTERS = 1 << 0; - private static final int TOO_SHORT = 1 << 1; - private static final int TOO_LONG = 1 << 2; - private static final int CONTAIN_NON_DIGITS = 1 << 3; - private static final int CONTAIN_SEQUENTIAL_DIGITS = 1 << 4; - private static final int RECENTLY_USED = 1 << 5; - private static final int NOT_ENOUGH_LETTER = 1 << 6; - private static final int NOT_ENOUGH_UPPER_CASE = 1 << 7; - private static final int NOT_ENOUGH_LOWER_CASE = 1 << 8; - private static final int NOT_ENOUGH_DIGITS = 1 << 9; - private static final int NOT_ENOUGH_SYMBOLS = 1 << 10; - private static final int NOT_ENOUGH_NON_LETTER = 1 << 11; + static final int NO_ERROR = 0; + static final int CONTAIN_INVALID_CHARACTERS = 1 << 0; + static final int TOO_SHORT = 1 << 1; + static final int TOO_LONG = 1 << 2; + static final int CONTAIN_NON_DIGITS = 1 << 3; + static final int CONTAIN_SEQUENTIAL_DIGITS = 1 << 4; + static final int RECENTLY_USED = 1 << 5; + static final int NOT_ENOUGH_LETTER = 1 << 6; + static final int NOT_ENOUGH_UPPER_CASE = 1 << 7; + static final int NOT_ENOUGH_LOWER_CASE = 1 << 8; + static final int NOT_ENOUGH_DIGITS = 1 << 9; + static final int NOT_ENOUGH_SYMBOLS = 1 << 10; + static final int NOT_ENOUGH_NON_LETTER = 1 << 11; + static final int BLACKLISTED = 1 << 12; /** * Keep track internally of where the user is in choosing a pattern. @@ -720,6 +721,17 @@ public class ChooseLockPassword extends SettingsActivity { break; } } + + // Only check the blacklist if the password is otherwise valid. Checking the blacklist + // can be expensive and it is not useful to report the fact it is on a blacklist if it + // couldn't be set anyway. + if (errorCode == NO_ERROR) { + if (mLockPatternUtils.getDevicePolicyManager() + .isPasswordBlacklisted(mUserId, password)) { + errorCode |= BLACKLISTED; + } + } + return errorCode; } @@ -787,7 +799,7 @@ public class ChooseLockPassword extends SettingsActivity { * @param errorCode error code returned from {@link #validatePassword(String)}. * @return an array of messages describing the error, important messages come first. */ - private String[] convertErrorCodeToMessages(int errorCode) { + String[] convertErrorCodeToMessages(int errorCode) { List messages = new ArrayList<>(); if ((errorCode & CONTAIN_INVALID_CHARACTERS) > 0) { messages.add(getString(R.string.lockpassword_illegal_character)); @@ -842,6 +854,11 @@ public class ChooseLockPassword extends SettingsActivity { messages.add(getString((mIsAlphaMode) ? R.string.lockpassword_password_recently_used : R.string.lockpassword_pin_recently_used)); } + if ((errorCode & BLACKLISTED) > 0) { + messages.add(getString((mIsAlphaMode) + ? R.string.lockpassword_password_blacklisted_by_admin + : R.string.lockpassword_pin_blacklisted_by_admin)); + } return messages.toArray(new String[0]); } diff --git a/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java b/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java index c0036037875..70e68fe25e1 100644 --- a/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java +++ b/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java @@ -127,6 +127,36 @@ public class ChooseLockPasswordTest { .isEqualTo(123); } + @Test + public void blacklist_addsErrorMessageForPin() { + final ChooseLockPassword activity = buildChooseLockPasswordActivity( + new IntentBuilder(application) + .setUserId(UserHandle.myUserId()) + // Set to numeric for a PIN + .setPasswordQuality(DevicePolicyManager.PASSWORD_QUALITY_NUMERIC) + .build()); + final ChooseLockPasswordFragment fragment = getChooseLockPasswordFragment(activity); + final int errors = ChooseLockPasswordFragment.BLACKLISTED; + final String[] messages = fragment.convertErrorCodeToMessages(errors); + assertThat(messages).isEqualTo(new String[] { + activity.getString(R.string.lockpassword_pin_blacklisted_by_admin) }); + } + + @Test + public void blacklist_addsErrorMessageForPassword() { + final ChooseLockPassword activity = buildChooseLockPasswordActivity( + new IntentBuilder(application) + .setUserId(UserHandle.myUserId()) + // Set to alphabetic for a password + .setPasswordQuality(DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC) + .build()); + final ChooseLockPasswordFragment fragment = getChooseLockPasswordFragment(activity); + final int errors = ChooseLockPasswordFragment.BLACKLISTED; + final String[] messages = fragment.convertErrorCodeToMessages(errors); + assertThat(messages).isEqualTo(new String[] { + activity.getString(R.string.lockpassword_password_blacklisted_by_admin) }); + } + @Test public void assertThat_chooseLockIconChanged_WhenFingerprintExtraSet() { ShadowDrawable drawable = setActivityAndGetIconDrawable(true); @@ -139,17 +169,22 @@ public class ChooseLockPasswordTest { assertThat(drawable.getCreatedFromResId()).isNotEqualTo(R.drawable.ic_fingerprint_header); } + private ChooseLockPassword buildChooseLockPasswordActivity(Intent intent) { + return Robolectric.buildActivity(ChooseLockPassword.class, intent).setup().get(); + } + + private ChooseLockPasswordFragment getChooseLockPasswordFragment(ChooseLockPassword activity) { + return (ChooseLockPasswordFragment) + activity.getFragmentManager().findFragmentById(R.id.main_content); + } + private ShadowDrawable setActivityAndGetIconDrawable(boolean addFingerprintExtra) { - ChooseLockPassword passwordActivity = - Robolectric.buildActivity( - ChooseLockPassword.class, - new IntentBuilder(application) - .setUserId(UserHandle.myUserId()) - .setForFingerprint(addFingerprintExtra) - .build()) - .setup().get(); - ChooseLockPasswordFragment fragment = (ChooseLockPasswordFragment) - passwordActivity.getFragmentManager().findFragmentById(R.id.main_content); + ChooseLockPassword passwordActivity = buildChooseLockPasswordActivity( + new IntentBuilder(application) + .setUserId(UserHandle.myUserId()) + .setForFingerprint(addFingerprintExtra) + .build()); + ChooseLockPasswordFragment fragment = getChooseLockPasswordFragment(passwordActivity); return Shadows.shadowOf(((GlifLayout) fragment.getView()).getIcon()); } }