diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 11c99c929e2a5..7cc207204e63b 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -29,14 +29,23 @@ module ClassMethods # it). When this attribute has a +nil+ value, the validation will not be # triggered. # - # For further customizability, it is possible to suppress the default - # validations by passing validations: false as an argument. + # Additionally, a +XXX_challenge+ attribute is created. When set to a + # value other than +nil+, it will validate against the currently persisted + # password. This validation relies on dirty tracking, as provided by + # ActiveModel::Dirty; if dirty tracking methods are not defined, this + # validation will fail. # - # Add bcrypt (~> 3.1.7) to Gemfile to use #has_secure_password: + # All of the above validations can be omitted by passing + # validations: false as an argument. This allows complete + # customizability of validation behavior. + # + # To use +has_secure_password+, add bcrypt (~> 3.1.7) to your Gemfile: # # gem 'bcrypt', '~> 3.1.7' # - # Example using Active Record (which automatically includes ActiveModel::SecurePassword): + # ==== Examples + # + # Using Active Record, which automatically includes ActiveModel::SecurePassword: # # # Schema: User(name:string, password_digest:string, recovery_password_digest:string) # class User < ActiveRecord::Base @@ -44,20 +53,30 @@ module ClassMethods # has_secure_password :recovery_password, validations: false # end # - # user = User.new(name: 'david', password: '', password_confirmation: 'nomatch') - # user.save # => false, password required - # user.password = 'mUc3m00RsqyRe' - # user.save # => false, confirmation doesn't match - # user.password_confirmation = 'mUc3m00RsqyRe' - # user.save # => true + # user = User.new(name: "david", password: "", password_confirmation: "nomatch") + # + # user.save # => false, password required + # user.password = "vr00m" + # user.save # => false, confirmation doesn't match + # user.password_confirmation = "vr00m" + # user.save # => true + # + # user.authenticate("notright") # => false + # user.authenticate("vr00m") # => user + # User.find_by(name: "david")&.authenticate("notright") # => false + # User.find_by(name: "david")&.authenticate("vr00m") # => user + # # user.recovery_password = "42password" - # user.recovery_password_digest # => "$2a$04$iOfhwahFymCs5weB3BNH/uXkTG65HR.qpW.bNhEjFP3ftli3o5DQC" - # user.save # => true - # user.authenticate('notright') # => false - # user.authenticate('mUc3m00RsqyRe') # => user - # user.authenticate_recovery_password('42password') # => user - # User.find_by(name: 'david')&.authenticate('notright') # => false - # User.find_by(name: 'david')&.authenticate('mUc3m00RsqyRe') # => user + # user.recovery_password_digest # => "$2a$04$iOfhwahFymCs5weB3BNH/uXkTG65HR.qpW.bNhEjFP3ftli3o5DQC" + # user.save # => true + # + # user.authenticate_recovery_password("42password") # => user + # + # user.update(password: "pwn3d", password_challenge: "") # => false, challenge doesn't authenticate + # user.update(password: "nohack4u", password_challenge: "vr00m") # => true + # + # user.authenticate("vr00m") # => false, old password + # user.authenticate("nohack4u") # => user def has_secure_password(attribute = :password, validations: true) # Load bcrypt gem only when has_secure_password is used. # This is to avoid ActiveModel (and by extension the entire framework) @@ -65,7 +84,7 @@ def has_secure_password(attribute = :password, validations: true) begin require "bcrypt" rescue LoadError - $stderr.puts "You don't have bcrypt installed in your application. Please add it to your Gemfile and run bundle install" + $stderr.puts "You don't have bcrypt installed in your application. Please add it to your Gemfile and run bundle install." raise end @@ -82,6 +101,16 @@ def has_secure_password(attribute = :password, validations: true) record.errors.add(attribute, :blank) unless record.public_send("#{attribute}_digest").present? end + validate do |record| + if challenge = record.public_send(:"#{attribute}_challenge") + digest_was = record.public_send(:"#{attribute}_digest_was") if record.respond_to?(:"#{attribute}_digest_was") + + unless digest_was.present? && BCrypt::Password.new(digest_was).is_password?(challenge) + record.errors.add(:"#{attribute}_challenge") + end + end + end + validates_length_of attribute, maximum: ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED validates_confirmation_of attribute, allow_blank: true end @@ -103,9 +132,7 @@ def initialize(attribute) end end - define_method("#{attribute}_confirmation=") do |unencrypted_password| - instance_variable_set("@#{attribute}_confirmation", unencrypted_password) - end + attr_accessor :"#{attribute}_confirmation", :"#{attribute}_challenge" # Returns +self+ if the password is correct, otherwise +false+. # diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index fab5589bf9fd3..c404561f33bad 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -16,6 +16,7 @@ class SecurePasswordTest < ActiveModel::TestCase # Simulate loading an existing user from the DB @existing_user = User.new @existing_user.password_digest = BCrypt::Password.create("password", cost: BCrypt::Engine::MIN_COST) + @existing_user.changes_applied end teardown do @@ -166,6 +167,47 @@ class SecurePasswordTest < ActiveModel::TestCase assert_equal ["doesn't match Password"], @existing_user.errors[:password_confirmation] end + test "updating an existing user with validation and a correct password challenge" do + @existing_user.password = "new password" + @existing_user.password_challenge = "password" + assert @existing_user.valid?(:update), "user should be valid" + end + + test "updating an existing user with validation and a nil password challenge" do + @existing_user.password = "new password" + @existing_user.password_challenge = nil + assert @existing_user.valid?(:update), "user should be valid" + end + + test "updating an existing user with validation and a blank password challenge" do + @existing_user.password = "new password" + @existing_user.password_challenge = "" + assert_not @existing_user.valid?(:update), "user should be invalid" + assert_equal 1, @existing_user.errors.count + assert_equal ["is invalid"], @existing_user.errors[:password_challenge] + end + + test "updating an existing user with validation and an incorrect password challenge" do + @existing_user.password = "new password" + @existing_user.password_challenge = "new password" + assert_not @existing_user.valid?(:update), "user should be invalid" + assert_equal 1, @existing_user.errors.count + assert_equal ["is invalid"], @existing_user.errors[:password_challenge] + end + + test "updating a user without dirty tracking and a correct password challenge" do + validatable_visitor = Class.new(Visitor) do + attr_accessor :untracked_digest + has_secure_password :untracked + end.new + + validatable_visitor.untracked = "password" + assert validatable_visitor.valid?(:update), "user should be valid" + + validatable_visitor.untracked_challenge = "password" + assert_not validatable_visitor.valid?(:update), "user should be invalid" + end + test "updating an existing user with validation and a blank password digest" do @existing_user.password_digest = "" assert_not @existing_user.valid?(:update), "user should be invalid" diff --git a/activemodel/test/models/user.rb b/activemodel/test/models/user.rb index fc4a9e433489b..b607308c188fc 100644 --- a/activemodel/test/models/user.rb +++ b/activemodel/test/models/user.rb @@ -2,14 +2,18 @@ class User extend ActiveModel::Callbacks + include ActiveModel::Attributes + include ActiveModel::Dirty include ActiveModel::SecurePassword define_model_callbacks :create + attribute :password_digest has_secure_password + + attribute :recovery_password_digest has_secure_password :recovery_password, validations: false - attr_accessor :password_digest, :recovery_password_digest attr_accessor :password_called def password=(unencrypted_password)