From 1a59adde37a07359568b6a79a55c8dc1a0e96d5c Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Fri, 8 Jul 2011 22:14:52 -0700 Subject: [PATCH 1/5] No need to use Engine::generate_salt Instead of using Engine::generate_salt and Engine::hash_secret, we can replace them with Password.create --- lib/generators/nifty/authentication/templates/user.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/generators/nifty/authentication/templates/user.rb b/lib/generators/nifty/authentication/templates/user.rb index ec18524..7af9f02 100644 --- a/lib/generators/nifty/authentication/templates/user.rb +++ b/lib/generators/nifty/authentication/templates/user.rb @@ -19,18 +19,17 @@ class <%= user_class_name %> < ActiveRecord::Base # login can be either username or email address def self.authenticate(login, pass) <%= user_singular_name %> = find_by_username(login) || find_by_email(login) - return <%= user_singular_name %> if <%= user_singular_name %> && <%= user_singular_name %>.password_hash == <%= user_singular_name %>.encrypt_password(pass) + return <%= user_singular_name %> if <%= user_singular_name %> && BCrypt::Password.new(<%= user_singular_name %>.password_hash) == pass end def encrypt_password(pass) - BCrypt::Engine.hash_secret(pass, password_salt) + BCrypt::Password.create(pass) end private def prepare_password unless password.blank? - self.password_salt = BCrypt::Engine.generate_salt self.password_hash = encrypt_password(password) end end From 41d51a2d7b980fd50d8698f26d127c61e2c77414 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Fri, 8 Jul 2011 22:19:56 -0700 Subject: [PATCH 2/5] No need set the salt separately in the fixtures --- lib/generators/nifty/authentication/templates/fixtures.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/generators/nifty/authentication/templates/fixtures.yml b/lib/generators/nifty/authentication/templates/fixtures.yml index c52532b..c17be7a 100644 --- a/lib/generators/nifty/authentication/templates/fixtures.yml +++ b/lib/generators/nifty/authentication/templates/fixtures.yml @@ -8,7 +8,6 @@ foo: password_salt: n6z_wtpWoIsHgQb5IcFd <%- else -%> password_hash: 3488f5f7efecab14b91eb96169e5e1ee518a569f - password_salt: bef65e058905c379436d80d1a32e7374b139e7b0 <%- end -%> bar: @@ -20,5 +19,4 @@ bar: password_salt: UiAh9ejabnKRxqsiK0xO <%- else -%> password_hash: 3488f5f7efecab14b91eb96169e5e1ee518a569f - password_salt: bef65e058905c379436d80d1a32e7374b139e7b0 <%- end -%> From 2c91c0de1a58c9f0e90aa292bdcf7b60c3fa4259 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Fri, 8 Jul 2011 22:21:03 -0700 Subject: [PATCH 3/5] Amend the test cases: we no longer generate a salt --- .../nifty/authentication/templates/tests/rspec/user.rb | 3 +-- .../nifty/authentication/templates/tests/shoulda/user.rb | 3 +-- .../nifty/authentication/templates/tests/testunit/user.rb | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/generators/nifty/authentication/templates/tests/rspec/user.rb b/lib/generators/nifty/authentication/templates/tests/rspec/user.rb index a3f7e92..c2f720b 100644 --- a/lib/generators/nifty/authentication/templates/tests/rspec/user.rb +++ b/lib/generators/nifty/authentication/templates/tests/rspec/user.rb @@ -52,11 +52,10 @@ def new_<%= user_singular_name %>(attributes = {}) new_<%= user_singular_name %>(:password_confirmation => 'nonmatching').should have(1).error_on(:password) end - it "should generate password hash and salt on create" do + it "should generate password hash on create" do <%= user_singular_name %> = new_<%= user_singular_name %> <%= user_singular_name %>.save! <%= user_singular_name %>.password_hash.should_not be_nil - <%= user_singular_name %>.password_salt.should_not be_nil end it "should authenticate by username" do diff --git a/lib/generators/nifty/authentication/templates/tests/shoulda/user.rb b/lib/generators/nifty/authentication/templates/tests/shoulda/user.rb index beb8bf4..bc18e08 100644 --- a/lib/generators/nifty/authentication/templates/tests/shoulda/user.rb +++ b/lib/generators/nifty/authentication/templates/tests/shoulda/user.rb @@ -54,11 +54,10 @@ def setup assert_equal ["doesn't match confirmation"], new_<%= user_singular_name %>(:password_confirmation => 'nonmatching').errors[:password] end - should "generate password hash and salt on create" do + should "generate password hash on create" do <%= user_singular_name %> = new_<%= user_singular_name %> <%= user_singular_name %>.save! assert <%= user_singular_name %>.password_hash - assert <%= user_singular_name %>.password_salt end should "authenticate by username" do diff --git a/lib/generators/nifty/authentication/templates/tests/testunit/user.rb b/lib/generators/nifty/authentication/templates/tests/testunit/user.rb index c036cf1..d9368b0 100644 --- a/lib/generators/nifty/authentication/templates/tests/testunit/user.rb +++ b/lib/generators/nifty/authentication/templates/tests/testunit/user.rb @@ -54,11 +54,10 @@ def test_require_matching_password_confirmation assert_equal ["doesn't match confirmation"], new_<%= user_singular_name %>(:password_confirmation => 'nonmatching').errors[:password] end - def test_generate_password_hash_and_salt_on_create + def test_generate_password_hash_on_create <%= user_singular_name %> = new_<%= user_singular_name %> <%= user_singular_name %>.save! assert <%= user_singular_name %>.password_hash - assert <%= user_singular_name %>.password_salt end def test_authenticate_by_username From e7b03df4738d3d8efaea81ca6846ba705fc4335e Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Fri, 8 Jul 2011 22:25:17 -0700 Subject: [PATCH 4/5] Reduce the number of fields in the users table BCrypt::Password handles the salt directly so we don't need to store it in a separate column. --- lib/generators/nifty/authentication/templates/migration.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/generators/nifty/authentication/templates/migration.rb b/lib/generators/nifty/authentication/templates/migration.rb index c945df3..e15696a 100644 --- a/lib/generators/nifty/authentication/templates/migration.rb +++ b/lib/generators/nifty/authentication/templates/migration.rb @@ -9,7 +9,6 @@ def self.up <%- else -%> t.string :password_hash <%- end -%> - t.string :password_salt t.timestamps end end From bcb525fc55c5a4d4bbfaeceaa4fc5a24b4e99415 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Fri, 8 Jul 2011 22:29:22 -0700 Subject: [PATCH 5/5] Update the .gemspec with the new version of bcrypt --- nifty-generators.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nifty-generators.gemspec b/nifty-generators.gemspec index d9b6402..03bfdba 100644 --- a/nifty-generators.gemspec +++ b/nifty-generators.gemspec @@ -14,7 +14,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'cucumber', '~> 0.9.2' s.add_development_dependency 'rails', '~> 3.0.0' s.add_development_dependency 'mocha', '~> 0.9.8' - s.add_development_dependency 'bcrypt-ruby', '~> 2.1.2' + s.add_development_dependency 'bcrypt-ruby', '~> 2.1.4' s.add_development_dependency 'sqlite3-ruby', '~> 1.3.1' s.rubyforge_project = s.name