Simple Java password rule enforcing

Question

• rule 1: length must be between 8 and 50 chars.
• rule 2: at least 3 of the 4 following rules:
• --------2a: at least one upper case
• --------2b: at least one lower case
• --------2c: at least one numeral
• --------2d: at least one non-alphanumeric.

I have a function which will take in two Strings (the password and the confirmation) and then return a boolean depending on if it verified. It also sets a label on my JavaFX screen to the error or confirmation message. It works fine, but the code seems cumbersome. any thoughts?

public boolean validatePassword(String newPass, String confirmPass)
{
    if (newPass == null || 
        confirmPass == null || 
        newPass.length() == 0 || 
        confirmPass.length() == 0 )
    {
        responseLabel.setText("");
        return false;
    }

    boolean equalsRule = newPass.equals(confirmPass);
    boolean lengthRule = newPass.length() >= 8 && newPass.length() <= 50;

    boolean upperRule = !newPass.equals(newPass.toLowerCase());
    boolean lowerRule = !newPass.equals(newPass.toUpperCase());
    boolean numeralRule = newPass.matches("(.*)[0-9](.*)");
    boolean nonAlphaRule = newPass.matches("(.*)[^A-Za-z0-9](.*)");

    int ruleCount = (upperRule ? 1 : 0) + ( lowerRule ? 1 : 0) + ( numeralRule ? 1 : 0) + (nonAlphaRule ? 1 : 0);

    if (ruleCount >= 3 && equalsRule && lengthRule)
    {
        responseLabel.setText("Password set successfully.");
        responseLabel.setTextFill(Color.GREEN);
        return true;
    }
    else
    {
        if (!equalsRule)
        {
            responseLabel.setText("Passwords must match.");
        }
        else if (ruleCount < 3 || !lengthRule)
        {
            responseLabel.setText("Password does not follow instructions above.");
        }
        responseLabel.setTextFill(Color.RED);
        return false;
    }
}

Answer 1

I see at least one big problem in your method: there is no separation of concerns. Everything is mixed into this method: common rule for password double-checking, business rules for password validation and UI calls for user response.

Extracting the business logic

The business logic in your case if the password validation specific rules, as you named them rule1, rule2a, rule2b, rule2c and rule2d. Everytime you will need a password validation, you will need these rules to validate a password, therefore I would extract these rules into a BO.

import java.util.function.Predicate;
import java.util.stream.Stream;

public final class Password {
    private final String value;
    private final Predicate<String> rule;

    public Password(String value) {
        this.value = value;
        Predicate<String> rule1 = s -> s.length() >= 8 && s.length() <= 50;
        Predicate<String> rule2a = s -> !s.equals(s.toLowerCase());
        Predicate<String> rule2b = s -> !s.equals(s.toUpperCase());
        Predicate<String> rule2c = s -> s.codePoints().anyMatch(Character::isDigit);
        Predicate<String> rule2d = s -> s.codePoints().anyMatch(i -> !Character.isAlphabetic(i));
        Predicate<String> rule2 = s -> Stream.of(rule2a, rule2b, rule2c, rule2d)
                                             .filter(p -> p.test(s))
                                             .count() >= 3;
        this.rule = rule1.and(rule2);
    }

    public boolean isValid() {
        return rule.test(value);
    }
}

If the rules are likely to change, it would also be possible to refactor this code and take the "final predicate" as a constructor argument.

Extracting "standard" logic

In your case, it is only the rule that assert the password and the confirmation password are equals. This is not a business-specific rule but rather a commonly accepted rule everytime a user creates a password. Moreover it seems you have a "validation flow" defined (first check the equality between the two password, then check if the password in itself is valid) that can be extracted into a separate class

public final class PasswordInteractor {
    public ValidationResult validatePassword(String pwd, String confirmPwd) {
        if (!pwd.equals(confirmPwd)) {
            return new ValidationResult("Passwords must match.", false);
        }

        if (!new Password(pwd).isValid()) {
            return new ValidationResult("Password does not follow instructions above.", false);
        }

        return new ValidationResult("Password set successfully", true);
    }
}

I created a DTO ValidationResult to transmit to the upper layer a composite result since it can be more than true/false.

public final class ValidationResult {
    private final String message;
    private final boolean success;

    public ValidationResult(String message, boolean success) {
        this.message = message;
        this.success = success;
    }

    public String getMessage() {
        return message;
    }

    public boolean isSuccess() {
        return success;
    }
}

Extracting UI calls

