diff --git a/active_model_otp.gemspec b/active_model_otp.gemspec index 66fc920..759ceb8 100644 --- a/active_model_otp.gemspec +++ b/active_model_otp.gemspec @@ -21,6 +21,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 2.3" spec.add_dependency "activemodel" + spec.add_dependency "bcrypt", "~> 3.1" spec.add_dependency "rotp", "~> 6.2.0" spec.add_development_dependency "activerecord" diff --git a/lib/active_model/one_time_password.rb b/lib/active_model/one_time_password.rb index c926abc..e6a1b5d 100644 --- a/lib/active_model/one_time_password.rb +++ b/lib/active_model/one_time_password.rb @@ -2,6 +2,11 @@ module ActiveModel module OneTimePassword extend ActiveSupport::Concern + class << self + attr_accessor :min_bcrypt_cost # :nodoc: + end + self.min_bcrypt_cost = false + OTP_DEFAULT_COLUMN_NAME = 'otp_secret_key'.freeze OTP_DEFAULT_COUNTER_COLUMN_NAME = 'otp_counter'.freeze OTP_DEFAULT_BACKUP_CODES_COLUMN_NAME = 'otp_backup_codes'.freeze @@ -9,6 +14,7 @@ module OneTimePassword OTP_DEFAULT_BACKUP_CODES_COUNT = 12 OTP_COUNTER_ENABLED_BY_DEFAULT = false OTP_BACKUP_CODES_ENABLED_BY_DEFAULT = false + OTP_BACKUP_CODES_ENCRYPTED_BY_DEFAULT = true module ClassMethods def has_one_time_password(options = {}) @@ -16,7 +22,7 @@ def has_one_time_password(options = {}) :otp_backup_codes_column_name, :otp_after_column_name class_attribute :otp_digits, :otp_counter_based, :otp_backup_codes_count, :otp_one_time_backup_codes, - :otp_interval + :otp_interval, :otp_backup_codes_encrypted self.otp_column_name = (options[:column_name] || OTP_DEFAULT_COLUMN_NAME).to_s self.otp_digits = options[:length] || OTP_DEFAULT_DIGITS @@ -27,13 +33,14 @@ def has_one_time_password(options = {}) self.otp_backup_codes_column_name = (options[:backup_codes_column_name] || OTP_DEFAULT_BACKUP_CODES_COLUMN_NAME).to_s self.otp_backup_codes_count = options[:backup_codes_count] || OTP_DEFAULT_BACKUP_CODES_COUNT self.otp_one_time_backup_codes = options[:one_time_backup_codes] || OTP_BACKUP_CODES_ENABLED_BY_DEFAULT + self.otp_backup_codes_encrypted = options.fetch(:backup_codes_encrypted, OTP_BACKUP_CODES_ENCRYPTED_BY_DEFAULT) include InstanceMethodsOnActivation before_create(**options.slice(:if, :unless)) do self.otp_regenerate_secret if !otp_column self.otp_regenerate_counter if otp_counter_based && !otp_counter - otp_regenerate_backup_codes if backup_codes_enabled? + self.otp_regenerate_backup_codes if backup_codes_enabled? end if respond_to?(:attributes_protected_by_default) @@ -51,6 +58,8 @@ def otp_random_secret(length = 20) end module InstanceMethodsOnActivation + attr_accessor :plain_backup_codes + def otp_regenerate_secret self.otp_column = self.class.otp_random_secret end @@ -127,6 +136,15 @@ def otp_regenerate_backup_codes otp.generate_otp((SecureRandom.random_number(9e5) + 1e5).to_i) end + if self.class.otp_backup_codes_encrypted + self.plain_backup_codes = backup_codes + + backup_codes = backup_codes.map do |code| + cost = ActiveModel::OneTimePassword.min_bcrypt_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost + BCrypt::Password.create(code, cost: cost) + end + end + public_send("#{self.class.otp_backup_codes_column_name}=", backup_codes) end @@ -180,10 +198,18 @@ def totp_code(options = {}) def authenticate_backup_code(code) backup_codes_column_name = self.class.otp_backup_codes_column_name backup_codes = public_send(backup_codes_column_name) - return false unless backup_codes.present? && backup_codes.include?(code) + + return false unless backup_codes.present? + + valid_code = backup_codes.find do |backup_code| + backup_code = BCrypt::Password.new(backup_code) if self.class.otp_backup_codes_encrypted + backup_code == code + end + + return false unless valid_code if self.class.otp_one_time_backup_codes - backup_codes.delete(code) + backup_codes.delete(valid_code) public_send("#{backup_codes_column_name}=", backup_codes) save if respond_to?(:changed?) && !new_record? end diff --git a/lib/active_model_otp.rb b/lib/active_model_otp.rb index e0524c0..0d0e53b 100644 --- a/lib/active_model_otp.rb +++ b/lib/active_model_otp.rb @@ -1,5 +1,6 @@ require "active_model" require "active_support/core_ext/module/attribute_accessors" +require "bcrypt" require "cgi" require "rotp" require "active_model/one_time_password" diff --git a/test/models/user.rb b/test/models/user.rb index 4c2659b..25e26f1 100644 --- a/test/models/user.rb +++ b/test/models/user.rb @@ -7,7 +7,7 @@ class User define_model_callbacks :create attr_accessor :otp_secret_key, :otp_backup_codes, :email - has_one_time_password one_time_backup_codes: true + has_one_time_password one_time_backup_codes: true, backup_codes_encrypted: false def attributes { "otp_secret_key" => otp_secret_key, "email" => email } diff --git a/test/models/user_with_encrypted_codes.rb b/test/models/user_with_encrypted_codes.rb new file mode 100644 index 0000000..c111256 --- /dev/null +++ b/test/models/user_with_encrypted_codes.rb @@ -0,0 +1,11 @@ +class UserWithEncryptedCodes + extend ActiveModel::Callbacks + include ActiveModel::Serializers::JSON + include ActiveModel::Validations + include ActiveModel::OneTimePassword + + define_model_callbacks :create + attr_accessor :otp_secret_key, :otp_backup_codes, :email + + has_one_time_password backup_codes_encrypted: true +end diff --git a/test/one_time_password_test.rb b/test/one_time_password_test.rb index f3e72bd..3d1c46a 100644 --- a/test/one_time_password_test.rb +++ b/test/one_time_password_test.rb @@ -29,6 +29,10 @@ def setup @after_user = AfterUser.new @after_user.email = 'roberto@heapsource.com' @after_user.run_callbacks :create + + @user_with_encrypted_code = UserWithEncryptedCodes.new + @user_with_encrypted_code.email = 'roberto@heapsource.com' + @user_with_encrypted_code.run_callbacks :create end def test_authenticate_with_otp @@ -112,13 +116,22 @@ def test_authenticate_with_backup_code backup_code = @user.public_send(@user.otp_backup_codes_column_name).last @user.otp_regenerate_backup_codes - assert_equal true, !@user.authenticate_otp(backup_code) + assert_equal false, @user.authenticate_otp(backup_code) + end + + def test_authenticate_with_encrypted_backup_code + backup_code = @user_with_encrypted_code.plain_backup_codes.first + assert_equal true, @user_with_encrypted_code.authenticate_otp(backup_code) + + backup_code = @user_with_encrypted_code.plain_backup_codes.last + @user_with_encrypted_code.otp_regenerate_backup_codes + assert_equal false, @user.authenticate_otp(backup_code) end def test_authenticate_with_one_time_backup_code backup_code = @user.public_send(@user.otp_backup_codes_column_name).first assert_equal true, @user.authenticate_otp(backup_code) - assert_equal true, !@user.authenticate_otp(backup_code) + assert_equal false, @user.authenticate_otp(backup_code) end def test_otp_code diff --git a/test/test_helper.rb b/test/test_helper.rb index aa398b3..f5c7f32 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -19,4 +19,6 @@ ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:" load "#{ File.dirname(__FILE__) }/schema.rb" +ActiveModel::OneTimePassword.min_bcrypt_cost = true + Dir["models/*.rb"].each {|file| require file }