Ruby on Rails: Why does the user's encrypted password get changed in the DB when I use the "toggle" method on the admin attribute?

I just finished Hartl's Rails Tutorial book and I'm using his account authentication logic in my first rails app. However, when I make a new user account and set it as an admin account by toggling the admin attribute in the console (e.g. User.find(5).toggle!(:admin)), the encrypted password that is stored in the DB gets modified. Why?

Here's the user model logic...

class User < ActiveRecord::Base

  #virtual attributes
  attr_accessor :password           

  #attrs modifiable by the outside world
  attr_accessible :name, :email, :password, :password_confirmation    

  email_regex = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i

  #validations
  validates :name,  :presence => true,
                    :length   => { :maximum => 50 }
  validates :email, :presence => true,
                    :format   => { :with => email_regex },
                    :uniqueness => true
  validates :password,  :presence     => true,
                        :confirmation => true,
                        :length       => { :within => 6..40 }

  #class method that authenticates a user, used to create a session cookie
  def self.authenticate(email, submitted_password)
    user = find_by_email(email)
    return nil if user.nil?
    return user if user.has_password?(submitted_password)
  end

  #used to authenticate a signed user from a signed cookie 
  def self.authenticate_with_salt(id, cookie_salt)
    user = find_by_id(id)
    return nil if user.nil?
    return user if user.salt == cookie_salt
  end

  #callback that occurs before a record is successfully saved (meaning it has a valud password)                      
  before_save :encrypt_password

  def has_password?(submitted_password)
    encrypted_password == encrypt(submitted_password)
  end

  private

    #self keyword is required when assigning to a instance attribute
    def encrypt_password
      self.salt = make_salt if new_record?
      self.encrypted_password = encrypt(password)
    end

    def encrypt(string)
      secure_hash("#{salt}--#{string}")
    end

    def make_salt
      secure_hash("#{Time.now.utc}--#{password}")      
    end

    def secure_hash(string)
      Digest::SHA2.hexdigest(string)
    end
end

and here's what the behavior looks like...

ruby-1.9.2-p180 :018 > User.last
 => #<User id: 12, name: "Test User A", email: "testusera@website.com", created_at: "2011-03-28 17:47:42", updated_at: "2011-03-28 17:47:42", salt: "23ca99e9848336a078e05ce9a8d904f9dfdff30dc7a38586f22...", admin: false, monthly_score: nil, encrypted_password: "50d17e6d6b581cfcfe84b61feb318705978cdf6c435626d10aa..."> 

ruby-1.9.2-p180 :019 > User.last.toggle!(:admin)
 => true 

ruby-1.9.2-p180 :020 > User.last
 => #<User id: 12, name: "Test User A", email: "testusera@website.com", created_at: "2011-03-28 17:47:42", updated开发者_如何学Go_at: "2011-03-28 17:49:06", salt: "23ca99e9848336a078e05ce9a8d904f9dfdff30dc7a38586f22...", admin: true, monthly_score: nil, encrypted_password: "5d6e17f7aa73925a0099da45807f5994fa8c368a5a12d187a7d...">

Thanks so much for your help!

---

try to change your before_save method:

def encrypt_password
  if password.present?
    self.salt = make_salt if new_record?
    self.encrypted_password = encrypt(password)
  end
end

UPD. or you can make it little shorter

def encrypt_password
  self.salt = make_salt if new_record?
  self.encrypted_password = encrypt(password) if password.present?
end

---

Not sure what is going on specifically. Could be that the password is getting encrypted again using a different salt (which is based on the time). Try adding debugger to the first line of encrypt_password and then run the same code from the console to see if the password is getting encrypted when you run toggle!

---

There's something odd going on in your code. A salt should be independent of the password, but your (Hartl's?) make_salt method says:

def make_salt
  secure_hash("#{Time.now.utc}--#{password}")      
end

This might have been the source of your nil problem, since you were accessing password inside make_salt; in any case this is bad crypto since it amounts to using Time.now as a "random" salt, which is much easier to crack (build rainbow tables for).

You should instead be using a good random number generator, e.g. Ruby's built-in SecureRandom:

def make_salt
  SecureRandom.hex(64)
end

Why such a long salt? According to https://crackstation.net/hashing-security.htm, "To make it impossible for an attacker to create a lookup table for every possible salt, the salt must be long. A good rule of thumb is to use a salt that is the same size as the output of the hash function. For example, the output of SHA256 is 256 bits (32 bytes), so the salt should be at least 32 random bytes." I don't want to use SecureRandom.random_bytes(32) to avoid potential database string encoding problems with non-ascii characters, and 64 random hex characters comprise 32 random bytes, which I think counts as the same entropy.