-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Provide more feedback #29
Comments
@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? |
With a password candidate like 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.
(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 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. |
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? |
Agreed, that's not helpful.
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: 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 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 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 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 Edit: added |
@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. |
By all means, take your time! |
Once I got back from Vacation, of course work got really crazy. Going to start looking at this again. |
@jdhoek said:
The text was updated successfully, but these errors were encountered: