Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

BCrypt for passwords

This commit makes BCrypt the default for new setups, and introduces a
strategy for converting existing infrastructure to BCrypt.

To switch to BCrypt now:

    Clearance.configure do |config|
      config.password_strategy = Clearance::PasswordStrategies::BCrypt
    end

To set the password strategy to the conversion layer:

    Clearance.configure do |config|
      config.password_strategy = Clearance::PasswordStrategies::BCryptMigrationFromSHA1
    end

To continue to use SHA1:

    Clearance.configure do |config|
      config.password_strategy = Clearance::PasswordStrategies::SHA1
    end
  • Loading branch information...
commit be37c354594a33333006dcdff38ffc46bdc55620 1 parent 3746806
Dan Croak and Gabe Berke-Williams authored mike-burns committed
Showing with 334 additions and 61 deletions.
  1. +2 −0  Gemfile.lock
  2. +6 −0 NEWS.md
  3. +19 −4 README.md
  4. +1 −0  clearance.gemspec
  5. +1 −1  features/engine/visitor_resets_password.feature
  6. +2 −2 features/step_definitions/engine/clearance_steps.rb
  7. +3 −1 gemfiles/3.0.12.gemfile.lock
  8. +2 −0  gemfiles/3.1.4.gemfile.lock
  9. +2 −0  gemfiles/3.2.3.gemfile.lock
  10. +3 −0  lib/clearance/password_strategies.rb
  11. +32 −0 lib/clearance/password_strategies/bcrypt.rb
  12. +52 −0 lib/clearance/password_strategies/bcrypt_migration_from_sha1.rb
  13. +6 −5 lib/clearance/password_strategies/blowfish.rb
  14. +34 −0 lib/clearance/password_strategies/fake.rb
  15. +10 −7 lib/clearance/password_strategies/sha1.rb
  16. +4 −0 lib/clearance/testing.rb
  17. +4 −9 lib/clearance/user.rb
  18. +0 −1  lib/generators/clearance/install/templates/db/migrate/upgrade_clearance_to_diesel.rb
  19. +6 −7 spec/controllers/passwords_controller_spec.rb
  20. +72 −0 spec/models/bcrypt_migration_from_sha1_spec.rb
  21. +41 −0 spec/models/bcrypt_spec.rb
  22. +9 −6 spec/models/blowfish_spec.rb
  23. +2 −2 spec/models/{clearance_user_spec.rb → password_strategies_spec.rb}
  24. +17 −11 spec/models/sha1_spec.rb
  25. +4 −5 spec/models/user_spec.rb
