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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use more semantic method to authenticate password in SecurePassword #18256

Merged
merged 1 commit into from
Dec 30, 2014
Merged

Use more semantic method to authenticate password in SecurePassword #18256

merged 1 commit into from
Dec 30, 2014

Conversation

rohitarondekar
Copy link
Contributor

-        BCrypt::Password.new(password_digest) == unencrypted_password && self
+        BCrypt::Password.new(password_digest).is_password?(unencrypted_password) && self

The original == method looks suspicious because it looks like the unencrypted password is being compared to a hashed password. The bcrypt-ruby gem provides an alias is_password? that reads better IMHO.

Feel free to close if this was already considered and == was chosen. 馃槃

rafaelfranca added a commit that referenced this pull request Dec 30, 2014
Use more semantic method to authenticate password in SecurePassword
@rafaelfranca rafaelfranca merged commit 426da75 into rails:master Dec 30, 2014
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

2 participants