Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony Bundle Reference #44

Open
AlexLisenkov opened this issue May 15, 2020 · 5 comments
Open

Symfony Bundle Reference #44

AlexLisenkov opened this issue May 15, 2020 · 5 comments

Comments

@AlexLisenkov
Copy link
Contributor

Hello maintainers,

I've been working on a symfony bundle that enables localization and tagging matchers in symfony.
How do you feel about referencing https://github.com/createnl/zxcvbn-bundle in the readme for people that use symfony?

@adorbabble
Copy link

I’m not a maintainer, but thank you for this ! 😃 I think I’m going to use your bundle, and I could do a pull request for the translations into French. I have frequency lists of words and names for French, but I hadn’t dealt with the localization of the strings within zxcvbn yet, so your bundle comes at the perfect time ! And there is another localization issue that was not taken care of, as far as I know : the script for building the keyboard adjacency graphs can’t take into account European layouts (dropbox/zxcvbn#275). I’m thinking of improving that script, but I’m not sure when I’ll have the time to do that. The only thing I’ve done so far is adding some layouts and converting the scripts to Python 3. There’s a German version of zxcvbn (JS), but they seem to have only manually added the missing key combinations to the generated file

@AlexLisenkov
Copy link
Contributor Author

Hi @not-a-lot, feel free to open up an issue and pr on the repository for the French translations.

I recently extended Zxcvbn.php with the addMatcher method, which allows you to add custom matchers. I don't think we should alter any behavior but instead create new
matchers for requests like azerty keyboard layout.

@adorbabble
Copy link

adorbabble commented Sep 24, 2020

Hi @AlexLisenkov 😺 Thank you for your answer ! I have just done the French translations, but I’d like to wait a little before a pull request because I’m still improving them.

I don’t really understand why I should create a new Matcher, however. If I have my own adjacency_graphs.json and frequency_lists.json files and just want to use those files instead of the provided ones, that’s not really altering any behaviour, but rather using different data, isn’t it ? I’m not sure what to do because I know that replacing these files in vendor/bjeavons/zxcvbn-php/src/Matchers/ wouldn’t be a good practice, but it seems easier than creating a new Matcher, for which there’s no documentation. And if I extend DictionaryMatch.php to change the path of the JSON file to be read, wouldn’t I want to replace this Matcher instead of adding it (which isn’t currently possible since we only have an addMatcher method)? I don’t want to run two dictionary matchers, and my frequency_lists.json includes the original words in English

@AlexLisenkov
Copy link
Contributor Author

Hi @not-a-lot ,

Your conclusion is correct, there are two things that are impossible now.

  1. You cannot provide matchers with different data sources, as they're hardcoded (here).
  2. You cannot exclude matchers that are defined as default

You should not alter vendor code in your project, as they're not part of your vcs. The next person that clones your project will not have these changes.
You could think of a more durable solution an suggest some changes for a new version to the maintainers of this project, or if you aren't able to invest that much you can make a compromise and do tun two dictionary matchers.

Feel free to create a pr with your French translation whenever you feel like it!

@AlexLisenkov
Copy link
Contributor Author

There are already some issues related to your problems: #34 #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants