Remove duplicate regist stratery
I'm working on an app use two_factor_backupable of devise-two-factor. It really helpful, thank for that!
I detected there are two problems may need to be fixed.
Problem 01: Remove duplicate of registertwo_factor_authenticatable strategy.
The gem registers two_factor_authenticatable strategy on generations.
- The first one is add
two_factor_authenticatableinto model. - The second one is inject strategy into warden config.
Problem 02: It happens again with two_factor_backupable in another way.
The installation instruction of two_factor_backupable in README.md file requires to add two_factor_backupable to both model and Devise initializer.
Checking on waden, it is duplicated.
So I think the requirement to add `:two_factor_backupable` into devise initializer is redundant.
Why it should be fixed?
By default, thedevise gem will look through and run all of the strategies. So the strategy will be executed 2 times. It definitely takes time and may generate the unexpected problem.
EX: The devise-two-factor force cascade to the next strategy if the current one fails. This is the cause of failed attempt increase multi-time on one user request.
Ruby version: ruby 2.5.1p57
Rails version: Rails 5.2.0
This looks like a good catch! And I can confirm this finding.
Before:
Warden::Proxy:70296438841060 @config={:default_scope=>:admin, :scope_defaults=>{}, :default_strategies=>{:admin=>[
:two_factor_backupable,
:two_factor_authenticatable,
:two_factor_backupable,
:two_factor_authenticatable,
:rememberable
]}, :intercept_401=>false, :failure_app=>#<Devise::Delegator:0x00007fde51e18c78>}
After removing the calls from the initializer (but still listing them in the model):
@config={:default_scope=>:admin, :scope_defaults=>{}, :default_strategies=>{:admin=>[
:two_factor_backupable,
:two_factor_authenticatable,
:rememberable
]}, :intercept_401=>false, :failure_app=>#<Devise::Delegator:0x00007fa304fef660>}
This gem is missing an app with tests. If it had one, you could proof with tests, that your changes work as expected.