Password Validation service

Question

I have a requirement to write a password validation service, which is to accept certain rules: I wrote code:

@Service
public class PasswordValidatonServiceImpl implements PasswordValidationService {

    public static final String EMPTY_OR_NULL_PASSWORD = "Password Should not be empty";
    public static final String ERROR_PASSWORD_LENGTH = "Password must be betwee 5 and 12 characters long.";
    public static final String ERROR_PASSWORD_CASE = "Password must only contain lowercase letters.";
    public static final String ERROR_LETTER_AND_DIGIT = "Password must contain both a letter and a digit.";
    public static final String ERROR_PASSWORD_SEQUENCE_REPEATED = "Password must not contain any sequence of characters immediately followed by the same sequence.";


    private Pattern checkCasePattern = Pattern.compile("[A-Z]");
    private Pattern checkLetterAndDigit = Pattern
            .compile("(?=.*[a-z])(?=.*[0-9])");
    private Pattern checkSequenceRepetition = Pattern.compile("(\\w{2,})\\1");

    /**
     * @param password
     * @return List<String> This method calls 4 more methods which validates
     *         password and return list of errors if any.
     */
    public List<String> validatePassword(String password) {
        List<String> failures = new ArrayList<String>();
        if (StringUtils.isEmpty(password)) {
            failures.add(EMPTY_OR_NULL_PASSWORD);
            return failures;
        } else {
            checkLength(password, failures);
            checkCase(password, failures);
            checkLetterAndDigit(password, failures);
            checkSequenceRepetition(password, failures);
            return failures;
        }
    }

    /**
     * @param password
     * @param failures
     *            This method will validate if there are any repeated character
     *            sequence, if found it will add error message to failures list.
     */
    private void checkSequenceRepetition(String password, List<String> failures) {
        Matcher matcher = checkSequenceRepetition.matcher(password);
        if (matcher.find()) {
            failures.add(ERROR_PASSWORD_SEQUENCE_REPEATED);
        }
    }

    /**
     * @param password
     * @param failures
     *            This method will validate both letters and characters in
     *            password, if not found add a error message to the failures
     *            list.
     */
    private void checkLetterAndDigit(String password, List<String> failures) {
        Matcher matcher = checkLetterAndDigit.matcher(password);
        if (!matcher.find()) {
            failures.add(ERROR_LETTER_AND_DIGIT);
        }
    }

    /**
     * @param password
     * @param failures
     *            This Method checks upper case and lower case letters in the
     *            password if there are any Upper case letters it will add error
     *            message to failures list.
     */
    private void checkCase(String password, List<String> failures) {
        Matcher matcher = checkCasePattern.matcher(password);
        if (matcher.find()) {
            failures.add(ERROR_PASSWORD_CASE);
        }
    }

    /**
     * @param string
     * @param failures
     *            This Method will checks the length of the string, if string is
     *            less than 5 or more than 12 characters then it will add error
     *            message into failures list
     */
    private void checkLength(String string, List<String> failures) {
        if (string.length() < 5 || string.length() > 12) {
            failures.add(ERROR_PASSWORD_LENGTH);
        }
    }
}

Now my requirement is to make this class is extensible, so, in future, if I want to add more rules/take out some rules, code changes should be minimal. How can I achieve this? any suggestions are appreciated.

Answer 1

You might define the PasswordValidationService as some sort of list or set of a new, abstract, class PasswordRule.

This way, the PasswordValidationService would return "password is valid" if and only if every PasswordRule is satisfied.

If you were to add new rules, you'll simply have to define them as new PasswordRules and add them into your PasswordValidationService instance.

EDIT: added sample code

The abstract class every new rule would need to implement:

public abstract class PasswordRule{
    private String errorString;

    abstract public boolean check(String password){
        //implement the rule
    }

    public String getError(){
        return errorString;
    }
}

The class which extends the PasswordRule abstract class, i.e. password not being empty:

public class PasswordNotEmpty extends PasswordRule{
    private String errorString;

    public PasswordNotEmpty(){
        errorString = "Password Should not be empty";
    }

    public boolean check(String password){
        return StringUtils.isEmpty(password);
    }
}

And finally the PasswordValidationService:

public class PasswordValidator implements PasswordValidationService{
    private Set<PasswordRule> rules = new HashSet<PasswordRules>();

    public PasswordValidator(PasswordRule... args){
        for(PasswordRule r : args)
            rules.add(r);
    }

    public List<String> validate(String password){
        List<String> failures = new ArrayList<String>();
        for(PasswordRule r : rules)
            if(!r.check(password))
                failures.add(r.getError());
        return failures;
    }
}

Its use is going to be similar to this:

PasswordRule rule1 = new PasswordNotEmpty();
PasswordValidationService v = new PasswordValidator(rule1);
List<String> errors = v.validate("somePassword");

Answer 2

Firstly don't store password in String but as array of characters char[]. It's due the security reasons. Read more at f.e. here: Why is char[] preferred over String for passwords?

Secondly, the service and its method isValid(char[] password) is supposed to return boolean descirbing the validity of password itself. It would be:

public boolean isValid(char[] password) { ... }

Personally, I would create a field List or Set holding the current policy of validation (as String, Enum..). These criteria rules should be added to the instance of the service validating a password.

private Set<PasswordValidationPolicy> passwordValidationPolicy;

public void addPolicy(PasswordValidationPolicy policy) {
    this.passwordValidationPolicy.add(policy);
}

The validation itself would be driven according the items in the List or Set in the method isValid(...).

if (passwordValidationPolicy.contains(..)) { /* validate ... */}

This is just one of many possible implementations. Finally it's up to you to choose the one that fits your project and it should respect the common practices about passwords mentioned above.

Answer 3

I would suggest you to list all these methods you want to use in interface(or maybe generic interface, if you need more generic methods e.g.). And then implement that interface in your Class so you need to import them. Always override your methods and it will be really nice. Abstract class is also a great example, as Emanuele Giona wrote. Abstract methods must be overridden.