From dff1b87859cf366cd104745e10732311486d3fcd Mon Sep 17 00:00:00 2001 From: josiah Date: Tue, 19 Nov 2024 21:29:24 +0000 Subject: [PATCH] feat(mfa): Added support for TOTP code tolerances 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 Reviewed-on: https://codeberg.org/allauth/django-allauth/pulls/4183 Reviewed-by: pennersr Co-authored-by: josiah Co-committed-by: josiah --- allauth/mfa/app_settings.py | 7 +++++ allauth/mfa/totp/internal/auth.py | 15 +++++++--- allauth/mfa/totp/tests/test_unit.py | 44 +++++++++++++++++++++++++++++ docs/mfa/configuration.rst | 3 ++ 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 allauth/mfa/totp/tests/test_unit.py diff --git a/allauth/mfa/app_settings.py b/allauth/mfa/app_settings.py index b92b1b32fe..532be0126f 100644 --- a/allauth/mfa/app_settings.py +++ b/allauth/mfa/app_settings.py @@ -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"] diff --git a/allauth/mfa/totp/internal/auth.py b/allauth/mfa/totp/internal/auth.py index 7d704e8fd3..7b91374dce 100644 --- a/allauth/mfa/totp/internal/auth.py +++ b/allauth/mfa/totp/internal/auth.py @@ -4,6 +4,7 @@ import secrets import struct import time +from typing import Iterator from django.core.cache import cache @@ -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: @@ -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: diff --git a/allauth/mfa/totp/tests/test_unit.py b/allauth/mfa/totp/tests/test_unit.py new file mode 100644 index 0000000000..8d7b567fe8 --- /dev/null +++ b/allauth/mfa/totp/tests/test_unit.py @@ -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) diff --git a/docs/mfa/configuration.rst b/docs/mfa/configuration.rst index 6772118469..4c00d65e6a 100644 --- a/docs/mfa/configuration.rst +++ b/docs/mfa/configuration.rst @@ -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.