Finally the UI calls must stay in the UI-part, namely into the JavaFX controller. Typically it could looks like this (simplified):

import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.paint.Color;

public final class PasswordController {
    @FXML
    private Label responseLabel;

    @FXML
    private TextField pwd;

    @FXML
    private TextField confirmPwd;

    public void validateForm() {
        // validate other parts of the form

        ValidationResult result = new PasswordInteractor().validatePassword(pwd.getText(), confirmPwd.getText());

        if (result.isSuccess()) {
            responseLabel.setTextFill(Color.GREEN);
        } else {
            responseLabel.setTextFill(Color.RED);
        }
        responseLabel.setText(result.getMessage());
    }
}

Answer 2

Start writing some utility classes, you're likely going to be checking if strings are nonempty in other places:

public final class StringUtils {
    public static boolean isNullOrEmpty(String string) {
        return string == null || string.length() == 0;
    }

    public static boolean isNullOrWhitespace(String string) {
        if (string == null) {
            return true;
        }

        for (int i = 0; i < string.length(); i++) {
            if (!Character.isWhitespace(string.charAt(i))) {
                return false;
            }
        }

        return true;
    }
}

You can try to generalize your password rules with an interface. A very simple approach could be just method to determine if the rule is satisfied:

public interface PasswordRule {
    boolean isSatisfied(String password);
}

And then you can implement some of your rules:

public class HasDigitRule implements PasswordRule {
    @Override
    public boolean isSatisfied(String password) {
        if(password == null) throw new IllegalArgumentException("password cannot be null.");

        for(int i = 0; i < password.length(); i++) {
            if(Character.isDigit(password.charAt(i))) {
                return true;
            }
        }

        return false;
    }
}

public class HasLowerCaseRule implements PasswordRule {
    @Override
    public boolean isSatisfied(String password) {
        if(password == null) throw new IllegalArgumentException("password cannot be null.");

        for(int i = 0; i < password.length(); i++) {
            if(Character.isLowerCase(password.charAt(i))) {
                return true;
            }
        }

        return false;
    }
}

public class HasNonAlphanumericRule implements PasswordRule {
    @Override
    public boolean isSatisfied(String password) {
        if(password == null) throw new IllegalArgumentException("password cannot be null.");

        for(int i = 0; i < password.length(); i++) {
            if(!Character.isLetterOrDigit(password.charAt(i))) {
                return true;
            }
        }

        return false;
    }
}

public class HasUpperCaseRule implements PasswordRule {
    @Override
    public boolean isSatisfied(String password) {
        if(password == null) throw new IllegalArgumentException("password cannot be null.");

        for(int i = 0; i < password.length(); i++) {
            if(Character.isUpperCase(password.charAt(i))) {
                return true;
            }
        }

        return false;
    }
}

public class LengthRule implements PasswordRule {
    @Override
    public boolean isSatisfied(String password) {
        if(password == null) throw new IllegalArgumentException("password cannot be null.");
        return password.length() >= 8 && password.length() <= 50;
    }
}

Now you can restructure you validation method to check all the critical cases first and exit early. Then count the amount of requirements met:

public boolean validatePassword(String newPassword, String confirmPassword) {
    if (StringUtils.isNullOrEmpty(newPassword)
            || StringUtils.isNullOrEmpty(confirmPassword)) {
        responseLabel.setText("");
        return false;
    }

    if (!newPassword.equals(confirmPassword)) {
        responseLabel.setText("Passwords must match.");
        responseLabel.setTextFill(Color.RED);
        return false;
    }

    PasswordRule lengthRule = new LengthRule();
    if (!lengthRule.isSatisfied(newPassword)) {
        responseLabel.setText("Passwords must be between 8 and 50 characters long.");
        responseLabel.setTextFill(Color.RED);
        return false;
    }

    PasswordRule[] additionalRules = {
            new HasDigitRule(),
            new HasLowerCaseRule(),
            new HasNonAlphanumericRule(),
            new HasUpperCaseRule(),
    };

    long satisfiedRulesCount =
            Stream.of(additionalRules)
                    .filter(r -> r.isSatisfied(newPassword))
                    .count();

    if (satisfiedRulesCount < 3) {
        responseLabel.setText("Password does not follow instructions above.");
        responseLabel.setTextFill(Color.RED);
        return false;
    }

    responseLabel.setText("Password set successfully.");
    responseLabel.setTextFill(Color.GREEN);
    return true;
}

Going forward you should consider extracting the error messages, maybe to the PasswordRule interface. Then you could perhaps return a list of unsatisfied conditions instead of a boolean.