Skip to content

Commit

Permalink
feat(mfa): Added support for TOTP code tolerances
Browse files Browse the repository at this point in the history
This PR adds support for a TOTP tolerance range using a new `TOTP_TOLERANCE` MFA setting. It is very similar to https://django-otp-official.readthedocs.io/en/stable/overview.html#django_otp.plugins.otp_totp.models.TOTPDevice.tolerance.

The idea is rather simple: if `TOTP_TOLERANCE` is non-zero we allow codes equal to the expected counter +/- TOTP_TOLERANCE.

This should address #3954

Co-authored-by: Josiah Bruner <josiah.bruner@jellyfish.co>
Reviewed-on: https://codeberg.org/allauth/django-allauth/pulls/4183
Reviewed-by: pennersr <pennersr@noreply.codeberg.org>
Co-authored-by: josiah <josiah@noreply.codeberg.org>
Co-committed-by: josiah <josiah@noreply.codeberg.org>
  • Loading branch information
2 people authored and pennersr committed Nov 19, 2024
1 parent db7e4bd commit dff1b87
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 4 deletions.
7 changes: 7 additions & 0 deletions allauth/mfa/app_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ def TOTP_INSECURE_BYPASS_CODE(self):
)
return code

@property
def TOTP_TOLERANCE(self):
"""
The number of time steps in the past or future to allow. Lower values are more secure, but more likely to fail due to clock drift.
"""
return self._setting("TOTP_TOLERANCE", 0)

@property
def SUPPORTED_TYPES(self):
dflt = ["recovery_codes", "totp"]
Expand Down
15 changes: 11 additions & 4 deletions allauth/mfa/totp/internal/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import secrets
import struct
import time
from typing import Iterator

from django.core.cache import cache

Expand All @@ -30,9 +31,11 @@ def get_totp_secret(regenerate: bool = False) -> str:
return secret


def hotp_counter_from_time() -> int:
def yield_hotp_counters_from_time() -> Iterator[int]:
current_time = int(time.time()) # Get the current Unix timestamp
return current_time // app_settings.TOTP_PERIOD
counter = current_time // app_settings.TOTP_PERIOD
for i in range(-app_settings.TOTP_TOLERANCE, app_settings.TOTP_TOLERANCE + 1):
yield counter + i


def hotp_value(secret: str, counter: int) -> int:
Expand Down Expand Up @@ -64,8 +67,12 @@ def _is_insecure_bypass(code: str) -> bool:
def validate_totp_code(secret: str, code: str) -> bool:
if _is_insecure_bypass(code):
return True
value = hotp_value(secret, hotp_counter_from_time())
return code == format_hotp_value(value)
counters = yield_hotp_counters_from_time()
for counter in counters:
value = hotp_value(secret, counter)
if code == format_hotp_value(value):
return True
return False


class TOTP:
Expand Down
44 changes: 44 additions & 0 deletions allauth/mfa/totp/tests/test_unit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
from unittest import mock

from allauth.mfa import app_settings
from allauth.mfa.totp.internal.auth import (
format_hotp_value,
generate_totp_secret,
hotp_value,
validate_totp_code,
yield_hotp_counters_from_time,
)


@mock.patch("time.time", mock.MagicMock(return_value=1731948631))
def test_totp_counters_from_time():
app_settings.TOTP_TOLERANCE = 0
counters = list(yield_hotp_counters_from_time())
assert len(counters) == 1


@mock.patch("time.time", mock.MagicMock(return_value=1731948631))
def test_totp_counters_from_time_with_tolerance():
app_settings.TOTP_TOLERANCE = 1
counters = list(yield_hotp_counters_from_time())
assert len(counters) == 3


@mock.patch("time.time", mock.MagicMock(return_value=1731948631))
def test_validate_with_tolerance():
app_settings.TOTP_TOLERANCE = 1
test_secret = generate_totp_secret()
expected_value = format_hotp_value(hotp_value(test_secret, 57731621))
assert validate_totp_code(test_secret, expected_value)

before_value = format_hotp_value(hotp_value(test_secret, 57731620))
assert validate_totp_code(test_secret, before_value)

after_value = format_hotp_value(hotp_value(test_secret, 57731622))
assert validate_totp_code(test_secret, after_value)

two_before_value = format_hotp_value(hotp_value(test_secret, 57731619))
assert not validate_totp_code(test_secret, two_before_value)

two_after_value = format_hotp_value(hotp_value(test_secret, 57731623))
assert not validate_totp_code(test_secret, two_after_value)
3 changes: 3 additions & 0 deletions docs/mfa/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ Available settings:
``MFA_TOTP_PERIOD`` (default: ``30``)
The period that a TOTP code will be valid for, in seconds.

``MFA_TOTP_TOLERANCE`` (default: ``0``)
The number of time steps in the past or future to allow. Lower values are more secure, but more likely to fail due to clock drift.

``MFA_TOTP_DIGITS`` (default: ``6``)
The number of digits for TOTP codes.

Expand Down

0 comments on commit dff1b87

Please sign in to comment.