diff --git a/Changelog.md b/Changelog.md index 2b8f903..4159255 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,8 @@ * Support Rails 4.1 - 7.0, dropped support for Rails <= 4.0. (same support as Devise as of v4.8) * Support Ruby 2.1 - 3.1, dropped support for Ruby <= 2.0. (same support as Devise as of v4.8) +* Add support to roll legacy encryptors +* Add support for rolling from Devise BCrypt ### 0.2.0 diff --git a/lib/devise/encryptable/encryptable.rb b/lib/devise/encryptable/encryptable.rb index cfd5af3..441ed4e 100644 --- a/lib/devise/encryptable/encryptable.rb +++ b/lib/devise/encryptable/encryptable.rb @@ -13,11 +13,17 @@ module Devise mattr_accessor :encryptor @@encryptor = nil + # Used to define a legacy algorithm that needs to be rolled to a new one. + mattr_accessor :transition_from_encryptor + @@transition_from_encryptor = nil + module Encryptable module Encryptors autoload :AuthlogicSha512, 'devise/encryptable/encryptors/authlogic_sha512' autoload :Base, 'devise/encryptable/encryptors/base' autoload :ClearanceSha1, 'devise/encryptable/encryptors/clearance_sha1' + autoload :DeviseBcrypt, 'devise/encryptable/encryptors/devise_bcrypt' + autoload :Pbkdf2, 'devise/encryptable/encryptors/pbkdf2' autoload :RestfulAuthenticationSha1, 'devise/encryptable/encryptors/restful_authentication_sha1' autoload :Sha1, 'devise/encryptable/encryptors/sha1' autoload :Sha512, 'devise/encryptable/encryptors/sha512' @@ -25,4 +31,4 @@ module Encryptors end end -Devise.add_module(:encryptable, :model => 'devise/encryptable/model') \ No newline at end of file +Devise.add_module(:encryptable, :model => 'devise/encryptable/model') diff --git a/lib/devise/encryptable/encryptors/devise_bcrypt.rb b/lib/devise/encryptable/encryptors/devise_bcrypt.rb new file mode 100644 index 0000000..2c767af --- /dev/null +++ b/lib/devise/encryptable/encryptors/devise_bcrypt.rb @@ -0,0 +1,33 @@ +require 'bcrypt' + +begin + module Devise + module Encryptable + module Encryptors + # Adapted from + # https://github.com/heartcombo/devise/blob/8593801130f2df94a50863b5db535c272b00efe1/lib/devise/encryptor.rb + class DeviseBcrypt < Base + def self.compare(hashed_password, password, stretches, _salt, pepper) + return false if hashed_password.blank? + bcrypt = ::BCrypt::Password.new(hashed_password) + if pepper.present? + password = "#{password}#{pepper}" + end + password = ::BCrypt::Engine.hash_secret(password, bcrypt.salt) + Devise.secure_compare(password, hashed_password) + rescue BCrypt::Errors::InvalidHash + # this probably means the password has already been migrated + false + end + + def self.digest(password, stretches, _salt, pepper) + if pepper.present? + password = "#{password}#{pepper}" + end + ::BCrypt::Password.create(password, cost: stretches).to_s + end + end + end + end + end +end diff --git a/lib/devise/encryptable/encryptors/pbkdf2.rb b/lib/devise/encryptable/encryptors/pbkdf2.rb new file mode 100644 index 0000000..cf0a3d8 --- /dev/null +++ b/lib/devise/encryptable/encryptors/pbkdf2.rb @@ -0,0 +1,25 @@ +begin + module Devise + module Encryptable + module Encryptors + class Pbkdf2 < Base + def self.compare(encrypted_password, password, stretches, salt, pepper) + value_to_test = self.digest(password, stretches, salt, pepper) + Devise.secure_compare(encrypted_password, value_to_test) + end + + def self.digest(password, stretches, salt, pepper) + hash = OpenSSL::Digest.new('SHA512').new + OpenSSL::KDF.pbkdf2_hmac( + password.to_s, + salt: "#{[salt].pack('H*')}#{pepper}", + iterations: stretches, + hash: hash, + length: hash.digest_length, + ).unpack1('H*') + end + end + end + end + end +end diff --git a/lib/devise/encryptable/model.rb b/lib/devise/encryptable/model.rb index c84beca..b23f552 100644 --- a/lib/devise/encryptable/model.rb +++ b/lib/devise/encryptable/model.rb @@ -13,9 +13,13 @@ module Models # # * +encryptor+: the encryptor going to be used. By default is nil. # + # * +transition_from_encryptor+: the legacy encryptor that needs to rolled to the +encryptor+. + # # == Examples # - # User.find(1).valid_password?('password123') # returns true/false + # User.find(1).valid_password?('password123') + # #=> returns true/false + # #=> if true and using a legacy encryptor, it will update the user to the new encryptor. # module Encryptable extend ActiveSupport::Concern @@ -38,7 +42,21 @@ def password=(new_password) # Validates the password considering the salt. def valid_password?(password) return false if encrypted_password.blank? - encryptor_class.compare(encrypted_password, password, self.class.stretches, authenticatable_salt, self.class.pepper) + + encryptor_arguments = [ + encrypted_password, + password, + self.class.stretches, + authenticatable_salt, + self.class.pepper + ] + + if transition_from_encryptor_class.try(:compare, *encryptor_arguments) + update_attribute(:password, password) + return true + end + + encryptor_class.compare(*encryptor_arguments) end # Overrides authenticatable salt to use the new password_salt @@ -62,24 +80,40 @@ def encryptor_class self.class.encryptor_class end + def transition_from_encryptor_class + self.class.transition_from_encryptor_class + end + module ClassMethods Devise::Models.config(self, :encryptor) + Devise::Models.config(self, :transition_from_encryptor) # Returns the class for the configured encryptor. def encryptor_class - @encryptor_class ||= case encryptor - when :bcrypt - raise "In order to use bcrypt as encryptor, simply remove :encryptable from your devise model" - when nil - raise "You need to give an :encryptor as option in order to use :encryptable" - else - Devise::Encryptable::Encryptors.const_get(encryptor.to_s.classify) - end + @encryptor_class ||= compute_encryptor_class(encryptor) + end + + # Returns the class for the configured transition from encryptor. + def transition_from_encryptor_class + @transition_from_encryptor_class ||= compute_encryptor_class(transition_from_encryptor) if transition_from_encryptor end def password_salt self.encryptor_class.salt(self.stretches) end + + private + + def compute_encryptor_class(encryptor) + case encryptor + when :bcrypt + raise "In order to use bcrypt as encryptor, simply remove :encryptable from your devise model" + when nil + raise "You need to give an :encryptor as option in order to use :encryptable" + else + Devise::Encryptable::Encryptors.const_get(encryptor.to_s.classify) + end + end end end end diff --git a/test/devise/encryptable/encryptable_test.rb b/test/devise/encryptable/encryptable_test.rb index 68bb125..3e474d6 100644 --- a/test/devise/encryptable/encryptable_test.rb +++ b/test/devise/encryptable/encryptable_test.rb @@ -48,6 +48,21 @@ def encrypt_password(admin, pepper=Admin.pepper, stretches=Admin.stretches, encr assert_not_equal encrypted_password, admin.encrypted_password end + test 'should roll from legacy encryptor to the new one' do + admin = nil + + swap_with_encryptor Admin, :sha1 do + admin = create_admin + assert admin.valid_password?('123456') + assert_equal admin.encrypted_password, encrypt_password(admin, Admin.pepper, Admin.stretches, Devise::Encryptable::Encryptors::Sha1) + end + + swap_with_encryptor Admin, :sha512, transition_from_encryptor: :sha1 do + assert admin.valid_password?('123456') + assert_equal admin.encrypted_password, encrypt_password(admin, Admin.pepper, Admin.stretches, Devise::Encryptable::Encryptors::Sha512) + end + end + test 'should respect encryptor configuration' do swap_with_encryptor Admin, :sha512 do admin = create_admin diff --git a/test/devise/encryptable/encryptors_test.rb b/test/devise/encryptable/encryptors_test.rb index 49a9323..35b6ced 100644 --- a/test/devise/encryptable/encryptors_test.rb +++ b/test/devise/encryptable/encryptors_test.rb @@ -1,4 +1,5 @@ require "test_helper" +require "ostruct" class Encryptors < ActiveSupport::TestCase include Support::Swappers @@ -21,6 +22,44 @@ class Encryptors < ActiveSupport::TestCase assert_equal clearance, encryptor end + test 'Devise can verify passwords generated by DeviseBcrypt' do + password = '123mudar' + klass = OpenStruct.new(pepper: '65c58472c207c829f28c68619d3e3aefed18ab3f', stretches: 10) + + hashed_password = Devise::Encryptable::Encryptors::DeviseBcrypt.digest(password, klass.stretches, nil, klass.pepper) + + assert Devise::Encryptor.compare(klass, hashed_password, password) + end + + test 'DeviseBcrypt can verify bcrypt hashes created by Devise' do + password = '123mudar' + klass = OpenStruct.new(pepper: '65c58472c207c829f28c68619d3e3aefed18ab3f', stretches: 10) + + devise_hash = Devise::Encryptor.digest(klass, password) + + assert Devise::Encryptable::Encryptors::DeviseBcrypt.compare(devise_hash, password, klass.stretches, nil, klass.pepper) + end + + test 'Devise fails to verify passwords generated by DeviseBcrypt when the password is wrong' do + password = '123mudar' + different_password = 'theskyisfalling123' + klass = OpenStruct.new(pepper: '65c58472c207c829f28c68619d3e3aefed18ab3f', stretches: 10) + + hashed_password = Devise::Encryptable::Encryptors::DeviseBcrypt.digest(password, klass.stretches, nil, klass.pepper) + + refute Devise::Encryptor.compare(klass, hashed_password, different_password) + end + + test 'DeviseBcrypt fails to verify bcrypt hashes created by Devise when the password is wrong' do + password = '123mudar' + different_password = 'theskyisfalling123' + klass = OpenStruct.new(pepper: '65c58472c207c829f28c68619d3e3aefed18ab3f', stretches: 10) + + devise_hash = Devise::Encryptor.digest(klass, password) + + refute Devise::Encryptable::Encryptors::DeviseBcrypt.compare(devise_hash, different_password, klass.stretches, nil, klass.pepper) + end + test 'digest should raise NotImplementedError if not implemented in subclass' do c = Class.new(Devise::Encryptable::Encryptors::Base) assert_raise(NotImplementedError) do diff --git a/test/support/swappers.rb b/test/support/swappers.rb index a2d8715..acabca6 100644 --- a/test/support/swappers.rb +++ b/test/support/swappers.rb @@ -2,6 +2,7 @@ module Support module Swappers def swap_with_encryptor(klass, encryptor, options={}) klass.instance_variable_set(:@encryptor_class, nil) + klass.instance_variable_set(:@transition_from_encryptor, nil) swap klass, options.merge(:encryptor => encryptor) do begin