django-two-factor-auth
django-two-factor-auth copied to clipboard
jazzband
No message due to failed attempts
Expected Behavior
When settings OTP_HOTP_THROTTLE_FACTOR is not 0 user after failed attempts should get error message:
Verification temporarily disabled because of {n} failed attempts, please try again soon.
Current Behavior
User gets misleading error:
Invalid token. Please make sure you have entered it correctly.
Possible Solution
Problem is that in AuthenticationTokenForm we don't have a field for user selected device which OTPAuthenticationFormMixin requires:
#. Define three additional form fields::
otp_device = forms.CharField(required=False, widget=forms.Select)
and therefore later in method _verify_token it never triggers device.verify_is_allowed() since device at this point will always be None and it tries to match the token with all devices, and if it fails return generic Invalid token error whereas it should return Verification disabled due to failed attempts.
To Fix this issue, we could simply override in AuthenticationTokenForm method _chosen_device:
def _chosen_device(self, user):
device = super()._chosen_device(user)
if not device:
device_qs = TOTPDevice.objects.devices_for_user(user, confirmed=True)
return device_qs.last()
return device
In that case, when device is not specified it takes the last confirmed one (if exists) and later properly triggers for it verify_is_allowed. Therefore when user already had few failed attempts doesn't get a misleading Invalid token when she is sure the token is correct.
There is also a second way, we already pass initial_device to AuthenticationTokenForm we could simply save it:
self.initial_device = initial_device
and overload _chosen_device this way:
def _chosen_device(self, user):
device = super()._chosen_device(user)
if not device:
device = self.initial_device
return device
If someone says that this wouldn't work properly if someone has multiple devices, we can even go a step further and add:
def _chosen_device(self, user):
device = super()._chosen_device(user)
if not device and count(TOTPDevice.objects.devices_for_user(user, confirmed=True)) == 1:
device = self.initial_device
return device
To make sure that if user has more than one device and didn't select any match_token method will be invoked.
I think we should implement something along this lines since I guess most implementation are single device only, and for those user shouldn't get invalid token error when she should get Too many failed attempts. Wait a moment.
What is the status of this? How can I get correct failure message when throttle limit os exceeded?
@claudep I think this issue is solved by https://github.com/jazzband/django-two-factor-auth/pull/626 too and can be closed
