Custom feedback messages

[paulballesty]

I need to use the library in an internationalized website, which means that I need the feedback messages in other languages. I added the optional third parameter options, in where you can pass a feedback_messages Array-like object with custom messages.

Do you guys think it worth adding this to the library?

Example usages:

Redifining all the messages

zxcvbn('example', ['paul'], {feedback_messages: ['Utilice varias palabras, evite frases comunes', ...]});

Changing an specific message

zxcvbn('example', ['paul'], {feedback_messages: {9: 'Sequences are easy to guess'}});

[peterver]

+1

definitely an upide to this merge as 2 issues are open regarding this ( #118 and #126 ), can someone take a look at why the travis ci builds are failing ? :]

[9and3r]

+1

This would be great to use it with other languges

[thedotedge]

👍

[wje97951]

+1

[paulballesty]

@lowe any interest on this? I am getting emails from people asking me how to use it. Based on the +1s in this PR, it seems users need a way to modify the feedback messages.

Happy to change the code if you don't like the approach.

[konradaadamczyk]

+1

[ck-ws]

+1

[lowe]

Hi @paulballesty,

Thanks for the PR. I'm happy to merge but have three requests to work through first:

• Can you add short documentation in the README that includes a brief example, under the "Usage" section (before "Performance")?

• Instead of an array object, what do you think about using an object? I think that will be more self-documenting and also make it easier to add messages in the middle of the list. Something like:

messages =
  use_a_few_words: "Use a few words, avoid common phrases"
  no_need_for_mixed_chars: "No need for symbols, digits, or uppercase letters"
  ...

# instead of -- 
# extra_feedback = @get_message(2) # Add another word or two. Uncommon words are better.
extra_feedback = messages.use_few_words # no need to copy/paste the message, which is costlier to maintain as messages change. 

• Lastly, I would suggest that if a key is missing from the optional messages object, it should result in an absence of that message in the corresponding suggestions array. That way, if a user doesn't like a particular feedback string, they can exclude it in their rewording/translation. When a code path would have added that feedback string, it'll instead do nothing if it can't find the key.

[cdroulers]

I agree with @lowe for the dictionary, otherwise numbers aren't very descriptive when overriding!

[the-question]

@paulballesty's PR (now with a dictionnary thanks to 55b9ce5) looks nice and is a must have! Any chance having it merged soon @lowe?

[paulballesty]

@the-question I didn't have time to test the last changes properly. I'll try to find some time this week. Sorry for the delay!

[lpavlicek]

Thank @paulballesty for PR. I tried to use it but I encountered a few problems:

• default value for options in zxcvbn function:
  zxcvbn = (password, user_inputs = [], options = {})
  Empty object means empty feedback or default feedback ? I suggest{}for empty feedback and null for default feedback, so I recommend change function header to:
  zxcvbn = (password, user_inputs = [], options = null)

• I'm beginner in Javscript/Coffeescript and I don't understand next construction from main function:
  { feedback_messages } = options
  In my code I change it to
  feedback_messages = options

• If I disable some suggestion then feedback contains empty string. Next code:

feedback.get_feedback(0, '', {
      'no_need_for_mixed_chars' : 'modified suggestion',
      'use_a_few_words' : null
      }),

returns empty string in suggestions unnecessary:

 {
      suggestions: [ '', 'modified suggestion' ],
      warning: ''
    }

so in function build_feedback i changed
suggestions.push message if message?
to
suggestions.push message if message? and message != ''

I attached feedback.patch and also tape tests for feedback (test/test-feedback.coffee).

zxcvbn_pr_patch.zip

[paulballesty]

@lowe finally found time to work on this 😄 (public holiday in Australia 🇦🇺 ).

Can you add short documentation in the README that includes a brief example, under the "Usage" section (before "Performance")?

In the last commits I moved the user_inputs parameter inside an options parameter (I made it backward compatible checking the type of the object passed by the consumer). I did this because I wouldn't like to force the user to pass something on user_inputs when they just want to use feedback_messages. I think this could be helpful if there's any intention of adding another optional parameter in the future. LMK your opinion on this.

Instead of an array object, what do you think about using an object?

Done

Lastly, I would suggest that if a key is missing from the optional messages object, it should result in an absence of that message in the corresponding suggestions array. That way, if a user doesn't like a particular feedback string, they can exclude it in their rewording/translation. When a code path would have added that feedback string, it'll instead do nothing if it can't find the key.

I did something a bit different here. Instead of excluding the message if it's not present in feedback_messages, I am only excluding it if the key is present but the value is falsy (null, undefined, false, etc) . If the key is missing, then we default to the in-app message. The argument for this decision is that I think it would be much more common to want to delete or modify a specific message, and we would be forcing users to copy and paste all the other messages that they don't want to modify.

I included tests for the feedback component, I think that's gonna make things easier in the future 😄 . Also cherry-picked those tests in another branch without all the custom feedback logic to check that these changes aren't breaking anything in the feedback logic, you can check that here.

[paulballesty]

Was this used somewhere?

[paulballesty]

^ this is the code moving user_inputs in a options object

[lpavlicek]

options are a bit better because they make future changes easier. I get error when compile, so I suggest the following version:

zxcvbn = (password, options = {}) ->
  user_inputs = []
  feedback_messages = {}
  if options instanceof Array
    user_inputs = options # backward-compatibility
  else
    user_inputs = options.user_inputs if options.user_inputs
    feedback_messages = options.feedback_messages if options.feedback_messages

[lpavlicek]

Please add message to suggestions only if not empty:
suggestions.push message if message? and message != ''

Array of suggestions with empty string like [ '' ] or [ '', 'Avoid sequences'] is complication for displaying feedback. Empty string occurs if message is present in custom messages and has a falsy value.

[clarkewing]

Hi @lowe, any chance of merging this in the near future? Seems ready to me!

[olegomon]

@paulballesty I don't know CoffeeScript well but strict equality checks using "===" is not valid syntax in CoffeeScript my local build failed using your PR. CoffeeScript converts all loose checks into strict equality checks by default so using "===" is not needed here.

[olegomon]

@paulballesty In that case user_input should be initialised with an empty array to mimic the old behaviour. Otherwise an exception is thrown in main.coffee L20 when you trying to iterate over the values of an undefined user_input array.

[ghost]

Hi, any chance to have t his merged any time soon?

[wwelles]

Also would love to see this in master.

[ctavan]

In case anyone is interested, we have temporarily released this branch as npm package: @contentpass/zxcvbn

[Pierre-Sassoulas]

Here's a MIT licensed french translation of most of the feedback strings. It's a .po file for another project, but you can probably adapt it quickly.

[revington]

Please merge this PR as soon as possible! the world is full of non english speakers who need secure passwords! :)

