devise-two-factor / devise-two-factor

Barebones two-factor authentication with Devise
MIT License
1.2k stars 240 forks source link

Duplicate Warden strategy :two_factor_authenticatable #228

Open evgenyneu opened 1 year ago

evgenyneu commented 1 year ago

Hi,

When I run the generator

./bin/rails generate devise_two_factor user

It adds the following code to config/initializers/devise.rb file:

config.warden do |manager|
    manager.default_strategies(:scope => :user).unshift :two_factor_authenticatable
end

However, everything works fine if I comment it out. Moreover, if I print out the array returned by manager.default_strategies(:scope => :user) it shows two duplicate items [:two_factor_authenticatable, :two_factor_authenticatable]. I'm using this code to print it out:

config.warden do |manager|
    manager.default_strategies(:scope => :user).unshift :two_factor_authenticatable
    pp 'manager.default_strategies', manager.default_strategies(:scope => :user)
end

It looks like the generator for the config/initializers/devise.rb can be removed since the :two_factor_authenticatable Warden strategy for the User is already added when we do

class User < ApplicationRecord
  devise :two_factor_authenticatable

?

I'm using devise-two-factor (5.0.0), devise (4.8.1) and rails (7.0.3.1).

abhishek-unthinkable commented 1 year ago

+1 found the same on version 2.2.1

dan-jensen commented 1 month ago

This is also true of two_factor_backupable. If you follow the README and add unshift(:two_factor_backupable), the default_strategies includes it twice. Like with authenticatable, declaring devise :two_factor_backupable in the model is sufficient.

I think these strategies are automatically added to Warden by these lines.

Seems like the best solution is to simplify:

  1. Authenticatable: Remove inject_strategies_into_warden_config from the generator.
  2. Backupable: Remove the unshift instruction from the README.