Skip to content

Commit

Permalink
Fix SystemStackError with ActiveRecord
Browse files Browse the repository at this point in the history
If you use the default `otp_counter` column name,
InstanceMethodsOnActivation#otp_counter calls `otp_counter` resulting in
a stack overflow.
  • Loading branch information
garethrees authored and Gareth Rees committed Oct 8, 2015
1 parent 03c114b commit a2d02ff
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 2 deletions.
2 changes: 2 additions & 0 deletions active_model_otp.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ Gem::Specification.new do |spec|
spec.add_dependency "activemodel"
spec.add_dependency "rotp"

spec.add_development_dependency "activerecord"
spec.add_development_dependency "bundler", "~> 1.3"
spec.add_development_dependency "rake"
spec.add_development_dependency "minitest", "~> 5.4.2"
spec.add_development_dependency "sqlite3"
end
12 changes: 10 additions & 2 deletions lib/active_model/one_time_password.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,19 @@ def otp_column=(attr)
end

def otp_counter
self.public_send(self.class.otp_counter_column_name)
if self.class.otp_counter_column_name != 'otp_counter'
self.public_send(self.class.otp_counter_column_name)
else
super
end
end

def otp_counter=(attr)
self.public_send("#{self.class.otp_counter_column_name}=", attr)
if self.class.otp_counter_column_name != 'otp_counter'
self.public_send("#{self.class.otp_counter_column_name}=", attr)
else
super
end
end
end
end
Expand Down
3 changes: 3 additions & 0 deletions test/models/activerecord_user.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class ActiverecordUser < ActiveRecord::Base
has_one_time_password counter_based: true
end
15 changes: 15 additions & 0 deletions test/one_time_password_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ def setup
@member = Member.new
@member.email = nil
@member.run_callbacks :create

@ar_user = ActiverecordUser.new
@ar_user.email = 'roberto@heapsource.com'
@ar_user.run_callbacks :create
end

def test_authenticate_with_otp
Expand All @@ -34,6 +38,17 @@ def test_counter_based_otp
assert code != @member.otp_code(auto_increment: true)
end

def test_counter_based_otp_active_record
code = @ar_user.otp_code
assert @ar_user.authenticate_otp(code)
assert @ar_user.authenticate_otp(code, auto_increment: true)
assert !@ar_user.authenticate_otp(code)
@ar_user.otp_counter -= 1
assert @ar_user.authenticate_otp(code)
assert code == @ar_user.otp_code
assert code != @ar_user.otp_code(auto_increment: true)
end

def test_authenticate_with_otp_when_drift_is_allowed
code = @user.otp_code(Time.now - 30)
assert @user.authenticate_otp(code, drift: 60)
Expand Down
12 changes: 12 additions & 0 deletions test/schema.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
ActiveRecord::Schema.define do
self.verbose = false

create_table :activerecord_users, :force => true do |t|
t.string :key
t.string :email
t.integer :otp_counter
t.string :otp_secret_key
t.timestamps
end

end
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,9 @@
require "active_model_otp"
require "minitest/autorun"
require "minitest/unit"
require "active_record"

ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:"
load "#{ File.dirname(__FILE__) }/schema.rb"

Dir["models/*.rb"].each {|file| require file }

0 comments on commit a2d02ff

Please sign in to comment.