[jeroencornelissen]

+1

[tracker1]

NOTE: given that this changes the interface, it should be published as a major version bump, so it doesn't interfere with exiting users of yarn/npm.

[tracker1]

should probably be userInputs and feedbackMessages respectively... this is JavaScript.

[tracker1]

userInputs and feedbackMessages ?

[paulballesty]

Closing this PR. It's been open for more than 2 years.

[L3o-pold]

This need to be merged not closed!

[cdroulers]

@tracker1 Looks like it was backwards compatible with line 9 of src/main.coffee, no?

[tracker1]

@cdroulers It does seem that way... the issue, to me is the prior version didn't force a convention, as it was an argument. Now that you're passing an object, imho the object structure should conform to JS naming conventions (camel-case as opposed to snake-case).

[cdroulers]

@tracker1 Oh, absolutely, I posted this in reference to your previous comment!

[xtrasimplicity]

@paulballesty, are you open to this sort of feature being merged (whether in this form or another)? I understand wanting to tidy up a bit, but it looked like it was starting to gain some traction again?

[paulballesty]

@xtrasimplicity I am not an owner of the repo so I cannot merge the feature myself. I left the PR open for more than 2 years. I closed it assuming there's no intention of merging it.

[xtrasimplicity]

Thanks for clarifying, @paulballesty - much appreciated. :)

@lowe, Are you able to shed some light on this?

[kasperwandahl]

@lowe @paulballesty Just found this PR, and this is something I need 👍 Can I do anything to help test, tidy up, etc? Would be nice to have it merged and bumped to a new version.

[Aggror]

Translation options would be a very common thing in my opinion. I thought I found a decent library, but seeing a PR like this not being used to its potential is a bit of a let down.

[jesobreira]

Is this going somewhere?

For now I'm using this package published by @ctavan (thanks!).

[bernardsaucier]

Yet another one of those "looks good, looks good, looks good, oh crap, need to find another library"...

[Saggre]

I need this feature and I'd be ready to rebase this onto the current master. @lowe would you check and merge the resulting pr?