vt-middleware / passay

Password policy enforcement for Java.
http://www.passay.org
Other
281 stars 64 forks source link

Create mechanism to easily expose validation error types #123

Closed cribbstechnologies closed 3 years ago

cribbstechnologies commented 4 years ago

First, great library. It helped me reduce a ton of code into far fewer lines.

It would be nice to have a mechanism on the RuleResult from the validate method that would allow lookup of error codes in the returned collection of error codes without iterating the collection like a Map.

Use case: UI has a display with each of the rules with a "X" next to them if they're not met and a check mark if they have been met. Calls to the server are REST/JSON.

In version 1.4.0 which I'm currently using the RuleResultDetail has only a single error code. It looks like it has been updated since to include potentially multiple error codes.

Would it be possible to change the implementation of validate to collect the results from the RuleResults into both the current list as well as a Map hashed by the error code(s)? I can do the work, I just don't know if this would cause any problems with the project's roadmap.

Potential Issues: There's no implementation of hashCode or equals in the RuleResultDetail which could possibly create a problem with overwriting values in the map due to duplicate/overlapping keys.

dfish3r commented 4 years ago

You should take a look at the v1.6 API before you start any work.

We've added several convenience methods over time to support various workflows and this doesn't sound any different. That is, this sort of thing tends to define the roadmap, not disrupt it.

cribbstechnologies commented 4 years ago

You should take a look at the v1.6 API before you start any work.

We've added several convenience methods over time to support various workflows and this doesn't sound any different. That is, this sort of thing tends to define the roadmap, not disrupt it.

I had already looked at 1.6 before I raised this issue and noticed the difference between how error codes were handled.

What I'd need defined is how the validator should treat the RuleResultDetails in the event that there is overlap in the superset of error codes. If I introduced a method that returned a Map of RuleResultDetails hashed by error code in my specific case I wouldn't care if an error code was duplicated, only that an error exists with that code. I don't know if that would be acceptable for other potential uses.

Another option would be simply to return a Set of the error codes. Either of these would solve my use case.

dfish3r commented 4 years ago

Based on your comments I'm not sure any API changes are needed here. Just use the streams API to convert the list however you like:

RuleResult result = validator.validate(new PasswordData("somepassword"));
Map<String[], RuleResultDetail> myMap =
      result.getDetails().stream().collect(Collectors.toMap(r -> r.getErrorCodes(), r -> r));

You can decide how to resolve conflicting error codes by providing a merge function.

cribbstechnologies commented 4 years ago

That's essentially what this does but it was added to the API

https://github.com/vt-middleware/passay/blob/master/src/main/java/org/passay/PasswordValidator.java#L155

The mapped objects could still benefit from optionally having their messages resolved in the same manner.

dfish3r commented 4 years ago

Feel free to submit a PR. If the API changes provide sufficient value we'll consider it for inclusion.

dfish3r commented 3 years ago

Did you decide against this feature? If so I'll close this issue. It can always be reopened.