Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e38292a2 authored by Eran Messeri's avatar Eran Messeri
Browse files

DPMS: Fix password complexity comparison

The method for validating whether a given password is sufficient for a
given complexity was returning the wrong value in the general case.
validatePasswordMetrics should return true when the actualMetrics of
a password satisfy the minComplexity passed in.

However, the isPin argument, skews the result such that an all-digit
PIN is considered not sufficient, because the only credential type
recognized in that method is the "password" type, and for that
credential type a non-digit character is required.

This worked for checking whether the password is sufficient in the
UI code, where isPin indicated whether the user chose a PIN or password.
However it does not work for the general case, for example for enforcing
complexity.

This change gets rid of the 'allowsNumericPassword()' criteria for
complexity buckets. Instead, the minimal length is calculated based on
whether the actualMetrics already have a non-numeric character, OR
a non-numeric character is required by the admin-set password quality
requirements (in the case when merging complexity and quality).

Bug: 172312413
Bug: 165573442
Test: atest FrameworksServicesTests:DevicePolicyManagerTest
Test: atest MixedDeviceOwnerTest MixedProfileOwnerTest OrgOwnedProfileOwnerTest  MixedDeviceOwnerTestApi25 MixedManagedProfileOwnerTest
Change-Id: I3227d4d8e6825b5c4ea525828d7e09f52702065b
parent a8a31234
Loading
Loading
Loading
Loading
+16 −30
Original line number Diff line number Diff line
@@ -406,11 +406,6 @@ public final class PasswordMetrics implements Parcelable {
                return containsNonNumeric ? 6 : 8;
            }

            @Override
            boolean allowsNumericPassword() {
                return false;
            }

            @Override
            boolean allowsCredType(int credType) {
                return credType == CREDENTIAL_TYPE_PASSWORD;
@@ -427,11 +422,6 @@ public final class PasswordMetrics implements Parcelable {
                return 4;
            }

            @Override
            boolean allowsNumericPassword() {
                return false;
            }

            @Override
            boolean allowsCredType(int credType) {
                return credType == CREDENTIAL_TYPE_PASSWORD;
@@ -448,11 +438,6 @@ public final class PasswordMetrics implements Parcelable {
                return 0;
            }

            @Override
            boolean allowsNumericPassword() {
                return true;
            }

            @Override
            boolean allowsCredType(int credType) {
                return credType != CREDENTIAL_TYPE_NONE;
@@ -469,11 +454,6 @@ public final class PasswordMetrics implements Parcelable {
                return 0;
            }

            @Override
            boolean allowsNumericPassword() {
                return true;
            }

            @Override
            boolean allowsCredType(int credType) {
                return true;
@@ -484,7 +464,6 @@ public final class PasswordMetrics implements Parcelable {

        abstract boolean canHaveSequence();
        abstract int getMinimumLength(boolean containsNonNumeric);
        abstract boolean allowsNumericPassword();
        abstract boolean allowsCredType(int credType);

        ComplexityBucket(int complexityLevel) {
@@ -591,7 +570,14 @@ public final class PasswordMetrics implements Parcelable {
            result.add(new PasswordValidationError(TOO_LONG, MAX_PASSWORD_LENGTH));
        }

        final PasswordMetrics minMetrics = applyComplexity(adminMetrics, isPin, bucket);
        // A flag indicating whether the provided password already has non-numeric characters in
        // it or if the admin imposes the requirement of any non-numeric characters.
        final boolean hasOrWouldNeedNonNumeric =
                actualMetrics.nonNumeric > 0 || adminMetrics.nonNumeric > 0
                        || adminMetrics.letters > 0 || adminMetrics.lowerCase > 0
                        || adminMetrics.upperCase > 0 || adminMetrics.symbols > 0;
        final PasswordMetrics minMetrics =
                applyComplexity(adminMetrics, hasOrWouldNeedNonNumeric, bucket);

        // Clamp required length between maximum and minimum valid values.
        minMetrics.length = Math.min(MAX_PASSWORD_LENGTH,
@@ -684,23 +670,23 @@ public final class PasswordMetrics implements Parcelable {
     * TODO: move to PasswordPolicy
     */
    public static PasswordMetrics applyComplexity(
            PasswordMetrics adminMetrics, boolean isPin, int complexity) {
        return applyComplexity(adminMetrics, isPin, ComplexityBucket.forComplexity(complexity));
            PasswordMetrics adminMetrics, boolean withNonNumericCharacters,
            int complexity) {
        return applyComplexity(adminMetrics, withNonNumericCharacters,
                ComplexityBucket.forComplexity(complexity));
    }

    private static PasswordMetrics applyComplexity(
            PasswordMetrics adminMetrics, boolean isPin, ComplexityBucket bucket) {
            PasswordMetrics adminMetrics, boolean withNonNumericCharacters,
            ComplexityBucket bucket) {
        final PasswordMetrics minMetrics = new PasswordMetrics(adminMetrics);

        if (!bucket.canHaveSequence()) {
            minMetrics.seqLength = Math.min(minMetrics.seqLength, MAX_ALLOWED_SEQUENCE);
        }

        minMetrics.length = Math.max(minMetrics.length, bucket.getMinimumLength(!isPin));

        if (!isPin && !bucket.allowsNumericPassword()) {
            minMetrics.nonNumeric = Math.max(minMetrics.nonNumeric, 1);
        }
        minMetrics.length = Math.max(minMetrics.length,
                bucket.getMinimumLength(withNonNumericCharacters));

        return minMetrics;
    }