mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Add authenticate_by when using has_secure_password
This method is intended to replace code like the following, which returns early when a user with a matching email is not found: ```ruby User.find_by(email: "...")&.authenticate("...") ``` Such code is vulnerable to timing-based enumeration attacks, wherein an attacker can determine if a user account with a given email exists. After confirming that an account exists, the attacker can try passwords associated with that email address from other leaked databases, in case the user re-used a password across multiple sites (a common practice). Additionally, knowing an account email address allows the attacker to attempt a targeted phishing ("spear phishing") attack. `authenticate_by` addresses the vulnerability by taking the same amount of time regardless of whether a user with a matching email is found.
This commit is contained in:
parent
0b10efcc4c
commit
9becc41df9
7 changed files with 154 additions and 1 deletions
|
@ -1,3 +1,29 @@
|
|||
* Add `authenticate_by` when using `has_secure_password`.
|
||||
|
||||
`authenticate_by` is intended to replace code like the following, which
|
||||
returns early when a user with a matching email is not found:
|
||||
|
||||
```ruby
|
||||
User.find_by(email: "...")&.authenticate("...")
|
||||
```
|
||||
|
||||
Such code is vulnerable to timing-based enumeration attacks, wherein an
|
||||
attacker can determine if a user account with a given email exists. After
|
||||
confirming that an account exists, the attacker can try passwords associated
|
||||
with that email address from other leaked databases, in case the user
|
||||
re-used a password across multiple sites (a common practice). Additionally,
|
||||
knowing an account email address allows the attacker to attempt a targeted
|
||||
phishing ("spear phishing") attack.
|
||||
|
||||
`authenticate_by` addresses the vulnerability by taking the same amount of
|
||||
time regardless of whether a user with a matching email is found:
|
||||
|
||||
```ruby
|
||||
User.authenticate_by(email: "...", password: "...")
|
||||
```
|
||||
|
||||
*Jonathan Hefner*
|
||||
|
||||
* Remove deprecated `ActiveRecord::DatabaseConfigurations::DatabaseConfig#spec_name`.
|
||||
|
||||
*Rafael Mendonça França*
|
||||
|
|
|
@ -68,6 +68,7 @@ module ActiveRecord
|
|||
autoload :SchemaDumper
|
||||
autoload :SchemaMigration
|
||||
autoload :Scoping
|
||||
autoload :SecurePassword
|
||||
autoload :SecureToken
|
||||
autoload :Serialization
|
||||
autoload :SignedId
|
||||
|
|
|
@ -315,7 +315,7 @@ module ActiveRecord # :nodoc:
|
|||
include Callbacks
|
||||
include Timestamp
|
||||
include Associations
|
||||
include ActiveModel::SecurePassword
|
||||
include SecurePassword
|
||||
include AutosaveAssociation
|
||||
include NestedAttributes
|
||||
include Transactions
|
||||
|
|
54
activerecord/lib/active_record/secure_password.rb
Normal file
54
activerecord/lib/active_record/secure_password.rb
Normal file
|
@ -0,0 +1,54 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/core_ext/hash/except"
|
||||
|
||||
module ActiveRecord
|
||||
module SecurePassword
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
include ActiveModel::SecurePassword
|
||||
|
||||
module ClassMethods
|
||||
# Given a set of attributes, finds a record using the non-password
|
||||
# attributes, and then authenticates that record using the password
|
||||
# attributes. Returns the record if authentication succeeds; otherwise,
|
||||
# returns +nil+.
|
||||
#
|
||||
# Regardless of whether a record is found or authentication succeeds,
|
||||
# +authenticate_by+ will take the same amount of time. This prevents
|
||||
# timing-based enumeration attacks, wherein an attacker can determine if a
|
||||
# passworded record exists even without knowing the password.
|
||||
#
|
||||
# Raises an ArgumentError if the set of attributes doesn't contain at
|
||||
# least one password and one non-password attribute.
|
||||
#
|
||||
# ==== Examples
|
||||
#
|
||||
# class User < ActiveRecord::Base
|
||||
# has_secure_password
|
||||
# end
|
||||
#
|
||||
# User.create(name: "John Doe", email: "jdoe@example.com", password: "abc123")
|
||||
#
|
||||
# User.authenticate_by(email: "jdoe@example.com", password: "abc123").name # => "John Doe" (in 373.4ms)
|
||||
# User.authenticate_by(email: "jdoe@example.com", password: "wrong") # => nil (in 373.9ms)
|
||||
# User.authenticate_by(email: "wrong@example.com", password: "abc123") # => nil (in 373.6ms)
|
||||
#
|
||||
# User.authenticate_by(email: "jdoe@example.com") # => ArgumentError
|
||||
# User.authenticate_by(password: "abc123") # => ArgumentError
|
||||
def authenticate_by(attributes)
|
||||
passwords = attributes.select { |name, value| !has_attribute?(name) && has_attribute?("#{name}_digest") }
|
||||
|
||||
raise ArgumentError, "One or more password arguments are required" if passwords.empty?
|
||||
raise ArgumentError, "One or more finder arguments are required" if passwords.size == attributes.size
|
||||
|
||||
if record = find_by(attributes.except(*passwords.keys))
|
||||
record if passwords.count { |name, value| record.public_send(:"authenticate_#{name}", value) } == passwords.size
|
||||
else
|
||||
self.new(passwords)
|
||||
nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
67
activerecord/test/cases/secure_password_test.rb
Normal file
67
activerecord/test/cases/secure_password_test.rb
Normal file
|
@ -0,0 +1,67 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require "cases/helper"
|
||||
require "models/user"
|
||||
|
||||
class SecurePasswordTest < ActiveRecord::TestCase
|
||||
setup do
|
||||
# Speed up tests
|
||||
@original_min_cost = ActiveModel::SecurePassword.min_cost
|
||||
ActiveModel::SecurePassword.min_cost = true
|
||||
|
||||
@user = User.create(password: "abc123", recovery_password: "123abc")
|
||||
end
|
||||
|
||||
teardown do
|
||||
ActiveModel::SecurePassword.min_cost = @original_min_cost
|
||||
end
|
||||
|
||||
test "authenticate_by authenticates when password is correct" do
|
||||
assert_equal @user, User.authenticate_by(token: @user.token, password: @user.password)
|
||||
end
|
||||
|
||||
test "authenticate_by does not authenticate when password is incorrect" do
|
||||
assert_nil User.authenticate_by(token: @user.token, password: "wrong")
|
||||
end
|
||||
|
||||
test "authenticate_by takes the same amount of time regardless of whether record is found" do
|
||||
# Benchmark.realtime returns fractional seconds. Thus, summing over 1000
|
||||
# iterations is equivalent to averaging over 1000 iterations and then
|
||||
# multiplying by 1000 to convert to milliseconds.
|
||||
found_average_time_in_ms = 1000.times.sum do
|
||||
Benchmark.realtime do
|
||||
User.authenticate_by(token: @user.token, password: @user.password)
|
||||
end
|
||||
end
|
||||
|
||||
not_found_average_time_in_ms = 1000.times.sum do
|
||||
Benchmark.realtime do
|
||||
User.authenticate_by(token: "wrong", password: @user.password)
|
||||
end
|
||||
end
|
||||
|
||||
assert_in_delta found_average_time_in_ms, not_found_average_time_in_ms, 0.5
|
||||
end
|
||||
|
||||
test "authenticate_by finds record using multiple attributes" do
|
||||
assert_equal @user, User.authenticate_by(token: @user.token, auth_token: @user.auth_token, password: @user.password)
|
||||
assert_nil User.authenticate_by(token: @user.token, auth_token: "wrong", password: @user.password)
|
||||
end
|
||||
|
||||
test "authenticate_by authenticates using multiple passwords" do
|
||||
assert_equal @user, User.authenticate_by(token: @user.token, password: @user.password, recovery_password: @user.recovery_password)
|
||||
assert_nil User.authenticate_by(token: @user.token, password: @user.password, recovery_password: "wrong")
|
||||
end
|
||||
|
||||
test "authenticate_by requires at least one password" do
|
||||
assert_raises ArgumentError do
|
||||
User.authenticate_by(token: @user.token)
|
||||
end
|
||||
end
|
||||
|
||||
test "authenticate_by requires at least one attribute" do
|
||||
assert_raises ArgumentError do
|
||||
User.authenticate_by(password: @user.password)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -3,6 +3,9 @@
|
|||
require "models/job"
|
||||
|
||||
class User < ActiveRecord::Base
|
||||
has_secure_password validations: false
|
||||
has_secure_password :recovery_password, validations: false
|
||||
|
||||
has_secure_token
|
||||
has_secure_token :auth_token, length: 36
|
||||
|
||||
|
|
|
@ -1274,6 +1274,8 @@ ActiveRecord::Schema.define do
|
|||
create_table :users, force: true do |t|
|
||||
t.string :token
|
||||
t.string :auth_token
|
||||
t.string :password_digest
|
||||
t.string :recovery_password_digest
|
||||
end
|
||||
|
||||
create_table :test_with_keyword_column_name, force: true do |t|
|
||||
|
|
Loading…
Reference in a new issue