Fix acceptance of invalid otp attempts

[kmwhite]

In the event that an OTP attempt is invalid, the super method is never
called. If the authentication_hash did describe a record, it was not
rejecting the authentication. The find_for_database_authentication
method is inherited from Devise::Strategies::DatabaseAuthenticatable
which pays no attention to the otp_attempt, considering this pry
session:

From: /usr/local/bundle/bundler/gems/devise-two-factor-07acb0ae5068/lib/devise_two_factor/strategies/two_factor_authenticatable.rb @ line 12 Devise::Strategies::TwoFactorAuthenticatable#authenticate!:

     1: module Devise
     2:   module Strategies
     3:     class TwoFactorAuthenticatable < Devise::Strategies::DatabaseAuthenticatable
     4:
     5:       def authenticate!
     6:         resource = mapping.to.find_for_database_authentication(authentication_hash)
     7:         # We authenticate in two cases:
     8:         # 1. The password and the OTP are correct
     9:         # 2. The password is correct, and OTP is not required for login
    10:         # We check the OTP, then defer to DatabaseAuthenticatable
    11:         require "pry"; binding.pry
 => 12:         if validate(resource) { validate_otp(resource) }
    13:           super
    14:         end
    15:
    16:         fail(Devise.paranoid ? :invalid : :not_found_in_database) unless resource
    17:
    18:         # We want to cascade to the next strategy if this one fails,
    19:         # but database authenticatable automatically halts on a bad password
    20:         @Halted = false if @Result == :failure
    21:       end
    22:
    23:       def validate_otp(resource)
    24:         return true unless resource.otp_required_for_login
    25:         return if params[scope]['otp_attempt'].nil?
    26:         resource.validate_and_consume_otp!(params[scope]['otp_attempt'])
    27:       end
    28:     end
    29:   end
    30: end
    31:
    32: Warden::Strategies.add(:two_factor_authenticatable, Devise::Strategies::TwoFactorAuthenticatable)

[6] pry(#<Devise::Strategies::TwoFactorAuthenticatable>)> params[scope]
=> {"login"=>"miranda", "password"=>"password", "otp_attempt"=>"123456"}
[7] pry(#<Devise::Strategies::TwoFactorAuthenticatable>)> validate(resource)
=> true
[8] pry(#<Devise::Strategies::TwoFactorAuthenticatable>)> validate_otp(resource)
=> false
[9] pry(#<Devise::Strategies::TwoFactorAuthenticatable>)> next

From: /usr/local/bundle/bundler/gems/devise-two-factor-07acb0ae5068/lib/devise_two_factor/strategies/two_factor_authenticatable.rb @ line 16 Devise::Strategies::TwoFactorAuthenticatable#authenticate!:

     5: def authenticate!
     6:   resource = mapping.to.find_for_database_authentication(authentication_hash)
     7:   # We authenticate in two cases:
     8:   # 1. The password and the OTP are correct
     9:   # 2. The password is correct, and OTP is not required for login
    10:   # We check the OTP, then defer to DatabaseAuthenticatable
    11:   require "pry"; binding.pry
    12:   if validate(resource) { validate_otp(resource) }
    13:     super
    14:   end
    15:
 => 16:   fail(Devise.paranoid ? :invalid : :not_found_in_database) unless resource
    17:
    18:   # We want to cascade to the next strategy if this one fails,
    19:   # but database authenticatable automatically halts on a bad password
    20:   @Halted = false if @Result == :failure
    21: end

[9] pry(#<Devise::Strategies::TwoFactorAuthenticatable>):1> resource.present?
=> true
[10] pry(#<Devise::Strategies::TwoFactorAuthenticatable>):1> resource.login
=> "miranda"
[11] pry(#<Devise::Strategies::TwoFactorAuthenticatable>):1>

While the super is not called to set up the session, but it does not
necessarily fail the login. This change ensures that fail() is called
when this case is hit.

[kmwhite]

I ran into this issue in a Rails 4.2 (through Rails 5.2) app we run at Buildout Inc. These changes ensured that our authentication flow did correctly abort when the wrong OTP attempt was provided when required. I also added a set of specs for the strategy itself to verify this.

If there's any feedback or requested changes, I'm happy to adjust this PR! Thanks!

[kmwhite]

Oof. Saw CI failed. Fixing the faker callers.