heapsource / active_model_otp

Adds methods to set and authenticate against one time passwords (Two-Factor Authentication). Inspired in AM::SecurePassword
MIT License
773 stars 81 forks source link

HOTP, default name for otp_counter yield "stack level too deep" #32

Closed petRUShka closed 3 years ago

petRUShka commented 8 years ago

I have pretty simple user model with otp_counter:integer as provided in help:

class User < ActiveRecord::Base
  has_one_time_password counter_based: true
end

If I try to create user I catch an error:

[1] pry(main)> user = User.create
   (0.2ms)  BEGIN
   (0.2ms)  ROLLBACK
SystemStackError: stack level too deep
from /home/user/.rvm/gems/ruby-2.3.0/gems/active_model_otp-1.2.0/lib/active_model/one_time_password.rb:100:in `otp_counter'
[2] pry(main)>

The problem is obviously in that lines:

def otp_counter
  self.send(self.class.otp_counter_column_name)
end

Which basically call otp_counter method recursively again and again. Probably solution is to rename otp_counter method to something like __otp_counter.

Workaround is to rename otp_counter column to something else (both in migration and in model):

class User < ActiveRecord::Base
  has_one_time_password counter_based: true, counter_column_name: :hotp_counter
end
petRUShka commented 8 years ago

I forget to add my Rails is 4.2.

23ranjan commented 7 years ago

+1

MeySalehi commented 7 years ago

+1

xDAGRONx commented 7 years ago

It looks like this is actually fixed on the master branch, but the latest version on RubyGems does not include the change.

Any news on a future release with this fix?

justinbkay commented 6 years ago

This is still an issue. Is there a library that is being maintained that should be used instead of this one for one time passwords?

kunalbhagawati commented 5 years ago

A similar thing will also happen if the otp secret column name is otp_column.

In the RubyGems repo:

      def otp_column
        self.send(self.class.otp_column_name)
      end

      def otp_column=(attr)
        self.send("#{self.class.otp_column_name}=", attr)
      end

and in master:

      def otp_column
        self.public_send(self.class.otp_column_name)
      end

      def otp_column=(attr)
        self.public_send("#{self.class.otp_column_name}=", attr)
      end

In both cases, self.class.otp_column_name would trigger self.otp_column recursively right? I see that this is not marked private in the module so it would be accessible even with self.public_send? I'm sorry if this is a little noob, I'm new to Ruby, and I'd be thankful if someone can point out if and why I could be wrong here. Cheers! :)

khemraj016-zz commented 5 years ago

I have pretty simple user model with otp_counter:integer as provided in help:

class User < ActiveRecord::Base
  has_one_time_password counter_based: true
end

If I try to create user I catch an error:

[1] pry(main)> user = User.create
   (0.2ms)  BEGIN
   (0.2ms)  ROLLBACK
SystemStackError: stack level too deep
from /home/user/.rvm/gems/ruby-2.3.0/gems/active_model_otp-1.2.0/lib/active_model/one_time_password.rb:100:in `otp_counter'
[2] pry(main)>

The problem is obviously in that lines:

def otp_counter
  self.send(self.class.otp_counter_column_name)
end

Which basically call otp_counter method recursively again and again. Probably solution is to rename otp_counter method to something like __otp_counter.

Workaround is to rename otp_counter column to something else (both in migration and in model):

class User < ActiveRecord::Base
  has_one_time_password counter_based: true, counter_column_name: :hotp_counter
end

+1

JanBussieck commented 5 years ago

@robertomiranda, This is a pretty severe bug, could you release a new version with the fix that is already on master? Is there a particular reason why you haven't done so, if you need help with anything I am happy to chip in :)

pedrofurtado commented 3 years ago

Hey guys!

After upgrading to gem in v2.1.1 version, this issue still occurs? 🤝

pedrofurtado commented 3 years ago

For a while, we will close it. Anyway, if necessary, we can reopen later