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

Refactor to allow integration with other plugins #1

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

therabidbanana
Copy link

Lots of stuff going on here:

  • Refactored to allow creating a standalone tester object that can return raw entropy for a given value. $.entropyTestFactory() returns this object, then the $.fn.passwordEntropy object just initializes a tester and handles displaying the messages.
  • Lowered thresholds that seemed a little high - made thresholds configurable
  • Changed wording of classes.
  • Added examples integrating the standalone entropy tester with jquery.validation.js library.
  • Begin allowing username to be passed in as part of the test on the raw entropy tester (to set entropy to zero if username == password), this needs more work.

Should note that the demo.html and packaged jquery.validation.js libraries are currently based on my fork of that library, which I'm working on cleaning up and hopefully submitting a pull request for.

* Should now be possible to call the entropy test from another jquery
  plugin by taking a function returned from $.entropyTestFactory(options)
  The entropy tester returns a 0-5 strength rating that you can classify.
@erikbrannstrom
Copy link
Owner

Thanks for your pull request! See comments below.

Refactored to allow creating a standalone tester object that can return raw entropy for a given value. $.entropyTestFactory() returns this object, then the $.fn.passwordEntropy object just initializes a tester and handles displaying the messages.

Not convinced about this one. This plugin is not intended to be used for validation, only information. I'll have to go through the code more closely though, since I do think that some refactoring might be needed for aesthetics ;)

Lowered thresholds that seemed a little high - made thresholds configurable

Don't agree that they are low, however making them editable by the user is a good idea, as well as the for-loop instead of the current if-statements.

Changed wording of classes.

I'll give it some thought.

Added examples integrating the standalone entropy tester with jquery.validation.js library.

This won't happen for the reason stated in the first point. I might consider the refactoring so it can be used for validation, however validation will never be a part of this plugin.

Begin allowing username to be passed in as part of the test on the raw entropy tester (to set entropy to zero if username == password), this needs more work.

This is a no-go, since it should be done by the user of the plugin by passing in such a function in the options at initialization. In fact, that is exactly how I do it on one of my live sites. Might include such a function in the examples though!

Feel free to share your thoughts on this.

@therabidbanana
Copy link
Author

Not convinced about this one. This plugin is not intended to be used for validation, only information. I'll have to go through the code more closely though, since I do think that some refactoring might be needed for aesthetics ;)

Obviously the "validation" side of things is only client side, which doesn't really have security value - it's just there more as a sanity check to help keep people from putting "password" as their password, which I'd want to validate clientside anyway before allowing form submission. (The goal in my fork is to be able to emulate the Twitter signup form - which basically takes the same approach, though it is more lax on minimum required strength.)

Regardless, I think a refactoring is useful in the fact it separates the display of the entropy and the calculation of entropy into separate modules, so you could create several display options that all use the same underlying logic. It may be that my way of refactoring is haphazard - I spend most of my time in Ruby, so the way I did that may not make much sense. :)

Don't agree that they are low, however making them editable by the user is a good idea, as well as the for-loop instead of the current if-statements.

I actually agree - but my boss was angry his favorite "secure" password was labeled weak and didn't want to be scaring potential users. :D

Changed wording of classes. I'll give it some thought.

See above - mainly to make the wording more end-user friendly.

This won't happen for the reason stated in the first point. I might consider the refactoring so it can be used for validation, however validation will never be a part of this plugin.

I agree it doesn't belong as part of the plugin, but it seemed like a good example to show a possible case of standalone usage (and it's the way I'm using it in my project - rather than requiring a bunch of character types and password length and doing a dictionary check in my jquery validation, I can just make a single call to the entropy tester and get the same result).

This is a no-go, since it should be done by the user of the plugin by passing in such a function in the options at initialization. In fact, that is exactly how I do it on one of my live sites. Might include such a function in the examples though!

My intention wasn't to actually add any functions in doing this, just to make it so that custom functions could optionally be passed a secondary username argument (if it was passed in to the standalone tester), rather than requiring each of the custom functions to handle finding the username in the DOM.

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

Successfully merging this pull request may close these issues.

2 participants