Permalink
Browse files

Use bcrypt instead of salted-SHA1 for hashes

  • Loading branch information...
jeremyevans committed Aug 13, 2012
1 parent 78d953e commit 5d56c7d82a0c75571334ff13b83e1c0fbe8c313d
Showing with 49 additions and 30 deletions.
  1. +2 −0 .gems
  2. +16 −0 migrate/002_bcrypt.rb
  3. +12 −2 models.rb
  4. +3 −10 models/user.rb
  5. +16 −18 spec/unit.rb
View
2 .gems
@@ -1,5 +1,7 @@
pg
scaffolding_extensions
sequel --version '>= 3.32.0'
+sequel_pg
sequel_postgresql_triggers
sinatra
+bcrypt-ruby
View
@@ -0,0 +1,16 @@
+Sequel.migration do
+ up do
+ drop_column(:users, :password)
+ drop_column(:users, :salt)
+
+ add_column(:users, :password_hash, :text)
+ # Bcrypt hash of: demo by default
+ from(:users).update(:password_hash=>"$2a$04$X5QQ9b3cKu8ReRsz8HM8W.jhRX2XJM4wB.n2xjgU9I6b5RZHR0Z4W")
+ alter_table(:users){set_column_allow_null :password_hash, false}
+ end
+ down do
+ drop_column(:users, :password_hash)
+ add_column(:users, :salt, :text)
+ add_column(:users, :password, :text)
+ end
+end
View
@@ -1,7 +1,10 @@
+Encoding.default_internal = Encoding.default_external = 'UTF-8' if RUBY_VERSION >= '1.9'
+$: << '.'
+
require 'rubygems'
-require 'digest/sha1'
+require 'bcrypt'
require 'logger'
-require 'sequel'
+require 'sequel/no_core_ext'
unless defined?(GIFTSMAS_ENV)
GIFTSMAS_ENV = ENV['GIFTSMAS_TEST'] ? :test : :production
@@ -12,6 +15,13 @@
rescue LoadError
DB = Sequel.connect(ENV['DATABASE_URL'] || "postgres:///giftsmas#{'_test' if GIFTSMAS_ENV != :production}")
end
+
+if GIFTSMAS_ENV == :production
+ BCRYPT_COST = BCrypt::Engine::DEFAULT_COST
+else
+ BCRYPT_COST = BCrypt::Engine::MIN_COST
+end
+
Sequel::Model.plugin :prepared_statements
Sequel::Model.plugin :prepared_statements_associations
View
@@ -4,18 +4,11 @@ class User < Sequel::Model
def self.login_user_id(username, password)
return unless username && password
return unless u = filter(:name=>username).first
- return unless u.password == ::Digest::SHA1.new.update(u.salt).update(password).hexdigest
+ return unless BCrypt::Password.new(u.password_hash) == password
u.id
end
- def password=(pass)
- self.salt = new_salt
- self[:password] = ::Digest::SHA1.new.update(salt).update(pass).hexdigest
- end
-
- private
-
- def new_salt
- (0...40).map{(i = Kernel.rand(62); i += ((i < 10) ? 48 : ((i < 36) ? 55 : 61 ))).chr}.join
+ def password=(new_password)
+ self.password_hash = BCrypt::Password.create(new_password, :cost=>BCRYPT_COST)
end
end
View
@@ -13,7 +13,7 @@ def execute(*args, &block)
describe Event do
before do
- @user = User.create(:name=>'test', :salt=>'', :password=>'')
+ @user = User.create(:name=>'test', :password=>'')
@event = Event.create(:name=>'Christmas', :user_id=>@user.id)
@sender = Person.create(:name=>'S', :user_id=>@user.id)
@receiver = Person.create(:name=>'R', :user_id=>@user.id)
@@ -79,7 +79,7 @@ def execute(*args, &block)
describe Gift do
before do
- @user = User.create(:name=>'test', :salt=>'', :password=>'')
+ @user = User.create(:name=>'test', :password_hash=>'')
@event = Event.create(:name=>'Christmas', :user_id=>@user.id)
@sender = Person.create(:name=>'S', :user_id=>@user.id)
@receiver = Person.create(:name=>'R', :user_id=>@user.id)
@@ -121,7 +121,7 @@ def execute(*args, &block)
describe Person do
before do
- @user = User.create(:name=>'test', :salt=>'', :password=>'')
+ @user = User.create(:name=>'test', :password_hash=>'')
@event = Event.create(:name=>'Christmas', :user_id=>@user.id)
@person = Person.create(:name=>'P', :user_id=>@user.id)
end
@@ -166,42 +166,40 @@ def execute(*args, &block)
describe User do
before do
- @user = User.create(:name=>'test', :salt=>'', :password=>'')
+ @user = User.create(:name=>'test', :password=>'blah')
+ end
+ after do
+ @user.delete
end
specify "associations should be correct" do
@user.events.should == []
end
- specify "#password= should create a new salt" do
- salt = @user.salt
- @user.password = 'blah'
- @user.salt.should_not == salt
- @user.salt.should =~ /\A[0-9a-zA-Z]{40}\z/
- end
-
- specify "#password= should set the SHA1 password hash based on the salt and password" do
- @user.password = 'blah'
- @user.password.should == Digest::SHA1.new.update(@user.salt).update('blah').hexdigest
+ specify "#password= should set a new password hash" do
+ pw = @user.password_hash
+ @user.password = 'foo'
+ @user.password_hash.should_not == pw
+ User.login_user_id('test', 'foo').should == nil
+ @user.save
+ User.login_user_id('test', 'foo').should == @user.id
end
specify ".login_user_id should return nil unless both username and password are present" do
User.login_user_id(nil, nil).should == nil
- User.login_user_id('default', nil).should == nil
+ User.login_user_id('test', nil).should == nil
User.login_user_id(nil, 'blah').should == nil
end
specify ".login_user_id should return nil unless a user with a given username exists" do
- User.login_user_id('blah', nil).should == nil
+ User.login_user_id('foo', nil).should == nil
end
specify ".login_user_id should return nil unless the password matches for that username" do
User.login_user_id('test', 'wrong').should == nil
end
specify ".login_user_id should return the user's id if the password matches " do
- @user.password = 'blah'
- @user.save
User.login_user_id('test', 'blah').should == @user.id
end
end

0 comments on commit 5d56c7d

Please sign in to comment.