Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double failed attempts and add error code for invalid otp #136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pdkproitf
Copy link

@pdkproitf pdkproitf commented Aug 28, 2018

Fixed: #127 and #51

Add the following message into locales/devise.en.yml:

devise:
    failure:
       invalid_otp: "2FA verification code failed."

If you are enabling use backup code, the parameter otp_backup should be attached in your parameters request instead of otp_attempt.

@pdkproitf pdkproitf force-pushed the fix-double-failed-attempts-and-code-error-message branch 4 times, most recently from 686a9e3 to 6ac2bb3 Compare August 28, 2018 18:08
@pdkproitf pdkproitf closed this Aug 28, 2018
@pdkproitf pdkproitf reopened this Aug 28, 2018
@pdkproitf pdkproitf force-pushed the fix-double-failed-attempts-and-code-error-message branch 7 times, most recently from 6dff6ec to afc7aad Compare August 28, 2018 19:29
@verenion
Copy link

I can confirm that this fixes the issues #28 and #127

@ChielHackman
Copy link

For me it fixed the issue to, but it breaks displaying the last_attempt warning.

Fixed it by adding the following to lib/devise_two_factor/strategies/two_factor_authenticatable.rb

if resource.failed_attempts == 2
  return fail!(:last_attempt)
else
  return fail!(:not_found_in_database)
end

This assumes you got max 3 attempts. Not sure how to get the max attempts from the devise initializer.

@pdkproitf
Copy link
Author

@ChielHackman @ChielHackman You can use maximum_attempts to check resource maximum attempts.

@mediafinger
Copy link
Contributor

Is this a different implementation for the same issue: PR #130 ?

@jeremy04
Copy link

Is this a different implementation for the same issue: PR #130 ?

Yes

@pdkproitf
Copy link
Author

The master branch's code was changed from the time I submitted this pull request. So anyone who wants to fix this problem should consider the solution instead of using it.

@williantenfen
Copy link

williantenfen commented Aug 24, 2023

any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lockable with two factor backupable causing double failed attempts
7 participants