Commit 74d6f92d54feaf59525695b650197903e9fe5724
1 parent
b4d9cf76
Exists in
master
and in
28 other branches
ActionItem44: changing user to accomodate different types of encryption
for password git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@1849 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
2 changed files
with
141 additions
and
6 deletions
Show diff stats
app/models/user.rb
| ... | ... | @@ -52,18 +52,63 @@ class User < ActiveRecord::Base |
| 52 | 52 | u && u.authenticated?(password) ? u : nil |
| 53 | 53 | end |
| 54 | 54 | |
| 55 | - # Encrypts some data with the salt. | |
| 56 | - def self.encrypt(password, salt) | |
| 57 | - Digest::SHA1.hexdigest("--#{salt}--#{password}--") | |
| 55 | + class UnsupportedEncryptionType < Exception; end | |
| 56 | + | |
| 57 | + def self.system_encryption_method | |
| 58 | + @system_encryption_method || :salted_sha1 | |
| 59 | + end | |
| 60 | + | |
| 61 | + def self.system_encryption_method=(method) | |
| 62 | + @system_encryption_method = method | |
| 63 | + end | |
| 64 | + | |
| 65 | + # a Hash containing the available encryption methods. Keys are symbols, | |
| 66 | + # values are Proc objects that contain the actual encryption code. | |
| 67 | + def self.encryption_methods | |
| 68 | + @encryption_methods ||= {} | |
| 69 | + end | |
| 70 | + | |
| 71 | + # adds a new encryption method. | |
| 72 | + def self.add_encryption_method(sym, &block) | |
| 73 | + encryption_methods[sym] = block | |
| 74 | + end | |
| 75 | + | |
| 76 | + # the encryption method used for this instance | |
| 77 | + def encryption_method | |
| 78 | + (password_type || User.system_encryption_method).to_sym | |
| 58 | 79 | end |
| 59 | 80 | |
| 60 | - # Encrypts the password with the user salt | |
| 81 | + # Encrypts the password using the chosen method | |
| 61 | 82 | def encrypt(password) |
| 62 | - self.class.encrypt(password, salt) | |
| 83 | + method = self.class.encryption_methods[encryption_method] | |
| 84 | + if method | |
| 85 | + method.call(password, salt) | |
| 86 | + else | |
| 87 | + raise UnsupportedEncryptionType, "Unsupported encryption type: #{encryption_method}" | |
| 88 | + end | |
| 89 | + end | |
| 90 | + | |
| 91 | + add_encryption_method :salted_sha1 do |password, salt| | |
| 92 | + Digest::SHA1.hexdigest("--#{salt}--#{password}--") | |
| 93 | + end | |
| 94 | + | |
| 95 | + add_encryption_method :md5 do |password, salt| | |
| 96 | + Digest::MD5.hexdigest(password) | |
| 97 | + end | |
| 98 | + | |
| 99 | + add_encryption_method :clear do |password, salt| | |
| 100 | + password | |
| 63 | 101 | end |
| 64 | 102 | |
| 65 | 103 | def authenticated?(password) |
| 66 | - crypted_password == encrypt(password) | |
| 104 | + result = (crypted_password == encrypt(password)) | |
| 105 | + if (encryption_method != User.system_encryption_method) && result | |
| 106 | + self.password_type = User.system_encryption_method.to_s | |
| 107 | + self.password = password | |
| 108 | + self.password_confirmation = password | |
| 109 | + self.save! | |
| 110 | + end | |
| 111 | + result | |
| 67 | 112 | end |
| 68 | 113 | |
| 69 | 114 | def remember_token? |
| ... | ... | @@ -120,6 +165,7 @@ class User < ActiveRecord::Base |
| 120 | 165 | def encrypt_password |
| 121 | 166 | return if password.blank? |
| 122 | 167 | self.salt = Digest::SHA1.hexdigest("--#{Time.now.to_s}--#{login}--") if new_record? |
| 168 | + self.password_type ||= User.system_encryption_method.to_s | |
| 123 | 169 | self.crypted_password = encrypt(password) |
| 124 | 170 | end |
| 125 | 171 | ... | ... |
test/unit/user_test.rb
| ... | ... | @@ -162,6 +162,95 @@ class UserTest < Test::Unit::TestCase |
| 162 | 162 | assert !Person.find_by_identifier('lalala') |
| 163 | 163 | end |
| 164 | 164 | |
| 165 | + def test_should_encrypt_password_with_salted_sha1 | |
| 166 | + user = User.new(:login => 'lalala', :email => 'lalala@example.com', :password => 'test', :password_confirmation => 'test') | |
| 167 | + user.expects(:salt).returns('testsalt') | |
| 168 | + user.save! | |
| 169 | + | |
| 170 | + # SHA1+salt crypted form for password 'test', and salt 'testsalt', | |
| 171 | + # calculated by hand at IRB | |
| 172 | + crypted_password = '77606e8e9227f73618eefdfd36f8eb1b8b52ca5f' | |
| 173 | + | |
| 174 | + assert_equal crypted_password, user.crypted_password | |
| 175 | + end | |
| 176 | + | |
| 177 | + def test_should_support_md5_passwords | |
| 178 | + # ATTENTION this test explicitly exposes the crypted form of 'test'. This | |
| 179 | + # makes 'test' a terrible password. :) | |
| 180 | + user = create_user(:login => 'lalala', :email => 'lalala@example.com', :password => 'test', :password_confirmation => 'test', :password_type => 'md5') | |
| 181 | + assert_equal '098f6bcd4621d373cade4e832627b4f6', user.crypted_password | |
| 182 | + end | |
| 183 | + | |
| 184 | + def test_should_support_clear_passwords | |
| 185 | + assert_equal 'test', create_user(:password => 'test', :password_confirmation => 'test', :password_type => 'clear').crypted_password | |
| 186 | + end | |
| 187 | + | |
| 188 | + def test_should_only_allow_know_encryption_methods | |
| 189 | + assert_raise User::UnsupportedEncryptionType do | |
| 190 | + User.create( | |
| 191 | + :login => 'lalala', | |
| 192 | + :email => 'lalala@example.com', | |
| 193 | + :password => 'test', | |
| 194 | + :password_confirmation => 'test', | |
| 195 | + :password_type => 'AN_ENCRYPTION_METHOD_NOT_LIKELY_TO_EXIST' # <<<< | |
| 196 | + ) | |
| 197 | + end | |
| 198 | + end | |
| 199 | + | |
| 200 | + def test_should_use_salted_sha1_by_default | |
| 201 | + assert_equal :salted_sha1, User.system_encryption_method | |
| 202 | + end | |
| 203 | + | |
| 204 | + def test_should_be_able_to_set_system_encryption_method | |
| 205 | + # save | |
| 206 | + saved = User.system_encryption_method | |
| 207 | + | |
| 208 | + User.system_encryption_method = :some_method | |
| 209 | + assert_equal :some_method, User.system_encryption_method | |
| 210 | + | |
| 211 | + # restore | |
| 212 | + User.system_encryption_method = saved | |
| 213 | + end | |
| 214 | + | |
| 215 | + def test_new_instances_should_use_system_encryption_method | |
| 216 | + User.expects(:system_encryption_method).returns(:clear) | |
| 217 | + assert_equal 'clear', create_user.password_type | |
| 218 | + end | |
| 219 | + | |
| 220 | + def test_should_reencrypt_password_when_using_different_encryption_method_from_the_system_default | |
| 221 | + User.stubs(:system_encryption_method).returns(:salted_sha1) | |
| 222 | + | |
| 223 | + # a user was created ... | |
| 224 | + user = create_user(:login => 'lalala', :email => 'lalala@example.com', :password => 'test', :password_confirmation => 'test', :password_type => 'salted_sha1') | |
| 225 | + | |
| 226 | + # then the sysadmin decided to change the encryption method | |
| 227 | + User.expects(:system_encryption_method).returns(:md5).at_least_once | |
| 228 | + | |
| 229 | + # when the user logs in, her password must be reencrypted with the new | |
| 230 | + # method | |
| 231 | + user.authenticated?('test') | |
| 232 | + | |
| 233 | + # and the new password must be saved back to the database | |
| 234 | + user.reload | |
| 235 | + assert_equal '098f6bcd4621d373cade4e832627b4f6', user.crypted_password | |
| 236 | + end | |
| 237 | + | |
| 238 | + def test_should_not_update_encryption_if_password_incorrect | |
| 239 | + # a user was created | |
| 240 | + User.stubs(:system_encryption_method).returns(:salted_sha1) | |
| 241 | + user = create_user(:login => 'lalala', :email => 'lalala@example.com', :password => 'test', :password_confirmation => 'test', :password_type => 'salted_sha1') | |
| 242 | + crypted_password = user.crypted_password | |
| 243 | + | |
| 244 | + # then the sysadmin deciced to change the encryption method | |
| 245 | + User.expects(:system_encryption_method).returns(:md5).at_least_once | |
| 246 | + | |
| 247 | + # but the user provided the wrong password | |
| 248 | + user.authenticated?('WRONG_PASSWORD') | |
| 249 | + | |
| 250 | + # and then her password is not updated | |
| 251 | + assert_equal crypted_password, user.crypted_password | |
| 252 | + end | |
| 253 | + | |
| 165 | 254 | protected |
| 166 | 255 | def create_user(options = {}) |
| 167 | 256 | User.create({ :login => 'quire', :email => 'quire@example.com', :password => 'quire', :password_confirmation => 'quire' }.merge(options)) | ... | ... |