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 - Validate password must be less than or equal to 72 #15708

Merged
merged 1 commit into from Jun 14, 2014

Conversation

@akshay-vishnoi
Copy link
Contributor

akshay-vishnoi commented Jun 14, 2014

See #14591, Reason - BCrypt hash function can handle maximum 72 characters.

@akshay-vishnoi
Copy link
Contributor Author

akshay-vishnoi commented Jun 14, 2014

cc: @matthewd

@chancancode
chancancode reviewed Jun 14, 2014
View changes
activemodel/lib/active_model/secure_password.rb Outdated
@@ -2,6 +2,8 @@ module ActiveModel
module SecurePassword
extend ActiveSupport::Concern

MAX_PASSWORD_LENGTH_ALLOWED = 72

This comment has been minimized.

Copy link
@chancancode

chancancode Jun 14, 2014

Member

Can you put a comment above this constant explaining why it is necessary?

See #14591, Reason - BCrypt hash function can handle maximum 72 characters.
@akshay-vishnoi
Copy link
Contributor Author

akshay-vishnoi commented Jun 14, 2014

Please check the updated code.

chancancode added a commit that referenced this pull request Jun 14, 2014
SecurePassword - Validate password must be less than or equal to 72
@chancancode chancancode merged commit 6fa7726 into rails:master Jun 14, 2014
1 check was pending
1 check was pending
continuous-integration/travis-ci The Travis CI build is in progress
Details
@akshay-vishnoi
Copy link
Contributor Author

akshay-vishnoi commented Jun 14, 2014

Hey I forgot, does this need CHANGELOG entry?

chancancode added a commit that referenced this pull request Jun 14, 2014
@chancancode
Copy link
Member

chancancode commented Jun 14, 2014

No worries, I fixed it for you 😄

@akshay-vishnoi
Copy link
Contributor Author

akshay-vishnoi commented Jun 14, 2014

Thanks 💚 😄

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.