From 5f139101e28ef975ef1e1a4a9193d5eb7392bc92 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Fri, 4 May 2018 17:09:28 +0100 Subject: [PATCH] Simplify password length constraints checking Currently minimum password length policy is queried twice: 1. When constructiong the intent in ChooseLockGenericFragment.getIntentForUnlockMethod and then passed into setPasswordLengthRange in getLockPasswordIntent 2. in ChooseLockPasswordFragment.processPasswordRequirements via LockPatternUtils.getRequestedMinimumPasswordLength(). These two values are then combined in processPasswordRequirements using Math.max(), which doesn't make sense since it is the same value. With this CL it is only queried once in processPasswordRequirements. + cleaned up code filling in unused list. + removed unused extras, since they are never set anywhere. Bug: 30558331 Test: atest ChooseLockPasswordTest Test: atest SetupChooseLockPasswordTest Test: atest ChooseLockGenericTest Test: manual, set password policy and change password. Change-Id: Ifc4946d5b3b26131da01178fa9c827de7a52c7c6 --- .../settings/password/ChooseLockGeneric.java | 14 +---- .../settings/password/ChooseLockPassword.java | 71 +++------------------- .../settings/password/SetupChooseLockGeneric.java | 4 +- .../settings/password/ChooseLockPasswordTest.java | 14 ----- 4 files changed, 14 insertions(+), 89 deletions(-) diff --git a/src/com/android/settings/password/ChooseLockGeneric.java b/src/com/android/settings/password/ChooseLockGeneric.java index add05e4e4a..4d0165e152 100644 --- a/src/com/android/settings/password/ChooseLockGeneric.java +++ b/src/com/android/settings/password/ChooseLockGeneric.java @@ -38,7 +38,6 @@ import android.os.Bundle; import android.os.UserHandle; import android.os.UserManager; import android.os.storage.StorageManager; -import android.security.KeyStore; import androidx.annotation.StringRes; import androidx.preference.Preference; import androidx.preference.PreferenceScreen; @@ -97,7 +96,6 @@ public class ChooseLockGeneric extends SettingsActivity { public static class ChooseLockGenericFragment extends SettingsPreferenceFragment { private static final String TAG = "ChooseLockGenericFragment"; - private static final int MIN_PASSWORD_LENGTH = 4; private static final String KEY_SKIP_FINGERPRINT = "unlock_skip_fingerprint"; private static final String PASSWORD_CONFIRMED = "password_confirmed"; private static final String WAITING_FOR_CONFIRMATION = "waiting_for_confirmation"; @@ -136,7 +134,6 @@ public class ChooseLockGeneric extends SettingsActivity { private ChooseLockSettingsHelper mChooseLockSettingsHelper; private DevicePolicyManager mDPM; - private KeyStore mKeyStore; private boolean mHasChallenge = false; private long mChallenge; private boolean mPasswordConfirmed = false; @@ -168,7 +165,6 @@ public class ChooseLockGeneric extends SettingsActivity { String chooseLockAction = getActivity().getIntent().getAction(); mFingerprintManager = Utils.getFingerprintManagerOrNull(getActivity()); mDPM = (DevicePolicyManager) getSystemService(Context.DEVICE_POLICY_SERVICE); - mKeyStore = KeyStore.getInstance(); mChooseLockSettingsHelper = new ChooseLockSettingsHelper(this.getActivity()); mLockPatternUtils = new LockPatternUtils(getActivity()); mIsSetNewPassword = ACTION_SET_NEW_PARENT_PROFILE_PASSWORD.equals(chooseLockAction) @@ -585,11 +581,10 @@ public class ChooseLockGeneric extends SettingsActivity { return mManagedPasswordProvider.createIntent(false, password); } - protected Intent getLockPasswordIntent(int quality, int minLength, int maxLength) { + protected Intent getLockPasswordIntent(int quality) { ChooseLockPassword.IntentBuilder builder = new ChooseLockPassword.IntentBuilder(getContext()) .setPasswordQuality(quality) - .setPasswordLengthRange(minLength, maxLength) .setForFingerprint(mForFingerprint) .setUserId(mUserId); if (mHasChallenge) { @@ -668,12 +663,7 @@ public class ChooseLockGeneric extends SettingsActivity { if (quality >= DevicePolicyManager.PASSWORD_QUALITY_MANAGED) { intent = getLockManagedPasswordIntent(mUserPassword); } else if (quality >= DevicePolicyManager.PASSWORD_QUALITY_NUMERIC) { - int minLength = mDPM.getPasswordMinimumLength(null, mUserId); - if (minLength < MIN_PASSWORD_LENGTH) { - minLength = MIN_PASSWORD_LENGTH; - } - final int maxLength = mDPM.getPasswordMaximumLength(quality); - intent = getLockPasswordIntent(quality, minLength, maxLength); + intent = getLockPasswordIntent(quality); } else if (quality == DevicePolicyManager.PASSWORD_QUALITY_SOMETHING) { intent = getLockPatternIntent(); } diff --git a/src/com/android/settings/password/ChooseLockPassword.java b/src/com/android/settings/password/ChooseLockPassword.java index 23c50226be..1013c44700 100644 --- a/src/com/android/settings/password/ChooseLockPassword.java +++ b/src/com/android/settings/password/ChooseLockPassword.java @@ -73,15 +73,6 @@ import java.util.ArrayList; import java.util.List; public class ChooseLockPassword extends SettingsActivity { - public static final String PASSWORD_MIN_KEY = "lockscreen.password_min"; - public static final String PASSWORD_MAX_KEY = "lockscreen.password_max"; - public static final String PASSWORD_MIN_LETTERS_KEY = "lockscreen.password_min_letters"; - public static final String PASSWORD_MIN_LOWERCASE_KEY = "lockscreen.password_min_lowercase"; - public static final String PASSWORD_MIN_UPPERCASE_KEY = "lockscreen.password_min_uppercase"; - public static final String PASSWORD_MIN_NUMERIC_KEY = "lockscreen.password_min_numeric"; - public static final String PASSWORD_MIN_SYMBOLS_KEY = "lockscreen.password_min_symbols"; - public static final String PASSWORD_MIN_NONLETTER_KEY = "lockscreen.password_min_nonletter"; - private static final String TAG = "ChooseLockPassword"; @Override @@ -113,12 +104,6 @@ public class ChooseLockPassword extends SettingsActivity { return this; } - public IntentBuilder setPasswordLengthRange(int min, int max) { - mIntent.putExtra(PASSWORD_MIN_KEY, min); - mIntent.putExtra(PASSWORD_MAX_KEY, max); - return this; - } - public IntentBuilder setUserId(int userId) { mIntent.putExtra(Intent.EXTRA_USER_ID, userId); return this; @@ -454,54 +439,32 @@ public class ChooseLockPassword extends SettingsActivity { } private void setupPasswordRequirementsView(View view) { - // Construct passwordRequirements and requirementDescriptions. - List passwordRequirements = new ArrayList<>(); - List requirementDescriptions = new ArrayList<>(); + final List passwordRequirements = new ArrayList<>(); if (mPasswordMinUpperCase > 0) { passwordRequirements.add(MIN_UPPER_LETTERS_IN_PASSWORD); - requirementDescriptions.add(getResources().getQuantityString( - R.plurals.lockpassword_password_requires_uppercase, mPasswordMinUpperCase, - mPasswordMinUpperCase)); } if (mPasswordMinLowerCase > 0) { passwordRequirements.add(MIN_LOWER_LETTERS_IN_PASSWORD); - requirementDescriptions.add(getResources().getQuantityString( - R.plurals.lockpassword_password_requires_lowercase, mPasswordMinLowerCase, - mPasswordMinLowerCase)); } if (mPasswordMinLetters > 0) { if (mPasswordMinLetters > mPasswordMinUpperCase + mPasswordMinLowerCase) { passwordRequirements.add(MIN_LETTER_IN_PASSWORD); - requirementDescriptions.add(getResources().getQuantityString( - R.plurals.lockpassword_password_requires_letters, mPasswordMinLetters, - mPasswordMinLetters)); } } if (mPasswordMinNumeric > 0) { passwordRequirements.add(MIN_NUMBER_IN_PASSWORD); - requirementDescriptions.add(getResources().getQuantityString( - R.plurals.lockpassword_password_requires_numeric, mPasswordMinNumeric, - mPasswordMinNumeric)); } if (mPasswordMinSymbols > 0) { passwordRequirements.add(MIN_SYMBOLS_IN_PASSWORD); - requirementDescriptions.add(getResources().getQuantityString( - R.plurals.lockpassword_password_requires_symbols, mPasswordMinSymbols, - mPasswordMinSymbols)); } if (mPasswordMinNonLetter > 0) { if (mPasswordMinNonLetter > mPasswordMinNumeric + mPasswordMinSymbols) { passwordRequirements.add(MIN_NON_LETTER_IN_PASSWORD); - requirementDescriptions.add(getResources().getQuantityString( - R.plurals.lockpassword_password_requires_nonletter, mPasswordMinNonLetter, - - mPasswordMinNonLetter)); } } // Convert list to array. mPasswordRequirements = passwordRequirements.stream().mapToInt(i -> i).toArray(); - mPasswordRestrictionView = - (RecyclerView) view.findViewById(R.id.password_requirements_view); + mPasswordRestrictionView = view.findViewById(R.id.password_requirements_view); mPasswordRestrictionView.setLayoutManager(new LinearLayoutManager(getActivity())); mPasswordRequirementAdapter = new PasswordRequirementAdapter(); mPasswordRestrictionView.setAdapter(mPasswordRequirementAdapter); @@ -582,29 +545,15 @@ public class ChooseLockPassword extends SettingsActivity { final int dpmPasswordQuality = mLockPatternUtils.getRequestedPasswordQuality(mUserId); mRequestedQuality = Math.max(intent.getIntExtra(LockPatternUtils.PASSWORD_TYPE_KEY, mRequestedQuality), dpmPasswordQuality); - mPasswordMinLength = Math.max(Math.max( - LockPatternUtils.MIN_LOCK_PASSWORD_SIZE, - intent.getIntExtra(PASSWORD_MIN_KEY, mPasswordMinLength)), + mPasswordMinLength = Math.max(LockPatternUtils.MIN_LOCK_PASSWORD_SIZE, mLockPatternUtils.getRequestedMinimumPasswordLength(mUserId)); - mPasswordMaxLength = intent.getIntExtra(PASSWORD_MAX_KEY, mPasswordMaxLength); - mPasswordMinLetters = Math.max(intent.getIntExtra(PASSWORD_MIN_LETTERS_KEY, - mPasswordMinLetters), mLockPatternUtils.getRequestedPasswordMinimumLetters( - mUserId)); - mPasswordMinUpperCase = Math.max(intent.getIntExtra(PASSWORD_MIN_UPPERCASE_KEY, - mPasswordMinUpperCase), mLockPatternUtils.getRequestedPasswordMinimumUpperCase( - mUserId)); - mPasswordMinLowerCase = Math.max(intent.getIntExtra(PASSWORD_MIN_LOWERCASE_KEY, - mPasswordMinLowerCase), mLockPatternUtils.getRequestedPasswordMinimumLowerCase( - mUserId)); - mPasswordMinNumeric = Math.max(intent.getIntExtra(PASSWORD_MIN_NUMERIC_KEY, - mPasswordMinNumeric), mLockPatternUtils.getRequestedPasswordMinimumNumeric( - mUserId)); - mPasswordMinSymbols = Math.max(intent.getIntExtra(PASSWORD_MIN_SYMBOLS_KEY, - mPasswordMinSymbols), mLockPatternUtils.getRequestedPasswordMinimumSymbols( - mUserId)); - mPasswordMinNonLetter = Math.max(intent.getIntExtra(PASSWORD_MIN_NONLETTER_KEY, - mPasswordMinNonLetter), mLockPatternUtils.getRequestedPasswordMinimumNonLetter( - mUserId)); + mPasswordMaxLength = mLockPatternUtils.getMaximumPasswordLength(mRequestedQuality); + mPasswordMinLetters = mLockPatternUtils.getRequestedPasswordMinimumLetters(mUserId); + mPasswordMinUpperCase = mLockPatternUtils.getRequestedPasswordMinimumUpperCase(mUserId); + mPasswordMinLowerCase = mLockPatternUtils.getRequestedPasswordMinimumLowerCase(mUserId); + mPasswordMinNumeric = mLockPatternUtils.getRequestedPasswordMinimumNumeric(mUserId); + mPasswordMinSymbols = mLockPatternUtils.getRequestedPasswordMinimumSymbols(mUserId); + mPasswordMinNonLetter = mLockPatternUtils.getRequestedPasswordMinimumNonLetter(mUserId); // Modify the value based on dpm policy. switch (dpmPasswordQuality) { diff --git a/src/com/android/settings/password/SetupChooseLockGeneric.java b/src/com/android/settings/password/SetupChooseLockGeneric.java index a72047308c..3edb08301b 100644 --- a/src/com/android/settings/password/SetupChooseLockGeneric.java +++ b/src/com/android/settings/password/SetupChooseLockGeneric.java @@ -170,9 +170,9 @@ public class SetupChooseLockGeneric extends ChooseLockGeneric { } @Override - protected Intent getLockPasswordIntent(int quality, int minLength, int maxLength) { + protected Intent getLockPasswordIntent(int quality) { final Intent intent = SetupChooseLockPassword.modifyIntentForSetup( - getContext(), super.getLockPasswordIntent(quality, minLength, maxLength)); + getContext(), super.getLockPasswordIntent(quality)); SetupWizardUtils.copySetupExtras(getActivity().getIntent(), intent); return intent; } diff --git a/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java b/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java index 75b6bb4b14..7ee9ea51c4 100644 --- a/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java +++ b/tests/robotests/src/com/android/settings/password/ChooseLockPasswordTest.java @@ -67,7 +67,6 @@ public class ChooseLockPasswordTest { Intent intent = new IntentBuilder(application) .setPassword("password") .setPasswordQuality(DevicePolicyManager.PASSWORD_QUALITY_NUMERIC) - .setPasswordLengthRange(123, 456) .setUserId(123) .build(); @@ -77,12 +76,6 @@ public class ChooseLockPasswordTest { assertThat(intent.getStringExtra(ChooseLockSettingsHelper.EXTRA_KEY_PASSWORD)) .named("EXTRA_KEY_PASSWORD") .isEqualTo("password"); - assertThat(intent.getIntExtra(ChooseLockPassword.PASSWORD_MIN_KEY, 0)) - .named("PASSWORD_MIN_KEY") - .isEqualTo(123); - assertThat(intent.getIntExtra(ChooseLockPassword.PASSWORD_MAX_KEY, 0)) - .named("PASSWORD_MAX_KEY") - .isEqualTo(456); assertThat(intent.getIntExtra(LockPatternUtils.PASSWORD_TYPE_KEY, 0)) .named("PASSWORD_TYPE_KEY") .isEqualTo(DevicePolicyManager.PASSWORD_QUALITY_NUMERIC); @@ -96,7 +89,6 @@ public class ChooseLockPasswordTest { Intent intent = new IntentBuilder(application) .setChallenge(12345L) .setPasswordQuality(DevicePolicyManager.PASSWORD_QUALITY_ALPHANUMERIC) - .setPasswordLengthRange(123, 456) .setUserId(123) .build(); @@ -106,12 +98,6 @@ public class ChooseLockPasswordTest { assertThat(intent.getLongExtra(ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE, 0L)) .named("EXTRA_KEY_CHALLENGE") .isEqualTo(12345L); - assertThat(intent.getIntExtra(ChooseLockPassword.PASSWORD_MIN_KEY, 0)) - .named("PASSWORD_MIN_KEY") - .isEqualTo(123); - assertThat(intent.getIntExtra(ChooseLockPassword.PASSWORD_MAX_KEY, 0)) - .named("PASSWORD_MAX_KEY") - .isEqualTo(456); assertThat(intent.getIntExtra(LockPatternUtils.PASSWORD_TYPE_KEY, 0)) .named("PASSWORD_TYPE_KEY") .isEqualTo(DevicePolicyManager.PASSWORD_QUALITY_ALPHANUMERIC); -- 2.11.0