View
2  Gemfile.lock
@@ -2,6 +2,7 @@ PATH
remote: .
specs:
clearance (0.16.2)
+ bcrypt-ruby
diesel (~> 0.1.5)
rails (>= 3.0)
@@ -45,6 +46,7 @@ GEM
cucumber (>= 1.1.1)
ffi (>= 1.0.11)
rspec (>= 2.7.0)
+ bcrypt-ruby (3.0.1)
bourne (1.1.2)
mocha (= 0.10.5)
builder (3.0.0)
View
6 NEWS.md
@@ -1,3 +1,9 @@
+HEAD:
+
+* Change default password strategy to BCrypt
+* Provide BCryptMigrationFromSHA1 password strategy to help people migrate from
+ SHA1 (the old default password strategy) to BCrypt (the new default).
+
New for 0.16.2:
* Change default email sender to deploy@example.com .
View
23 README.md
@@ -188,15 +188,27 @@ If you want to override the **model** behavior, you can include sub-modules of `
Overriding the password strategy
--------------------------------
-By default, Clearance uses SHA1 encryption of the user's password. You can provide your own password strategy by creating a module that conforms to an API of two instance methods:
+By default, Clearance uses BCrypt encryption of the user's password. You can provide your own password strategy by creating a module that conforms to an API of two instance methods:
def authenticated?
end
- def encrypt_password
+ def password=(new_password)
end
-See [lib/clearance/password_strategies/sha1.rb](https://github.com/thoughtbot/clearance/blob/master/lib/clearance/password_strategies/sha1.rb) for the default behavior. Also see [lib/clearance/password_strategies/blowfish.rb](https://github.com/thoughtbot/clearance/blob/master/lib/clearance/password_strategies/blowfish.rb) for another password strategy. Switching password strategies will cause your existing users' passwords to not work.
+The previous default password strategy was SHA1. To keep using SHA1, use this
+code:
+
+ Clearance.configure do |config|
+ config.password_strategy = Clearance::PasswordStrategies::SHA1
+ end
+
+See [lib/clearance/password_strategies/bcrypt.rb](https://github.com/thoughtbot/clearance/blob/master/lib/clearance/password_strategies/bcrypt.rb) for the default behavior.
+Also see [lib/clearance/password_strategies/blowfish.rb](https://github.com/thoughtbot/clearance/blob/master/lib/clearance/password_strategies/blowfish.rb) for another password strategy.
+Switching password strategies will cause your existing users' passwords to not
+work. If you are currently using the SHA1 strategy (the previous default), and
+want to transparently switch to BCrypt, use the [BCryptMigrationFromSHA1 strategy](https://github.com/thoughtbot/clearance/blob/master/lib/clearance/password_strategies/bcrypt_migration_from_sha1.rb).
+
Once you have an API-compliant module, load it with:
@@ -207,11 +219,14 @@ Once you have an API-compliant module, load it with:
For example:
# default
+ config.password_strategy = Clearance::PasswordStrategies::BCrypt
+ # use this strategy if you used to use SHA1, and now you want to use BCrypt
+ config.password_strategy = Clearance::PasswordStrategies::BCryptMigrationFromSHA1
+ # SHA1 (the previous default)
config.password_strategy = Clearance::PasswordStrategies::SHA1
# Blowfish
config.password_strategy = Clearance::PasswordStrategies::Blowfish
-
Routing Constraints
-------------------
View
1  clearance.gemspec
@@ -21,6 +21,7 @@ Gem::Specification.new do |s|
s.add_dependency('rails', '>= 3.0')
s.add_dependency('diesel', '~> 0.1.5')
+ s.add_dependency('bcrypt-ruby')
s.add_development_dependency('bundler', '~> 1.1')
s.add_development_dependency('appraisal', '~> 0.4.1')
View
2  features/engine/visitor_resets_password.feature
@@ -33,7 +33,7 @@ Feature: Password reset
Then I should be signed in
Scenario: User who was created before Clearance was installed creates password for first time
- Given a user "email@example.com" exists without a salt, remember token, or password
+ Given a user "email@example.com" exists without a remember token or password
When I reset the password for "email@example.com"
When I follow the password reset link sent to "email@example.com"
And I update my password with "newpassword"
View
4 features/step_definitions/engine/clearance_steps.rb
@@ -6,9 +6,9 @@
FactoryGirl.create(:user, :email => email)
end
-Given /^a user "([^"]*)" exists without a salt, remember token, or password$/ do |email|
+Given /^a user "([^"]*)" exists without a remember token or password$/ do |email|
user = FactoryGirl.create(:user, :email => email)
- sql = "update users set salt = NULL, encrypted_password = NULL, remember_token = NULL where id = #{user.id}"
+ sql = "update users set encrypted_password = NULL, remember_token = NULL where id = #{user.id}"
ActiveRecord::Base.connection.update(sql)
end
View
4 gemfiles/3.0.12.gemfile.lock
@@ -1,7 +1,8 @@
PATH
- remote: /home/mike/clearance
+ remote: /Users/gabe/thoughtbot/open-source/clearance
specs:
clearance (0.16.2)
+ bcrypt-ruby
diesel (~> 0.1.5)
rails (>= 3.0)
@@ -45,6 +46,7 @@ GEM
cucumber (>= 1.1.1)
ffi (>= 1.0.11)
rspec (>= 2.7.0)
+ bcrypt-ruby (3.0.1)
bourne (1.1.2)
mocha (= 0.10.5)
builder (2.1.2)
View
2  gemfiles/3.1.4.gemfile.lock
@@ -2,6 +2,7 @@ PATH
remote: /Users/gabe/thoughtbot/open-source/clearance
specs:
clearance (0.16.2)
+ bcrypt-ruby
diesel (~> 0.1.5)
rails (>= 3.0)
@@ -46,6 +47,7 @@ GEM
cucumber (>= 1.1.1)
ffi (>= 1.0.11)
rspec (>= 2.7.0)
+ bcrypt-ruby (3.0.1)
bourne (1.1.2)
mocha (= 0.10.5)
builder (3.0.0)
View
2  gemfiles/3.2.3.gemfile.lock
@@ -2,6 +2,7 @@ PATH
remote: /Users/gabe/thoughtbot/open-source/clearance
specs:
clearance (0.16.2)
+ bcrypt-ruby
diesel (~> 0.1.5)
rails (>= 3.0)
@@ -45,6 +46,7 @@ GEM
cucumber (>= 1.1.1)
ffi (>= 1.0.11)
rspec (>= 2.7.0)
+ bcrypt-ruby (3.0.1)
bourne (1.1.2)
mocha (= 0.10.5)
builder (3.0.0)
View
3  lib/clearance/password_strategies.rb
@@ -2,5 +2,8 @@ module Clearance
module PasswordStrategies
autoload :SHA1, 'clearance/password_strategies/sha1'
autoload :Blowfish, 'clearance/password_strategies/blowfish'
+ autoload :BCrypt, 'clearance/password_strategies/bcrypt'
+ autoload :Fake, 'clearance/password_strategies/fake'
+ autoload :BCryptMigrationFromSHA1, 'clearance/password_strategies/bcrypt_migration_from_sha1'
end
end
View
32 lib/clearance/password_strategies/bcrypt.rb
@@ -0,0 +1,32 @@
+module Clearance
+ module PasswordStrategies
+ module BCrypt
+ require 'bcrypt'
+
+ extend ActiveSupport::Concern
+
+ # Am I authenticated with given password?
+ #
+ # @param [String] plain-text password
+ # @return [true, false]
+ # @example
+ # user.authenticated?('password')
+ def authenticated?(password)
+ ::BCrypt::Password.new(encrypted_password) == password
+ end
+
+ def password=(new_password)
+ @password = new_password
+ if new_password.present?
+ self.encrypted_password = encrypt(new_password)
+ end
+ end
+
+ private
+
+ def encrypt(password)
+ ::BCrypt::Password.create(password)
+ end
+ end
+ end
+end
View
52 lib/clearance/password_strategies/bcrypt_migration_from_sha1.rb
@@ -0,0 +1,52 @@
+module Clearance
+ module PasswordStrategies
+ module BCryptMigrationFromSHA1
+ class BCryptUser
+ include Clearance::PasswordStrategies::BCrypt
+
+ def initialize(user)
+ @user = user
+ end
+
+ delegate :encrypted_password, :encrypted_password=, to: :@user
+ end
+
+ class SHA1User
+ include Clearance::PasswordStrategies::SHA1
+
+ def initialize(user)
+ @user = user
+ end
+
+ delegate :salt, :salt=, :encrypted_password, :encrypted_password=, to: :@user
+ end
+
+ def authenticated?(password)
+ authenticated_with_sha1?(password) || authenticated_with_bcrypt?(password)
+ end
+
+ def password=(new_password)
+ BCryptUser.new(self).password = new_password
+ end
+
+ private
+
+ def authenticated_with_sha1?(password)
+ if sha1_password?
+ if SHA1User.new(self).authenticated?(password)
+ self.password = password
+ true
+ end
+ end
+ end
+
+ def authenticated_with_bcrypt?(password)
+ BCryptUser.new(self).authenticated?(password)
+ end
+
+ def sha1_password?
+ self.encrypted_password =~ /^[a-f0-9]{40}$/
+ end
+ end
+ end
+end
View
11 lib/clearance/password_strategies/blowfish.rb
@@ -13,15 +13,16 @@ def authenticated?(password)
encrypted_password == encrypt(password)
end
- protected
-
- def encrypt_password
+ def password=(new_password)
+ @password = new_password
initialize_salt_if_necessary
- if password.present?
- self.encrypted_password = encrypt(password)
+ if new_password.present?
+ self.encrypted_password = encrypt(new_password)
end
end
+ protected
+
def generate_hash(string)
cipher = OpenSSL::Cipher::Cipher.new('bf-cbc').encrypt
cipher.key = Digest::SHA256.digest(salt)
View
34 lib/clearance/password_strategies/fake.rb
@@ -0,0 +1,34 @@
+module Clearance
+ module PasswordStrategies
+ # The Clearance::PasswordStrategies::Fake module is meant to be used in test suites.
+ # It stores passwords as plain text so your test suite doesn't pay the time cost
+ # of any hashing algorithm.
+ #
+ # Use the fake in your test suite by requiring Clearance's testing helpers:
+ #
+ # require 'clearance/testing'
+ #
+ # The usual places you'd require it are:
+ #
+ # spec/support/clearance.rb
+ # features/support/clearance.rb
+ module Fake
+ extend ActiveSupport::Concern
+
+ def authenticated?(password)
+ encrypted_password == password
+ end
+
+ def password=(new_password)
+ @password = new_password
+ if new_password.present?
+ self.encrypted_password = encrypt(password)
+ end
+ end
+
+ def encrypt(password)
+ password
+ end
+ end
+ end
+end
View
17 lib/clearance/password_strategies/sha1.rb
@@ -1,8 +1,10 @@
-require 'digest/sha1'
-
module Clearance
module PasswordStrategies
module SHA1
+ require 'digest/sha1'
+
+ extend ActiveSupport::Concern
+
# Am I authenticated with given password?
#
# @param [String] plain-text password
@@ -13,15 +15,16 @@ def authenticated?(password)
encrypted_password == encrypt(password)
end
- protected
-
- def encrypt_password
+ def password=(new_password)
+ @password = new_password
initialize_salt_if_necessary
- if password.present?
- self.encrypted_password = encrypt(password)
+ if new_password.present?
+ self.encrypted_password = encrypt(new_password)
end
end
+ private
+
def generate_hash(string)
if RUBY_VERSION >= '1.9'
Digest::SHA1.hexdigest(string).encode('UTF-8')
View
4 lib/clearance/testing.rb
@@ -2,6 +2,10 @@
require 'clearance/testing/deny_access_matcher'
require 'clearance/testing/helpers'
+Clearance.configure do |config|
+ config.password_strategy = Clearance::PasswordStrategies::Fake
+end
+
if defined?(ActionController::TestCase)
ActionController::TestCase.extend Clearance::Testing::Matchers
class ActionController::TestCase
View
13 lib/clearance/user.rb
@@ -16,12 +16,12 @@ module User
# @see Callbacks
included do
attr_accessor :password_changing
- attr_reader :password
+ attr_reader :password
include Validations
include Callbacks
- include (Clearance.configuration.password_strategy || Clearance::PasswordStrategies::SHA1)
+ include (Clearance.configuration.password_strategy || Clearance::PasswordStrategies::BCrypt)
end
module ClassMethods
@@ -63,7 +63,7 @@ module Callbacks
# salt, token, password encryption are handled before_save.
included do
before_validation :downcase_email
- before_create :generate_remember_token
+ before_create :generate_remember_token
end
end
@@ -108,12 +108,7 @@ def update_password(new_password)
save
end
- def password=(unencrypted_password)
- @password = unencrypted_password
- encrypt_password
- end
-
- protected
+ private
def generate_random_code(length = 20)
if RUBY_VERSION >= '1.9'
View
1  lib/generators/clearance/install/templates/db/migrate/upgrade_clearance_to_diesel.rb
@@ -5,7 +5,6 @@ def self.up
columns = [
[:email, 't.string :email'],
[:encrypted_password, 't.string :encrypted_password, :limit => 128'],
- [:salt, 't.string :salt, :limit => 128'],
[:confirmation_token, 't.string :confirmation_token, :limit => 128'],
[:remember_token, 't.string :remember_token, :limit => 128']
].delete_if {|c| existing_columns.include?(c.first.to_s)}
View
13 spec/controllers/passwords_controller_spec.rb
@@ -98,21 +98,20 @@
describe "on PUT to #update with password" do
before do
- new_password = "new_password"
- @encrypted_new_password = @user.send(:encrypt, new_password)
- @user.encrypted_password.should_not == @encrypted_new_password
+ @new_password = "new_password"
+ @user.encrypted_password.should_not == @new_password
put(:update,
:user_id => @user,
:token => @user.confirmation_token,
:user => {
- :password => new_password
+ :password => @new_password
})
@user.reload
end
it "should update password" do
- @user.encrypted_password.should == @encrypted_new_password
+ @user.encrypted_password.should == @new_password
end
it "should clear confirmation token" do
@@ -137,8 +136,8 @@
@user.reload
end
- it "should not update password" do
- @user.encrypted_password.should_not == @encrypted_new_password
+ it "should not update password to be blank" do
+ @user.encrypted_password.should_not be_blank
end
it "should not clear token" do
View
72 spec/models/bcrypt_migration_from_sha1_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper'
+
+describe Clearance::PasswordStrategies::BCryptMigrationFromSHA1 do
+ subject do
+ Class.new do
+ attr_accessor :encrypted_password
+ attr_accessor :salt
+ include Clearance::PasswordStrategies::BCryptMigrationFromSHA1
+ end.new
+ end
+
+ describe "#password=" do
+ let(:salt) { "salt" }
+ let(:password) { "password" }
+ let(:encrypted_password) { stub("encrypted password") }
+
+ before do
+ subject.salt = salt
+ subject.encrypted_password = Digest::SHA1.hexdigest("--#{salt}--#{password}--")
+
+ BCrypt::Password.stubs(:create => encrypted_password)
+ subject.password = password
+ end
+
+ it "encrypts the password into a BCrypt-encrypted encrypted_password" do
+ subject.encrypted_password.should == encrypted_password
+ end
+
+ it "encrypts with BCrypt" do
+ BCrypt::Password.should have_received(:create).with(password)
+ end
+ end
+
+ describe "#authenticated?" do
+ let(:password) { "password" }
+ let(:salt) { "salt" }
+ let(:sha1_hash) { Digest::SHA1.hexdigest("--#{salt}--#{password}--") }
+
+ context 'with a SHA1-encrypted password' do
+ before do
+ subject.salt = salt
+ subject.encrypted_password = sha1_hash
+ end
+
+ it "is authenticated" do
+ subject.should be_authenticated(password)
+ end
+
+ it "changes the hash into a BCrypt-encrypted one" do
+ subject.authenticated?(password)
+ subject.encrypted_password.should_not == sha1_hash
+ end
+ end
+
+ context "with a BCrypt-encrypted password" do
+ let(:bcrypt_hash) { ::BCrypt::Password.create(password) }
+
+ before do
+ subject.encrypted_password = bcrypt_hash
+ end
+
+ it "is authenticated" do
+ subject.should be_authenticated(password)
+ end
+
+ it "does not change the hash" do
+ subject.authenticated?(password)
+ subject.encrypted_password.to_s.should == bcrypt_hash.to_s
+ end
+ end
+ end
+end
View
41 spec/models/bcrypt_spec.rb
@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+describe Clearance::PasswordStrategies::BCrypt do
+ subject do
+ Class.new do
+ attr_accessor :encrypted_password
+ include Clearance::PasswordStrategies::BCrypt
+ end.new
+ end
+
+ describe "#password=" do
+ let(:password) { "password" }
+ let(:encrypted_password) { stub("encrypted password") }
+
+ before do
+ BCrypt::Password.stubs(:create => encrypted_password)
+
+ subject.password = password
+ end
+
+ it "encrypts the password into encrypted_password" do
+ subject.encrypted_password.should == encrypted_password
+ end
+
+ it "encrypts with BCrypt" do
+ BCrypt::Password.should have_received(:create).with(password)
+ end
+ end
+
+ describe "#authenticated?" do
+ let(:password) { "password" }
+
+ before do
+ subject.password = password
+ end
+
+ it "is authenticated with BCrypt" do
+ subject.should be_authenticated(password)
+ end
+ end
+end
View
15 spec/models/blowfish_spec.rb
@@ -3,14 +3,14 @@
describe Clearance::PasswordStrategies::Blowfish do
subject do
Class.new do
- attr_accessor :salt, :password, :encrypted_password
+ attr_accessor :salt, :encrypted_password
include Clearance::PasswordStrategies::Blowfish
def generate_random_code; "code"; end
end.new
end
- describe "#encrypt_password" do
+ describe "#password=" do
context "when the password is set" do
let(:salt) { "salt" }
let(:password) { "password" }
@@ -18,14 +18,17 @@ def generate_random_code; "code"; end
before do
subject.salt = salt
subject.password = password
- subject.send(:encrypt_password)
end
- it "should encrypt the password using Blowfish into encrypted_password" do
+ it "doesn't initialize the salt" do
+ subject.salt.should == salt
+ end
+
+ it "encrypts the password using Blowfish into encrypted_password" do
cipher = OpenSSL::Cipher::Cipher.new('bf-cbc').encrypt
cipher.key = Digest::SHA256.digest(salt)
expected = cipher.update("--#{salt}--#{password}--") << cipher.final
-
+
subject.encrypted_password.should == expected
end
end
@@ -34,7 +37,7 @@ def generate_random_code; "code"; end
before do
subject.salt = nil
- subject.send(:encrypt_password)
+ subject.password = 'whatever'
end
it "should initialize the salt" do
View
4 spec/models/clearance_user_spec.rb → spec/models/password_strategies_spec.rb
@@ -26,8 +26,8 @@ def self.before_create(*args); end
describe "when Clearance.configuration.password_strategy is not set" do
before { Clearance.configuration.password_strategy = nil }
- it "includes Clearance::PasswordStrategies::SHA1" do
- subject.should be_kind_of(Clearance::PasswordStrategies::SHA1)
+ it "includes Clearance::PasswordStrategies::BCrypt" do
+ subject.should be_kind_of(Clearance::PasswordStrategies::BCrypt)
end
end
end
View
28 spec/models/sha1_spec.rb
@@ -3,41 +3,47 @@
describe Clearance::PasswordStrategies::SHA1 do
subject do
Class.new do
- attr_accessor :salt, :password, :encrypted_password
+ attr_accessor :salt, :encrypted_password
include Clearance::PasswordStrategies::SHA1
def generate_random_code; "code"; end
end.new
end
- describe "#encrypt_password" do
+ describe "#password=" do
context "when the password is set" do
- let(:salt) { "salt" }
+ let(:salt) { "salt" }
let(:password) { "password" }
before do
- subject.salt = salt
+ subject.salt = salt
subject.password = password
- subject.send(:encrypt_password)
end
- it "should encrypt the password using SHA1 into encrypted_password" do
+ it "doesn't initialize the salt" do
+ subject.salt.should == salt
+ end
+
+ it "encrypts the password using SHA1 and the existing salt into encrypted_password" do
expected = Digest::SHA1.hexdigest("--#{salt}--#{password}--")
subject.encrypted_password.should == expected
end
end
- context "when the salt is not set" do
+ context "when the password is not set" do
before do
- subject.salt = nil
-
- subject.send(:encrypt_password)
+ subject.salt = nil
+ subject.password = ""
end
- it "should initialize the salt" do
+ it "initializes the salt" do
subject.salt.should_not be_nil
end
+
+ it "doesn't encrpt the password" do
+ subject.encrypted_password.should be_nil
+ end
end
end
end
View
9 spec/models/user_spec.rb
@@ -98,7 +98,7 @@
@user.forgot_password!
end
- it "should generate confirmation token" do
+ it "generates confirmation token" do
@user.confirmation_token.should_not be_nil
end
@@ -108,11 +108,11 @@
@user.update_password("new_password")
end
- it "should change encrypted password" do
+ it "changes encrypted password" do
@user.encrypted_password.should_not == @old_encrypted_password
end
- it "should clear confirmation token" do
+ it "clears confirmation token" do
@user.confirmation_token.should be_nil
end
end
@@ -182,9 +182,8 @@ def password_optional?
@user.reload.remember_token.should be_nil
end
- it "should initialize salt, generate remember token, and save encrypted password on update_password" do
+ it "should generate remember token and save encrypted password on update_password" do
@user.update_password('password')
- @user.salt.should_not be_nil
@user.encrypted_password.should_not be_nil
@user.remember_token.should_not be_nil
end
Please sign in to comment.
Something went wrong with that request. Please try again.