HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaspringModerate

Spring password validator library

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
springvalidatorlibrarypassword

Problem

I recently got rejected at a job interview for submitting this:

https://bitbucket.org/gnerr/password-validator

The interviewer asked for a password validation library that was configurable via Spring, with the following default validations:



  • Must consist of a mixture of lowercase letters and numerical digits only, with at least one of each.



  • Must be between 5 and 12 characters in length.



  • Must not contain any sequence of characters immediately followed by the same sequence.




Unfortunately they never told me why my implementation is bad, so would someone be kind enough to tell me what I did wrong here?

Here's the main validation class for the library:

```
package dm.passwordvalidator;

import java.util.ArrayList;
import java.util.List;

/**
* @author dm
*
*/
public class PasswordValidator {

private List rules;

/**
* Runs all your validation rules and returns true if it's valid, false otherwise
* @param inPassword
* @return boolean true if valid, false if otherwise
*/
public boolean validate(String inPassword) {
for (ValidationRule rule : this.getRules()) {
if (!rule.validate(inPassword)) {
return false;
}
}
return true;
}

/**
* Runs all your validation rules and returns a {@link ValidationResult} with messages if
* the password was invalid
* @param inPassword
* @return {@link ValidationResult} The validation result
*/
public ValidationResult validateWithMessages(String inPassword) {
ValidationResult returnValue = new ValidationResult();
List messages = new ArrayList();
for (ValidationRule rule : this.getRules()) {
if (!rule.validate(inPassword)) {
messages.add(rule.getMessage());
}
}
if (messages.size() > 0) {
returnValue.setValid(false);
}
returnValue.setMessages(messages);
return returnValue;

Solution

Putting this as an anwer, because it took a while to find it:

I took your code, and put it through some tests..... I made the tests up based on the 'rules'. This is the sort of thing any interviewer will do:

public static void main(String[] args) {
    String[] positive = {"12345abc", "1a2b3c4d", "1234567890ab", "1234a"};
    String[] negative = {"", "1", "1a", "123567890", "abcdefghij", "abcdEfgh1", "abcabc123", "123a", "1234567890abc"};

    PasswordValidator validator = new PasswordValidator();

    for (String pos : positive) {
        ValidationResult result = validator.validateWithMessages(pos);
        if (!result.isValid()) {
            System.out.println("Claims that '" + pos + "' is not valid!: " + result.getMessages());
        }
    }

    for (String neg : negative) {
        if (validator.validate(neg)) {
            System.out.println("Claims that '" + neg + "' is valid (but it is not)!");
        }
    }
}


When I ran that test, it passed the password: "1234567890abc" which is 13 characters long, and the rules say the limit is 12.

Checking your Regex, I see you mis-typed the range-limit on the match: ".{5,15}"

Hate to be the bearer of bad news, but an interviewer could look at this as being an indication that you are not 'detail-oriented', and dismiss the application immediately.

For what it's worth, I look at your code and feel it has a strong structure, a reusable and extensible model. Generally good. Perhaps overkill, but that's OK.

But, bottom line, if your code does not meet the specification, you can expect to be moved down the pile of applicants ......

P.S. It also blew up on a null password, which I removed from my test because I did not think it was 'fair', but regardless, you should handle null input gracefully.

Code Snippets

public static void main(String[] args) {
    String[] positive = {"12345abc", "1a2b3c4d", "1234567890ab", "1234a"};
    String[] negative = {"", "1", "1a", "123567890", "abcdefghij", "abcdEfgh1", "abcabc123", "123a", "1234567890abc"};


    PasswordValidator validator = new PasswordValidator();

    for (String pos : positive) {
        ValidationResult result = validator.validateWithMessages(pos);
        if (!result.isValid()) {
            System.out.println("Claims that '" + pos + "' is not valid!: " + result.getMessages());
        }
    }

    for (String neg : negative) {
        if (validator.validate(neg)) {
            System.out.println("Claims that '" + neg + "' is valid (but it is not)!");
        }
    }
}

Context

StackExchange Code Review Q#41082, answer score: 11

Revisions (0)

No revisions yet.