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

SecurePassword with a standalone PasswordValidator #135

Closed
wants to merge 2 commits into from

Conversation

franckverrot
Copy link
Contributor

I took some pieces of (ActiveValidators)[https://github.com/cesario/activevalidators] and added it to ActiveModel.

has_secure_password :strength => :strong

now accepts an option:strength, which can be weak, medium and strong

By default the 7 chars policy + WEAK_PASSWORD array are evaluated, if they are all OK, the strength will use the right regex.

@paneq
Copy link
Contributor

paneq commented Dec 19, 2010

This is really poorly implemented.

record.errors.add(:password, "is too weak and common")
  1. Errors are added on :password attribute instead of the attribute user wanted to validate
  2. Previous implementation was I18n aware. This is not. English message is always added.

Nice idea but needs much better implementation in my opinion.

@franckverrot
Copy link
Contributor Author

Agreed. Do you see anything else that should be improved?

Thanks for your review!

@josevalim
Copy link
Contributor

Thanks for the pull requests, the SecurePassword module is meant to be very very simple. We have a huge amount of authentication solutions today and if someone wants something more robust, with several options, I recommend them to use it, not Rails one.

@franckverrot
Copy link
Contributor Author

Alright, thanks anyway for reviewing it :)

vijaydev pushed a commit that referenced this pull request Apr 11, 2013
Change user_class to author_class in Engine guide
This pull request was closed.
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.

None yet

3 participants