Provide more feedback

[gosimplellc]

Provide more feedback

[Tostino]

@jdhoek I haven't forgotten about this, my day job just got really crazy for the past couple weeks, and now i've got a couple weeks of vacation planned where i'm not going to be near a computer. I'll dedicate a little time to knock this out once back.

[Tostino]

@jdhoek Was wondering on this, should we just leave it at a single warning for the longest match (as it is currently), but add additional suggestions based on the other match types the user hits?

[jdhoek]

With a password candidate like apples1980 I can imagine wanting to present the feedback to the user something like this:

---

This password is too weak ← Often this will be represented by some form of bar. I intend to base this on the entropy number returned in the feedback.

⚠️ 'apples' is a very common password
⚠️ Recent years are often easy to guess, especially if they are relevant to you
ℹ️ Add a few more words for a stronger password
ℹ️ Instead of a password, create a passphrase — it's okay to include spaces and punctuation as well

(Well, with friendlier formatting obviously, but I'll just use these GitHub icons for the example.)

---

So basically relevant warnings and relevant suggestions; although the latter tend to boil down to 'add more words'. By providing all warnings at once, the user gets positive feedback as they improve their password (the warnings disappear one by one). So you would get a list of warnings in the feedback if multiple matches are found (except for BruteForceMatch I guess?).

If only one warning is returned, the user might fix one issue (replacing 'apples' with 'snozberries') and frustratingly have the next one pop up (don't use years).

Having the 'secondary' warnings as a suggestion would be a bit weird, because they would go from a suggestion to a warning once the primary warning is resolved.

I hope this helps.

[Tostino]

The issue I see with returning all feedback all the time, is in passwords where a few patterns are hit, you'll hit a point where you are just returning a wall of text. Also, there is nothing inherently wrong about using any of the match types within a password, other than if the password as a whole is not deemed to have enough entropy. That is why after getting it to that point (the minimum entropy that is), the feedback doesn't keep returning more suggestions or warnings.

I'm thinking keeping it to the longest portion of the password for the warning, and adding suggestions for the other reasons is a good compromise. It also cuts down on API changes, and how much code has to be touched. It also does give the user feedback that we know about those other portions of their password, and gives them suggestions on how to improve it.

Thoughts?

[jdhoek]

The issue I see with returning all feedback all the time, is in passwords where a few patterns are hit, you'll hit a point where you are just returning a wall of text.

Agreed, that's not helpful.

I'm thinking keeping it to the longest portion of the password for the warning, and adding suggestions for the other reasons is a good compromise.

What does this mean for consumers of the library? The warning would be something you'd always show I suppose (provided the minimum entropy has not been met), but what about the suggestions? Are all equally relevant, or can they be sorted in decreasing order of relevancy?

If the user acts on the warning ('apples is a commons password', so: apples1980 → snozberries1980) then if I understand correctly, if the minimum entropy is not met, the 'don't use a year' suggestion turns into a warning?

It looks like the warning/suggestion division may not be as clear-cut, but perhaps that is not so bad? Instead of a single warning and unordered list of suggestions, might it be useful to return an ordered lists of weighted suggestions?

That is, all suggestions get a relevance value (perhaps a double or float between 0.0 and 1.0), and Feedback would get a getter method that returns List<Suggestion>:

public class Suggestion implements Comparable<Suggestion>
{
    String messageKey;
    String translatedMessage;
    String token; // The part of the candidate password relevant to this suggestion?
    float relevance;

    // Implement Comparable<Suggestion> to order by relevance, descending.
}

The relevance of a suggestion could be determined in part by using the length of the match (so the longest match will have a high relevance) and any other heuristics you care to put in (perhaps emphasize commons password warnings).

Consumers of the API can choose to show only the top n of the list as feedback (perhaps two or three) so the user is never overwhelmed with suggestions. Consumers can also choose to emphasize highly relevant (e.g., relevance > 0.8) suggestions (effectively treating them as warnings) by using different styling.

Any consumers who would like more details (automated password analysis maybe?) can just use the complete list of suggestions.

That would mean an API change though, but I wouldn't worry to much about doing that (just version releases accordingly).

Anyone who really wants the one warning, multiple suggestions strings can just take the 'heaviest' suggestion as warning and use the rest as is (you could even leave the current methods as @Deprecated and do just that).

The benefit of this approach is that you can easily introduce improved and/or additional suggestions in the future without breaking the API again, and if it turns out that certain suggestions are more relevant than others, you can just tweak the algorithm assigning their relevance.

Edit: added messageKey field to Suggestion class.

[jdhoek]

By all means, take your time!

[Tostino]

Once I got back from Vacation, of course work got really crazy. Going to start looking at this again.